Skip to content

Fix InlineArray and some other corner cases in Swift lowering #99337

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Mar 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

namespace System.Runtime.CompilerServices
{
[AttributeUsage(AttributeTargets.Struct, AllowMultiple = false)]
public sealed class InlineArrayAttribute : Attribute
{
public InlineArrayAttribute(int length)
{
Length = length;
}

public int Length { get; }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,7 @@
<Compile Include="Internal\Runtime\MethodTable.Runtime.cs" />
<Compile Include="System\Runtime\CompilerServices\CastCache.cs" />
<Compile Include="System\Runtime\CompilerServices\ClassConstructorRunner.cs" />
<Compile Include="System\Runtime\CompilerServices\InlineArrayAttribute.cs" />
<Compile Include="System\Runtime\CompilerServices\StaticClassConstructionContext.cs" />
<Compile Include="System\Runtime\InteropServices\InAttribute.cs" />
<Compile Include="System\Diagnostics\CodeAnalysis\DoesNotReturnIfAttribute.cs" />
Expand Down
27 changes: 26 additions & 1 deletion src/coreclr/tools/Common/JitInterface/CorInfoTypes.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using System.Diagnostics.CodeAnalysis;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
using System.Text;
using Internal.Pgo;

namespace Internal.JitInterface
Expand Down Expand Up @@ -1507,11 +1508,22 @@ private struct SwiftLoweredTypes
public CorInfoType type;
}

[InlineArray(4)]
private struct LoweredOffsets
{
public uint offset;
}

private SwiftLoweredTypes _loweredElements;

[UnscopedRef]
public Span<CorInfoType> LoweredElements => _loweredElements;

private LoweredOffsets _offsets;

[UnscopedRef]
public Span<uint> Offsets => _offsets;

public nint numLoweredElements;

// Override for a better unit test experience
Expand All @@ -1522,7 +1534,20 @@ public override string ToString()
return "byReference";
}

return string.Join(", ", LoweredElements[0..(int)numLoweredElements].ToArray());
var stringBuilder = new StringBuilder();
stringBuilder.Append('{');
for (int i = 0; i < numLoweredElements; i++)
{
if (i != 0)
{
stringBuilder.Append(", ");
}
stringBuilder.Append(LoweredElements[i]);
stringBuilder.Append(": ");
stringBuilder.Append(Offsets[i]);
}
stringBuilder.Append('}');
return stringBuilder.ToString();
}
}
}
135 changes: 85 additions & 50 deletions src/coreclr/tools/Common/JitInterface/SwiftPhysicalLowering.cs
Original file line number Diff line number Diff line change
@@ -1,11 +1,8 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics;
using System.Linq;
using System.Runtime.InteropServices;
using Internal.TypeSystem;

Expand All @@ -29,7 +26,19 @@ private sealed class LoweringVisitor(int pointerSize) : FieldLayoutIntervalCalcu
protected override bool IntervalsHaveCompatibleTags(LoweredType existingTag, LoweredType nextTag)
{
// Adjacent ranges mapped to opaque or empty can be combined.
return existingTag is LoweredType.Opaque or LoweredType.Empty && nextTag is LoweredType.Opaque or LoweredType.Empty;
if (existingTag is LoweredType.Empty
&& nextTag is LoweredType.Empty)
{
return true;
}

if (existingTag is LoweredType.Opaque
&& nextTag is LoweredType.Opaque)
{
return true;
}

return false;
}

