Skip to content

Commit

Permalink
JIT: Support delegate GDV guards in loop cloning (#75140)
Browse files Browse the repository at this point in the history 
Add support for cloning loops based on delegate GDV guards. Mark delegate address loads as invariant to allow VN and CSE of them.

Remove exceptions on indirs in guards after loop cloning, which otherwise prevents RBO from optimizing them away if the loop is inside an EH handler (e.g. foreach loop).
  • Loading branch information
@jakobbotsch
jakobbotsch authored Oct 20, 2022
1 parent 12f9f91 commit 17d613e
Show file tree
Hide file tree
Showing 6 changed files with 462 additions and 255 deletions.
8 changes: 5 additions & 3 deletions src/coreclr/jit/compiler.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7375,17 +7375,17 @@ class Compiler
Statement* stmt;
const unsigned loopNum;
const bool cloneForArrayBounds;
const bool cloneForTypeTests;
const bool cloneForGDVTests;
LoopCloneVisitorInfo(LoopCloneContext* context,
unsigned loopNum,
Statement* stmt,
bool cloneForArrayBounds,
bool cloneForTypeTests)
bool cloneForGDVTests)
: context(context)
, stmt(nullptr)
, loopNum(loopNum)
, cloneForArrayBounds(cloneForArrayBounds)
, cloneForTypeTests(cloneForTypeTests)
, cloneForGDVTests(cloneForGDVTests)
{
}
};
Expand All @@ -7399,6 +7399,8 @@ class Compiler
fgWalkResult optCanOptimizeByLoopCloning(GenTree* tree, LoopCloneVisitorInfo* info);
bool optObtainLoopCloningOpts(LoopCloneContext* context);
bool optIsLoopClonable(unsigned loopInd);
bool optCheckLoopCloningGDVTestProfitable(GenTreeOp* guard, LoopCloneVisitorInfo* info);
bool optIsHandleOrIndirOfHandle(GenTree* tree, GenTreeFlags handleType);

bool optLoopCloningEnabled();

Expand Down
76 changes: 2 additions & 74 deletions src/coreclr/jit/indirectcalltransformer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -450,11 +450,9 @@ class IndirectCallTransformer

