Skip to content

Commit

Permalink
JIT: block general cloning of candidate calls (dotnet/coreclr#21418)
Browse files Browse the repository at this point in the history 
Follow-up from dotnet/coreclr#21270 and dotnet/coreclr#21414.

Block general cloning from inadvertently cloning a candidate call.

Add a separate path for cloning calls that are inline and guarded
devirtualization candidates for use by guarded devirtualization.
Callers need to take extra steps after cloning one of these calls
to properly fix everything up.

Also fix up some issues in the large comment for the fat calli
transformation.


Commit migrated from dotnet/coreclr@08be98c
  • Loading branch information
@AndyAyersMS
AndyAyersMS authored Dec 7, 2018
1 parent 44b8f2a commit 44f7fb3
Show file tree
Hide file tree
Showing 3 changed files with 159 additions and 87 deletions.
11 changes: 10 additions & 1 deletion src/coreclr/src/jit/compiler.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2482,11 +2482,20 @@ class Compiler

// Create a copy of `tree`, optionally adding specifed flags, and optionally mapping uses of local
// `varNum` to int constants with value `varVal`.
GenTree* gtCloneExpr(GenTree* tree, unsigned addFlags = 0, unsigned varNum = (unsigned)-1, int varVal = 0)
GenTree* gtCloneExpr(GenTree* tree, unsigned addFlags = 0, unsigned varNum = BAD_VAR_NUM, int varVal = 0)
{
return gtCloneExpr(tree, addFlags, varNum, varVal, varNum, varVal);
}

// Internal helper for cloning a call
GenTreeCall* gtCloneExprCallHelper(GenTreeCall* call,
unsigned addFlags = 0,
unsigned deepVarNum = BAD_VAR_NUM,
int deepVarVal = 0);

// Create copy of an inline or guarded devirtualization candidate tree.
GenTreeCall* gtCloneCandidateCall(GenTreeCall* call);

GenTree* gtReplaceTree(GenTree* stmt, GenTree* tree, GenTree* replacementTree);

void gtUpdateSideEffects(GenTree* stmt, GenTree* tree);
Expand Down
205 changes: 131 additions & 74 deletions src/coreclr/src/jit/gentree.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7273,82 +7273,14 @@ GenTree* Compiler::gtCloneExpr(

case GT_CALL:

copy = new (this, GT_CALL) GenTreeCall(tree->TypeGet());

copy->gtCall.gtCallObjp = tree->gtCall.gtCallObjp
? gtCloneExpr(tree->gtCall.gtCallObjp, addFlags, deepVarNum, deepVarVal)
: nullptr;
copy->gtCall.gtCallArgs =
tree->gtCall.gtCallArgs
? gtCloneExpr(tree->gtCall.gtCallArgs, addFlags, deepVarNum, deepVarVal)->AsArgList()
: nullptr;
copy->gtCall.gtCallMoreFlags = tree->gtCall.gtCallMoreFlags;
copy->gtCall.gtCallLateArgs =
tree->gtCall.gtCallLateArgs
? gtCloneExpr(tree->gtCall.gtCallLateArgs, addFlags, deepVarNum, deepVarVal)->AsArgList()
: nullptr;

#if !FEATURE_FIXED_OUT_ARGS
copy->gtCall.regArgList = tree->gtCall.regArgList;
copy->gtCall.regArgListCount = tree->gtCall.regArgListCount;
#endif

// The call sig comes from the EE and doesn't change throughout the compilation process, meaning
// we only really need one physical copy of it. Therefore a shallow pointer copy will suffice.
// (Note that this still holds even if the tree we are cloning was created by an inlinee compiler,
// because the inlinee still uses the inliner's memory allocator anyway.)
copy->gtCall.callSig = tree->gtCall.callSig;

copy->gtCall.gtCallType = tree->gtCall.gtCallType;
copy->gtCall.gtReturnType = tree->gtCall.gtReturnType;
copy->gtCall.gtControlExpr = tree->gtCall.gtControlExpr;

/* Copy the union */
if (tree->gtCall.gtCallType == CT_INDIRECT)
{
copy->gtCall.gtCallCookie =
tree->gtCall.gtCallCookie ? gtCloneExpr(tree->gtCall.gtCallCookie, addFlags, deepVarNum, deepVarVal)
: nullptr;
copy->gtCall.gtCallAddr = tree->gtCall.gtCallAddr
? gtCloneExpr(tree->gtCall.gtCallAddr, addFlags, deepVarNum, deepVarVal)
: nullptr;
}
else if (tree->gtCall.IsVirtualStub())
{
copy->gtCall.gtCallMethHnd = tree->gtCall.gtCallMethHnd;
copy->gtCall.gtStubCallStubAddr = tree->gtCall.gtStubCallStubAddr;
}
else
// We can't safely clone calls that have GT_RET_EXPRs via gtCloneExpr.
// You must use gtCloneCandidateCall for these calls (and then do appropriate other fixup)
if (tree->gtCall.IsInlineCandidate() || tree->gtCall.IsGuardedDevirtualizationCandidate())
{
copy->gtCall.gtCallMethHnd = tree->gtCall.gtCallMethHnd;
copy->gtCall.gtInlineCandidateInfo = tree->gtCall.gtInlineCandidateInfo;
NO_WAY("Cloning of calls with associated GT_RET_EXPR nodes is not supported");
}

if (tree->gtCall.fgArgInfo)
{
// Create and initialize the fgArgInfo for our copy of the call tree
copy->gtCall.fgArgInfo = new (this, CMK_Unknown) fgArgInfo(copy->AsCall(), tree->AsCall());
}
else
{
copy->gtCall.fgArgInfo = nullptr;
}
copy->gtCall.gtRetClsHnd = tree->gtCall.gtRetClsHnd;

#if FEATURE_MULTIREG_RET
copy->gtCall.gtReturnTypeDesc = tree->gtCall.gtReturnTypeDesc;
#endif

#ifdef FEATURE_READYTORUN_COMPILER
copy->gtCall.setEntryPoint(tree->gtCall.gtEntryPoint);
#endif

#if defined(DEBUG) || defined(INLINE_DATA)
copy->gtCall.gtInlineObservation = tree->gtCall.gtInlineObservation;
copy->gtCall.gtRawILOffset = tree->gtCall.gtRawILOffset;
#endif

copy->AsCall()->CopyOtherRegFlags(tree->AsCall());
copy = gtCloneExprCallHelper(tree->AsCall(), addFlags, deepVarNum, deepVarVal);
break;

case GT_FIELD:
Expand Down Expand Up @@ -7486,6 +7418,131 @@ GenTree* Compiler::gtCloneExpr(
return copy;
}

//------------------------------------------------------------------------
// gtCloneExprCallHelper: clone a call tree
//
// Notes:
// Do not invoke this method directly, instead call either gtCloneExpr
// or gtCloneCandidateCall, as appropriate.
//
// Arguments:
// tree - the call to clone
// addFlags - GTF_* flags to add to the copied tree nodes
// deepVarNum - lclNum to replace uses of beyond the root, or BAD_VAR_NUM for no replacement
// deepVarVal - If replacing beyond root, replace `deepVarNum` with IntCns `deepVarVal`
//
// Returns:
// Cloned copy of call and all subtrees.

GenTreeCall* Compiler::gtCloneExprCallHelper(GenTreeCall* tree, unsigned addFlags, unsigned deepVarNum, int deepVarVal)
{
GenTreeCall* copy = new (this, GT_CALL) GenTreeCall(tree->TypeGet());

copy->gtCallObjp = tree->gtCallObjp ? gtCloneExpr(tree->gtCallObjp, addFlags, deepVarNum, deepVarVal) : nullptr;
copy->gtCallArgs =
tree->gtCallArgs ? gtCloneExpr(tree->gtCallArgs, addFlags, deepVarNum, deepVarVal)->AsArgList() : nullptr;
copy->gtCallMoreFlags = tree->gtCallMoreFlags;
copy->gtCallLateArgs = tree->gtCallLateArgs
? gtCloneExpr(tree->gtCallLateArgs, addFlags, deepVarNum, deepVarVal)->AsArgList()
: nullptr;

#if !FEATURE_FIXED_OUT_ARGS
copy->regArgList = tree->regArgList;
copy->regArgListCount = tree->regArgListCount;
#endif

// The call sig comes from the EE and doesn't change throughout the compilation process, meaning
// we only really need one physical copy of it. Therefore a shallow pointer copy will suffice.
// (Note that this still holds even if the tree we are cloning was created by an inlinee compiler,
// because the inlinee still uses the inliner's memory allocator anyway.)
copy->callSig = tree->callSig;

copy->gtCallType = tree->gtCallType;
copy->gtReturnType = tree->gtReturnType;
copy->gtControlExpr = tree->gtControlExpr;

/* Copy the union */
if (tree->gtCallType == CT_INDIRECT)
{
copy->gtCallCookie =
tree->gtCallCookie ? gtCloneExpr(tree->gtCallCookie, addFlags, deepVarNum, deepVarVal) : nullptr;
copy->gtCallAddr = tree->gtCallAddr ? gtCloneExpr(tree->gtCallAddr, addFlags, deepVarNum, deepVarVal) : nullptr;
}
else if (tree->IsVirtualStub())
{
copy->gtCallMethHnd = tree->gtCallMethHnd;
copy->gtStubCallStubAddr = tree->gtStubCallStubAddr;
}
else
{
copy->gtCallMethHnd = tree->gtCallMethHnd;
copy->gtInlineCandidateInfo = nullptr;
}

if (tree->fgArgInfo)
{
// Create and initialize the fgArgInfo for our copy of the call tree
copy->fgArgInfo = new (this, CMK_Unknown) fgArgInfo(copy, tree);
}
else
{
copy->fgArgInfo = nullptr;
}

copy->gtRetClsHnd = tree->gtRetClsHnd;

#if FEATURE_MULTIREG_RET
copy->gtReturnTypeDesc = tree->gtReturnTypeDesc;
#endif

#ifdef FEATURE_READYTORUN_COMPILER
copy->setEntryPoint(tree->gtEntryPoint);
#endif

#if defined(DEBUG) || defined(INLINE_DATA)
copy->gtInlineObservation = tree->gtInlineObservation;
copy->gtRawILOffset = tree->gtCall.gtRawILOffset;
#endif

copy->CopyOtherRegFlags(tree);

return copy;
}

//------------------------------------------------------------------------
// gtCloneCandidateCall: clone a call that is an inline or guarded
// devirtualization candidate (~ any call that can have a GT_RET_EXPR)
//
// Notes:
// If the call really is a candidate, the caller must take additional steps
// after cloning to re-establish candidate info and the relationship between
// the candidate and any associated GT_RET_EXPR.
//
// Arguments:
// call - the call to clone
//
// Returns:
// Cloned copy of call and all subtrees.

GenTreeCall* Compiler::gtCloneCandidateCall(GenTreeCall* call)
{
assert(call->IsInlineCandidate() || call->IsGuardedDevirtualizationCandidate());

GenTreeCall* result = gtCloneExprCallHelper(call);

// There is some common post-processing in gtCloneExpr that we reproduce
// here, for the fields that make sense for candidate calls.
result->gtFlags |= call->gtFlags;

#if defined(DEBUG)
result->gtDebugFlags |= (call->gtDebugFlags & ~GTF_DEBUG_NODE_MASK);
#endif

result->CopyReg(call);

return result;
}

//------------------------------------------------------------------------
// gtReplaceTree: Replace a tree with a new tree.
//
Expand Down Expand Up @@ -12079,7 +12136,7 @@ GenTree* Compiler::gtFoldTypeCompare(GenTree* tree)
// Compare the two method tables
GenTree* const compare = gtCreateHandleCompare(oper, objMT, knownMT, typeCheckInliningResult);

// Drop any any now irrelevant flags
// Drop any now irrelevant flags
compare->gtFlags |= tree->gtFlags & (GTF_RELOP_JMP_USED | GTF_RELOP_QMARK | GTF_DONT_CSE);

// And we're done
Expand Down
30 changes: 18 additions & 12 deletions src/coreclr/src/jit/indirectcalltransformer.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,26 @@
#pragma hdrstop
#endif

// The IndirectCallTransformer transforms indirect calls that involve fat pointers
// or guarded devirtualization candidates.
// The IndirectCallTransformer transforms indirect calls that involve fat function
// pointers or guarded devirtualization candidates. These transformations introduce
// control flow and so can't easily be done in the importer.
//
// A fat function pointer is pointer with the second least significant bit set,
// if the bit is set, the pointer (after clearing the bit) actually points to
// a tuple <method pointer, instantiation argument pointer> where
// A fat function pointer is a pointer with the second least significant bit
// (aka FAT_POINTER_MASK) set. If the bit is set, the pointer (after clearing the bit)
// actually points to a tuple <method pointer, instantiation argument pointer> where
// instantiationArgument is a hidden first argument required by method pointer.
//
// Fat pointers are used in CoreRT as a replacement for instantiating stubs,
// because CoreRT can't generate stubs in runtime.
//
// Jit is responsible for the checking the bit, do the regular call if it is not set
// or load hidden argument, fix the pointer and make a call with the fixed pointer and
// the instantiation argument.
// The JIT is responsible for emitting code to check the bit at runtime, branching
// to one of two call sites.
//
// When the bit is not set, the code should execute the original indirect call.
//
// When the bit is set, the code should mask off the bit, use the resulting pointer
// to load the real target address and the extra argument, and then call indirect
// via the target, passing the extra argument.
//
// before:
// current block
Expand All @@ -40,15 +46,15 @@
// } BBJ_NONE check block
// check block
// {
// jump to else if function ptr has GTF_CALL_M_FAT_POINTER_CHECK set.
// jump to else if function ptr has the FAT_POINTER_MASK bit set.
// } BBJ_COND then block, else block
// then block
// {
// original statement
// } BBJ_ALWAYS remainder block
// else block
// {
// unset GTF_CALL_M_FAT_POINTER_CHECK
// clear FAT_POINTER_MASK bit
// load actual function pointer
// load instantiation argument
// create newArgList = (instantiation argument, original argList)
Expand Down Expand Up @@ -663,8 +669,8 @@ class IndirectCallTransformer
GenTreeStmt* assignStmt = compiler->gtNewStmt(assign);
compiler->fgInsertStmtAtEnd(thenBlock, assignStmt);

// Clone call
GenTreeCall* call = compiler->gtCloneExpr(origCall)->AsCall();
// Clone call. Note we must use the special candidate helper.
GenTreeCall* call = compiler->gtCloneCandidateCall(origCall);
call->gtCallObjp = compiler->gtNewLclvNode(thisTemp, TYP_REF);
call->SetIsGuarded();

Expand Down

0 comments on commit 44f7fb3

Please sign in to comment.