protected override FieldLayoutInterval CombineIntervals(FieldLayoutInterval firstInterval, FieldLayoutInterval nextInterval)
Expand Down Expand Up @@ -94,13 +103,39 @@ protected override LoweredType GetIntervalDataForType(int offset, TypeDesc field

protected override bool NeedsRecursiveLayout(int offset, TypeDesc fieldType) => fieldType.IsValueType && !fieldType.IsPrimitiveNumeric;

public List<CorInfoType> GetLoweredTypeSequence()
private List<FieldLayoutInterval> CreateConsolidatedIntervals()
{
// We need to track the sequence size to ensure we break up the opaque ranges
// into correctly-sized integers that do not require padding.
int loweredSequenceSize = 0;
List<CorInfoType> loweredTypes = new();
// First, let's make a list of exclusively non-empty intervals.
List<FieldLayoutInterval> consolidatedIntervals = new(Intervals.Count);
foreach (var interval in Intervals)
{
if (interval.Tag != LoweredType.Empty)
{
consolidatedIntervals.Add(interval);
}
}

// Now, we'll look for adjacent opaque intervals and combine them.
for (int i = 0; i < consolidatedIntervals.Count - 1; i++)
{
// Only merge sequential opaque intervals that are within the same PointerSize-sized chunk.
if (consolidatedIntervals[i].Tag == LoweredType.Opaque
&& consolidatedIntervals[i + 1].Tag == LoweredType.Opaque
&& (consolidatedIntervals[i].EndSentinel - 1) / PointerSize == consolidatedIntervals[i + 1].Start / PointerSize)
{
consolidatedIntervals[i] = CombineIntervals(consolidatedIntervals[i], consolidatedIntervals[i + 1]);
consolidatedIntervals.RemoveAt(i + 1);
i--;
}
}

return consolidatedIntervals;
}

public List<(CorInfoType, int)> GetLoweredTypeSequence()
{
List<(CorInfoType, int)> loweredTypes = new();
foreach (var interval in CreateConsolidatedIntervals())
{
// Empty intervals at this point don't need to be represented in the lowered type sequence.
// We want to skip over them.
Expand All @@ -109,26 +144,20 @@ public List<CorInfoType> GetLoweredTypeSequence()

if (interval.Tag == LoweredType.Float)
{
loweredTypes.Add(CorInfoType.CORINFO_TYPE_FLOAT);
loweredSequenceSize = loweredSequenceSize.AlignUp(4);
loweredSequenceSize += 4;
loweredTypes.Add((CorInfoType.CORINFO_TYPE_FLOAT, interval.Start));
}

if (interval.Tag == LoweredType.Double)
{
loweredTypes.Add(CorInfoType.CORINFO_TYPE_DOUBLE);
loweredSequenceSize = loweredSequenceSize.AlignUp(8);
loweredSequenceSize += 8;
loweredTypes.Add((CorInfoType.CORINFO_TYPE_DOUBLE, interval.Start));
}

if (interval.Tag == LoweredType.Int64)
{
loweredTypes.Add(CorInfoType.CORINFO_TYPE_LONG);
loweredSequenceSize = loweredSequenceSize.AlignUp(8);
loweredSequenceSize += 8;
loweredTypes.Add((CorInfoType.CORINFO_TYPE_LONG, interval.Start));
}

if (interval.Tag == LoweredType.Opaque)
if (interval.Tag is LoweredType.Opaque)
{
// We need to split the opaque ranges into integer parameters.
// As part of this splitting, we must ensure that we don't introduce alignment padding.
Expand All @@ -138,40 +167,42 @@ public List<CorInfoType> GetLoweredTypeSequence()
// The lowered range is allowed to extend past the end of the opaque range (including past the end of the struct),
// but not into the next non-empty interval.
// However, due to the properties of the lowering (the only non-8 byte elements of the lowering are 4-byte floats),
// we'll never encounter a scneario where we need would need to account for a correctly-aligned
// we'll never encounter a scenario where we need would need to account for a correctly-aligned
// opaque range of > 4 bytes that we must not pad to 8 bytes.


// As long as we need to fill more than 4 bytes and the sequence is currently 8-byte aligned, we'll split into 8-byte integers.
// If we have more than 2 bytes but less than 4 and the sequence is 4-byte aligned, we'll use a 4-byte integer to represent the rest of the parameters.
// If we have 2 bytes and the sequence is 2-byte aligned, we'll use a 2-byte integer to represent the rest of the parameters.
// If we have 1 byte, we'll use a 1-byte integer to represent the rest of the parameters.
// While we need to fill more than 4 bytes and the sequence is currently 8-byte aligned, we'll split into 8-byte integers.
// While we need to fill more than 2 bytes but less than 4 and the sequence is 4-byte aligned, we'll use a 4-byte integer to represent the rest of the parameters.
// While we need to fill more than 1 bytes and the sequence is 2-byte aligned, we'll use a 2-byte integer to represent the rest of the parameters.
// While we need to fill at least 1 byte, we'll use a 1-byte integer to represent the rest of the parameters.
int opaqueIntervalStart = interval.Start;
int remainingIntervalSize = interval.Size;
while (remainingIntervalSize > 4 && loweredSequenceSize == loweredSequenceSize.AlignUp(8))
while (remainingIntervalSize > 0)
{
loweredTypes.Add(CorInfoType.CORINFO_TYPE_LONG);
loweredSequenceSize += 8;
remainingIntervalSize -= 8;
}

if (remainingIntervalSize > 2 && loweredSequenceSize == loweredSequenceSize.AlignUp(4))
{
loweredTypes.Add(CorInfoType.CORINFO_TYPE_INT);
loweredSequenceSize += 4;
remainingIntervalSize -= 4;
}

if (remainingIntervalSize > 1 && loweredSequenceSize == loweredSequenceSize.AlignUp(2))
{
loweredTypes.Add(CorInfoType.CORINFO_TYPE_SHORT);
loweredSequenceSize += 2;
remainingIntervalSize -= 2;
}

if (remainingIntervalSize == 1)
{
loweredSequenceSize += 1;
loweredTypes.Add(CorInfoType.CORINFO_TYPE_BYTE);
if (remainingIntervalSize > 4 && opaqueIntervalStart == opaqueIntervalStart.AlignUp(8))
{
loweredTypes.Add((CorInfoType.CORINFO_TYPE_LONG, opaqueIntervalStart));
opaqueIntervalStart += 8;
remainingIntervalSize -= 8;
}
else if (remainingIntervalSize > 2 && opaqueIntervalStart == opaqueIntervalStart.AlignUp(4))
{
loweredTypes.Add((CorInfoType.CORINFO_TYPE_INT, opaqueIntervalStart));
opaqueIntervalStart += 4;
remainingIntervalSize -= 4;
}
else if (remainingIntervalSize > 1 && opaqueIntervalStart == opaqueIntervalStart.AlignUp(2))
{
loweredTypes.Add((CorInfoType.CORINFO_TYPE_SHORT, opaqueIntervalStart));
opaqueIntervalStart += 2;
remainingIntervalSize -= 2;
}
else
{
opaqueIntervalStart++;
remainingIntervalSize--;
loweredTypes.Add((CorInfoType.CORINFO_TYPE_BYTE, opaqueIntervalStart));
}
}
}
}
Expand All @@ -190,7 +221,7 @@ public static CORINFO_SWIFT_LOWERING LowerTypeForSwiftSignature(TypeDesc type)
LoweringVisitor visitor = new(type.Context.Target.PointerSize);
visitor.AddFields(type, addTrailingEmptyInterval: false);

List<CorInfoType> loweredTypes = visitor.GetLoweredTypeSequence();
List<(CorInfoType type, int offset)> loweredTypes = visitor.GetLoweredTypeSequence();

// If a type has a primitive sequence with more than 4 elements, Swift passes it by reference.
if (loweredTypes.Count > 4)
Expand All @@ -204,7 +235,11 @@ public static CORINFO_SWIFT_LOWERING LowerTypeForSwiftSignature(TypeDesc type)
numLoweredElements = loweredTypes.Count
};

CollectionsMarshal.AsSpan(loweredTypes).CopyTo(lowering.LoweredElements);
for (int i = 0; i < loweredTypes.Count; i++)
{
lowering.LoweredElements[i] = loweredTypes[i].type;
lowering.Offsets[i] = (uint)loweredTypes[i].offset;
}

return lowering;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,11 @@ public void AddToFieldLayout(int offset, TypeDesc fieldType, bool addTrailingEmp
{
if (NeedsRecursiveLayout(offset, fieldType))
{
if (fieldType is MetadataType { IsInlineArray: true } mdType)
{
fieldType = new TypeWithRepeatedFields(mdType);
}

List<FieldLayoutInterval> nestedIntervals = new List<FieldLayoutInterval>();
foreach (FieldDesc field in fieldType.GetFields())
{
Expand Down Expand Up @@ -123,6 +128,11 @@ private void AddToFieldLayout(List<FieldLayoutInterval> fieldLayout, int offset,
{
if (NeedsRecursiveLayout(offset, fieldType))
{
if (fieldType is MetadataType { IsInlineArray: true } mdType)
{
fieldType = new TypeWithRepeatedFields(mdType);
}

foreach (FieldDesc field in fieldType.GetFields())
{
int fieldOffset = offset + field.Offset.AsInt;
Expand Down Expand Up @@ -246,7 +256,7 @@ private void MergeIntervalWithNeighboringIntervals(List<FieldLayoutInterval> fie
break;
}

if ((previousInterval.EndSentinel == expandedInterval.Start) && !IntervalsHaveCompatibleTags(expandedInterval.Tag, previousInterval.Tag))
if ((previousInterval.EndSentinel == expandedInterval.Start) && !IntervalsHaveCompatibleTags(previousInterval.Tag, expandedInterval.Tag))
{
// Expanded interval starts just after previous interval, but does not match tag. Expansion succeeded
break;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

namespace Internal.TypeSystem
{
public sealed partial class ImpliedRepeatedFieldDesc : FieldDesc
{
protected internal override int CompareToImpl(FieldDesc other, TypeSystemComparer comparer)
{
var impliedRepeatedFieldDesc = (ImpliedRepeatedFieldDesc)other;

int result = comparer.Compare(_underlyingFieldDesc, impliedRepeatedFieldDesc._underlyingFieldDesc);

if (result != 0)
{
return result;
}

return FieldIndex.CompareTo(impliedRepeatedFieldDesc.FieldIndex);
}

protected internal override int ClassCode => 31666958;
}
}
Original file line number Diff line number Diff line change
@@ -1,11 +1,9 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using Internal.TypeSystem;

namespace ILCompiler
namespace Internal.TypeSystem
{
public sealed class ImpliedRepeatedFieldDesc : FieldDesc
public sealed partial class ImpliedRepeatedFieldDesc : FieldDesc
{
private readonly FieldDesc _underlyingFieldDesc;

Expand Down Expand Up @@ -36,26 +34,10 @@ public ImpliedRepeatedFieldDesc(DefType owningType, FieldDesc underlyingFieldDes

public int FieldIndex { get; }

protected override int ClassCode => 31666958;

public override EmbeddedSignatureData[] GetEmbeddedSignatureData() => _underlyingFieldDesc.GetEmbeddedSignatureData();

public override bool HasCustomAttribute(string attributeNamespace, string attributeName) => _underlyingFieldDesc.HasCustomAttribute(attributeNamespace, attributeName);

protected override int CompareToImpl(FieldDesc other, TypeSystemComparer comparer)
{
var impliedRepeatedFieldDesc = (ImpliedRepeatedFieldDesc)other;

int result = comparer.Compare(_underlyingFieldDesc, impliedRepeatedFieldDesc._underlyingFieldDesc);

if (result != 0)
{
return result;
}

return FieldIndex.CompareTo(impliedRepeatedFieldDesc.FieldIndex);
}

public override MarshalAsDescriptor GetMarshalAsDescriptor() => _underlyingFieldDesc.GetMarshalAsDescriptor();

public override string Name => $"{_underlyingFieldDesc.Name}[{FieldIndex}]";
Expand Down
Loading