From 3e5a6d0dd9ffc0077e78420ed99eaff0162abcf7 Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Fri, 28 Mar 2025 10:10:54 -0400 Subject: [PATCH 1/9] Check BBJ_CALLFINALLYRET weight correctly --- src/coreclr/jit/fgprofile.cpp | 30 ++---------------------------- src/coreclr/jit/importer.cpp | 18 ++++++++++++++++++ 2 files changed, 20 insertions(+), 28 deletions(-) diff --git a/src/coreclr/jit/fgprofile.cpp b/src/coreclr/jit/fgprofile.cpp index f034fe3e83c490..e8cce4d952dd71 100644 --- a/src/coreclr/jit/fgprofile.cpp +++ b/src/coreclr/jit/fgprofile.cpp @@ -4787,20 +4787,12 @@ bool Compiler::fgDebugCheckIncomingProfileData(BasicBlock* block, ProfileChecks weight_t incomingLikelyWeight = 0; unsigned missingLikelyWeight = 0; bool foundPreds = false; - bool foundEHPreds = false; for (FlowEdge* const predEdge : block->PredEdges()) { if (predEdge->hasLikelihood()) { - if (BasicBlock::sameHndRegion(block, predEdge->getSourceBlock())) - { - incomingLikelyWeight += predEdge->getLikelyWeight(); - } - else - { - foundEHPreds = true; - } + incomingLikelyWeight += predEdge->getLikelyWeight(); } else { @@ -4812,29 +4804,11 @@ bool Compiler::fgDebugCheckIncomingProfileData(BasicBlock* block, ProfileChecks foundPreds = true; } - // We almost certainly won't get the likelihoods on a BBJ_EHFINALLYRET right, - // so special-case BBJ_CALLFINALLYRET incoming flow. - // - if (block->isBBCallFinallyPairTail()) - { - incomingLikelyWeight = block->Prev()->bbWeight; - foundEHPreds = false; - } - - // We almost certainly won't get the likelihoods on a BBJ_EHFINALLYRET right, - // so special-case BBJ_CALLFINALLYRET incoming flow. - // - if (block->isBBCallFinallyPairTail()) - { - incomingLikelyWeight = block->Prev()->bbWeight; - foundEHPreds = false; - } - bool likelyWeightsValid = true; // If we have EH preds we may not have consistent incoming flow. // - if (foundPreds && !foundEHPreds) + if (foundPreds) { if (verifyLikelyWeights) { diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index cb9d578d19b0bf..c211d37f8d13f6 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -12656,6 +12656,24 @@ void Compiler::impFixPredLists() } finallyBlock->SetEhfTargets(jumpEhf); + + if (finallyBlock->hasProfileWeight()) + { + bool profileConsistent = false; + for (BasicBlock* const succBlock : finallyBlock->Succs()) + { + const weight_t oldWeight = succBlock->bbWeight; + const weight_t newWeight = succBlock->computeIncomingWeight(); + succBlock->setBBProfileWeight(newWeight); + profileConsistent &= fgProfileWeightsEqual(oldWeight, newWeight); + } + + if (!profileConsistent) + { + JITDUMP("Flow propagation out of " FMT_BB " might have introduced inconsistency. Data %s inconsistent.\n", finallyBlock->bbNum, fgPgoConsistent ? "is now" : "was already"); + fgPgoConsistent = false; + } + } } } } From 1816412ba42441de4602e89cde5c50c9c5496419 Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Fri, 28 Mar 2025 12:04:19 -0400 Subject: [PATCH 2/9] Propagate flow into finally regions correctly --- src/coreclr/jit/fgprofilesynthesis.cpp | 35 +++++++++++--------------- src/coreclr/jit/importer.cpp | 4 ++- 2 files changed, 18 insertions(+), 21 deletions(-) diff --git a/src/coreclr/jit/fgprofilesynthesis.cpp b/src/coreclr/jit/fgprofilesynthesis.cpp index ff4b0ac8dda869..c3f2a8f62fd7f1 100644 --- a/src/coreclr/jit/fgprofilesynthesis.cpp +++ b/src/coreclr/jit/fgprofilesynthesis.cpp @@ -168,7 +168,10 @@ void ProfileSynthesis::Run(ProfileSynthesisOption option) if (m_approximate) { JITDUMP("Profile is inconsistent. Bypassing post-phase consistency checks.\n"); - m_comp->Metrics.ProfileInconsistentInitially++; + if (!m_comp->fgImportDone) + { + m_comp->Metrics.ProfileInconsistentInitially++; + } } // Derive the method's call count from the entry block's weight @@ -828,10 +831,7 @@ void ProfileSynthesis::ComputeCyclicProbabilities(FlowGraphNaturalLoop* loop) for (FlowEdge* const edge : nestedLoop->EntryEdges()) { - if (BasicBlock::sameHndRegion(block, edge->getSourceBlock())) - { - newWeight += edge->getLikelyWeight(); - } + newWeight += edge->getLikelyWeight(); } newWeight *= m_cyclicProbabilities[nestedLoop->GetIndex()]; @@ -847,10 +847,10 @@ void ProfileSynthesis::ComputeCyclicProbabilities(FlowGraphNaturalLoop* loop) { BasicBlock* const sourceBlock = edge->getSourceBlock(); - // Ignore flow across EH, or from preds not in the loop. - // Latter can happen if there are unreachable blocks that flow into the loop. + // Ignore flow from preds not in the loop. + // This can happen if there are unreachable blocks that flow into the loop. // - if (BasicBlock::sameHndRegion(block, sourceBlock) && loop->ContainsBlock(sourceBlock)) + if (loop->ContainsBlock(sourceBlock)) { newWeight += edge->getLikelyWeight(); } @@ -1214,7 +1214,8 @@ void ProfileSynthesis::GaussSeidelSolver() const FlowGraphDfsTree* const dfs = m_loops->GetDfsTree(); unsigned const blockCount = dfs->GetPostOrderCount(); bool checkEntryExitWeight = true; - bool showDetails = false; + bool const showDetails = false; + bool const callFinalliesCreated = m_comp->fgImportDone; JITDUMP("Synthesis solver: flow graph has %u improper loop headers\n", m_improperLoopHeaders); @@ -1290,9 +1291,10 @@ void ProfileSynthesis::GaussSeidelSolver() { newWeight = block->bbWeight; - // Finallies also add in the weight of their try. + // If we haven't added flow edges into/out of finallies yet, + // add in the weight of their corresponding try regions. // - if (ehDsc->HasFinallyHandler()) + if (!callFinalliesCreated && ehDsc->HasFinallyHandler()) { newWeight += countVector[ehDsc->ebdTryBeg->bbNum]; } @@ -1318,11 +1320,7 @@ void ProfileSynthesis::GaussSeidelSolver() for (FlowEdge* const edge : loop->EntryEdges()) { BasicBlock* const predBlock = edge->getSourceBlock(); - - if (BasicBlock::sameHndRegion(block, predBlock)) - { - newWeight += edge->getLikelihood() * countVector[predBlock->bbNum]; - } + newWeight += edge->getLikelihood() * countVector[predBlock->bbNum]; } // Scale by cyclic probability @@ -1354,10 +1352,7 @@ void ProfileSynthesis::GaussSeidelSolver() continue; } - if (BasicBlock::sameHndRegion(block, predBlock)) - { - newWeight += edge->getLikelihood() * countVector[predBlock->bbNum]; - } + newWeight += edge->getLikelihood() * countVector[predBlock->bbNum]; } if (selfEdge != nullptr) diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index c211d37f8d13f6..a5e5d48347d2d0 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -12670,7 +12670,9 @@ void Compiler::impFixPredLists() if (!profileConsistent) { - JITDUMP("Flow propagation out of " FMT_BB " might have introduced inconsistency. Data %s inconsistent.\n", finallyBlock->bbNum, fgPgoConsistent ? "is now" : "was already"); + JITDUMP("Flow propagation out of " FMT_BB + " might have introduced inconsistency. Data %s inconsistent.\n", + finallyBlock->bbNum, fgPgoConsistent ? "is now" : "was already"); fgPgoConsistent = false; } } From 50abab8c45ad61e116e7e9844ef6445198026395 Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Fri, 28 Mar 2025 12:11:12 -0400 Subject: [PATCH 3/9] Allocate cyclic probabilities array once --- src/coreclr/jit/fgprofilesynthesis.cpp | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/coreclr/jit/fgprofilesynthesis.cpp b/src/coreclr/jit/fgprofilesynthesis.cpp index c3f2a8f62fd7f1..70d074637edfd2 100644 --- a/src/coreclr/jit/fgprofilesynthesis.cpp +++ b/src/coreclr/jit/fgprofilesynthesis.cpp @@ -41,6 +41,11 @@ void ProfileSynthesis::Run(ProfileSynthesisOption option) assert(m_loops != nullptr); } + if (m_loops->NumLoops() > 0) + { + m_cyclicProbabilities = new (m_comp, CMK_Pgo) weight_t[m_loops->NumLoops()]; + } + // Profile synthesis can be run before or after morph, so tolerate (non-)canonical method entries // m_entryBlock = (m_comp->opts.IsOSR() && (m_comp->fgEntryBB != nullptr)) ? m_comp->fgEntryBB : m_comp->fgFirstBB; @@ -748,13 +753,6 @@ void ProfileSynthesis::RandomizeLikelihoods() // void ProfileSynthesis::ComputeCyclicProbabilities() { - m_cyclicProbabilities = nullptr; - if (m_loops->NumLoops() == 0) - { - return; - } - - m_cyclicProbabilities = new (m_comp, CMK_Pgo) weight_t[m_loops->NumLoops()]; // Walk loops in post order to visit inner loops before outer loops. for (FlowGraphNaturalLoop* loop : m_loops->InPostOrder()) { From 4dfddffa07411973afd3dc3fa8942d38f404944c Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Fri, 28 Mar 2025 22:30:51 -0400 Subject: [PATCH 4/9] Compute finally successor edge likelihoods using entry weights --- src/coreclr/jit/fgbasic.cpp | 20 +++++++++++++ src/coreclr/jit/fgehopt.cpp | 57 ++++++++++++------------------------ src/coreclr/jit/importer.cpp | 57 ++++++++++++++++++++++++------------ 3 files changed, 77 insertions(+), 57 deletions(-) diff --git a/src/coreclr/jit/fgbasic.cpp b/src/coreclr/jit/fgbasic.cpp index f6dda4d6a85764..0f2c8865457e63 100644 --- a/src/coreclr/jit/fgbasic.cpp +++ b/src/coreclr/jit/fgbasic.cpp @@ -368,6 +368,16 @@ void Compiler::fgRemoveEhfSuccessor(BasicBlock* block, const unsigned succIndex) (succCount - succIndex - 1) * sizeof(FlowEdge*)); } + // If the block has other successors, distribute the removed edge's likelihood among the remaining successor edges. + if (succCount > 1) + { + const weight_t likelihoodIncrease = succEdge->getLikelihood() / (succCount - 1); + for (unsigned i = 0; i < (succCount - 1); i++) + { + succTab[i]->addLikelihood(likelihoodIncrease); + } + } + #ifdef DEBUG // We only expect to see a successor once in the table. for (unsigned i = succIndex; i < (succCount - 1); i++) @@ -427,6 +437,16 @@ void Compiler::fgRemoveEhfSuccessor(FlowEdge* succEdge) } } + // If the block has other successors, distribute the removed edge's likelihood among the remaining successor edges. + if (succCount > 1) + { + const weight_t likelihoodIncrease = succEdge->getLikelihood() / (succCount - 1); + for (unsigned i = 0; i < (succCount - 1); i++) + { + succTab[i]->addLikelihood(likelihoodIncrease); + } + } + assert(found); ehfDesc->bbeCount--; } diff --git a/src/coreclr/jit/fgehopt.cpp b/src/coreclr/jit/fgehopt.cpp index 4dc20dcd8d2511..f6a80492993519 100644 --- a/src/coreclr/jit/fgehopt.cpp +++ b/src/coreclr/jit/fgehopt.cpp @@ -1644,27 +1644,28 @@ PhaseStatus Compiler::fgCloneFinally() for (BasicBlock* const block : Blocks(firstBlock, lastBlock)) { - if (block->hasProfileWeight()) - { - weight_t const blockWeight = block->bbWeight; - block->setBBProfileWeight(blockWeight * originalScale); - JITDUMP("Set weight of " FMT_BB " to " FMT_WT "\n", block->bbNum, block->bbWeight); + weight_t const blockWeight = block->bbWeight; + block->setBBProfileWeight(blockWeight * originalScale); + JITDUMP("Set weight of " FMT_BB " to " FMT_WT "\n", block->bbNum, block->bbWeight); - BasicBlock* const clonedBlock = blockMap[block]; - clonedBlock->setBBProfileWeight(blockWeight * clonedScale); - JITDUMP("Set weight of " FMT_BB " to " FMT_WT "\n", clonedBlock->bbNum, clonedBlock->bbWeight); - } + BasicBlock* const clonedBlock = blockMap[block]; + clonedBlock->setBBProfileWeight(blockWeight * clonedScale); + JITDUMP("Set weight of " FMT_BB " to " FMT_WT "\n", clonedBlock->bbNum, clonedBlock->bbWeight); + } + + if (!retargetedAllCalls) + { + JITDUMP( + "Reduced flow out of EH%u needs to be propagated to continuation block(s). Data %s inconsistent.\n", + XTnum, fgPgoConsistent ? "is now" : "was already"); + fgPgoConsistent = false; } } // Update flow into normalCallFinallyReturn if (normalCallFinallyReturn->hasProfileWeight()) { - normalCallFinallyReturn->bbWeight = BB_ZERO_WEIGHT; - for (FlowEdge* const predEdge : normalCallFinallyReturn->PredEdges()) - { - normalCallFinallyReturn->increaseBBProfileWeight(predEdge->getLikelyWeight()); - } + normalCallFinallyReturn->setBBProfileWeight(normalCallFinallyReturn->computeIncomingWeight()); } // Done! @@ -2185,33 +2186,13 @@ bool Compiler::fgRetargetBranchesToCanonicalCallFinally(BasicBlock* block, // if (block->hasProfileWeight()) { - // Add weight to the canonical call finally pair. + // Add weight to the canonical call-finally. // - weight_t const canonicalWeight = - canonicalCallFinally->hasProfileWeight() ? canonicalCallFinally->bbWeight : BB_ZERO_WEIGHT; - weight_t const newCanonicalWeight = block->bbWeight + canonicalWeight; - - canonicalCallFinally->setBBProfileWeight(newCanonicalWeight); + canonicalCallFinally->increaseBBProfileWeight(block->bbWeight); - BasicBlock* const canonicalLeaveBlock = canonicalCallFinally->Next(); - - weight_t const canonicalLeaveWeight = - canonicalLeaveBlock->hasProfileWeight() ? canonicalLeaveBlock->bbWeight : BB_ZERO_WEIGHT; - weight_t const newLeaveWeight = block->bbWeight + canonicalLeaveWeight; - - canonicalLeaveBlock->setBBProfileWeight(newLeaveWeight); - - // Remove weight from the old call finally pair. + // Remove weight from the old call-finally. // - if (callFinally->hasProfileWeight()) - { - callFinally->decreaseBBProfileWeight(block->bbWeight); - } - - if (leaveBlock->hasProfileWeight()) - { - leaveBlock->decreaseBBProfileWeight(block->bbWeight); - } + callFinally->decreaseBBProfileWeight(block->bbWeight); } return true; diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index a5e5d48347d2d0..37dee07ed1578f 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -12576,8 +12576,9 @@ void Compiler::impImport() // void Compiler::impFixPredLists() { - unsigned XTnum = 0; - bool added = false; + unsigned XTnum = 0; + bool added = false; + const bool usingProfileWeights = fgIsUsingProfileWeights(); for (EHblkDsc* HBtab = compHndBBtab; XTnum < compHndBBtabCount; XTnum++, HBtab++) { @@ -12586,6 +12587,7 @@ void Compiler::impFixPredLists() BasicBlock* const finallyBegBlock = HBtab->ebdHndBeg; BasicBlock* const finallyLastBlock = HBtab->ebdHndLast; unsigned predCount = (unsigned)-1; + const weight_t finallyWeight = finallyBegBlock->bbWeight; for (BasicBlock* const finallyBlock : BasicBlockRangeList(finallyBegBlock, finallyLastBlock)) { @@ -12629,7 +12631,8 @@ void Compiler::impFixPredLists() jumpEhf->bbeCount = predCount; jumpEhf->bbeSuccs = new (this, CMK_FlowEdge) FlowEdge*[predCount]; - unsigned predNum = 0; + unsigned predNum = 0; + weight_t remainingLikelihood = 1.0; for (BasicBlock* const predBlock : finallyBegBlock->PredBlocks()) { // We only care about preds that are callfinallies. @@ -12643,6 +12646,22 @@ void Compiler::impFixPredLists() FlowEdge* const newEdge = fgAddRefPred(continuation, finallyBlock); newEdge->setLikelihood(1.0 / predCount); + if (usingProfileWeights && (finallyWeight != BB_ZERO_WEIGHT)) + { + // Derive edge likelihood from the entry block's weight relative to other entries. + // + const weight_t callFinallyWeight = predBlock->bbWeight; + const weight_t likelihood = min(callFinallyWeight / finallyWeight, 1.0); + newEdge->setLikelihood(min(likelihood, remainingLikelihood)); + remainingLikelihood = max(BB_ZERO_WEIGHT, remainingLikelihood - likelihood); + } + else + { + // If we don't have profile data, evenly distribute the likelihoods. + // + newEdge->setLikelihood(1.0 / predCount); + } + jumpEhf->bbeSuccs[predNum] = newEdge; ++predNum; @@ -12656,25 +12675,25 @@ void Compiler::impFixPredLists() } finallyBlock->SetEhfTargets(jumpEhf); + } - if (finallyBlock->hasProfileWeight()) + if (usingProfileWeights) + { + // Compute new flow into the finally region's continuation successors. + // + bool profileConsistent = true; + for (BasicBlock* const callFinally : finallyBegBlock->PredBlocks()) { - bool profileConsistent = false; - for (BasicBlock* const succBlock : finallyBlock->Succs()) - { - const weight_t oldWeight = succBlock->bbWeight; - const weight_t newWeight = succBlock->computeIncomingWeight(); - succBlock->setBBProfileWeight(newWeight); - profileConsistent &= fgProfileWeightsEqual(oldWeight, newWeight); - } + BasicBlock* const callFinallyRet = callFinally->Next(); + callFinallyRet->setBBProfileWeight(callFinallyRet->computeIncomingWeight()); + profileConsistent &= fgProfileWeightsEqual(callFinally->bbWeight, callFinallyRet->bbWeight); + } - if (!profileConsistent) - { - JITDUMP("Flow propagation out of " FMT_BB - " might have introduced inconsistency. Data %s inconsistent.\n", - finallyBlock->bbNum, fgPgoConsistent ? "is now" : "was already"); - fgPgoConsistent = false; - } + if (!profileConsistent) + { + JITDUMP("Flow into finally handler EH%u does not match outgoing flow. Data %s inconsistent.\n", + XTnum, fgPgoConsistent ? "is now" : "was already"); + fgPgoConsistent = false; } } } From 5a39850adef8df3f61d1b0a992b0b6520b90886a Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Mon, 31 Mar 2025 15:35:30 -0400 Subject: [PATCH 5/9] Feedback --- src/coreclr/jit/fgbasic.cpp | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/src/coreclr/jit/fgbasic.cpp b/src/coreclr/jit/fgbasic.cpp index 0f2c8865457e63..ea0372d0c5c9ea 100644 --- a/src/coreclr/jit/fgbasic.cpp +++ b/src/coreclr/jit/fgbasic.cpp @@ -368,14 +368,13 @@ void Compiler::fgRemoveEhfSuccessor(BasicBlock* block, const unsigned succIndex) (succCount - succIndex - 1) * sizeof(FlowEdge*)); } - // If the block has other successors, distribute the removed edge's likelihood among the remaining successor edges. - if (succCount > 1) + // Recompute the likelihoods of the block's other successor edges. + const weight_t removedLikelihood = succEdge->getLikelihood(); + for (unsigned i = 0; (removedLikelihood != 1.0) && (i < (succCount - 1)); i++) { - const weight_t likelihoodIncrease = succEdge->getLikelihood() / (succCount - 1); - for (unsigned i = 0; i < (succCount - 1); i++) - { - succTab[i]->addLikelihood(likelihoodIncrease); - } + const weight_t currLikelihood = succTab[i]->getLikelihood(); + const weight_t newLikelihood = currLikelihood / (1.0 - removedLikelihood); + succTab[i]->setLikelihood(min(1.0, newLikelihood)); } #ifdef DEBUG @@ -437,14 +436,13 @@ void Compiler::fgRemoveEhfSuccessor(FlowEdge* succEdge) } } - // If the block has other successors, distribute the removed edge's likelihood among the remaining successor edges. - if (succCount > 1) + // Recompute the likelihoods of the block's other successor edges. + const weight_t removedLikelihood = succEdge->getLikelihood(); + for (unsigned i = 0; (removedLikelihood != 1.0) && (i < (succCount - 1)); i++) { - const weight_t likelihoodIncrease = succEdge->getLikelihood() / (succCount - 1); - for (unsigned i = 0; i < (succCount - 1); i++) - { - succTab[i]->addLikelihood(likelihoodIncrease); - } + const weight_t currLikelihood = succTab[i]->getLikelihood(); + const weight_t newLikelihood = currLikelihood / (1.0 - removedLikelihood); + succTab[i]->setLikelihood(min(1.0, newLikelihood)); } assert(found); From e3f44629c517f46a4737f705733539333b4f59e0 Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Mon, 31 Mar 2025 15:48:43 -0400 Subject: [PATCH 6/9] Enable likelihood checks for BBJ_EHFINALLYRET blocks --- src/coreclr/jit/fgprofile.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/fgprofile.cpp b/src/coreclr/jit/fgprofile.cpp index e8cce4d952dd71..ba6a17697b6413 100644 --- a/src/coreclr/jit/fgprofile.cpp +++ b/src/coreclr/jit/fgprofile.cpp @@ -4864,7 +4864,7 @@ bool Compiler::fgDebugCheckOutgoingProfileData(BasicBlock* block, ProfileChecks // const unsigned numSuccs = block->NumSucc(this); - if ((numSuccs > 0) && !block->KindIs(BBJ_EHFINALLYRET, BBJ_EHFAULTRET, BBJ_EHFILTERRET)) + if ((numSuccs > 0) && !block->KindIs(BBJ_EHFAULTRET, BBJ_EHFILTERRET)) { weight_t const blockWeight = block->bbWeight; weight_t outgoingLikelihood = 0; From 66be06987bdafb0d3483e0f60dcce9a911003533 Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Tue, 1 Apr 2025 13:53:19 -0400 Subject: [PATCH 7/9] Enable post-synthesis consistency checking --- src/coreclr/jit/fgprofilesynthesis.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/coreclr/jit/fgprofilesynthesis.cpp b/src/coreclr/jit/fgprofilesynthesis.cpp index 70d074637edfd2..65b1499d731131 100644 --- a/src/coreclr/jit/fgprofilesynthesis.cpp +++ b/src/coreclr/jit/fgprofilesynthesis.cpp @@ -216,7 +216,7 @@ void ProfileSynthesis::Run(ProfileSynthesisOption option) // Leave a note so we can bypass the post-phase check, and // instead assert at the end of fgImport, if we make it that far. // - if (!isConsistent) + if (!isConsistent && !m_comp->fgImportDone) { m_comp->fgPgoDeferredInconsistency = true; JITDUMP("Will defer asserting until after importation\n"); @@ -1437,10 +1437,12 @@ void ProfileSynthesis::GaussSeidelSolver() } } - // If there were no improper headers, we will have converged in one pass. + // If there were no improper headers, we will have converged in one pass // (profile may still be inconsistent, if there were capped cyclic probabilities). + // After the importer runs, we require that synthesis achieves profile consistency + // unless the resultant profile is approximate, so don't skip the below checks. // - if (m_improperLoopHeaders == 0) + if ((m_improperLoopHeaders == 0) && !m_comp->fgImportDone) { converged = true; break; From b419dfd445893c5563d02e6f1ec1a91d441546e6 Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Wed, 2 Apr 2025 15:48:13 -0400 Subject: [PATCH 8/9] Handle removing dominant successor edge --- src/coreclr/jit/fgbasic.cpp | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/coreclr/jit/fgbasic.cpp b/src/coreclr/jit/fgbasic.cpp index ea0372d0c5c9ea..cb9319f0c45c5b 100644 --- a/src/coreclr/jit/fgbasic.cpp +++ b/src/coreclr/jit/fgbasic.cpp @@ -370,10 +370,14 @@ void Compiler::fgRemoveEhfSuccessor(BasicBlock* block, const unsigned succIndex) // Recompute the likelihoods of the block's other successor edges. const weight_t removedLikelihood = succEdge->getLikelihood(); - for (unsigned i = 0; (removedLikelihood != 1.0) && (i < (succCount - 1)); i++) + const unsigned newSuccCount = succCount - 1; + + for (unsigned i = 0; i < newSuccCount; i++) { + // If we removed all of the flow out of 'block', distribute flow among the remaining edges evenly. const weight_t currLikelihood = succTab[i]->getLikelihood(); - const weight_t newLikelihood = currLikelihood / (1.0 - removedLikelihood); + const weight_t newLikelihood = + currLikelihood / ((removedLikelihood == 1.0) ? newSuccCount : (1.0 - removedLikelihood)); succTab[i]->setLikelihood(min(1.0, newLikelihood)); } @@ -438,10 +442,14 @@ void Compiler::fgRemoveEhfSuccessor(FlowEdge* succEdge) // Recompute the likelihoods of the block's other successor edges. const weight_t removedLikelihood = succEdge->getLikelihood(); - for (unsigned i = 0; (removedLikelihood != 1.0) && (i < (succCount - 1)); i++) + const unsigned newSuccCount = succCount - 1; + + for (unsigned i = 0; i < newSuccCount; i++) { + // If we removed all of the flow out of 'block', distribute flow among the remaining edges evenly. const weight_t currLikelihood = succTab[i]->getLikelihood(); - const weight_t newLikelihood = currLikelihood / (1.0 - removedLikelihood); + const weight_t newLikelihood = + currLikelihood / ((removedLikelihood == 1.0) ? newSuccCount : (1.0 - removedLikelihood)); succTab[i]->setLikelihood(min(1.0, newLikelihood)); } From 25dec79d1e2c4c307af1fa5f0eaa5ce0e8581eed Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Wed, 2 Apr 2025 18:13:40 -0400 Subject: [PATCH 9/9] Feedback --- src/coreclr/jit/fgbasic.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/fgbasic.cpp b/src/coreclr/jit/fgbasic.cpp index cb9319f0c45c5b..7c11369d3a53f5 100644 --- a/src/coreclr/jit/fgbasic.cpp +++ b/src/coreclr/jit/fgbasic.cpp @@ -377,7 +377,7 @@ void Compiler::fgRemoveEhfSuccessor(BasicBlock* block, const unsigned succIndex) // If we removed all of the flow out of 'block', distribute flow among the remaining edges evenly. const weight_t currLikelihood = succTab[i]->getLikelihood(); const weight_t newLikelihood = - currLikelihood / ((removedLikelihood == 1.0) ? newSuccCount : (1.0 - removedLikelihood)); + (removedLikelihood == 1.0) ? (1.0 / newSuccCount) : (currLikelihood / (1.0 - removedLikelihood)); succTab[i]->setLikelihood(min(1.0, newLikelihood)); } @@ -449,7 +449,7 @@ void Compiler::fgRemoveEhfSuccessor(FlowEdge* succEdge) // If we removed all of the flow out of 'block', distribute flow among the remaining edges evenly. const weight_t currLikelihood = succTab[i]->getLikelihood(); const weight_t newLikelihood = - currLikelihood / ((removedLikelihood == 1.0) ? newSuccCount : (1.0 - removedLikelihood)); + (removedLikelihood == 1.0) ? (1.0 / newSuccCount) : (currLikelihood / (1.0 - removedLikelihood)); succTab[i]->setLikelihood(min(1.0, newLikelihood)); }