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.

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

Jump to bottom

JIT: profile checking through loop opts #99367

Merged
merged 1 commit into from
Mar 7, 2024
Merged

JIT: profile checking through loop opts #99367

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
36 changes: 35 additions & 1 deletion src/coreclr/jit/block.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ unsigned SsaStressHashHelper()
#endif

//------------------------------------------------------------------------
// setLikelihood: set the likelihood of aflow edge
// setLikelihood: set the likelihood of a flow edge
//
// Arguments:
// likelihood -- value in range [0.0, 1.0] indicating how likely
Expand All @@ -86,6 +86,40 @@ void FlowEdge::setLikelihood(weight_t likelihood)
m_likelihood);
}

//------------------------------------------------------------------------
// addLikelihood: adjust the likelihood of a flow edge
//
// Arguments:
// addedLikelihood -- value in range [-likelihood, 1.0 - likelihood]
// to add to current likelihood.
//
void FlowEdge::addLikelihood(weight_t addedLikelihood)
{
assert(m_likelihoodSet);

weight_t newLikelihood = m_likelihood + addedLikelihood;

// Tolerate slight overflow or underflow
//
const weight_t eps = 0.0001;

if ((newLikelihood < 0) && (newLikelihood > -eps))
{
newLikelihood = 0.0;
}
else if ((newLikelihood > 1) && (newLikelihood < 1 + eps))
{
newLikelihood = 1.0;
}

assert(newLikelihood >= 0.0);
assert(newLikelihood <= 1.0);
m_likelihood = newLikelihood;

JITDUMP("updating likelihood of " FMT_BB " -> " FMT_BB " to " FMT_WT "\n", m_sourceBlock->bbNum, m_destBlock->bbNum,
m_likelihood);
}

//------------------------------------------------------------------------
// AllSuccessorEnumerator: Construct an instance of the enumerator.
//
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/jit/block.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -671,6 +671,7 @@ struct FlowEdge
}

void setLikelihood(weight_t likelihood);
void addLikelihood(weight_t addedLikelihod);

void clearLikelihood()
{
Expand Down
10 changes: 5 additions & 5 deletions src/coreclr/jit/compiler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4610,11 +4610,6 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl
//
DoPhase(this, PHASE_MORPH_ADD_INTERNAL, &Compiler::fgAddInternal);

// Disable profile checks now.
// Over time we will move this further and further back in the phase list, as we fix issues.
//
activePhaseChecks &= ~PhaseChecks::CHECK_PROFILE;

// Remove empty try regions
//
DoPhase(this, PHASE_EMPTY_TRY, &Compiler::fgRemoveEmptyTry);
Expand Down Expand Up @@ -4855,6 +4850,11 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl
DoPhase(this, PHASE_COMPUTE_DOMINATORS, &Compiler::fgComputeDominators);
}

// Disable profile checks now.
// Over time we will move this further and further back in the phase list, as we fix issues.
//
activePhaseChecks &= ~PhaseChecks::CHECK_PROFILE;

