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

Guarded devirtualization: multiple type checks #86551

Merged
merged 18 commits into from
May 26, 2023
8 changes: 4 additions & 4 deletions src/coreclr/jit/gentree.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2164,12 +2164,12 @@ GenTree* Compiler::getArrayLengthFromAllocation(GenTree* tree DEBUGARG(BasicBloc
}

//-------------------------------------------------------------------------
// SetSingleInlineCadidateInfo: set a single inline candidate info in the current call.
// SetSingleInlineCandidateInfo: set a single inline candidate info in the current call.
//
// Arguments:
// candidateInfo - inline candidate info
//
void GenTreeCall::SetSingleInlineCadidateInfo(InlineCandidateInfo* candidateInfo)
void GenTreeCall::SetSingleInlineCandidateInfo(InlineCandidateInfo* candidateInfo)
{
if (candidateInfo != nullptr)
{
Expand Down Expand Up @@ -2199,7 +2199,7 @@ InlineCandidateInfo* GenTreeCall::GetGDVCandidateInfo(uint8_t index)

//-------------------------------------------------------------------------
// AddGDVCandidateInfo: Record a guarded devirtualization (GDV) candidate info
// for this call. For now, we only support one GDV candidate per call.
// for this call. A call can't have more than MAX_GDV_TYPE_CHECKS number of candidates
//
// Arguments:
// comp - Compiler instance
EgorBo marked this conversation as resolved.
Show resolved Hide resolved
Expand Down Expand Up @@ -9352,8 +9352,8 @@ GenTreeCall* Compiler::gtCloneExprCallHelper(GenTreeCall* tree,
{
copy->gtCallMethHnd = tree->gtCallMethHnd;
copy->gtInlineCandidateInfo = tree->gtInlineCandidateInfo;
copy->gtInlineInfoCount = tree->gtInlineInfoCount;
}
copy->gtInlineInfoCount = tree->gtInlineInfoCount;

copy->gtCallType = tree->gtCallType;
copy->gtReturnType = tree->gtReturnType;
Expand Down
15 changes: 11 additions & 4 deletions src/coreclr/jit/gentree.h
Original file line number Diff line number Diff line change
Expand Up @@ -5423,22 +5423,29 @@ struct GenTreeCall final : public GenTree

InlineCandidateInfo* GetSingleInlineCandidateInfo()
{
if (gtInlineInfoCount > 1)
// gtInlineInfoCount can be 0 (not an inline candidate) or 1
if (gtInlineInfoCount == 0)
{
assert(!"Call has multiple inline candidates - use GetGDVCandidateInfo instead");
assert(!IsInlineCandidate());
assert(gtInlineCandidateInfo == nullptr);
return nullptr;
}
else if (gtInlineInfoCount > 1)
{
assert(!"Call has multiple inline candidates");
}
return gtInlineCandidateInfo;
}

void SetSingleInlineCadidateInfo(InlineCandidateInfo* candidateInfo);
void SetSingleInlineCandidateInfo(InlineCandidateInfo* candidateInfo);

InlineCandidateInfo* GetGDVCandidateInfo(uint8_t index);
EgorBo marked this conversation as resolved.
Show resolved Hide resolved

void AddGDVCandidateInfo(Compiler* comp, InlineCandidateInfo* candidateInfo);

void ClearInlineInfo()
{
SetSingleInlineCadidateInfo(nullptr);
SetSingleInlineCandidateInfo(nullptr);
}

uint8_t GetInlineCandidatesCount()
Expand Down
7 changes: 2 additions & 5 deletions src/coreclr/jit/importercalls.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5903,12 +5903,9 @@ void Compiler::considerGuardedDevirtualization(GenTreeCall* call,
}
else
{
assert(numExactClasses <= maxTypeChecks);
assert((numExactClasses > 0) && (numExactClasses <= maxTypeChecks));
JITDUMP("We have exactly %d classes implementing %s:\n", numExactClasses, eeGetClassName(baseClass));

// NOTE: The case where we have only a single implementation is handled naturally
// through gtGetClassHandle without help of GDV.
//
int skipped = 0;
for (int exactClsIdx = 0; exactClsIdx < numExactClasses; exactClsIdx++)
{
Expand Down Expand Up @@ -6523,7 +6520,7 @@ bool Compiler::impMarkInlineCandidateHelper(GenTreeCall* call,
else
{
assert(candidateIndex == 0);
call->SetSingleInlineCadidateInfo(inlineCandidateInfo);
call->SetSingleInlineCandidateInfo(inlineCandidateInfo);
}

// Let the strategy know there's another candidate.
Expand Down
26 changes: 11 additions & 15 deletions src/coreclr/jit/indirectcalltransformer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -588,14 +588,8 @@ class IndirectCallTransformer
compiler->fgAddRefPred(checkBlock, prevCheckBlock);

// Weight for the new secondary check is the difference between the previous check and the thenBlock.
weight_t newWeight = prevCheckBlock->bbWeight - thenBlock->bbWeight;
if (newWeight < 0)
{
// There could be a small error leading to e.g. -0.00001 instead of 0.0 here
// Mostly because of inheritWeightPercentage for thenBlock;
newWeight = 0;
}
checkBlock->setBBProfileWeight(newWeight);
checkBlock->inheritWeightPercentage(prevCheckBlock,
100 - origCall->GetGDVCandidateInfo(checkIdx)->likelihood);
}

// Find last arg with a side effect. All args with any effect
Expand Down Expand Up @@ -932,7 +926,7 @@ class IndirectCallTransformer
inlineInfo->clsHandle = compiler->info.compCompHnd->getMethodClass(methodHnd);
inlineInfo->exactContextHnd = context;
inlineInfo->preexistingSpillTemp = returnTemp;
call->SetSingleInlineCadidateInfo(inlineInfo);
call->SetSingleInlineCandidateInfo(inlineInfo);

// If there was a ret expr for this call, we need to create a new one
// and append it just after the call.
Expand Down Expand Up @@ -990,15 +984,17 @@ class IndirectCallTransformer
compiler->fgAddRefPred(remainderBlock, elseBlock);
compiler->fgAddRefPred(elseBlock, checkBlock);

weight_t newWeight = checkBlock->bbWeight - thenBlock->bbWeight;
if (newWeight < 0)
// Calculate the likelihood of the else block as a remainder of the sum
// of all the other likelihoods.
unsigned elseLikelihood = 100;
for (uint8_t i = 0; i < origCall->GetInlineCandidatesCount(); i++)
{
// There could be a small error leading to e.g. -0.00001 instead of 0.0 here
// Mostly because of inheritWeightPercentage for thenBlock;
newWeight = 0;
elseLikelihood -= origCall->GetGDVCandidateInfo(i)->likelihood;
}
// Make sure it didn't overflow
assert(elseLikelihood <= 100);

elseBlock->setBBProfileWeight(newWeight);
elseBlock->inheritWeightPercentage(currBlock, elseLikelihood);
Copy link
Member

@AndyAyersMS AndyAyersMS May 25, 2023

Choose a reason for hiding this comment

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

For the multi-guess exact case this likelihood should end up at zero, right? We think we know all the possibilities.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, but the logic is shared with non-exact case where fallback may have non-zero likelihood (Dynamic PGO)

I plan to enable "no fallback needed" separately, I am thinking of doing something like "we're importing the last type-check and the call has GTF_M_EXACT_GDV flag --> convert the last pair of check-then to elseBock.


GenTreeCall* call = origCall;
Statement* newStmt = compiler->gtNewStmt(call, stmt->GetDebugInfo());
Expand Down