Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JIT: Evaluate GDV call args early #75634

Merged
merged 5 commits into from
Oct 18, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 47 additions & 15 deletions src/coreclr/jit/indirectcalltransformer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -540,28 +540,44 @@ class IndirectCallTransformer
checkBlock = currBlock;
checkBlock->bbJumpKind = BBJ_COND;

CallArg* thisArg = origCall->gtArgs.GetThisArg();
GenTree* thisTree = thisArg->GetNode();

// Create temp for this if the tree is costly.
if (thisTree->IsLocal())
// Find last arg with a side effect. All args with any effect
// before that will need to be spilled.
CallArg* lastSideEffArg = nullptr;
for (CallArg& arg : origCall->gtArgs.Args())
{
thisTree = compiler->gtCloneExpr(thisTree);
if ((arg.GetNode()->gtFlags & GTF_SIDE_EFFECT) != 0)
{
lastSideEffArg = &arg;
}
}
else

if (lastSideEffArg != nullptr)
{
const unsigned thisTempNum = compiler->lvaGrabTemp(true DEBUGARG("guarded devirt this temp"));
GenTree* asgTree = compiler->gtNewTempAssign(thisTempNum, thisTree);
Statement* asgStmt = compiler->fgNewStmtFromTree(asgTree, stmt->GetDebugInfo());
compiler->fgInsertStmtAtEnd(checkBlock, asgStmt);
for (CallArg& arg : origCall->gtArgs.Args())
{
GenTree* argNode = arg.GetNode();
if (((argNode->gtFlags & GTF_ALL_EFFECT) != 0) || compiler->gtHasLocalsWithAddrOp(argNode))
{
SpillArgToTempBeforeGuard(&arg);
}

thisTree = compiler->gtNewLclvNode(thisTempNum, TYP_REF);
if (&arg == lastSideEffArg)
{
break;
}
}
}

// Propagate the new this to the call. Must be a new expr as the call
// will live on in the else block and thisTree is used below.
thisArg->SetEarlyNode(compiler->gtNewLclvNode(thisTempNum, TYP_REF));
CallArg* thisArg = origCall->gtArgs.GetThisArg();
// We spill 'this' if it is complex, regardless of side effects. It
// is going to be used multiple times due to the guard.
if (!thisArg->GetNode()->IsLocal())
{
SpillArgToTempBeforeGuard(thisArg);
}

GenTree* thisTree = compiler->gtCloneExpr(thisArg->GetNode());

// Remember the current last statement. If we're doing a chained GDV, we'll clone/copy
// all the code in the check block up to and including this statement.
//
Expand Down Expand Up @@ -663,6 +679,22 @@ class IndirectCallTransformer
compiler->fgInsertStmtAtEnd(checkBlock, jmpStmt);
}

//------------------------------------------------------------------------
// SpillArgToTempBeforeGuard: spill an argument into a temp in the guard/check block.
//
// Parameters
// arg - The arg to create a temp and assignment for.
//
void SpillArgToTempBeforeGuard(CallArg* arg)
{
unsigned tmpNum = compiler->lvaGrabTemp(true DEBUGARG("guarded devirt arg temp"));
GenTree* asgTree = compiler->gtNewTempAssign(tmpNum, arg->GetNode());
Statement* asgStmt = compiler->fgNewStmtFromTree(asgTree, stmt->GetDebugInfo());
compiler->fgInsertStmtAtEnd(checkBlock, asgStmt);

arg->SetEarlyNode(compiler->gtNewLclvNode(tmpNum, genActualType(arg->GetNode())));
}

//------------------------------------------------------------------------
// FixupRetExpr: set up to repair return value placeholder from call
//
Expand Down
63 changes: 63 additions & 0 deletions src/tests/JIT/Regression/JitBlue/Runtime_75607/Runtime_75607.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
using System;
using System.Runtime.CompilerServices;
using System.Threading;

public class Program
{
private static int s_result;
public static int Main()
{
C c = new();
for (int i = 0; i < 100; i++)
{
Foo(c);
Thread.Sleep(15);
}

s_result = -1;
try
{
Foo(null);
Console.WriteLine("FAIL: No exception thrown");
return -2;
}
catch (NullReferenceException)
{
if (s_result == 100)
{
Console.WriteLine("PASS");
}
else
{
Console.WriteLine("FAIL: Result is {0}", s_result);
}

return s_result;
}
}

[MethodImpl(MethodImplOptions.NoInlining)]
private static void Foo(Base b)
{
b.Test(SideEffect(10));
}

[MethodImpl(MethodImplOptions.NoInlining)]
private static long SideEffect(long i)
{
s_result = 100;
return i;
}
}

public interface Base
{
void Test(long arg);
}

public class C : Base
{
public void Test(long arg)
{
}
}
Original file line number Diff line number Diff line change
@@ -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>