Skip to content

Commit 47e82c1

Browse files
authored
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.
1 parent a81fee2 commit 47e82c1

File tree

3 files changed

+105
-0
lines changed

3 files changed

+105
-0
lines changed

src/coreclr/jit/importer.cpp

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6808,6 +6808,38 @@ void Compiler::impImportAndPushBox(CORINFO_RESOLVED_TOKEN* pResolvedToken)
68086808

68096809
if (varTypeIsStruct(exprToBox))
68106810
{
6811+
// Workaround for GitHub issue 53549.
6812+
//
6813+
// If the struct being boxed is returned via hidden buffer and comes from an inline/gdv candidate,
6814+
// the IR we produce after importation is out of order:
6815+
//
6816+
// call (&(box-temp + 8), ....)
6817+
// box-temp = newobj
6818+
// ret-val from call (void)
6819+
// ... box-temp (on stack)
6820+
//
6821+
// For inline candidates this bad ordering gets fixed up during inlining, but for GDV candidates
6822+
// the GDV expansion is such that the newobj follows the call as in the above.
6823+
//
6824+
// This is nontrivial to fix in GDV, so in these (rare) cases we simply disable GDV.
6825+
//
6826+
if (exprToBox->OperIs(GT_RET_EXPR))
6827+
{
6828+
GenTreeCall* const call = exprToBox->AsRetExpr()->gtInlineCandidate->AsCall();
6829+
6830+
if (call->IsGuardedDevirtualizationCandidate() && call->HasRetBufArg())
6831+
{
6832+
JITDUMP("Disabling GDV for [%06u] because of in-box struct return\n");
6833+
call->ClearGuardedDevirtualizationCandidate();
6834+
if (call->IsVirtualStub())
6835+
{
6836+
JITDUMP("Restoring stub addr %p from guarded devirt candidate info\n",
6837+
dspPtr(call->gtGuardedDevirtualizationCandidateInfo->stubAddr));
6838+
call->gtStubCallStubAddr = call->gtGuardedDevirtualizationCandidateInfo->stubAddr;
6839+
}
6840+
}
6841+
}
6842+
68116843
assert(info.compCompHnd->getClassSize(pResolvedToken->hClass) == info.compCompHnd->getClassSize(operCls));
68126844
op1 = impAssignStructPtr(op1, exprToBox, operCls, (unsigned)CHECK_SPILL_ALL);
68136845
}
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
// Licensed to the .NET Foundation under one or more agreements.
2+
// The .NET Foundation licenses this file to you under the MIT license.
3+
4+
using System;
5+
using System.Runtime.CompilerServices;
6+
using System.Threading;
7+
8+
interface I
9+
{
10+
public Decimal F();
11+
}
12+
13+
class Runtime_53549 : I
14+
{
15+
Decimal z;
16+
17+
public Decimal F() => z;
18+
19+
public static bool G(object o)
20+
{
21+
return ((decimal) o).Equals(100M);
22+
}
23+
24+
// This method will have bad codegen if
25+
// we allow GDV on i.F().
26+
//
27+
[MethodImpl(MethodImplOptions.NoInlining)]
28+
public static int H(I i)
29+
{
30+
return G(i.F()) ? 100 : -1;
31+
}
32+
33+
public static int Main()
34+
{
35+
Runtime_53549 x = new Runtime_53549();
36+
x.z = 100M;
37+
38+
for (int i = 0; i < 100; i++)
39+
{
40+
_ = H(x);
41+
Thread.Sleep(15);
42+
}
43+
44+
return H(x);
45+
}
46+
}
47+
48+
49+
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
<Project Sdk="Microsoft.NET.Sdk">
2+
<PropertyGroup>
3+
<OutputType>Exe</OutputType>
4+
</PropertyGroup>
5+
<PropertyGroup>
6+
<DebugType>None</DebugType>
7+
<Optimize>True</Optimize>
8+
</PropertyGroup>
9+
<PropertyGroup>
10+
<CLRTestBatchPreCommands><![CDATA[
11+
$(CLRTestBatchPreCommands)
12+
set COMPlus_TieredPGO=1
13+
set COMPlus_TC_QuickJitForLoops=1
14+
]]></CLRTestBatchPreCommands>
15+
<BashCLRTestPreCommands><![CDATA[
16+
$(BashCLRTestPreCommands)
17+
export COMPlus_TieredPGO=1
18+
export COMPlus_TC_QuickJitForLoops=1
19+
]]></BashCLRTestPreCommands>
20+
</PropertyGroup>
21+
<ItemGroup>
22+
<Compile Include="$(MSBuildProjectName).cs" />
23+
</ItemGroup>
24+
</Project>

0 commit comments

Comments
 (0)