JIT: Improve parameter register field extraction and reconstruction

src/coreclr/jit/lower.cpp

@@ -5199,6 +5199,29 @@ struct LowerFieldListRegisterInfo
     }
 };
 
+#if defined(TARGET_XARCH)
+static bool IsFloatPairFieldListRegister(GenTreeFieldList::Use* use,
+                                         unsigned               regStart,
+                                         unsigned               regEnd,
+                                         var_types              regType)
+{
+    if ((regType != TYP_DOUBLE) || (use == nullptr) || (use->GetType() != TYP_FLOAT) || (use->GetOffset() != regStart))
+    {
+        return false;
+    }
+
+    GenTreeFieldList::Use* secondUse = use->GetNext();
+    if ((secondUse == nullptr) || (secondUse->GetType() != TYP_FLOAT) ||
+        (secondUse->GetOffset() != regStart + genTypeSize(TYP_FLOAT)))
+    {
+        return false;
+    }
+
+    GenTreeFieldList::Use* nextUse = secondUse->GetNext();
+    return (nextUse == nullptr) || (nextUse->GetOffset() >= regEnd);
+}
+#endif // TARGET_XARCH
+
 //----------------------------------------------------------------------------------------------
 // LowerRetFieldList:
 //   Lower a returned FIELD_LIST node.
@@ -5372,6 +5395,12 @@ bool Lowering::IsFieldListCompatibleWithRegisters(GenTreeFieldList*   fieldList,
             return false;
         }
 
