Skip to content

Runtime lookup clean up, enable for helper-based tail calls #83430

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.

Sign up for GitHub

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

Merged
EgorBo merged 4 commits into from
Mar 17, 2023
Merged

Runtime lookup clean up, enable for helper-based tail calls #83430

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
459edc7
Runtime lookup clean up
EgorBo Mar 14, 2023
06ca378
Remove comment
EgorBo Mar 15, 2023
0e95e4b
Remove testForFixup
EgorBo Mar 15, 2023
5745393
Update prestub.cpp
EgorBo Mar 15, 2023
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
21 changes: 0 additions & 21 deletions src/coreclr/jit/compiler.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1029,27 +1029,6 @@ inline GenTreeCall* Compiler::gtNewHelperCallNode(
return result;
}

//------------------------------------------------------------------------------
// gtNewRuntimeLookupHelperCallNode : Helper to create a runtime lookup call helper node.
//
//
// Arguments:
// helper - Call helper
// type - Type of the node
// args - Call args
//
// Return Value:
// New CT_HELPER node

inline GenTreeCall* Compiler::gtNewRuntimeLookupHelperCallNode(CORINFO_RUNTIME_LOOKUP* pRuntimeLookup,
GenTree* ctxTree,
void* compileTimeHandle)
{
GenTree* argNode = gtNewIconEmbHndNode(pRuntimeLookup->signature, nullptr, GTF_ICON_GLOBAL_PTR, compileTimeHandle);
GenTreeCall* call = gtNewHelperCallNode(pRuntimeLookup->helper, TYP_I_IMPL, ctxTree, argNode);
return call;
}

