Skip to content

Commit

Permalink
JIT: refactor fields of GenTreeCall (#63402)
Browse files Browse the repository at this point in the history
Move `gtStubCallStubAddr` out of the union it shares with class probe,
inline, and guarded devirt information, so that we no longer need to
save and restore the stub address when we want to probe, guardedly
devirtualize, or inline calls.

Delete all the code that did saving and restoring, and the field in the
inline info struct that held the saved copy.
  • Loading branch information
AndyAyersMS authored Jan 5, 2022
1 parent ec5471c commit b28c38d
Show file tree
Hide file tree
Showing 6 changed files with 14 additions and 118 deletions.
60 changes: 0 additions & 60 deletions src/coreclr/jit/fgprofile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1566,38 +1566,10 @@ class ClassProbeInserter
JITDUMP("Modified call is now\n");
DISPTREE(call);

// Restore the stub address on the call
//
call->gtStubCallStubAddr = call->gtClassProfileCandidateInfo->stubAddr;

m_instrCount++;
}
};

//------------------------------------------------------------------------
// SuppressProbesFunctor: functor that resets IR back to the state
// it had if there were no class probes.
//
class SuppressProbesFunctor
{
private:
unsigned& m_cleanupCount;

public:
SuppressProbesFunctor(unsigned& cleanupCount) : m_cleanupCount(cleanupCount)
{
}

void operator()(Compiler* compiler, GenTreeCall* call)
{
// Restore the stub address on the call
//
call->gtStubCallStubAddr = call->gtClassProfileCandidateInfo->stubAddr;

m_cleanupCount++;
}
};

//------------------------------------------------------------------------
// ClassProbeInstrumentor: instrumentor that adds a class probe to each
// virtual call in the basic block
Expand All @@ -1615,7 +1587,6 @@ class ClassProbeInstrumentor : public Instrumentor
void Prepare(bool isPreImport) override;
void BuildSchemaElements(BasicBlock* block, Schema& schema) override;
void Instrument(BasicBlock* block, Schema& schema, uint8_t* profileMemory) override;
void SuppressProbes() override;
};

//------------------------------------------------------------------------
Expand Down Expand Up @@ -1703,37 +1674,6 @@ void ClassProbeInstrumentor::Instrument(BasicBlock* block, Schema& schema, uint8
}
}

//------------------------------------------------------------------------
// ClassProbeInstrumentor::SuppressProbes: clean up if we're not instrumenting
//
// Notes:
// Currently we're hijacking the gtCallStubAddr of the call node to hold
// a pointer to the profile candidate info.
//
// We must undo this, if not instrumenting.
//
void ClassProbeInstrumentor::SuppressProbes()
{
unsigned cleanupCount = 0;
SuppressProbesFunctor suppressProbes(cleanupCount);
ClassProbeVisitor<SuppressProbesFunctor> visitor(m_comp, suppressProbes);

for (BasicBlock* const block : m_comp->Blocks())
{
if ((block->bbFlags & BBF_HAS_CLASS_PROFILE) == 0)
{
continue;
}

for (Statement* const stmt : block->Statements())
{
visitor.WalkTree(stmt->GetRootNodePointer(), nullptr);
}
}

assert(cleanupCount == m_comp->info.compClassProbeCount);
}

