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

Author: jkoritzinsky

src/coreclr/nativeaot/Test.CoreLib/src/System/Runtime/CompilerServices/InlineArrayAttribute.cs

@@ -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; }
+    }
+}

src/coreclr/nativeaot/Test.CoreLib/src/Test.CoreLib.csproj

@@ -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" />

src/coreclr/tools/Common/JitInterface/CorInfoTypes.cs

@@ -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
@@ -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
@@ -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();
         }
     }
 }

src/coreclr/tools/Common/JitInterface/SwiftPhysicalLowering.cs

@@ -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;
 
@@ -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)
@@ -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.
@@ -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.
@@ -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));
+                            }
                         }
                     }
                 }
@@ -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)
@@ -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;
         }

src/coreclr/tools/Common/TypeSystem/Common/FieldLayoutIntervalCalculator.cs

@@ -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())
                 {
@@ -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;
@@ -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;

src/coreclr/tools/Common/TypeSystem/Common/ImpliedRepeatedFieldDesc.Sorting.cs

@@ -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;
+    }
+}

src/coreclr/tools/Common/Compiler/ImpliedRepeatedFieldDesc.cs renamed to src/coreclr/tools/Common/TypeSystem/Common/ImpliedRepeatedFieldDesc.cs

@@ -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;
 
@@ -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}]";