Skip to content

Commit

Permalink
JIT: Move profile consistency checks to after loop opts (#111285)
Browse files Browse the repository at this point in the history 
Part of #107749.
  • Loading branch information
@amanasifkhalid
amanasifkhalid authored Jan 21, 2025
1 parent ba82203 commit ccc9c52
Show file tree
Hide file tree
Showing 6 changed files with 125 additions and 136 deletions.
13 changes: 6 additions & 7 deletions src/coreclr/jit/compiler.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4814,11 +4814,6 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl
//
DoPhase(this, PHASE_GS_COOKIE, &Compiler::gsPhase);

// Drop back to just checking profile likelihoods.
//
activePhaseChecks &= ~PhaseChecks::CHECK_PROFILE;
activePhaseChecks |= PhaseChecks::CHECK_LIKELIHOODS;

if (opts.OptimizationEnabled())
{
// Compute the block weights
Expand Down Expand Up @@ -4855,8 +4850,7 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl
//
DoPhase(this, PHASE_DFS_BLOCKS3, &Compiler::fgDfsBlocksAndRemove);

// Discover and classify natural loops (e.g. mark iterative loops as such). Also marks loop blocks
// and sets bbWeight to the loop nesting levels.
// Discover and classify natural loops (e.g. mark iterative loops as such).
//
DoPhase(this, PHASE_FIND_LOOPS, &Compiler::optFindLoopsPhase);

Expand All @@ -4877,6 +4871,11 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl
DoPhase(this, PHASE_COMPUTE_DOMINATORS, &Compiler::fgComputeDominators);
}

// Drop back to just checking profile likelihoods.
//
activePhaseChecks &= ~PhaseChecks::CHECK_PROFILE;
activePhaseChecks |= PhaseChecks::CHECK_LIKELIHOODS;

#ifdef DEBUG
fgDebugCheckLinks();
#endif
Expand Down
22 changes: 16 additions & 6 deletions src/coreclr/jit/fgehopt.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,12 @@ PhaseStatus Compiler::fgRemoveEmptyFinally()
currentBlock->SetKind(BBJ_ALWAYS);
currentBlock->RemoveFlags(BBF_RETLESS_CALL); // no longer a BBJ_CALLFINALLY

// Update profile data into postTryFinallyBlock
if (currentBlock->hasProfileWeight())
{
postTryFinallyBlock->increaseBBProfileWeight(currentBlock->bbWeight);
}

// Cleanup the postTryFinallyBlock
fgCleanupContinuation(postTryFinallyBlock);

Expand Down Expand Up @@ -672,12 +678,6 @@ PhaseStatus Compiler::fgRemoveEmptyTry()
fgPrepareCallFinallyRetForRemoval(leave);
fgRemoveBlock(leave, /* unreachable */ true);

// Remove profile weight into the continuation block
if (continuation->hasProfileWeight())
{
continuation->decreaseBBProfileWeight(leave->bbWeight);
}

// (3) Convert the callfinally to a normal jump to the handler
assert(callFinally->HasInitializedTarget());
callFinally->SetKind(BBJ_ALWAYS);
Expand Down Expand Up @@ -1674,6 +1674,16 @@ PhaseStatus Compiler::fgCloneFinally()
}
}

// Update flow into normalCallFinallyReturn
if (normalCallFinallyReturn->hasProfileWeight())
{
normalCallFinallyReturn->bbWeight = BB_ZERO_WEIGHT;
for (FlowEdge* const predEdge : normalCallFinallyReturn->PredEdges())
{
normalCallFinallyReturn->increaseBBProfileWeight(predEdge->getLikelyWeight());
}
}

// Done!
JITDUMP("\nDone with EH#%u\n\n", XTnum);
cloneCount++;
Expand Down
95 changes: 54 additions & 41 deletions src/coreclr/jit/fgopt.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1259,8 +1259,25 @@ void Compiler::fgUnreachableBlock(BasicBlock* block)
// Mark the block as removed
block->SetFlags(BBF_REMOVED);

// Update bbRefs and bbPreds for the blocks reached by this block
fgRemoveBlockAsPred(block);
// Update bbRefs and bbPreds for this block's successors
bool profileInconsistent = false;
for (BasicBlock* const succBlock : block->Succs(this))
{
FlowEdge* const succEdge = fgRemoveAllRefPreds(succBlock, block);

if (block->hasProfileWeight() && succBlock->hasProfileWeight())
{
succBlock->decreaseBBProfileWeight(succEdge->getLikelyWeight());
profileInconsistent |= (succBlock->NumSucc() > 0);
}
}

if (profileInconsistent)
{
JITDUMP("Flow removal of " FMT_BB " needs to be propagated. Data %s inconsistent.\n", block->bbNum,
fgPgoConsistent ? "is now" : "was already");
fgPgoConsistent = false;
}
}

