From 928ff3015d1936ca9985a0123754e13cbf47b237 Mon Sep 17 00:00:00 2001
From: Jakob Botsch Nielsen <Jakob.botsch.nielsen@gmail.com>
Date: Fri, 5 Jan 2024 15:23:30 +0100
Subject: [PATCH] JIT: Home float parameters before integer parameters (#96439)

Parameters that are going into float registers can come from integer
registers in the presence of struct promotion. We need to home those
before integer parameters or the source register could have been
overridden by the integer parameter homing logic.

Ideally it seems like the homing logic should be unified to handle all
parameters simultaneously, but this seems like a simple enough fix. I do
not think we have ABIs where we have the opposite kind constraint
(integer parameters coming from float registers).

Fix #96306
---
 src/coreclr/jit/codegencommon.cpp             |  8 +++-
 .../JitBlue/Runtime_96306/Runtime_96306.cs    | 37 +++++++++++++++++++
 .../Runtime_96306/Runtime_96306.csproj        | 10 +++++
 3 files changed, 54 insertions(+), 1 deletion(-)
 create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_96306/Runtime_96306.cs
 create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_96306/Runtime_96306.csproj

diff --git a/src/coreclr/jit/codegencommon.cpp b/src/coreclr/jit/codegencommon.cpp
index 5cde37749329c..09a28cc373bf4 100644
--- a/src/coreclr/jit/codegencommon.cpp
+++ b/src/coreclr/jit/codegencommon.cpp
@@ -6243,8 +6243,14 @@ void CodeGen::genFnProlog()
         };
 
 #if defined(TARGET_AMD64) || defined(TARGET_ARM64) || defined(TARGET_ARM)
-        assignIncomingRegisterArgs(&intRegState);
+        // Handle float parameters first; in the presence of struct promotion
+        // we can have parameters that are homed into float registers but
+        // passed in integer registers. So make sure we get those out of the
+        // integer registers before we potentially override those as part of
+        // handling integer parameters.
+
         assignIncomingRegisterArgs(&floatRegState);
+        assignIncomingRegisterArgs(&intRegState);
 #else
         assignIncomingRegisterArgs(&intRegState);
 #endif
diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_96306/Runtime_96306.cs b/src/tests/JIT/Regression/JitBlue/Runtime_96306/Runtime_96306.cs
new file mode 100644
index 0000000000000..043e15ad90522
--- /dev/null
+++ b/src/tests/JIT/Regression/JitBlue/Runtime_96306/Runtime_96306.cs
@@ -0,0 +1,37 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System.Numerics;
+using System.Runtime.CompilerServices;
+using Xunit;
+
+public class Runtime_96306
+{
+    [Fact]
+    public static int TestEntryPoint()
+    {
+        return Foo(new Point2D { V = new Vector2(101, -1) }, 100);
+    }
+
+    // 'a' is passed in rcx but homed into xmm1 after promotion.
+    // 'scale' is passed in xmm1 but spilled because of the call to Bar.
+    // We must take care that we spill 'scale' before we home 'a'.
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    private static int Foo(Point2D a, float scale)
+    {
+        Bar();
+        return ReturnValue(scale);
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    private static int ReturnValue(float value) => (int)value;
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    private static void Bar() { }
+
+    private struct Point2D
+    {
+        public Vector2 V;
+    }
+}
+
diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_96306/Runtime_96306.csproj b/src/tests/JIT/Regression/JitBlue/Runtime_96306/Runtime_96306.csproj
new file mode 100644
index 0000000000000..6c2e6c34ba403
--- /dev/null
+++ b/src/tests/JIT/Regression/JitBlue/Runtime_96306/Runtime_96306.csproj
@@ -0,0 +1,10 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <Optimize>True</Optimize>
+    <DebugType>None</DebugType>
+    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="$(MSBuildProjectName).cs" />
+  </ItemGroup>
+</Project>
\ No newline at end of file