Account for type mismatch of `FIELD_LIST` members in LSRA (#57450)

* Add the test

* Fix the bug

Assertion propagation can replace a TYP_UBYTE field
in a field list with a TYP_INT constant, so LSRA
must use the actual field type when deciding whether
an internal register to allocate must be byteable.

diff --git a/src/coreclr/jit/lowerxarch.cpp b/src/coreclr/jit/lowerxarch.cpp
index 43c0df6..38a7655 100644 (file)
--- a/src/coreclr/jit/lowerxarch.cpp
+++ b/src/coreclr/jit/lowerxarch.cpp
@@ -452,10 +452,10 @@ void Lowering::LowerPutArgStk(GenTreePutArgStk* putArgStk)
         unsigned prevOffset        = putArgStk->GetStackByteSize();
         for (GenTreeFieldList::Use& use : fieldList->Uses())
         {
-            GenTree* const  fieldNode   = use.GetNode();
-            const var_types fieldType   = fieldNode->TypeGet();
-            const unsigned  fieldOffset = use.GetOffset();
-            assert(fieldType != TYP_LONG);
+            GenTree* const fieldNode   = use.GetNode();
+            const unsigned fieldOffset = use.GetOffset();
+
+            assert(!fieldNode->TypeIs(TYP_LONG));
 
             // We can treat as a slot any field that is stored at a slot boundary, where the previous
             // field is not in the same slot. (Note that we store the fields in reverse order.)

diff --git a/src/coreclr/jit/lsraxarch.cpp b/src/coreclr/jit/lsraxarch.cpp
index 2d2c452..f440182 100644 (file)
--- a/src/coreclr/jit/lsraxarch.cpp
+++ b/src/coreclr/jit/lsraxarch.cpp
@@ -1501,18 +1501,18 @@ int LinearScan::BuildPutArgStk(GenTreePutArgStk* putArgStk)
         for (GenTreeFieldList::Use& use : putArgStk->gtOp1->AsFieldList()->Uses())
         {
             GenTree* const  fieldNode   = use.GetNode();
-            const var_types fieldType   = fieldNode->TypeGet();
             const unsigned  fieldOffset = use.GetOffset();
+            const var_types fieldType   = use.GetType();
 
 #ifdef TARGET_X86
             assert(fieldType != TYP_LONG);
 #endif // TARGET_X86
 
 #if defined(FEATURE_SIMD)
-            // Note that we need to check the GT_FIELD_LIST type, not 'fieldType'. This is because the
-            // GT_FIELD_LIST will be TYP_SIMD12 whereas the fieldType might be TYP_SIMD16 for lclVar, where
+            // Note that we need to check the field type, not the type of the node. This is because the
+            // field type will be TYP_SIMD12 whereas the node type might be TYP_SIMD16 for lclVar, where
             // we "round up" to 16.
-            if ((use.GetType() == TYP_SIMD12) && (simdTemp == nullptr))
+            if ((fieldType == TYP_SIMD12) && (simdTemp == nullptr))
             {
                 simdTemp = buildInternalFloatRegisterDefForNode(putArgStk);
             }

diff --git a/src/tests/JIT/Directed/StructABI/FieldListByteNodeTypeMismatchX86.cs b/src/tests/JIT/Directed/StructABI/FieldListByteNodeTypeMismatchX86.cs
new file mode 100644 (file)
index 0000000..1872ca3
--- /dev/null
+++ b/src/tests/JIT/Directed/StructABI/FieldListByteNodeTypeMismatchX86.cs
@@ -0,0 +1,75 @@
+using System.Runtime.CompilerServices;
+using System.Runtime.InteropServices;
+
+public unsafe class FieldListByteNodeTypeMismatchX86
+{
+    private static readonly byte* _addr = (byte*)Marshal.AllocHGlobal(20);
+    private static int _result = 100;
+
+    public static int Main()
+    {
+        int sum = 0;
+        Problem(&sum);
+
+        return _result;
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    private static void Problem(int* sum)
+    {
+        // Just a loop with some computations so that we can use
+        // callee-saved registers (which happen to be non-byteable ones).
+        for (int i = 0; i < 10; i++)
+        {
+            var i1 = i ^ i;
+            var i2 = i | i;
+            var i3 = i & i;
+            var i4 = i1 + i + i2 - i3;
+            i4 = i2 ^ i4;
+
+            *sum += i4;
+        }
+
+        Sx2x1 s;
+        byte* j = Addr();
+
+        int o1 = j[-2];
+        int o2 = j[-1];
+        s.Short = j[0];
+        s.Byte = j[1];
+
+        o1 = o1 + o1;
+        o2 = o2 + o2;
+
+        if (s.Byte != 1)
+        {
+            return;
+        }
+
+        // Here assertion propagation will make s.Byte into CNS_INT(TYP_INT), yet the RA must
+        // still allocate a byteable register so that it can be saved to the stack correctly.
+        Call(s, 0, 0, o2, o1, j);
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    private static byte* Addr()
+    {
+        _addr[11] = 1;
+        return _addr + 10;
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    private static void Call(Sx2x1 s, int i3, int i4, int i5, int i6, byte* i1)
+    {
+        if (s.Byte != 1)
+        {
+            _result = -1;
+        }
+    }
+
+    private struct Sx2x1
+    {
+        public short Short;
+        public byte Byte;
+    }
+}

diff --git a/src/tests/JIT/Directed/StructABI/FieldListByteNodeTypeMismatchX86.csproj b/src/tests/JIT/Directed/StructABI/FieldListByteNodeTypeMismatchX86.csproj
new file mode 100644 (file)
index 0000000..6d06d33
--- /dev/null
+++ b/src/tests/JIT/Directed/StructABI/FieldListByteNodeTypeMismatchX86.csproj
@@ -0,0 +1,13 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
+  </PropertyGroup>
+  <PropertyGroup>
+    <DebugType>None</DebugType>
+    <Optimize>True</Optimize>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="$(MSBuildProjectName).cs" />
+  </ItemGroup>
+</Project>