+#if defined(TARGET_XARCH)
+        bool supportsFloatPairInsertion = IsFloatPairFieldListRegister(use, regStart, regEnd, regType);
+#else
+        bool supportsFloatPairInsertion = false;
+#endif // TARGET_XARCH
+
         do
         {
             unsigned fieldStart = use->GetOffset();
@@ -5396,12 +5425,15 @@ bool Lowering::IsFieldListCompatibleWithRegisters(GenTreeFieldList*   fieldList,
                 return false;
             }
 
-            // float -> float insertions are not yet supported
+            // A pair of floats can be directly reconstructed in an 8-byte floating-point register.
             if (varTypeUsesFloatReg(use->GetNode()) && varTypeUsesFloatReg(regType) && (fieldStart != regStart))
             {
-                JITDUMP("it is not; field [%06u] requires an insertion into register %u\n",
-                        Compiler::dspTreeID(use->GetNode()), i);
-                return false;
+                if (!supportsFloatPairInsertion)
+                {
+                    JITDUMP("it is not; field [%06u] requires an insertion into register %u\n",
+                            Compiler::dspTreeID(use->GetNode()), i);
+                    return false;
+                }
             }
 
             // int -> float is currently only supported if we can do it as a single bitcast (i.e. without insertions
@@ -5464,6 +5496,28 @@ void Lowering::LowerFieldListToFieldListOfRegisters(GenTreeFieldList*   fieldLis
 
         GenTree* fieldListPrev = fieldList->gtPrev;
 
+#if defined(TARGET_XARCH)
+        if (IsFloatPairFieldListRegister(use, regStart, regEnd, regType))
+        {
+            GenTreeFieldList::Use* secondUse = use->GetNext();
+            GenTree* firstVector = InsertNewSimdCreateScalarUnsafeNode(TYP_SIMD16, use->GetNode(), TYP_FLOAT, 16);
+            GenTree* secondVector =
+                InsertNewSimdCreateScalarUnsafeNode(TYP_SIMD16, secondUse->GetNode(), TYP_FLOAT, 16);
+
+            GenTree* value = m_compiler->gtNewSimdHWIntrinsicNode(TYP_SIMD16, firstVector, secondVector,
+                                                                  NI_X86Base_UnpackLow, TYP_FLOAT, 16);
+            BlockRange().InsertBefore(fieldList, value);
+
+            regEntry->SetNode(value);
+            regEntry->SetType(TYP_SIMD16);
+            regEntry->SetNext(secondUse->GetNext());
+            use = regEntry->GetNext();
+
+            LowerNode(value);
+
+            continue;
+        }
+#endif // TARGET_XARCH
         do
         {
             unsigned fieldStart = use->GetOffset();
@@ -9214,13 +9268,23 @@ void Lowering::FindInducedParameterRegisterLocals()
                 continue;
             }
 
-            // TODO-CQ: Float -> !float extractions are not supported
-            // TODO-CQ: Float -> float extractions with non-zero offset is not supported
+#ifdef FEATURE_SIMD
+            if (varTypeIsSIMD(segment.GetRegisterType()) &&
+                (varTypeIsSIMD(fld) ? (fld->GetLclOffs() != segment.Offset)
+                                    : (((fld->GetLclOffs() - segment.Offset) % genTypeSize(fld)) != 0)))
+            {
+                continue;
+            }
+#endif // FEATURE_SIMD
+
+#ifdef TARGET_ARM
+            // The scalar extraction below can require TYP_LONG nodes, which are not legal after decomposition.
             if (genIsValidFloatReg(segment.GetRegister()) &&
                 (!varTypeUsesFloatReg(fld) || (fld->GetLclOffs() != segment.Offset)))
             {
                 continue;
             }
+#endif // TARGET_ARM
 
             // Found a register segment this field is contained in
             regSegment = &segment;
@@ -9289,26 +9353,39 @@ void Lowering::FindInducedParameterRegisterLocals()
 
         GenTree* value = m_compiler->gtNewLclVarNode(remappedLclNum);
 
-#ifdef TARGET_WASM
-        if (varTypeIsSIMD(value) && !varTypeIsSIMD(fld))
-        {
-            // Unlike native targets, wasm cannot reinterpret a v128 local access as a scalar.
-            const unsigned laneOffset = fld->GetLclOffs() - regSegment->Offset;
-            const unsigned scalarSize = genTypeSize(fld);
-            assert((laneOffset % scalarSize) == 0);
+        bool useSimdGetElement = false;
+#ifdef FEATURE_SIMD
+        useSimdGetElement = varTypeIsSIMD(value) && !varTypeIsSIMD(fld);
 
-            const unsigned laneIndex = laneOffset / scalarSize;
-            value                    = m_compiler->gtNewSimdGetElementNode(fld->TypeGet(), value,
-                                                                           m_compiler->gtNewIconNode(static_cast<ssize_t>(laneIndex)),
-                                                                           fld->TypeGet(), genTypeSize(value));
+#ifndef TARGET_WASM
+        if (!useSimdGetElement && varTypeIsFloating(value) && varTypeIsFloating(fld) &&
+            (fld->GetLclOffs() != regSegment->Offset) &&
+            (((fld->GetLclOffs() - regSegment->Offset) % genTypeSize(fld)) == 0))
+        {
+            assert(value->TypeIs(TYP_DOUBLE));
+            assert(fld->TypeIs(TYP_FLOAT));
+            value->gtType     = TYP_SIMD8;
+            useSimdGetElement = true;
         }
-        else if (varTypeUsesFloatReg(value))
-#else
-        if (varTypeUsesFloatReg(value))
-#endif // TARGET_WASM
+#endif // !TARGET_WASM
+#endif // FEATURE_SIMD
+
+        if (useSimdGetElement)
         {
-            assert(fld->GetLclOffs() == regSegment->Offset);
+#ifdef FEATURE_SIMD
+            unsigned laneOffset = fld->GetLclOffs() - regSegment->Offset;
+            assert((laneOffset % genTypeSize(fld)) == 0);
 
+            unsigned laneIndex = laneOffset / genTypeSize(fld);
+            value              = m_compiler->gtNewSimdGetElementNode(fld->TypeGet(), value,
+                                                                     m_compiler->gtNewIconNode((ssize_t)laneIndex), fld->TypeGet(),
+                                                                     genTypeSize(value));
+#else
+            unreached();
+#endif // FEATURE_SIMD
+        }
+        else if (varTypeUsesFloatReg(value) && varTypeUsesFloatReg(fld) && (fld->GetLclOffs() == regSegment->Offset))
+        {
             value->gtType = fld->TypeGet();
 
 #ifdef FEATURE_SIMD
@@ -9323,6 +9400,11 @@ void Lowering::FindInducedParameterRegisterLocals()
         }
         else
         {
+            if (varTypeUsesFloatReg(value))
+            {
+                value = m_compiler->gtNewBitCastNode(genTypeSize(value) == 8 ? TYP_LONG : TYP_INT, value);
+            }
+
             var_types registerType = value->TypeGet();
 
             if (fld->GetLclOffs() > regSegment->Offset)

src/coreclr/jit/promotion.cpp

@@ -3052,21 +3052,28 @@ bool Promotion::MapsToParameterRegister(Compiler* comp, unsigned lclNum, unsigne
 
     for (const ABIPassingSegment& seg : abiInfo.Segments())
     {
-        // This code corresponds to code in Lower::FindInducedParameterRegisterLocals
+        // This code corresponds to code in Lowering::FindInducedParameterRegisterLocals
         if ((offset < seg.Offset) || (offset + genTypeSize(accessType) > seg.Offset + seg.Size))
         {
             continue;
         }
 
-        if (!genIsValidIntReg(seg.GetRegister()) && varTypeUsesFloatReg(accessType))
+#ifdef FEATURE_SIMD
+        if (varTypeIsSIMD(seg.GetRegisterType()) &&
+            (varTypeIsSIMD(accessType) ? (offset != seg.Offset)
+                                       : (((offset - seg.Offset) % genTypeSize(accessType)) != 0)))
         {
             continue;
         }
+#endif // FEATURE_SIMD
 
-        if (genIsValidFloatReg(seg.GetRegister()) && (offset != seg.Offset))
+#ifdef TARGET_ARM
+        // The scalar extraction in lowering can require TYP_LONG nodes, which are not legal after decomposition.
+        if (genIsValidFloatReg(seg.GetRegister()) && (!varTypeUsesFloatReg(accessType) || (offset != seg.Offset)))
         {
             continue;
         }
+#endif // TARGET_ARM
 
         return true;
     }

src/tests/JIT/Directed/StructABI/FieldListFloatInsertion.cs

@@ -0,0 +1,51 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System.Runtime.CompilerServices;
+using Xunit;
+
+public class FieldListFloatInsertion
+{
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    private static FloatPair ReturnPair(float x, float y)
+    {
+        // X64-LINUX-FULL-LINE: {{v?unpcklps}} {{xmm[0-9]+}}, {{xmm[0-9]+}}
+        // X64-OSX-FULL-LINE: {{v?unpcklps}} {{xmm[0-9]+}}, {{xmm[0-9]+}}
+        return new FloatPair { X = x, Y = y };
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    private static float PassPair(float x, float y)
+    {
+        // X64-LINUX-FULL-LINE: {{v?unpcklps}} {{xmm[0-9]+}}, {{xmm[0-9]+}}
+        // X64-OSX-FULL-LINE: {{v?unpcklps}} {{xmm[0-9]+}}, {{xmm[0-9]+}}
+        return SumPair(new FloatPair { X = x, Y = y });
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    private static float PassPairWithConstant(float y)
+    {
+        // X64-LINUX-FULL-LINE: {{v?unpcklps}} {{xmm[0-9]+}}, {{xmm[0-9]+}}
+        // X64-OSX-FULL-LINE: {{v?unpcklps}} {{xmm[0-9]+}}, {{xmm[0-9]+}}
+        return SumPair(new FloatPair { X = 0.0f, Y = y });
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    private static float SumPair(FloatPair value) => value.X + value.Y;
+
+    [Fact]
+    public static int TestEntryPoint()
+    {
+        FloatPair value = ReturnPair(1.0f, 2.0f);
+        return (value.X == 1.0f) && (value.Y == 2.0f) && (PassPair(3.0f, 4.0f) == 7.0f) &&
+                       (PassPairWithConstant(5.0f) == 5.0f)
+                   ? 100
+                   : 0;
+    }
+
+    private struct FloatPair
+    {
+        public float X;
+        public float Y;
+    }
+}

src/tests/JIT/Directed/StructABI/FieldListFloatInsertion.csproj

@@ -0,0 +1,11 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <DebugType>None</DebugType>
+    <Optimize>True</Optimize>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="$(MSBuildProjectName).cs">
+      <HasDisasmCheck>true</HasDisasmCheck>
+    </Compile>
+  </ItemGroup>
+</Project>

src/tests/JIT/opt/Unsafe/Unsafe.cs

@@ -3,6 +3,7 @@
 
 using System;
 using System.Runtime.CompilerServices;
+using System.Runtime.Intrinsics;
 using Xunit;
 
 namespace CodeGenTests
@@ -30,6 +31,34 @@ static byte UnsafeAsNarrowCast_Long(long value)
             return Unsafe.As<long, byte>(ref value);
         }
 
+        [MethodImpl(MethodImplOptions.NoInlining)]
+        static float UnsafeAsSecondFloat_Double(double value)
+        {
+            // X64-FULL-LINE: {{v?movshdup}} {{xmm[0-9]+}}, {{xmm[0-9]+}}
+            // ARM64-FULL-LINE: dup {{s[0-9]+}}, {{v[0-9]+}}.s[1]
+            return Unsafe.Add(ref Unsafe.As<double, float>(ref value), 1);
+        }
+
+        [MethodImpl(MethodImplOptions.NoInlining)]
+        static float UnsafeAsMisalignedFloat_Double(double value)
+        {
+            return Unsafe.As<byte, float>(ref Unsafe.AddByteOffset(ref Unsafe.As<double, byte>(ref value), 2));
+        }
+
+        [MethodImpl(MethodImplOptions.NoInlining)]
+        static int UnsafeAsInt_Vector128(Vector128<float> value)
+        {
+            // ARM64-FULL-LINE: smov {{x[0-9]+}}, {{v[0-9]+}}.s[2]
+            return Unsafe.Add(ref Unsafe.As<Vector128<float>, int>(ref value), 2);
+        }
+
+        [MethodImpl(MethodImplOptions.NoInlining)]
+        static double UnsafeAsSecondDouble_Vector128(Vector128<double> value)
+        {
+            // ARM64-FULL-LINE: dup {{d[0-9]+}}, {{v[0-9]+}}.d[1]
+            return Unsafe.Add(ref Unsafe.As<Vector128<double>, double>(ref value), 1);
+        }
+
         [Fact]
         public static int TestEntryPoint()
         {
@@ -42,6 +71,23 @@ public static int TestEntryPoint()
             if (UnsafeAsNarrowCast_Long(255) != 255)
                 return 0;
 
+            double doubleValue = BitConverter.Int64BitsToDouble(
+                ((long)BitConverter.SingleToInt32Bits(2.5f) << 32) | (uint)BitConverter.SingleToInt32Bits(1.25f));
+            if (UnsafeAsSecondFloat_Double(doubleValue) != 2.5f)
+                return 0;
+
+            float expectedMisaligned = BitConverter.Int32BitsToSingle((int)(BitConverter.DoubleToInt64Bits(doubleValue) >> 16));
+            if (UnsafeAsMisalignedFloat_Double(doubleValue) != expectedMisaligned)
+                return 0;
+
+            Vector128<float> floatVector = Vector128.Create(1.0f, 2.0f, 3.0f, 4.0f);
+            if (UnsafeAsInt_Vector128(floatVector) != BitConverter.SingleToInt32Bits(3.0f))
+                return 0;
+
+            Vector128<double> doubleVector = Vector128.Create(5.0, 6.0);
+            if (UnsafeAsSecondDouble_Vector128(doubleVector) != 6.0)
+                return 0;
+
             return 100;
         }
     }