Skip to content

Commit

Permalink
Fix InlineArray and some other corner cases in Swift lowering (#99337)
Browse files Browse the repository at this point in the history
  • Loading branch information
jkoritzinsky authored Mar 8, 2024
1 parent fa49755 commit 3b8d8da
Show file tree
Hide file tree
Showing 18 changed files with 370 additions and 172 deletions.
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; }
}
}
1 change: 1 addition & 0 deletions src/coreclr/nativeaot/Test.CoreLib/src/Test.CoreLib.csproj
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

0 comments on commit 3b8d8da

Please sign in to comment.