//-------------------------------------------------------------
Expand Down Expand Up @@ -2592,13 +2609,13 @@ bool Compiler::fgOptimizeBranch(BasicBlock* bJump)
estDupCostSz += expr->GetCostSz();
}

bool allProfileWeightsAreValid = false;
weight_t weightJump = bJump->bbWeight;
weight_t weightDest = bDest->bbWeight;
weight_t weightNext = bJump->Next()->bbWeight;
bool rareJump = bJump->isRunRarely();
bool rareDest = bDest->isRunRarely();
bool rareNext = bJump->Next()->isRunRarely();
bool haveProfileWeights = false;
weight_t weightJump = bJump->bbWeight;
weight_t weightDest = bDest->bbWeight;
weight_t weightNext = bJump->Next()->bbWeight;
bool rareJump = bJump->isRunRarely();
bool rareDest = bDest->isRunRarely();
bool rareNext = bJump->Next()->isRunRarely();

// If we have profile data then we calculate the number of time
// the loop will iterate into loopIterations
Expand All @@ -2611,7 +2628,7 @@ bool Compiler::fgOptimizeBranch(BasicBlock* bJump)
bDest->HasAnyFlag(BBF_PROF_WEIGHT | BBF_RUN_RARELY) &&
bJump->Next()->HasAnyFlag(BBF_PROF_WEIGHT | BBF_RUN_RARELY))
{
allProfileWeightsAreValid = true;
haveProfileWeights = true;

if ((weightJump * 100) < weightDest)
{
Expand Down Expand Up @@ -2668,9 +2685,9 @@ bool Compiler::fgOptimizeBranch(BasicBlock* bJump)
if (verbose)
{
printf("\nDuplication of the conditional block " FMT_BB " (always branch from " FMT_BB
") %s, because the cost of duplication (%i) is %s than %i, validProfileWeights = %s\n",
") %s, because the cost of duplication (%i) is %s than %i, haveProfileWeights = %s\n",
bDest->bbNum, bJump->bbNum, costIsTooHigh ? "not done" : "performed", estDupCostSz,
costIsTooHigh ? "greater" : "less or equal", maxDupCostSz, allProfileWeightsAreValid ? "true" : "false");
costIsTooHigh ? "greater" : "less or equal", maxDupCostSz, dspBool(haveProfileWeights));
}
#endif // DEBUG

Expand Down Expand Up @@ -2772,7 +2789,8 @@ bool Compiler::fgOptimizeBranch(BasicBlock* bJump)

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

// bJump now jumps to bDest's normal jump target
//
Expand All @@ -2781,35 +2799,26 @@ bool Compiler::fgOptimizeBranch(BasicBlock* bJump)

bJump->SetCond(bJump->GetTargetEdge(), falseEdge);

if (weightJump > 0)
// Update profile data
//
if (haveProfileWeights)
{
if (allProfileWeightsAreValid)
{
if (weightDest > weightJump)
{
bDest->bbWeight = (weightDest - weightJump);
}
else if (!bDest->isRunRarely())
{
bDest->bbWeight = BB_UNITY_WEIGHT;
}
}
else
{
weight_t newWeightDest = 0;
// bJump no longer flows into bDest
//
bDest->decreaseBBProfileWeight(bJump->bbWeight);
bDestNormalTarget->decreaseBBProfileWeight(bJump->bbWeight * destFalseEdge->getLikelihood());
bDestFalseTarget->decreaseBBProfileWeight(bJump->bbWeight * destTrueEdge->getLikelihood());

if (weightDest > weightJump)
{
newWeightDest = (weightDest - weightJump);
}
if (weightDest >= (BB_LOOP_WEIGHT_SCALE * BB_UNITY_WEIGHT) / 2)
{
newWeightDest = (weightDest * 2) / (BB_LOOP_WEIGHT_SCALE * BB_UNITY_WEIGHT);
}
if (newWeightDest > 0)
{
bDest->bbWeight = newWeightDest;
}
// Propagate bJump's weight into its new successors
//
bDestNormalTarget->increaseBBProfileWeight(bJump->GetTrueEdge()->getLikelyWeight());
bDestFalseTarget->increaseBBProfileWeight(falseEdge->getLikelyWeight());

if ((bDestNormalTarget->NumSucc() > 0) || (bDestFalseTarget->NumSucc() > 0))
{
JITDUMP("fgOptimizeBranch: New flow out of " FMT_BB " needs to be propagated. Data %s inconsistent.\n",
fgPgoConsistent ? "is now" : "was already");
fgPgoConsistent = false;
}
}

Expand Down Expand Up @@ -2939,6 +2948,10 @@ bool Compiler::fgOptimizeSwitchJumps()
blockToTargetEdge->setLikelihood(fraction);
blockToNewBlockEdge->setLikelihood(max(0.0, 1.0 - fraction));

JITDUMP("fgOptimizeSwitchJumps: Updated flow into " FMT_BB " needs to be propagated. Data %s inconsistent.\n",
newBlock->bbNum, fgPgoConsistent ? "is now" : "was already");
fgPgoConsistent = false;

// For now we leave the switch as is, since there's no way
// to indicate that one of the cases is now unreachable.
//
Expand Down Expand Up @@ -3168,7 +3181,7 @@ bool Compiler::fgExpandRarelyRunBlocks()
// If block is not run rarely, then check to make sure that it has
// at least one non-rarely run block.

if (!block->isRunRarely())
if (!block->isRunRarely() && !block->isBBCallFinallyPairTail())
{
bool rare = true;

Expand Down
3 changes: 3 additions & 0 deletions src/coreclr/jit/fgprofile.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4463,6 +4463,9 @@ bool Compiler::fgComputeCalledCount(weight_t returnWeight)
{
fgFirstBB->setBBProfileWeight(fgCalledCount);
madeChanges = true;
JITDUMP("fgComputeCalledCount: Modified method entry weight. Data %s inconsistent.\n",
fgPgoConsistent ? "is now" : "was already");
fgPgoConsistent = false;
}

#if DEBUG
Expand Down
26 changes: 9 additions & 17 deletions src/coreclr/jit/loopcloning.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2011,21 +2011,8 @@ void Compiler::optCloneLoop(FlowGraphNaturalLoop* loop, LoopCloneContext* contex
bool cloneLoopsWithEH = false;
INDEBUG(cloneLoopsWithEH = (JitConfig.JitCloneLoopsWithEH() > 0);)

// Determine the depth of the loop, so we can properly weight blocks added (outside the cloned loop blocks).
unsigned depth = loop->GetDepth();
weight_t ambientWeight = 1;
for (unsigned j = 0; j < depth; j++)
{
weight_t lastWeight = ambientWeight;
ambientWeight *= BB_LOOP_WEIGHT_SCALE;
assert(ambientWeight > lastWeight);
}

assert(loop->EntryEdges().size() == 1);
BasicBlock* preheader = loop->EntryEdge(0)->getSourceBlock();
// The ambient weight might be higher than we computed above. Be safe by
// taking the max with the head block's weight.
ambientWeight = max(ambientWeight, preheader->bbWeight);

// We're going to transform this loop:
//
Expand All @@ -2043,8 +2030,7 @@ void Compiler::optCloneLoop(FlowGraphNaturalLoop* loop, LoopCloneContext* contex

BasicBlock* fastPreheader = fgNewBBafter(BBJ_ALWAYS, preheader, /*extendRegion*/ true);
JITDUMP("Adding " FMT_BB " after " FMT_BB "\n", fastPreheader->bbNum, preheader->bbNum);
fastPreheader->bbWeight = preheader->isRunRarely() ? BB_ZERO_WEIGHT : ambientWeight;
fastPreheader->CopyFlags(preheader, (BBF_PROF_WEIGHT | BBF_RUN_RARELY));
fastPreheader->inheritWeight(preheader);

assert(preheader->KindIs(BBJ_ALWAYS));
assert(preheader->TargetIs(loop->GetHeader()));
Expand Down Expand Up @@ -2092,8 +2078,7 @@ void Compiler::optCloneLoop(FlowGraphNaturalLoop* loop, LoopCloneContext* contex
const bool extendRegion = BasicBlock::sameEHRegion(beforeSlowPreheader, preheader);
BasicBlock* slowPreheader = fgNewBBafter(BBJ_ALWAYS, beforeSlowPreheader, extendRegion);
JITDUMP("Adding " FMT_BB " after " FMT_BB "\n", slowPreheader->bbNum, beforeSlowPreheader->bbNum);
slowPreheader->bbWeight = preheader->isRunRarely() ? BB_ZERO_WEIGHT : ambientWeight;
slowPreheader->CopyFlags(preheader, (BBF_PROF_WEIGHT | BBF_RUN_RARELY));
slowPreheader->inheritWeight(preheader);
slowPreheader->scaleBBWeight(LoopCloneContext::slowPathWeightScaleFactor);

// If we didn't extend the region above (because the beforeSlowPreheader
Expand Down Expand Up @@ -3091,6 +3076,13 @@ PhaseStatus Compiler::optCloneLoops()
m_dfsTree = fgComputeDfs();
m_loops = FlowGraphNaturalLoops::Find(m_dfsTree);
}

if (fgIsUsingProfileWeights())
{
JITDUMP("optCloneLoops: Profile data needs to be propagated through new loops. Data %s inconsistent.\n",
fgPgoConsistent ? "is now" : "was already");
fgPgoConsistent = false;
}
}

#ifdef DEBUG
Expand Down
Loading

0 comments on commit ccc9c52

Please sign in to comment.