[release/6.0] Add cast when replacing promoted struct with field in lowering (#58951)

* Add cast when replacing promoted struct with field in lowering

We may need to sign/zero-extend when we do this replacement very late.
Normally this cast would be inserted in morph.

Fix #58373

* Add test

* Fix formatting

* Fix test cases

* Type returned-struct-as-primitive correctly for normalization

Retyping it to the type of the ret node is not right when reinterpreting
small structs as a type that needs to be normalized.

* Fix assertion

* Run jit-format

Co-authored-by: Jakob Botsch Nielsen <jakob.botsch.nielsen@gmail.com>

diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp
index 08cbc8b..178217f 100644 (file)
--- a/src/coreclr/jit/lower.cpp
+++ b/src/coreclr/jit/lower.cpp
@@ -2984,9 +2984,8 @@ void Lowering::LowerRet(GenTreeUnOp* ret)
     // There are two kinds of retyping:
     // - A simple bitcast can be inserted when:
     //   - We're returning a floating type as an integral type or vice-versa, or
-    //   - We're returning a struct as a primitive type and using the old form of retyping.
-    // - If we're returning a struct as a primitive type and *not* using old retying, we change the type of
-    //   'retval' in 'LowerRetStructLclVar()'
+    // - If we're returning a struct as a primitive type, we change the type of
+    // 'retval' in 'LowerRetStructLclVar()'
     bool needBitcast =
         (ret->TypeGet() != TYP_VOID) && (varTypeUsesFloatReg(ret) != varTypeUsesFloatReg(ret->gtGetOp1()));
     bool doPrimitiveBitcast = false;
@@ -3408,6 +3407,7 @@ void Lowering::LowerRetSingleRegStructLclVar(GenTreeUnOp* ret)
     unsigned   lclNum = lclVar->GetLclNum();
     LclVarDsc* varDsc = comp->lvaGetDesc(lclNum);
 
+    bool replacedInLowering = false;
     if (varDsc->CanBeReplacedWithItsField(comp))
     {
         // We can replace the struct with its only field and keep the field on a register.
@@ -3420,8 +3420,9 @@ void Lowering::LowerRetSingleRegStructLclVar(GenTreeUnOp* ret)
                 "[%06u]\n",
                 lclNum, fieldLclNum, comp->dspTreeID(ret));
         lclVar->ChangeType(fieldDsc->lvType);
-        lclNum = fieldLclNum;
-        varDsc = comp->lvaGetDesc(lclNum);
+        lclNum             = fieldLclNum;
+        varDsc             = comp->lvaGetDesc(lclNum);
+        replacedInLowering = true;
     }
     else if (varDsc->lvPromoted)
     {
@@ -3432,20 +3433,52 @@ void Lowering::LowerRetSingleRegStructLclVar(GenTreeUnOp* ret)
 
     if (varDsc->lvDoNotEnregister)
     {
+        assert(!replacedInLowering);
         lclVar->ChangeOper(GT_LCL_FLD);
         lclVar->AsLclFld()->SetLclOffs(0);
-        lclVar->ChangeType(ret->TypeGet());
+
+        // We are returning as a primitive type and the lcl is of struct type.
+        assert(comp->info.compRetNativeType != TYP_STRUCT);
+        assert((genTypeSize(comp->info.compRetNativeType) == genTypeSize(ret)) ||
+               (varTypeIsIntegral(ret) && varTypeIsIntegral(comp->info.compRetNativeType) &&
+                (genTypeSize(comp->info.compRetNativeType) <= genTypeSize(ret))));
+        // If the actual return type requires normalization, then make sure we
+        // do so by using the correct small type for the GT_LCL_FLD. It would
+        // be conservative to check just compRetNativeType for this since small
+        // structs are normalized to primitive types when they are returned in
+        // registers, so we would normalize for them as well.
+        if (varTypeIsSmall(comp->info.compRetType))
+        {
+            assert(genTypeSize(comp->info.compRetNativeType) == genTypeSize(comp->info.compRetType));
+            lclVar->ChangeType(comp->info.compRetType);
+        }
+        else
+        {
+            // Otherwise we don't mind that we leave the upper bits undefined.
+            lclVar->ChangeType(ret->TypeGet());
+        }
     }
     else
     {
         const var_types lclVarType = varDsc->GetRegisterType(lclVar);
         assert(lclVarType != TYP_UNDEF);
+
+        if (varDsc->lvNormalizeOnLoad() && replacedInLowering)
+        {
+            // For a normalize-on-load var that we replaced late we need to insert a cast
+            // as morph would typically be responsible for this.
+            GenTreeCast* cast = comp->gtNewCastNode(TYP_INT, lclVar, false, lclVarType);
+            ret->gtOp1        = cast;
+            BlockRange().InsertBefore(ret, cast);
+            ContainCheckCast(cast);
+        }
+
         const var_types actualType = genActualType(lclVarType);
         lclVar->ChangeType(actualType);
 
         if (varTypeUsesFloatReg(ret) != varTypeUsesFloatReg(lclVarType))
         {
-            GenTree* bitcast = comp->gtNewBitCastNode(ret->TypeGet(), lclVar);
+            GenTree* bitcast = comp->gtNewBitCastNode(ret->TypeGet(), ret->gtOp1);
             ret->gtOp1       = bitcast;
             BlockRange().InsertBefore(ret, bitcast);
             ContainCheckBitCast(bitcast);

diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_58373/Runtime_58373_1.cs b/src/tests/JIT/Regression/JitBlue/Runtime_58373/Runtime_58373_1.cs
new file mode 100644 (file)
index 0000000..76a4607
--- /dev/null
+++ b/src/tests/JIT/Regression/JitBlue/Runtime_58373/Runtime_58373_1.cs
@@ -0,0 +1,29 @@
+// 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.Runtime.CompilerServices;
+
+public unsafe class Runtime_58373
+{
+    public static int Main()
+    {
+        short halfValue = HalfToInt16Bits(MakeHalf());
+        int x = halfValue;
+        short val2 = HalfToInt16Bits(*(Half*)&x);
+
+        return halfValue == val2 ? 100 : -1;
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    static Half MakeHalf()
+    {
+        return (Half)(-1.0f);
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    static short HalfToInt16Bits(Half h)
+    {
+        return *(short*)&h;
+    }
+}  
\ No newline at end of file

diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_58373/Runtime_58373_1.csproj b/src/tests/JIT/Regression/JitBlue/Runtime_58373/Runtime_58373_1.csproj
new file mode 100644 (file)
index 0000000..1a1d3ea
--- /dev/null
+++ b/src/tests/JIT/Regression/JitBlue/Runtime_58373/Runtime_58373_1.csproj
@@ -0,0 +1,13 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+  </PropertyGroup>
+  <PropertyGroup>
+    <DebugType>None</DebugType>
+    <Optimize>True</Optimize>
+    <AllowUnsafeBlocks>True</AllowUnsafeBlocks>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="$(MSBuildProjectName).cs" />
+  </ItemGroup>
+</Project>
\ No newline at end of file

diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_58373/Runtime_58373_2.cs b/src/tests/JIT/Regression/JitBlue/Runtime_58373/Runtime_58373_2.cs
new file mode 100644 (file)
index 0000000..c8d1286
--- /dev/null
+++ b/src/tests/JIT/Regression/JitBlue/Runtime_58373/Runtime_58373_2.cs
@@ -0,0 +1,47 @@
+// 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.Runtime.CompilerServices;
+
+public unsafe class Runtime_58373
+{
+    public static int Main()
+    {
+        // Use up a lot of registers
+        int a = GetVal();
+        int b = GetVal();
+        int c = GetVal();
+        int d = GetVal();
+        int e = GetVal();
+        int f = GetVal();
+        int g = GetVal();
+        int h = GetVal();
+        int i = GetVal();
+
+        short val1 = HalfToInt16Bits(MakeHalf());
+        Half half = MakeHalf();
+        MakeHalf(); // This will spill lower 16 bits of 'half' to memory
+        short val2 = HalfToInt16Bits(half); // This will pass 32 bits as arg with upper 16 bits undefined
+
+        return val1 == val2 ? 100 + a + b + c + d + e + f + g + h + i : -1;
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    static int GetVal()
+    {
+        return 0;
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    static Half MakeHalf()
+    {
+        return default;
+    }
+
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    static short HalfToInt16Bits(Half h)
+    {
+        return *(short*)&h;
+    }
+}  
\ No newline at end of file

diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_58373/Runtime_58373_2.csproj b/src/tests/JIT/Regression/JitBlue/Runtime_58373/Runtime_58373_2.csproj
new file mode 100644 (file)
index 0000000..1a1d3ea
--- /dev/null
+++ b/src/tests/JIT/Regression/JitBlue/Runtime_58373/Runtime_58373_2.csproj
@@ -0,0 +1,13 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+  </PropertyGroup>
+  <PropertyGroup>
+    <DebugType>None</DebugType>
+    <Optimize>True</Optimize>
+    <AllowUnsafeBlocks>True</AllowUnsafeBlocks>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="$(MSBuildProjectName).cs" />
+  </ItemGroup>
+</Project>
\ No newline at end of file