class GuardedDevirtualizationTransformer final : public Transformer
{
unsigned m_targetLclNum;

public:
GuardedDevirtualizationTransformer(Compiler* compiler, BasicBlock* block, Statement* stmt)
: Transformer(compiler, block, stmt), m_targetLclNum(BAD_VAR_NUM), returnTemp(BAD_VAR_NUM)
: Transformer(compiler, block, stmt), returnTemp(BAD_VAR_NUM)
{
}

Expand Down Expand Up @@ -607,25 +605,10 @@ class IndirectCallTransformer
// which case the check will be moved into the success case of
// a previous GDV and thus may not execute when we hit the cold
// path.
// TODO-GDV: Consider duplicating the store at the end of the
// cold case for the previous GDV. Then we can reuse the target
// if the second check of a chained GDV fails.
bool reuseTarget = (origCall->gtCallMoreFlags & GTF_CALL_M_GUARDED_DEVIRT_CHAIN) == 0;
if (origCall->IsVirtualVtable())
{
GenTree* tarTree = compiler->fgExpandVirtualVtableCallTarget(origCall);

if (reuseTarget)
{
m_targetLclNum = compiler->lvaGrabTemp(false DEBUGARG("guarded devirt call target temp"));

GenTree* asgTree = compiler->gtNewTempAssign(m_targetLclNum, tarTree);
Statement* asgStmt = compiler->fgNewStmtFromTree(asgTree, stmt->GetDebugInfo());
compiler->fgInsertStmtAtEnd(checkBlock, asgStmt);

tarTree = compiler->gtNewLclvNode(m_targetLclNum, TYP_I_IMPL);
}

CORINFO_METHOD_HANDLE methHnd = guardedInfo->guardedMethodHandle;
CORINFO_CONST_LOOKUP lookup;
compiler->info.compCompHnd->getFunctionEntryPoint(methHnd, &lookup);
Expand All @@ -635,35 +618,12 @@ class IndirectCallTransformer
}
else
{
// Reusing the call target for delegates is more
// complicated. Essentially we need to do the
// transformation done in LowerDelegateInvoke by converting
// the call to CT_INDIRECT and reusing the target address.
// We will do that transformation in CreateElse, but here
// we need to stash the target.
CLANG_FORMAT_COMMENT_ANCHOR;
#ifdef TARGET_ARM
// Not impossible to support, but would additionally
// require us to load the wrapper delegate cell when
// expanding.
reuseTarget &= (origCall->gtCallMoreFlags & GTF_CALL_M_WRAPPER_DELEGATE_INV) == 0;
#endif

GenTree* offset =
compiler->gtNewIconNode((ssize_t)compiler->eeGetEEInfo()->offsetOfDelegateFirstTarget,
TYP_I_IMPL);
GenTree* tarTree = compiler->gtNewOperNode(GT_ADD, TYP_BYREF, thisTree, offset);
tarTree = compiler->gtNewIndir(TYP_I_IMPL, tarTree);

if (reuseTarget)
{
m_targetLclNum = compiler->lvaGrabTemp(false DEBUGARG("guarded devirt call target temp"));

GenTree* asgTree = compiler->gtNewTempAssign(m_targetLclNum, tarTree);
Statement* asgStmt = compiler->fgNewStmtFromTree(asgTree, stmt->GetDebugInfo());
compiler->fgInsertStmtAtEnd(checkBlock, asgStmt);
tarTree = compiler->gtNewLclvNode(m_targetLclNum, TYP_I_IMPL);
}
tarTree->gtFlags |= GTF_IND_INVARIANT;

CORINFO_METHOD_HANDLE methHnd = guardedInfo->guardedMethodHandle;
CORINFO_CONST_LOOKUP lookup;
Expand Down Expand Up @@ -970,38 +930,6 @@ class IndirectCallTransformer
newStmt->SetRootNode(assign);
}

if (m_targetLclNum != BAD_VAR_NUM)
{
if (call->IsVirtualVtable())
{
// We already loaded the target once for the check, so reuse it from the temp.
call->gtControlExpr = compiler->gtNewLclvNode(m_targetLclNum, TYP_I_IMPL);
call->SetExpandedEarly();
}
else if (call->IsDelegateInvoke())
{
// Target was saved into a temp during check. We expand the
// delegate call to a CT_INDIRECT call that uses the target
// directly, somewhat similarly to LowerDelegateInvoke.
call->gtCallType = CT_INDIRECT;
call->gtCallAddr = compiler->gtNewLclvNode(m_targetLclNum, TYP_I_IMPL);
call->gtCallCookie = nullptr;
call->gtCallMoreFlags &= ~GTF_CALL_M_DELEGATE_INV;

GenTree* thisOffset =
compiler->gtNewIconNode((ssize_t)compiler->eeGetEEInfo()->offsetOfDelegateInstance, TYP_I_IMPL);
CallArg* thisArg = call->gtArgs.GetThisArg();
GenTree* delegateObj = thisArg->GetNode();

assert(delegateObj->OperIsLocal());
GenTree* newThis =
compiler->gtNewOperNode(GT_ADD, TYP_BYREF, compiler->gtCloneExpr(delegateObj), thisOffset);
newThis = compiler->gtNewIndir(TYP_REF, newThis);

thisArg->SetEarlyNode(newThis);
}
}

compiler->fgInsertStmtAtEnd(elseBlock, newStmt);

// Set the original statement to a nop.
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/jit/jitconfigvalues.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ CONFIG_INTEGER(JitBreakOnBadCode, W("JitBreakOnBadCode"), 0)
CONFIG_INTEGER(JitBreakOnMinOpts, W("JITBreakOnMinOpts"), 0) // Halt if jit switches to MinOpts
CONFIG_INTEGER(JitBreakOnUnsafeCode, W("JitBreakOnUnsafeCode"), 0)
CONFIG_INTEGER(JitCloneLoops, W("JitCloneLoops"), 1) // If 0, don't clone. Otherwise clone loops for optimizations.
CONFIG_INTEGER(JitCloneLoopsWithTypeTests, W("JitCloneLoopsWithTypeTests"), 1) // If 0, don't clone loops based on
// invariant type tests
CONFIG_INTEGER(JitCloneLoopsWithGdvTests, W("JitCloneLoopsWithGdvTests"), 1) // If 0, don't clone loops based on
// invariant type/method address tests
CONFIG_INTEGER(JitDebugLogLoopCloning, W("JitDebugLogLoopCloning"), 0) // In debug builds log places where loop cloning
// optimizations are performed on the fast path.
CONFIG_INTEGER(JitDefaultFill, W("JitDefaultFill"), 0xdd) // In debug builds, initialize the memory allocated by the nra
Expand Down
Loading

0 comments on commit 17d613e

Please sign in to comment.