//------------------------------------------------------------------------
// fgPrepareToInstrumentMethod: prepare for instrumentation
//
Expand Down
16 changes: 6 additions & 10 deletions src/coreclr/jit/gentree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7784,9 +7784,9 @@ GenTreeCall* Compiler::gtCloneExprCallHelper(GenTreeCall* tree,
// a shallow copy suffices.
copy->tailCallInfo = tree->tailCallInfo;

copy->gtCallType = tree->gtCallType;
copy->gtReturnType = tree->gtReturnType;
copy->gtControlExpr = gtCloneExpr(tree->gtControlExpr, addFlags, deepVarNum, deepVarVal);
copy->gtRetClsHnd = tree->gtRetClsHnd;
copy->gtControlExpr = gtCloneExpr(tree->gtControlExpr, addFlags, deepVarNum, deepVarVal);
copy->gtStubCallStubAddr = tree->gtStubCallStubAddr;

/* Copy the union */
if (tree->gtCallType == CT_INDIRECT)
Expand All @@ -7795,17 +7795,15 @@ GenTreeCall* Compiler::gtCloneExprCallHelper(GenTreeCall* tree,
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;
}

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

if (tree->fgArgInfo)
{
// Create and initialize the fgArgInfo for our copy of the call tree
Expand All @@ -7816,8 +7814,6 @@ GenTreeCall* Compiler::gtCloneExprCallHelper(GenTreeCall* tree,
copy->fgArgInfo = nullptr;
}

copy->gtRetClsHnd = tree->gtRetClsHnd;

#if FEATURE_MULTIREG_RET
copy->gtReturnTypeDesc = tree->gtReturnTypeDesc;
#endif
Expand Down
11 changes: 6 additions & 5 deletions src/coreclr/jit/gentree.h
Original file line number Diff line number Diff line change
Expand Up @@ -4729,10 +4729,11 @@ struct GenTreeCall final : public GenTree

void ResetArgInfo();

GenTreeCallFlags gtCallMoreFlags; // in addition to gtFlags
gtCallTypes gtCallType : 3; // value from the gtCallTypes enumeration
var_types gtReturnType : 5; // exact return type
CORINFO_CLASS_HANDLE gtRetClsHnd; // The return type handle of the call if it is a struct; always available
GenTreeCallFlags gtCallMoreFlags; // in addition to gtFlags
gtCallTypes gtCallType : 3; // value from the gtCallTypes enumeration
var_types gtReturnType : 5; // exact return type
CORINFO_CLASS_HANDLE gtRetClsHnd; // The return type handle of the call if it is a struct; always available
void* gtStubCallStubAddr; // GTF_CALL_VIRT_STUB - these are never inlined

union {
// only used for CALLI unmanaged calls (CT_INDIRECT)
Expand All @@ -4741,7 +4742,7 @@ struct GenTreeCall final : public GenTree
InlineCandidateInfo* gtInlineCandidateInfo;
GuardedDevirtualizationCandidateInfo* gtGuardedDevirtualizationCandidateInfo;
ClassProfileCandidateInfo* gtClassProfileCandidateInfo;
void* gtStubCallStubAddr; // GTF_CALL_VIRT_STUB - these are never inlined

CORINFO_GENERIC_HANDLE compileTimeHelperArgumentHandle; // Used to track type handle argument of dynamic helpers
void* gtDirectCallAddress; // Used to pass direct call address between lower and codegen
};
Expand Down
32 changes: 2 additions & 30 deletions src/coreclr/jit/importer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -19925,7 +19925,6 @@ void Compiler::impCheckCanInline(GenTreeCall* call,
pInfo->guardedClassHandle = nullptr;
pInfo->guardedMethodHandle = nullptr;
pInfo->guardedMethodUnboxedEntryHandle = nullptr;
pInfo->stubAddr = nullptr;
pInfo->likelihood = 0;
pInfo->requiresInstMethodTableArg = false;
}
Expand Down Expand Up @@ -20936,15 +20935,6 @@ void Compiler::impMarkInlineCandidate(GenTree* callNode,
dspTreeID(call));

call->ClearGuardedDevirtualizationCandidate();

// If we have a stub address, restore it back into the union that it shares
// with the candidate info.
if (call->IsVirtualStub())
{
JITDUMP("Restoring stub addr %p from guarded devirt candidate info\n",
dspPtr(call->gtGuardedDevirtualizationCandidateInfo->stubAddr));
call->gtStubCallStubAddr = call->gtGuardedDevirtualizationCandidateInfo->stubAddr;
}
}

//------------------------------------------------------------------------
Expand Down Expand Up @@ -21432,14 +21422,8 @@ void Compiler::impDevirtualizeCall(GenTreeCall* call,

// Record some info needed for the class profiling probe.
//
pInfo->ilOffset = ilOffset;
pInfo->probeIndex = info.compClassProbeCount++;
pInfo->stubAddr = call->gtStubCallStubAddr;

// note this overwrites gtCallStubAddr, so it needs to be undone
// during the instrumentation phase, or we won't generate proper
// code for vsd calls.
//
pInfo->ilOffset = ilOffset;
pInfo->probeIndex = info.compClassProbeCount++;
call->gtClassProfileCandidateInfo = pInfo;

// Flag block as needing scrutiny
Expand Down Expand Up @@ -22559,18 +22543,6 @@ void Compiler::addGuardedDevirtualizationCandidate(GenTreeCall* call,
}
}

// Save off the stub address since it shares a union with the candidate info.
//
if (call->IsVirtualStub())
{
JITDUMP("Saving stub addr %p in candidate info\n", dspPtr(call->gtStubCallStubAddr));
pInfo->stubAddr = call->gtStubCallStubAddr;
}
else
{
pInfo->stubAddr = nullptr;
}

call->gtGuardedDevirtualizationCandidateInfo = pInfo;
}

Expand Down
12 changes: 0 additions & 12 deletions src/coreclr/jit/indirectcalltransformer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -844,18 +844,6 @@ class IndirectCallTransformer
newStmt->SetRootNode(assign);
}

// For stub calls, restore the stub address. For everything else,
// null out the candidate info field.
if (call->IsVirtualStub())
{
JITDUMP("Restoring stub addr %p from candidate info\n", call->gtInlineCandidateInfo->stubAddr);
call->gtStubCallStubAddr = call->gtInlineCandidateInfo->stubAddr;
}
else
{
call->gtInlineCandidateInfo = nullptr;
}

compiler->fgInsertStmtAtEnd(elseBlock, newStmt);

// Set the original statement to a nop.
Expand Down
1 change: 0 additions & 1 deletion src/coreclr/jit/inline.h
Original file line number Diff line number Diff line change
Expand Up @@ -554,7 +554,6 @@ struct ClassProfileCandidateInfo
{
IL_OFFSET ilOffset;
unsigned probeIndex;
void* stubAddr;
};

// GuardedDevirtualizationCandidateInfo provides information about
Expand Down

0 comments on commit b28c38d

Please sign in to comment.