//------------------------------------------------------------------------
// gtNewAllocObjNode: A little helper to create an object allocation node.
//
Expand Down
3 changes: 3 additions & 0 deletions src/coreclr/jit/fgbasic.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4643,6 +4643,9 @@ BasicBlock* Compiler::fgSplitBlockBeforeTree(
BasicBlockFlags originalFlags = block->bbFlags;
BasicBlock* prevBb = block;

// We use fgSplitBlockAfterStatement() API here to split the block, however, we want to split
// it *Before* rather than *After* so if the current statement is the first in the
// current block - invoke fgSplitBlockAtBeginning
if (stmt == block->firstStmt())
{
block = fgSplitBlockAtBeginning(prevBb);
Expand Down
79 changes: 11 additions & 68 deletions src/coreclr/jit/importer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1782,36 +1782,22 @@ GenTree* Compiler::impRuntimeLookupToTree(CORINFO_RESOLVED_TOKEN* pResolvedToken

if (pRuntimeLookup->testForNull)
{
// Import just a helper call and mark it for late expansion in fgExpandRuntimeLookups phase
assert(pRuntimeLookup->indirections != 0);
GenTreeCall* helperCall = gtNewRuntimeLookupHelperCallNode(pRuntimeLookup, ctxTree, compileTimeHandle);

// Call the helper
// - Setup argNode with the pointer to the signature returned by the lookup
GenTree* argNode =
gtNewIconEmbHndNode(pRuntimeLookup->signature, nullptr, GTF_ICON_GLOBAL_PTR, compileTimeHandle);
GenTreeCall* helperCall = gtNewHelperCallNode(pRuntimeLookup->helper, TYP_I_IMPL, ctxTree, argNode);

// No need to perform CSE/hoisting for signature node - it is expected to end up in a rarely-taken block after
// "Expand runtime lookups" phase.
argNode->gtFlags |= GTF_DONT_CSE;

// Leave a note that this method has runtime lookups we might want to expand (nullchecks, size checks) later.
// We can also consider marking current block as a runtime lookup holder to improve TP for Tier0
impInlineRoot()->setMethodHasExpRuntimeLookup();
helperCall->SetExpRuntimeLookup();
if (!impInlineRoot()->GetSignatureToLookupInfoMap()->Lookup(pRuntimeLookup->signature))
{
JITDUMP("Registering %p in SignatureToLookupInfoMap\n", pRuntimeLookup->signature)
impInlineRoot()->GetSignatureToLookupInfoMap()->Set(pRuntimeLookup->signature, *pRuntimeLookup);
}
// Spilling it to a temp improves CQ (mainly in Tier0)
unsigned callLclNum = lvaGrabTemp(true DEBUGARG("spilling helperCall"));
impAssignTempGen(callLclNum, helperCall);
return gtNewLclvNode(callLclNum, helperCall->TypeGet());
}

// Size-check is not expected without testForNull
assert(pRuntimeLookup->sizeOffset == CORINFO_NO_SIZE_CHECK);

// Slot pointer
GenTree* slotPtrTree = ctxTree;
GenTree* indOffTree = nullptr;
GenTree* lastIndOfTree = nullptr;
GenTree* slotPtrTree = ctxTree;
GenTree* indOffTree = nullptr;

// Applied repeated indirections
for (WORD i = 0; i < pRuntimeLookup->indirections; i++)
Expand All @@ -1822,18 +1808,10 @@ GenTree* Compiler::impRuntimeLookupToTree(CORINFO_RESOLVED_TOKEN* pResolvedToken
nullptr DEBUGARG("impRuntimeLookup indirectOffset"));
}

// The last indirection could be subject to a size check (dynamic dictionary expansion)
bool isLastIndirectionWithSizeCheck =
((i == pRuntimeLookup->indirections - 1) && (pRuntimeLookup->sizeOffset != CORINFO_NO_SIZE_CHECK));

if (i != 0)
{
slotPtrTree = gtNewOperNode(GT_IND, TYP_I_IMPL, slotPtrTree);
slotPtrTree->gtFlags |= GTF_IND_NONFAULTING;
if (!isLastIndirectionWithSizeCheck)
{
slotPtrTree->gtFlags |= GTF_IND_INVARIANT;
}
slotPtrTree->gtFlags |= (GTF_IND_NONFAULTING | GTF_IND_INVARIANT);
}

if ((i == 1 && pRuntimeLookup->indirectFirstOffset) || (i == 2 && pRuntimeLookup->indirectSecondOffset))
Expand All @@ -1843,12 +1821,6 @@ GenTree* Compiler::impRuntimeLookupToTree(CORINFO_RESOLVED_TOKEN* pResolvedToken

if (pRuntimeLookup->offsets[i] != 0)
{
if (isLastIndirectionWithSizeCheck)
{
lastIndOfTree = impCloneExpr(slotPtrTree, &slotPtrTree, NO_CLASS_HANDLE, CHECK_SPILL_ALL,
nullptr DEBUGARG("impRuntimeLookup indirectOffset"));
}

slotPtrTree =
gtNewOperNode(GT_ADD, TYP_I_IMPL, slotPtrTree, gtNewIconNode(pRuntimeLookup->offsets[i], TYP_I_IMPL));
}
Expand All @@ -1865,37 +1837,8 @@ GenTree* Compiler::impRuntimeLookupToTree(CORINFO_RESOLVED_TOKEN* pResolvedToken
slotPtrTree = gtNewOperNode(GT_IND, TYP_I_IMPL, slotPtrTree);
slotPtrTree->gtFlags |= GTF_IND_NONFAULTING;

if (!pRuntimeLookup->testForFixup)
{
return slotPtrTree;
}

impSpillSideEffects(true, CHECK_SPILL_ALL DEBUGARG("bubbling QMark0"));

unsigned slotLclNum = lvaGrabTemp(true DEBUGARG("impRuntimeLookup test"));
impAssignTempGen(slotLclNum, slotPtrTree, NO_CLASS_HANDLE, CHECK_SPILL_ALL, nullptr, impCurStmtDI);

GenTree* slot = gtNewLclvNode(slotLclNum, TYP_I_IMPL);
// downcast the pointer to a TYP_INT on 64-bit targets
slot = impImplicitIorI4Cast(slot, TYP_INT);
// Use a GT_AND to check for the lowest bit and indirect if it is set
GenTree* test = gtNewOperNode(GT_AND, TYP_INT, slot, gtNewIconNode(1));
GenTree* relop = gtNewOperNode(GT_EQ, TYP_INT, test, gtNewIconNode(0));

// slot = GT_IND(slot - 1)
slot = gtNewLclvNode(slotLclNum, TYP_I_IMPL);
GenTree* add = gtNewOperNode(GT_ADD, TYP_I_IMPL, slot, gtNewIconNode(-1, TYP_I_IMPL));
GenTree* indir = gtNewOperNode(GT_IND, TYP_I_IMPL, add);
indir->gtFlags |= GTF_IND_NONFAULTING;
indir->gtFlags |= GTF_IND_INVARIANT;

slot = gtNewLclvNode(slotLclNum, TYP_I_IMPL);
GenTree* asg = gtNewAssignNode(slot, indir);
GenTreeColon* colon = new (this, GT_COLON) GenTreeColon(TYP_VOID, gtNewNothingNode(), asg);
GenTreeQmark* qmark = gtNewQmarkNode(TYP_VOID, relop, colon);
impAppendTree(qmark, CHECK_SPILL_NONE, impCurStmtDI);

return gtNewLclvNode(slotLclNum, TYP_I_IMPL);
assert(!pRuntimeLookup->testForFixup);
Copy link
Member

@jkotas jkotas Mar 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be deleted from JIT/EE interface.

return slotPtrTree;
}

struct RecursiveGuard
Expand Down
6 changes: 0 additions & 6 deletions src/coreclr/jit/morph.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7049,12 +7049,6 @@ GenTree* Compiler::getRuntimeLookupTree(CORINFO_RESOLVED_TOKEN* pResolvedToken,
if ((pRuntimeLookup->indirections == CORINFO_USEHELPER) || (pRuntimeLookup->indirections == CORINFO_USENULL) ||
pRuntimeLookup->testForNull || pRuntimeLookup->testForFixup)
{
// If the first condition is true, runtime lookup tree is available only via the run-time helper function.
// TODO-CQ If the second or third condition is true, we are always using the slow path since we can't
// introduce control flow at this point. See impRuntimeLookupToTree for the logic to avoid calling the helper.
// The long-term solution is to introduce a new node representing a runtime lookup, create instances
// of that node both in the importer and here, and expand the node in lower (introducing control flow if
// necessary).
return gtNewRuntimeLookupHelperCallNode(pRuntimeLookup,
getRuntimeContextTree(pLookup->lookupKind.runtimeLookupKind),
compileTimeHandle);
Expand Down
36 changes: 36 additions & 0 deletions src/coreclr/jit/runtimelookup.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,42 @@ static BasicBlock* CreateBlockFromTree(Compiler* comp,
return newBlock;
}

//------------------------------------------------------------------------------
// gtNewRuntimeLookupHelperCallNode : Helper to create a runtime lookup call helper node.
//
// Arguments:
// helper - Call helper
// type - Type of the node
// args - Call args
//
// Return Value:
// New CT_HELPER node
//
GenTreeCall* Compiler::gtNewRuntimeLookupHelperCallNode(CORINFO_RUNTIME_LOOKUP* pRuntimeLookup,
GenTree* ctxTree,
void* compileTimeHandle)
{
// Call the helper
// - Setup argNode with the pointer to the signature returned by the lookup
GenTree* argNode = gtNewIconEmbHndNode(pRuntimeLookup->signature, nullptr, GTF_ICON_GLOBAL_PTR, compileTimeHandle);
GenTreeCall* helperCall = gtNewHelperCallNode(pRuntimeLookup->helper, TYP_I_IMPL, ctxTree, argNode);

// No need to perform CSE/hoisting for signature node - it is expected to end up in a rarely-taken block after
// "Expand runtime lookups" phase.
argNode->gtFlags |= GTF_DONT_CSE;

// Leave a note that this method has runtime lookups we might want to expand (nullchecks, size checks) later.
// We can also consider marking current block as a runtime lookup holder to improve TP for Tier0
impInlineRoot()->setMethodHasExpRuntimeLookup();
helperCall->SetExpRuntimeLookup();
if (!impInlineRoot()->GetSignatureToLookupInfoMap()->Lookup(pRuntimeLookup->signature))
{
JITDUMP("Registering %p in SignatureToLookupInfoMap\n", pRuntimeLookup->signature)
impInlineRoot()->GetSignatureToLookupInfoMap()->Set(pRuntimeLookup->signature, *pRuntimeLookup);
}
return helperCall;
}

//------------------------------------------------------------------------------
// fgExpandRuntimeLookups : partially expand runtime lookups helper calls
// to add a nullcheck [+ size check] and a fast path
Expand Down