#ifdef DEBUG
fgDebugCheckLinks();
#endif
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/jit/fgflow.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,7 @@ FlowEdge* Compiler::fgAddRefPred(BasicBlock* block, BasicBlock* blockPred, FlowE
if (flow != nullptr)
{
// The predecessor block already exists in the flow list; simply add to its duplicate count.
//
noway_assert(flow->getDupCount());
flow->incrementDupCount();
}
Expand Down
61 changes: 53 additions & 8 deletions src/coreclr/jit/fgopt.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1891,6 +1891,27 @@ bool Compiler::fgOptimizeSwitchBranches(BasicBlock* block)
FlowEdge* const newEdge = fgAddRefPred(bNewDest, block, oldEdge);
*jmpTab = newEdge;

// Update edge likelihoods
// Note old edge may still be "in use" so we decrease its likelihood.
//
if (oldEdge->hasLikelihood())
{
// We want to move this much likelihood from old->new
//
const weight_t likelihoodFraction = oldEdge->getLikelihood() / (oldEdge->getDupCount() + 1);

if (newEdge->getDupCount() == 1)
{
newEdge->setLikelihood(likelihoodFraction);
}
else
{
newEdge->addLikelihood(likelihoodFraction);
}

oldEdge->addLikelihood(-likelihoodFraction);
}

// we optimized a Switch label - goto REPEAT_SWITCH to follow this new jump
modified = true;

Expand Down Expand Up @@ -2903,19 +2924,32 @@ bool Compiler::fgOptimizeBranch(BasicBlock* bJump)
// We need to update the following flags of the bJump block if they were set in the bDest block
bJump->CopyFlags(bDest, BBF_COPY_PROPAGATE);

/* Update bbRefs and bbPreds */
// Update bbRefs and bbPreds
//
// For now we set the likelihood of the new branch to match
// the likelihood of the old branch.
//
// This may or may not match the block weight adjustments we're
// making. All this becomes easier to reconcile once we rely on
// edge likelihoods more and have synthesis running.
//
// Until then we won't worry that edges and blocks are potentially
// out of sync.
//
FlowEdge* const destFalseEdge = bDest->GetFalseEdge();
FlowEdge* const destTrueEdge = bDest->GetTrueEdge();

// bJump now falls through into the next block
//
FlowEdge* const falseEdge = fgAddRefPred(bJump->Next(), bJump);
FlowEdge* const falseEdge = fgAddRefPred(bJump->Next(), bJump, destFalseEdge);

// bJump no longer jumps to bDest
//
fgRemoveRefPred(bJump->GetTargetEdge());

// bJump now jumps to bDest's normal jump target
//
FlowEdge* const trueEdge = fgAddRefPred(bDestNormalTarget, bJump);
FlowEdge* const trueEdge = fgAddRefPred(bDestNormalTarget, bJump, destTrueEdge);

bJump->SetCond(trueEdge, falseEdge);

Expand Down Expand Up @@ -3076,7 +3110,9 @@ bool Compiler::fgOptimizeSwitchJumps()
newBlock->setBBProfileWeight(blockToNewBlockWeight);

blockToTargetEdge->setEdgeWeights(blockToTargetWeight, blockToTargetWeight, dominantTarget);
blockToTargetEdge->setLikelihood(fraction);
blockToNewBlockEdge->setEdgeWeights(blockToNewBlockWeight, blockToNewBlockWeight, block);
blockToNewBlockEdge->setLikelihood(max(0, 1.0 - fraction));

// There may be other switch cases that lead to this same block, but there's just
// one edge in the flowgraph. So we need to subtract off the profile data that now flows
Expand Down Expand Up @@ -5020,11 +5056,20 @@ bool Compiler::fgUpdateFlowGraph(bool doTailDuplication /* = false */, bool isPh
}

// Optimize the Conditional JUMP to go to the new target
fgRemoveRefPred(block->GetFalseEdge());
fgRemoveRefPred(bNext->GetTargetEdge());
block->SetFalseEdge(block->GetTrueEdge());
FlowEdge* const newEdge = fgAddRefPred(bNext->GetTarget(), block, bNext->GetTargetEdge());
block->SetTrueEdge(newEdge);
//
FlowEdge* const oldFalseEdge = block->GetFalseEdge();
FlowEdge* const oldTrueEdge = block->GetTrueEdge();
FlowEdge* const oldNextEdge = bNext->GetTargetEdge();

// bNext no longer flows to target
//
fgRemoveRefPred(oldNextEdge);

// Rewire flow from block
//
block->SetFalseEdge(oldTrueEdge);
FlowEdge* const newTrueEdge = fgAddRefPred(bNext->GetTarget(), block, oldFalseEdge);
block->SetTrueEdge(newTrueEdge);

/*
Unlink bNext from the BasicBlock list; note that we can
Expand Down
22 changes: 22 additions & 0 deletions src/coreclr/jit/loopcloning.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -853,11 +853,27 @@ BasicBlock* LoopCloneContext::CondToStmtInBlock(Compiler*
noway_assert(conds.Size() > 0);
assert(slowPreheader != nullptr);

// For now assume high likelihood for the fast path,
// uniformly spread across the gating branches.
//
// For "normal" cloning this is probably ok. For GDV cloning this
// may be inaccurate. We should key off the type test likelihood(s).
//
// TODO: this is a bit of out sync with what we do for block weights.
// Reconcile.
//
const weight_t fastLikelihood = 0.999;
Copy link
Member

Choose a reason for hiding this comment

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

Out of curiosity, where does this number of significant figures come from?

Copy link
Member Author

Choose a reason for hiding this comment

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

It's just an arbitrary factor. For most cloned loops we actually never expect the cold loop to run, since we think it's execution may cause exceptions. But setting likelihood to zero seemed to drastic.

Copy link
Member

Choose a reason for hiding this comment

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

optCloneLoop scales the fast loop blocks by 0.99. There is a comment about it:

runtime/src/coreclr/jit/loopcloning.cpp

Lines 1944 to 1949 in 5f52977

// We assume that the fast path will run 99% of the time, and thus should get 99% of the block weights.
// The slow path will, correspondingly, get only 1% of the block weights. It could be argued that we should
// mark the slow path as "run rarely", since it really shouldn't execute (given the currently optimized loop
// conditions) except under exceptional circumstances.
const weight_t fastPathWeightScaleFactor = 0.99;
const weight_t slowPathWeightScaleFactor = 1.0 - fastPathWeightScaleFactor;

It seems like we should use the same likelihood in both places.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, good point. I will tweak this in the next round of changes.


// Choose how to generate the conditions
const bool generateOneConditionPerBlock = true;

if (generateOneConditionPerBlock)
{
// N = conds.Size() branches must all be true to execute the fast loop.
// Use the N'th root....
//
const weight_t fastLikelihoodPerBlock = exp(log(fastLikelihood) / (weight_t)conds.Size());

for (unsigned i = 0; i < conds.Size(); ++i)
{
BasicBlock* newBlk = comp->fgNewBBafter(BBJ_COND, insertAfter, /*extendRegion*/ true);
Expand All @@ -866,12 +882,14 @@ BasicBlock* LoopCloneContext::CondToStmtInBlock(Compiler*
JITDUMP("Adding " FMT_BB " -> " FMT_BB "\n", newBlk->bbNum, slowPreheader->bbNum);
FlowEdge* const trueEdge = comp->fgAddRefPred(slowPreheader, newBlk);
newBlk->SetTrueEdge(trueEdge);
trueEdge->setLikelihood(1 - fastLikelihoodPerBlock);

if (insertAfter->KindIs(BBJ_COND))
{
JITDUMP("Adding " FMT_BB " -> " FMT_BB "\n", insertAfter->bbNum, newBlk->bbNum);
FlowEdge* const falseEdge = comp->fgAddRefPred(newBlk, insertAfter);
insertAfter->SetFalseEdge(falseEdge);
falseEdge->setLikelihood(fastLikelihoodPerBlock);
}

JITDUMP("Adding conditions %u to " FMT_BB "\n", i, newBlk->bbNum);
Expand Down Expand Up @@ -901,12 +919,14 @@ BasicBlock* LoopCloneContext::CondToStmtInBlock(Compiler*
JITDUMP("Adding " FMT_BB " -> " FMT_BB "\n", newBlk->bbNum, slowPreheader->bbNum);
FlowEdge* const trueEdge = comp->fgAddRefPred(slowPreheader, newBlk);
newBlk->SetTrueEdge(trueEdge);
trueEdge->setLikelihood(1.0 - fastLikelihood);

if (insertAfter->KindIs(BBJ_COND))
{
JITDUMP("Adding " FMT_BB " -> " FMT_BB "\n", insertAfter->bbNum, newBlk->bbNum);
FlowEdge* const falseEdge = comp->fgAddRefPred(newBlk, insertAfter);
insertAfter->SetFalseEdge(falseEdge);
falseEdge->setLikelihood(fastLikelihood);
}

JITDUMP("Adding conditions to " FMT_BB "\n", newBlk->bbNum);
Expand Down Expand Up @@ -2069,6 +2089,8 @@ void Compiler::optCloneLoop(FlowGraphNaturalLoop* loop, LoopCloneContext* contex
assert(condLast->NextIs(fastPreheader));
FlowEdge* const falseEdge = fgAddRefPred(fastPreheader, condLast);
condLast->SetFalseEdge(falseEdge);
FlowEdge* const trueEdge = condLast->GetTrueEdge();
falseEdge->setLikelihood(max(0, 1.0 - trueEdge->getLikelihood()));
}

//-------------------------------------------------------------------------
Expand Down
39 changes: 28 additions & 11 deletions src/coreclr/jit/morph.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14679,15 +14679,20 @@ bool Compiler::fgExpandQmarkStmt(BasicBlock* block, Statement* stmt)
thenBlock->SetFlags(BBF_IMPORTED);
}

const unsigned thenLikelihood = qmark->ThenNodeLikelihood();
const unsigned elseLikelihood = qmark->ElseNodeLikelihood();

FlowEdge* const newEdge = fgAddRefPred(remainderBlock, thenBlock);
thenBlock->SetTargetEdge(newEdge);

assert(condBlock->TargetIs(elseBlock));
FlowEdge* const falseEdge = fgAddRefPred(thenBlock, condBlock);
condBlock->SetCond(condBlock->GetTargetEdge(), falseEdge);

thenBlock->inheritWeightPercentage(condBlock, qmark->ThenNodeLikelihood());
elseBlock->inheritWeightPercentage(condBlock, qmark->ElseNodeLikelihood());
FlowEdge* const elseEdge = fgAddRefPred(thenBlock, condBlock);
FlowEdge* const thenEdge = condBlock->GetTargetEdge();
condBlock->SetCond(thenEdge, elseEdge);
thenBlock->inheritWeightPercentage(condBlock, thenLikelihood);
elseBlock->inheritWeightPercentage(condBlock, elseLikelihood);
thenEdge->setLikelihood(thenLikelihood / 100.0);
elseEdge->setLikelihood(elseLikelihood / 100.0);
}
else if (hasTrueExpr)
{
Expand All @@ -14699,15 +14704,21 @@ bool Compiler::fgExpandQmarkStmt(BasicBlock* block, Statement* stmt)
//
gtReverseCond(condExpr);

const unsigned thenLikelihood = qmark->ThenNodeLikelihood();
const unsigned elseLikelihood = qmark->ElseNodeLikelihood();

assert(condBlock->TargetIs(elseBlock));
FlowEdge* const trueEdge = fgAddRefPred(remainderBlock, condBlock);
condBlock->SetCond(trueEdge, condBlock->GetTargetEdge());
FlowEdge* const thenEdge = fgAddRefPred(remainderBlock, condBlock);
FlowEdge* const elseEdge = condBlock->GetTargetEdge();
condBlock->SetCond(thenEdge, elseEdge);

// Since we have no false expr, use the one we'd already created.
thenBlock = elseBlock;
elseBlock = nullptr;

thenBlock->inheritWeightPercentage(condBlock, qmark->ThenNodeLikelihood());
thenBlock->inheritWeightPercentage(condBlock, thenLikelihood);
thenEdge->setLikelihood(thenLikelihood / 100.0);
elseEdge->setLikelihood(elseLikelihood / 100.0);
}
else if (hasFalseExpr)
{
Expand All @@ -14717,11 +14728,17 @@ bool Compiler::fgExpandQmarkStmt(BasicBlock* block, Statement* stmt)
// +-->------------+
// bbj_cond(true)
//
const unsigned thenLikelihood = qmark->ThenNodeLikelihood();
const unsigned elseLikelihood = qmark->ElseNodeLikelihood();

assert(condBlock->TargetIs(elseBlock));
FlowEdge* const trueEdge = fgAddRefPred(remainderBlock, condBlock);
condBlock->SetCond(trueEdge, condBlock->GetTargetEdge());
FlowEdge* const thenEdge = fgAddRefPred(remainderBlock, condBlock);
FlowEdge* const elseEdge = condBlock->GetTargetEdge();
condBlock->SetCond(thenEdge, elseEdge);

elseBlock->inheritWeightPercentage(condBlock, qmark->ElseNodeLikelihood());
elseBlock->inheritWeightPercentage(condBlock, elseLikelihood);
thenEdge->setLikelihood(thenLikelihood / 100.0);
elseEdge->setLikelihood(elseLikelihood / 100.0);
}

assert(condBlock->KindIs(BBJ_COND));
Expand Down
Loading
Loading