JIT: work around issue with GDV and Bboxing (#56126)

If a call is a GDV candidate and returns a struct via hidden buffer, and that
return value is immediately boxed, the GDV expansion will produce IR in
incorrect order, leading to bad codegen.

This seems to be a rare enough sequence that disabling GDV is a reasonable
workaround for now.

Actually the box expansion is producing IR in the wrong order and GDV fails
to fix the order (unlike inlining, which does fix the order).

Longer term we should avoid producing out of order IR. But that seems a bit
more complicated and may have other CQ impact.

Added a test case.

Closes #53549.

diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp
index f6ec7f3..27dd183 100644 (file)
--- a/src/coreclr/jit/importer.cpp
+++ b/src/coreclr/jit/importer.cpp
@@ -6808,6 +6808,38 @@ void Compiler::impImportAndPushBox(CORINFO_RESOLVED_TOKEN* pResolvedToken)
 
         if (varTypeIsStruct(exprToBox))
         {
+            // Workaround for GitHub issue 53549.
+            //
+            // If the struct being boxed is returned via hidden buffer and comes from an inline/gdv candidate,
+            // the IR we produce after importation is out of order:
+            //
+            //    call (&(box-temp + 8), ....)
+            //    box-temp = newobj
+            //    ret-val from call (void)
+            //        ... box-temp (on stack)
+            //
+            // For inline candidates this bad ordering gets fixed up during inlining, but for GDV candidates
+            // the GDV expansion is such that the newobj follows the call as in the above.
+            //
+            // This is nontrivial to fix in GDV, so in these (rare) cases we simply disable GDV.
+            //
+            if (exprToBox->OperIs(GT_RET_EXPR))
+            {
+                GenTreeCall* const call = exprToBox->AsRetExpr()->gtInlineCandidate->AsCall();
+
+                if (call->IsGuardedDevirtualizationCandidate() && call->HasRetBufArg())
+                {
+                    JITDUMP("Disabling GDV for [%06u] because of in-box struct return\n");
+                    call->ClearGuardedDevirtualizationCandidate();
+                    if (call->IsVirtualStub())
+                    {
+                        JITDUMP("Restoring stub addr %p from guarded devirt candidate info\n",
+                                dspPtr(call->gtGuardedDevirtualizationCandidateInfo->stubAddr));
+                        call->gtStubCallStubAddr = call->gtGuardedDevirtualizationCandidateInfo->stubAddr;
+                    }
+                }
+            }
+
             assert(info.compCompHnd->getClassSize(pResolvedToken->hClass) == info.compCompHnd->getClassSize(operCls));
             op1 = impAssignStructPtr(op1, exprToBox, operCls, (unsigned)CHECK_SPILL_ALL);
         }

diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_53549/Runtime_53549.cs b/src/tests/JIT/Regression/JitBlue/Runtime_53549/Runtime_53549.cs
new file mode 100644 (file)
index 0000000..5078556
--- /dev/null
+++ b/src/tests/JIT/Regression/JitBlue/Runtime_53549/Runtime_53549.cs
@@ -0,0 +1,49 @@
+// 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;
+using System.Threading;
+
+interface I
+{
+    public Decimal F();
+}
+
+class Runtime_53549 : I
+{
+    Decimal z;
+
+    public Decimal F() => z;
+
+    public static bool G(object o) 
+    {
+        return ((decimal) o).Equals(100M);
+    }
+
+    // This method will have bad codegen if
+    // we allow GDV on i.F().
+    //
+    [MethodImpl(MethodImplOptions.NoInlining)]
+    public static int H(I i)
+    {
+        return G(i.F()) ? 100 : -1;
+    }
+
+    public static int Main()
+    {
+        Runtime_53549 x = new Runtime_53549();
+        x.z = 100M;
+
+        for (int i = 0; i < 100; i++)
+        {
+            _ = H(x);
+            Thread.Sleep(15);
+        }
+
+        return H(x);
+    }
+}
+
+
+    

diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_53549/Runtime_53549.csproj b/src/tests/JIT/Regression/JitBlue/Runtime_53549/Runtime_53549.csproj
new file mode 100644 (file)
index 0000000..9a3423f
--- /dev/null
+++ b/src/tests/JIT/Regression/JitBlue/Runtime_53549/Runtime_53549.csproj
@@ -0,0 +1,24 @@
+<Project Sdk="Microsoft.NET.Sdk">
+  <PropertyGroup>
+    <OutputType>Exe</OutputType>
+  </PropertyGroup>
+  <PropertyGroup>
+    <DebugType>None</DebugType>
+    <Optimize>True</Optimize>
+  </PropertyGroup>
+  <PropertyGroup>
+    <CLRTestBatchPreCommands><![CDATA[
+$(CLRTestBatchPreCommands)
+set COMPlus_TieredPGO=1
+set COMPlus_TC_QuickJitForLoops=1
+]]></CLRTestBatchPreCommands>
+    <BashCLRTestPreCommands><![CDATA[
+$(BashCLRTestPreCommands)
+export COMPlus_TieredPGO=1
+export COMPlus_TC_QuickJitForLoops=1
+]]></BashCLRTestPreCommands>
+  </PropertyGroup>
+  <ItemGroup>
+    <Compile Include="$(MSBuildProjectName).cs" />
+  </ItemGroup>
+</Project>