diff --git a/src/coreclr/jit/block.h b/src/coreclr/jit/block.h index 9fa0bcf0f8d73..3556087253edb 100644 --- a/src/coreclr/jit/block.h +++ b/src/coreclr/jit/block.h @@ -2070,9 +2070,8 @@ struct FlowEdge // The count of duplicate "edges" (used for switch stmts or degenerate branches) unsigned m_dupCount; -#ifdef DEBUG + // True if likelihood has been set bool m_likelihoodSet; -#endif public: FlowEdge(BasicBlock* block, FlowEdge* rest) @@ -2081,7 +2080,8 @@ struct FlowEdge , m_edgeWeightMin(0) , m_edgeWeightMax(0) , m_likelihood(0) - , m_dupCount(0) DEBUGARG(m_likelihoodSet(false)) + , m_dupCount(0) + , m_likelihoodSet(false) { } @@ -2137,22 +2137,20 @@ struct FlowEdge { assert(likelihood >= 0.0); assert(likelihood <= 1.0); - INDEBUG(m_likelihoodSet = true); - m_likelihood = likelihood; + m_likelihoodSet = true; + m_likelihood = likelihood; } void clearLikelihood() { - m_likelihood = 0.0; - INDEBUG(m_likelihoodSet = false); + m_likelihood = 0.0; + m_likelihoodSet = false; } -#ifdef DEBUG bool hasLikelihood() const { return m_likelihoodSet; } -#endif weight_t getLikelyWeight() const { diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index f0d8e0e46fd72..5fac5c8e0704c 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/src/coreclr/jit/compiler.cpp @@ -4503,7 +4503,6 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl // activePhaseChecks |= PhaseChecks::CHECK_PROFILE; DoPhase(this, PHASE_INCPROFILE, &Compiler::fgIncorporateProfileData); - activePhaseChecks &= ~PhaseChecks::CHECK_PROFILE; // If we are doing OSR, update flow to initially reach the appropriate IL offset. // @@ -4606,6 +4605,11 @@ 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); diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index cfcd7dc31b968..4bf627cae9517 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -1580,9 +1580,10 @@ enum class ProfileChecks : unsigned int { CHECK_NONE = 0, CHECK_CLASSIC = 1 << 0, // check "classic" jit weights - CHECK_LIKELY = 1 << 1, // check likelihood based weights - RAISE_ASSERT = 1 << 2, // assert on check failure - CHECK_ALL_BLOCKS = 1 << 3, // check blocks even if bbHasProfileWeight is false + CHECK_HASLIKELIHOOD = 1 << 1, // check all FlowEdges for hasLikelihood + CHECK_LIKELY = 1 << 2, // fully check likelihood based weights + RAISE_ASSERT = 1 << 3, // assert on check failure + CHECK_ALL_BLOCKS = 1 << 4, // check blocks even if bbHasProfileWeight is false }; inline constexpr ProfileChecks operator ~(ProfileChecks a) diff --git a/src/coreclr/jit/fgbasic.cpp b/src/coreclr/jit/fgbasic.cpp index 6301f87be3d8c..c8a142f07b7a6 100644 --- a/src/coreclr/jit/fgbasic.cpp +++ b/src/coreclr/jit/fgbasic.cpp @@ -390,17 +390,29 @@ void Compiler::fgChangeSwitchBlock(BasicBlock* oldSwitchBlock, BasicBlock* newSw // fgRemoveRefPred()/fgAddRefPred() will do the right thing: the second and // subsequent duplicates will simply subtract from and add to the duplicate // count (respectively). - + // + // However this does the "wrong" thing with respect to edge profile + // data; the old edge is not returned by fgRemoveRefPred until it has + // a dup count of 0, and the fgAddRefPred only uses the optional + // old edge arg when the new edge is first created. // // Remove the old edge [oldSwitchBlock => bJump] // assert(bJump->countOfInEdges() > 0); - fgRemoveRefPred(bJump, oldSwitchBlock); + FlowEdge* const oldEdge = fgRemoveRefPred(bJump, oldSwitchBlock); // // Create the new edge [newSwitchBlock => bJump] // - fgAddRefPred(bJump, newSwitchBlock); + FlowEdge* const newEdge = fgAddRefPred(bJump, newSwitchBlock); + + // Handle the profile update, once we get our hands on the old edge. + // + if (oldEdge != nullptr) + { + assert(!newEdge->hasLikelihood()); + newEdge->setLikelihood(oldEdge->getLikelihood()); + } } if (m_switchDescMap != nullptr) @@ -608,10 +620,13 @@ void Compiler::fgReplaceJumpTarget(BasicBlock* block, BasicBlock* oldTarget, Bas case BBJ_EHCATCHRET: case BBJ_EHFILTERRET: case BBJ_LEAVE: // This function can be called before import, so we still have BBJ_LEAVE - assert(block->TargetIs(oldTarget)); - block->SetTarget(newTarget); - fgRemoveRefPred(oldTarget, block); - fgAddRefPred(newTarget, block); + + if (block->TargetIs(oldTarget)) + { + block->SetTarget(newTarget); + FlowEdge* const oldEdge = fgRemoveRefPred(oldTarget, block); + fgAddRefPred(newTarget, block, oldEdge); + } break; case BBJ_COND: @@ -637,7 +652,15 @@ void Compiler::fgReplaceJumpTarget(BasicBlock* block, BasicBlock* oldTarget, Bas // TODO-NoFallThrough: Proliferate weight from oldEdge // (as a quirk, we avoid doing so for the true target to reduce diffs for now) - fgAddRefPred(newTarget, block); + FlowEdge* const newEdge = fgAddRefPred(newTarget, block); + if (block->KindIs(BBJ_ALWAYS)) + { + newEdge->setLikelihood(1.0); + } + else if (oldEdge->hasLikelihood()) + { + newEdge->setLikelihood(oldEdge->getLikelihood()); + } } else { @@ -661,10 +684,21 @@ void Compiler::fgReplaceJumpTarget(BasicBlock* block, BasicBlock* oldTarget, Bas { if (jumpTab[i] == oldTarget) { - jumpTab[i] = newTarget; - changed = true; - fgRemoveRefPred(oldTarget, block); - fgAddRefPred(newTarget, block); + jumpTab[i] = newTarget; + changed = true; + FlowEdge* const oldEdge = fgRemoveRefPred(oldTarget, block); + FlowEdge* const newEdge = fgAddRefPred(newTarget, block, oldEdge); + + // Handle the profile update, once we get our hands on the old edge. + // (see notes in fgChangeSwitchBlock for why this extra step is necessary) + // + // We do it slightly differently here so we don't lose the old + // edge weight propagation that would sometimes happen + // + if ((oldEdge != nullptr) && !newEdge->hasLikelihood()) + { + newEdge->setLikelihood(oldEdge->getLikelihood()); + } } } @@ -4739,7 +4773,8 @@ BasicBlock* Compiler::fgSplitBlockAtEnd(BasicBlock* curr) curr->SetFlags(BBF_NONE_QUIRK); assert(curr->JumpsToNext()); - fgAddRefPred(newBlock, curr); + FlowEdge* const newEdge = fgAddRefPred(newBlock, curr); + newEdge->setLikelihood(1.0); return newBlock; } @@ -4981,12 +5016,15 @@ BasicBlock* Compiler::fgSplitEdge(BasicBlock* curr, BasicBlock* succ) // newBlock replaces succ as curr's successor. fgReplaceJumpTarget(curr, succ, newBlock); - // And succ has newBlock as a new predecessor. - fgAddRefPred(succ, newBlock); + // And 'succ' has 'newBlock' as a new predecessor. + FlowEdge* const newEdge = fgAddRefPred(succ, newBlock); + newEdge->setLikelihood(1.0); // This isn't accurate, but it is complex to compute a reasonable number so just assume that we take the // branch 50% of the time. // + // TODO: leverage edge likelihood. + // if (!curr->KindIs(BBJ_ALWAYS)) { newBlock->inheritWeightPercentage(curr, 50); diff --git a/src/coreclr/jit/fgflow.cpp b/src/coreclr/jit/fgflow.cpp index 5f6a754a99416..1321977e1fd07 100644 --- a/src/coreclr/jit/fgflow.cpp +++ b/src/coreclr/jit/fgflow.cpp @@ -187,6 +187,13 @@ FlowEdge* Compiler::fgAddRefPred(BasicBlock* block, BasicBlock* blockPred, FlowE block->bbLastPred = flow; } + // Copy likelihood from old edge, if any. + // + if ((oldEdge != nullptr) && oldEdge->hasLikelihood()) + { + flow->setLikelihood(oldEdge->getLikelihood()); + } + if (fgHaveValidEdgeWeights) { // We are creating an edge from blockPred to block diff --git a/src/coreclr/jit/fginline.cpp b/src/coreclr/jit/fginline.cpp index 4a4093492179d..54003637e7e36 100644 --- a/src/coreclr/jit/fginline.cpp +++ b/src/coreclr/jit/fginline.cpp @@ -1534,7 +1534,8 @@ void Compiler::fgInsertInlineeBlocks(InlineInfo* pInlineInfo) bottomBlock->bbNum); block->SetKindAndTarget(BBJ_ALWAYS, bottomBlock); - fgAddRefPred(bottomBlock, block); + FlowEdge* const newEdge = fgAddRefPred(bottomBlock, block); + newEdge->setLikelihood(1.0); if (block == InlineeCompiler->fgLastBB) { @@ -1553,8 +1554,8 @@ void Compiler::fgInsertInlineeBlocks(InlineInfo* pInlineInfo) topBlock->SetNext(InlineeCompiler->fgFirstBB); topBlock->SetTarget(topBlock->Next()); topBlock->SetFlags(BBF_NONE_QUIRK); - fgRemoveRefPred(bottomBlock, topBlock); - fgAddRefPred(InlineeCompiler->fgFirstBB, topBlock); + FlowEdge* const oldEdge = fgRemoveRefPred(bottomBlock, topBlock); + fgAddRefPred(InlineeCompiler->fgFirstBB, topBlock, oldEdge); InlineeCompiler->fgLastBB->SetNext(bottomBlock); // diff --git a/src/coreclr/jit/fgopt.cpp b/src/coreclr/jit/fgopt.cpp index fa3372e89bfb8..acce240204476 100644 --- a/src/coreclr/jit/fgopt.cpp +++ b/src/coreclr/jit/fgopt.cpp @@ -647,7 +647,8 @@ PhaseStatus Compiler::fgPostImportationCleanup() } else { - fgAddRefPred(newTryEntry->Next(), newTryEntry); + FlowEdge* const newEdge = fgAddRefPred(newTryEntry->Next(), newTryEntry); + newEdge->setLikelihood(1.0); } JITDUMP("OSR: changing start of try region #%u from " FMT_BB " to new " FMT_BB "\n", @@ -773,6 +774,7 @@ PhaseStatus Compiler::fgPostImportationCleanup() fromBlock->SetFlags(BBF_INTERNAL); newBlock->RemoveFlags(BBF_DONT_REMOVE); addedBlocks++; + FlowEdge* const normalTryEntryEdge = fgGetPredForBlock(newBlock, fromBlock); GenTree* const entryStateLcl = gtNewLclvNode(entryStateVar, TYP_INT); GenTree* const compareEntryStateToZero = @@ -781,9 +783,17 @@ PhaseStatus Compiler::fgPostImportationCleanup() fgNewStmtAtBeg(fromBlock, jumpIfEntryStateZero); fromBlock->SetCond(toBlock, newBlock); - fgAddRefPred(toBlock, fromBlock); + FlowEdge* const osrTryEntryEdge = fgAddRefPred(toBlock, fromBlock); newBlock->inheritWeight(fromBlock); + // Not sure what the correct edge likelihoods are just yet; + // for now we'll say the OSR path is the likely one. + // + // Todo: can we leverage profile data here to get a better answer? + // + osrTryEntryEdge->setLikelihood(0.9); + normalTryEntryEdge->setLikelihood(0.1); + entryJumpTarget = fromBlock; }; @@ -824,8 +834,8 @@ PhaseStatus Compiler::fgPostImportationCleanup() if (entryJumpTarget != osrEntry) { fgFirstBB->SetTarget(entryJumpTarget); - fgRemoveRefPred(osrEntry, fgFirstBB); - fgAddRefPred(entryJumpTarget, fgFirstBB); + FlowEdge* const oldEdge = fgRemoveRefPred(osrEntry, fgFirstBB); + fgAddRefPred(entryJumpTarget, fgFirstBB, oldEdge); JITDUMP("OSR: redirecting flow from method entry " FMT_BB " to OSR entry " FMT_BB " via step blocks.\n", diff --git a/src/coreclr/jit/fgprofile.cpp b/src/coreclr/jit/fgprofile.cpp index 4508f3c34d168..3e636b4d77c19 100644 --- a/src/coreclr/jit/fgprofile.cpp +++ b/src/coreclr/jit/fgprofile.cpp @@ -511,7 +511,8 @@ void BlockCountInstrumentor::RelocateProbes() m_comp->fgNewBBbefore(BBJ_ALWAYS, block, /* extendRegion */ true, /* jumpDest */ block); intermediary->SetFlags(BBF_IMPORTED | BBF_MARKED | BBF_NONE_QUIRK); intermediary->inheritWeight(block); - m_comp->fgAddRefPred(block, intermediary); + FlowEdge* const newEdge = m_comp->fgAddRefPred(block, intermediary); + newEdge->setLikelihood(1.0); SetModifiedFlow(); while (criticalPreds.Height() > 0) @@ -1682,7 +1683,8 @@ void EfficientEdgeCountInstrumentor::RelocateProbes() m_comp->fgNewBBbefore(BBJ_ALWAYS, block, /* extendRegion */ true, /* jumpDest */ block); intermediary->SetFlags(BBF_IMPORTED | BBF_NONE_QUIRK); intermediary->inheritWeight(block); - m_comp->fgAddRefPred(block, intermediary); + FlowEdge* const newEdge = m_comp->fgAddRefPred(block, intermediary); + newEdge->setLikelihood(1.0); NewRelocatedProbe(intermediary, probe->source, probe->target, &leader); SetModifiedFlow(); @@ -2798,6 +2800,7 @@ PhaseStatus Compiler::fgIncorporateProfileData() JITDUMP("JitStress -- incorporating random profile data\n"); fgIncorporateBlockCounts(); fgApplyProfileScale(); + ProfileSynthesis::Run(this, ProfileSynthesisOption::RepairLikelihoods); return PhaseStatus::MODIFIED_EVERYTHING; } @@ -4065,7 +4068,6 @@ void EfficientEdgeCountReconstructor::PropagateEdges(BasicBlock* block, BlockInf assert(block == edge->m_sourceBlock); FlowEdge* const flowEdge = m_comp->fgGetPredForBlock(edge->m_targetBlock, block); assert(flowEdge != nullptr); - assert(!flowEdge->hasLikelihood()); weight_t likelihood = 0; if (nEdges == 1) @@ -5274,20 +5276,22 @@ bool Compiler::fgProfileWeightsConsistent(weight_t weight1, weight_t weight2) // (or nearly so) // // Notes: -// Does nothing, if profile checks are disabled, or there are -// no profile weights or pred lists. +// By default, just checks for each flow edge having likelihood. +// Can be altered via external config. // void Compiler::fgDebugCheckProfileWeights() { - // Optionally check profile data, if we have any. - // - const bool enabled = (JitConfig.JitProfileChecks() > 0) && fgHaveProfileWeights() && fgPredsComputed; - if (!enabled) + const bool configEnabled = (JitConfig.JitProfileChecks() >= 0) && fgHaveProfileWeights() && fgPredsComputed; + + if (configEnabled) { - return; + fgDebugCheckProfileWeights((ProfileChecks)JitConfig.JitProfileChecks()); + } + else + { + ProfileChecks checks = ProfileChecks::CHECK_HASLIKELIHOOD | ProfileChecks::RAISE_ASSERT; + fgDebugCheckProfileWeights(checks); } - - fgDebugCheckProfileWeights((ProfileChecks)JitConfig.JitProfileChecks()); } //------------------------------------------------------------------------ @@ -5312,19 +5316,21 @@ void Compiler::fgDebugCheckProfileWeights(ProfileChecks checks) { // We can check classic (min/max, late computed) weights // and/or - // new likelyhood based weights. + // new likelihood based weights. // const bool verifyClassicWeights = fgEdgeWeightsComputed && hasFlag(checks, ProfileChecks::CHECK_CLASSIC); const bool verifyLikelyWeights = hasFlag(checks, ProfileChecks::CHECK_LIKELY); + const bool verifyHasLikelihood = hasFlag(checks, ProfileChecks::CHECK_HASLIKELIHOOD); const bool assertOnFailure = hasFlag(checks, ProfileChecks::RAISE_ASSERT); const bool checkAllBlocks = hasFlag(checks, ProfileChecks::CHECK_ALL_BLOCKS); - if (!(verifyClassicWeights || verifyLikelyWeights)) + if (!(verifyClassicWeights || verifyLikelyWeights || verifyHasLikelihood)) { + JITDUMP("[profile weight checks disabled]\n"); return; } - JITDUMP("Checking Profile Data\n"); + JITDUMP("Checking Profile Weights (flags:0x%x)\n", checks); unsigned problemBlocks = 0; unsigned unprofiledBlocks = 0; unsigned profiledBlocks = 0; @@ -5434,22 +5440,25 @@ void Compiler::fgDebugCheckProfileWeights(ProfileChecks checks) // Verify overall input-output balance. // - if (entryProfiled && exitProfiled) + if (verifyClassicWeights || verifyLikelyWeights) { - // Note these may not agree, if fgEntryBB is a loop header. - // - if (fgFirstBB->bbRefs > 1) - { - JITDUMP(" Method entry " FMT_BB " is loop head, can't check entry/exit balance\n"); - } - else if (!fgProfileWeightsConsistent(entryWeight, exitWeight)) + if (entryProfiled && exitProfiled) { - problemBlocks++; - JITDUMP(" Method entry " FMT_WT " method exit " FMT_WT " weight mismatch\n", entryWeight, exitWeight); + // Note these may not agree, if fgEntryBB is a loop header. + // + if (fgFirstBB->bbRefs > 1) + { + JITDUMP(" Method entry " FMT_BB " is loop head, can't check entry/exit balance\n"); + } + else if (!fgProfileWeightsConsistent(entryWeight, exitWeight)) + { + problemBlocks++; + JITDUMP(" Method entry " FMT_WT " method exit " FMT_WT " weight mismatch\n", entryWeight, exitWeight); + } } } - // Sum up what we discovered. + // Summarize what we discovered. // if (problemBlocks == 0) { @@ -5457,11 +5466,15 @@ void Compiler::fgDebugCheckProfileWeights(ProfileChecks checks) { JITDUMP("No blocks were profiled, so nothing to check\n"); } - else + else if (verifyClassicWeights || verifyLikelyWeights) { JITDUMP("Profile is self-consistent (%d profiled blocks, %d unprofiled)\n", profiledBlocks, unprofiledBlocks); } + else if (verifyHasLikelihood) + { + JITDUMP("All flow edges have likelihoods\n"); + } } else { @@ -5493,8 +5506,9 @@ bool Compiler::fgDebugCheckIncomingProfileData(BasicBlock* block, ProfileChecks { const bool verifyClassicWeights = fgEdgeWeightsComputed && hasFlag(checks, ProfileChecks::CHECK_CLASSIC); const bool verifyLikelyWeights = hasFlag(checks, ProfileChecks::CHECK_LIKELY); + const bool verifyHasLikelihood = hasFlag(checks, ProfileChecks::CHECK_HASLIKELIHOOD); - if (!(verifyClassicWeights || verifyLikelyWeights)) + if (!(verifyClassicWeights || verifyLikelyWeights || verifyHasLikelihood)) { return true; } @@ -5519,6 +5533,8 @@ bool Compiler::fgDebugCheckIncomingProfileData(BasicBlock* block, ProfileChecks } else { + JITDUMP("Missing likelihood on %p " FMT_BB "->" FMT_BB "\n", predEdge, predEdge->getSourceBlock()->bbNum, + block->bbNum); missingLikelyWeight++; } @@ -5562,7 +5578,10 @@ bool Compiler::fgDebugCheckIncomingProfileData(BasicBlock* block, ProfileChecks block->bbNum, blockWeight, incomingLikelyWeight); likelyWeightsValid = false; } + } + if (verifyHasLikelihood) + { if (missingLikelyWeight > 0) { JITDUMP(" " FMT_BB " -- %u incoming edges are missing likely weights\n", block->bbNum, @@ -5593,8 +5612,9 @@ bool Compiler::fgDebugCheckOutgoingProfileData(BasicBlock* block, ProfileChecks { const bool verifyClassicWeights = fgEdgeWeightsComputed && hasFlag(checks, ProfileChecks::CHECK_CLASSIC); const bool verifyLikelyWeights = hasFlag(checks, ProfileChecks::CHECK_LIKELY); + const bool verifyHasLikelihood = hasFlag(checks, ProfileChecks::CHECK_HASLIKELIHOOD); - if (!(verifyClassicWeights || verifyLikelyWeights)) + if (!(verifyClassicWeights || verifyLikelyWeights || verifyHasLikelihood)) { return true; } @@ -5639,6 +5659,7 @@ bool Compiler::fgDebugCheckOutgoingProfileData(BasicBlock* block, ProfileChecks } else { + JITDUMP("Missing likelihood on %p " FMT_BB "->" FMT_BB "\n", succEdge, block->bbNum, succBlock->bbNum); missingLikelihood++; } } @@ -5674,14 +5695,17 @@ bool Compiler::fgDebugCheckOutgoingProfileData(BasicBlock* block, ProfileChecks } } - if (verifyLikelyWeights) + if (verifyHasLikelihood) { if (missingLikelihood > 0) { JITDUMP(" " FMT_BB " - missing likelihood on %d successor edges\n", block->bbNum, missingLikelihood); likelyWeightsValid = false; } + } + if (verifyLikelyWeights) + { if (!fgProfileWeightsConsistent(outgoingLikelihood, 1.0)) { JITDUMP(" " FMT_BB " - outgoing likelihood " FMT_WT " should be 1.0\n", block->bbNum, diff --git a/src/coreclr/jit/fgprofilesynthesis.cpp b/src/coreclr/jit/fgprofilesynthesis.cpp index b7af7de958555..e315e33015e13 100644 --- a/src/coreclr/jit/fgprofilesynthesis.cpp +++ b/src/coreclr/jit/fgprofilesynthesis.cpp @@ -16,7 +16,6 @@ // * IR based heuristics (perhaps) // * During Cp, avoid repeatedly propagating through nested loops // * Fake BB0 or always force scratch BB -// * Reconcile with our other loop finding stuff // * Stop the upweight/downweight of loops in rest of jit // * Durable edge properties (exit, back) // * Tweak RunRarely to be at or near zero @@ -649,12 +648,6 @@ void ProfileSynthesis::RandomizeLikelihoods() for (BasicBlock* const block : m_comp->Blocks()) { unsigned const N = block->NumSucc(m_comp); - - if (N < 2) - { - continue; - } - likelihoods.clear(); likelihoods.reserve(N); diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp index 9ee019f7822ce..2291139538e12 100644 --- a/src/coreclr/jit/flowgraph.cpp +++ b/src/coreclr/jit/flowgraph.cpp @@ -1626,7 +1626,8 @@ void Compiler::fgConvertSyncReturnToLeave(BasicBlock* block) // Convert the BBJ_RETURN to BBJ_ALWAYS, jumping to genReturnBB. block->SetKindAndTarget(BBJ_ALWAYS, genReturnBB); - fgAddRefPred(genReturnBB, block); + FlowEdge* const newEdge = fgAddRefPred(genReturnBB, block); + newEdge->setLikelihood(1.0); #ifdef DEBUG if (verbose) @@ -2097,7 +2098,8 @@ class MergedReturns // Change BBJ_RETURN to BBJ_ALWAYS targeting const return block. assert((comp->info.compFlags & CORINFO_FLG_SYNCH) == 0); returnBlock->SetKindAndTarget(BBJ_ALWAYS, constReturnBlock); - comp->fgAddRefPred(constReturnBlock, returnBlock); + FlowEdge* const newEdge = comp->fgAddRefPred(constReturnBlock, returnBlock); + newEdge->setLikelihood(1.0); // Remove GT_RETURN since constReturnBlock returns the constant. assert(returnBlock->lastStmt()->GetRootNode()->OperIs(GT_RETURN)); diff --git a/src/coreclr/jit/importer.cpp b/src/coreclr/jit/importer.cpp index a5901e40d721b..69d623787fa35 100644 --- a/src/coreclr/jit/importer.cpp +++ b/src/coreclr/jit/importer.cpp @@ -2026,7 +2026,8 @@ BasicBlock* Compiler::impPushCatchArgOnStack(BasicBlock* hndBlk, CORINFO_CLASS_H newBlk->inheritWeight(hndBlk); newBlk->bbCodeOffs = hndBlk->bbCodeOffs; - fgAddRefPred(hndBlk, newBlk); + FlowEdge* const newEdge = fgAddRefPred(hndBlk, newBlk); + newEdge->setLikelihood(1.0); // Spill into a temp. unsigned tempNum = lvaGrabTemp(false DEBUGARG("SpillCatchArg")); @@ -4437,7 +4438,8 @@ void Compiler::impImportLeave(BasicBlock* block) // the previous call to a finally returns to this call (to the next finally in the chain) step->SetTarget(callBlock); - fgAddRefPred(callBlock, step); + FlowEdge* const newEdge = fgAddRefPred(callBlock, step); + newEdge->setLikelihood(1.0); // The new block will inherit this block's weight. callBlock->inheritWeight(block); @@ -4487,7 +4489,8 @@ void Compiler::impImportLeave(BasicBlock* block) assert(callBlock->KindIs(BBJ_CALLFINALLY)); assert(callBlock->TargetIs(HBtab->ebdHndBeg)); - fgAddRefPred(callBlock->GetTarget(), callBlock); + FlowEdge* const newEdge = fgAddRefPred(callBlock->GetTarget(), callBlock); + newEdge->setLikelihood(1.0); GenTree* endLFin = new (this, GT_END_LFIN) GenTreeVal(GT_END_LFIN, TYP_VOID, finallyNesting); endLFinStmt = gtNewStmt(endLFin); @@ -4537,7 +4540,10 @@ void Compiler::impImportLeave(BasicBlock* block) assert(!step->HasInitializedTarget()); step->SetTarget(finalStep); - fgAddRefPred(finalStep, step); + { + FlowEdge* const newEdge = fgAddRefPred(finalStep, step); + newEdge->setLikelihood(1.0); + } // The new block will inherit this block's weight. finalStep->inheritWeight(block); @@ -4566,7 +4572,10 @@ void Compiler::impImportLeave(BasicBlock* block) impEndTreeList(finalStep, endLFinStmt, lastStmt); // this is the ultimate destination of the LEAVE - fgAddRefPred(leaveTarget, finalStep); + { + FlowEdge* const newEdge = fgAddRefPred(leaveTarget, finalStep); + newEdge->setLikelihood(1.0); + } // Queue up the jump target for importing @@ -4684,7 +4693,8 @@ void Compiler::impImportLeave(BasicBlock* block) } step->SetTarget(exitBlock); // the previous step (maybe a call to a nested finally, or a nested catch // exit) returns to this block - fgAddRefPred(exitBlock, step); + FlowEdge* const newEdge = fgAddRefPred(exitBlock, step); + newEdge->setLikelihood(1.0); // The new block will inherit this block's weight. exitBlock->inheritWeight(block); @@ -4728,7 +4738,8 @@ void Compiler::impImportLeave(BasicBlock* block) // next block, and flow optimizations will remove it. fgRemoveRefPred(block->GetTarget(), block); block->SetKindAndTarget(BBJ_ALWAYS, callBlock); - fgAddRefPred(callBlock, block); + FlowEdge* const newEdge = fgAddRefPred(callBlock, block); + newEdge->setLikelihood(1.0); // The new block will inherit this block's weight. callBlock->inheritWeight(block); @@ -4797,7 +4808,8 @@ void Compiler::impImportLeave(BasicBlock* block) fgRemoveRefPred(step->GetTarget(), step); } step->SetTarget(step2); - fgAddRefPred(step2, step); + FlowEdge* const newEdge = fgAddRefPred(step2, step); + newEdge->setLikelihood(1.0); step2->inheritWeight(block); step2->CopyFlags(block, BBF_RUN_RARELY); step2->SetFlags(BBF_IMPORTED); @@ -4838,7 +4850,8 @@ void Compiler::impImportLeave(BasicBlock* block) } step->SetTarget(callBlock); // the previous call to a finally returns to this call (to the next // finally in the chain) - fgAddRefPred(callBlock, step); + FlowEdge* const newEdge = fgAddRefPred(callBlock, step); + newEdge->setLikelihood(1.0); // The new block will inherit this block's weight. callBlock->inheritWeight(block); @@ -4874,7 +4887,8 @@ void Compiler::impImportLeave(BasicBlock* block) assert(callBlock->KindIs(BBJ_CALLFINALLY)); assert(callBlock->TargetIs(HBtab->ebdHndBeg)); - fgAddRefPred(callBlock->GetTarget(), callBlock); + FlowEdge* const newEdge = fgAddRefPred(callBlock->GetTarget(), callBlock); + newEdge->setLikelihood(1.0); } else if (HBtab->HasCatchHandler() && jitIsBetween(blkAddr, tryBeg, tryEnd) && !jitIsBetween(jmpAddr, tryBeg, tryEnd)) @@ -4941,7 +4955,8 @@ void Compiler::impImportLeave(BasicBlock* block) fgRemoveRefPred(step->GetTarget(), step); } step->SetTarget(catchStep); - fgAddRefPred(catchStep, step); + FlowEdge* const newEdge = fgAddRefPred(catchStep, step); + newEdge->setLikelihood(1.0); // The new block will inherit this block's weight. catchStep->inheritWeight(block); @@ -4995,7 +5010,8 @@ void Compiler::impImportLeave(BasicBlock* block) fgRemoveRefPred(step->GetTarget(), step); } step->SetTarget(leaveTarget); // this is the ultimate destination of the LEAVE - fgAddRefPred(leaveTarget, step); + FlowEdge* const newEdge = fgAddRefPred(leaveTarget, step); + newEdge->setLikelihood(1.0); #ifdef DEBUG if (verbose) @@ -5056,7 +5072,8 @@ void Compiler::impResetLeaveBlock(BasicBlock* block, unsigned jmpAddr) { BasicBlock* dupBlock = BasicBlock::New(this, BBJ_CALLFINALLY, block->GetTarget()); dupBlock->CopyFlags(block); - fgAddRefPred(dupBlock->GetTarget(), dupBlock); + FlowEdge* const newEdge = fgAddRefPred(dupBlock->GetTarget(), dupBlock); + newEdge->setLikelihood(1.0); dupBlock->copyEHRegion(block); dupBlock->bbCatchTyp = block->bbCatchTyp; @@ -5087,7 +5104,8 @@ void Compiler::impResetLeaveBlock(BasicBlock* block, unsigned jmpAddr) fgRemoveRefPred(block->GetTarget(), block); block->SetKindAndTarget(BBJ_LEAVE, fgLookupBB(jmpAddr)); - fgAddRefPred(block->GetTarget(), block); + FlowEdge* const newEdge = fgAddRefPred(block->GetTarget(), block); + newEdge->setLikelihood(1.0); // We will leave the BBJ_ALWAYS block we introduced. When it's reimported // the BBJ_ALWAYS block will be unreachable, and will be removed after. The @@ -12244,7 +12262,8 @@ void Compiler::impFixPredLists() } BasicBlock* const continuation = predBlock->Next(); - fgAddRefPred(continuation, finallyBlock); + FlowEdge* const newEdge = fgAddRefPred(continuation, finallyBlock); + newEdge->setLikelihood(1.0); jumpEhf->bbeSuccs[predNum] = continuation; ++predNum; diff --git a/src/coreclr/jit/indirectcalltransformer.cpp b/src/coreclr/jit/indirectcalltransformer.cpp index 479b1385f773c..fbbe08d66fc3c 100644 --- a/src/coreclr/jit/indirectcalltransformer.cpp +++ b/src/coreclr/jit/indirectcalltransformer.cpp @@ -273,21 +273,32 @@ class IndirectCallTransformer { assert(currBlock->KindIs(BBJ_ALWAYS)); currBlock->SetTarget(checkBlock); - compiler->fgAddRefPred(checkBlock, currBlock); + FlowEdge* const newEdge = compiler->fgAddRefPred(checkBlock, currBlock); + newEdge->setLikelihood(1.0); } // checkBlock + // Todo: get likelihoods right + // assert(checkBlock->KindIs(BBJ_ALWAYS)); checkBlock->SetCond(elseBlock, thenBlock); - compiler->fgAddRefPred(elseBlock, checkBlock); - compiler->fgAddRefPred(thenBlock, checkBlock); + FlowEdge* const thenEdge = compiler->fgAddRefPred(thenBlock, checkBlock); + thenEdge->setLikelihood(0.5); + FlowEdge* const elseEdge = compiler->fgAddRefPred(elseBlock, checkBlock); + elseEdge->setLikelihood(0.5); // thenBlock assert(thenBlock->TargetIs(remainderBlock)); - compiler->fgAddRefPred(remainderBlock, thenBlock); + { + FlowEdge* const newEdge = compiler->fgAddRefPred(remainderBlock, thenBlock); + newEdge->setLikelihood(1.0); + } // elseBlock - compiler->fgAddRefPred(remainderBlock, elseBlock); + { + FlowEdge* const newEdge = compiler->fgAddRefPred(remainderBlock, elseBlock); + newEdge->setLikelihood(1.0); + } } Compiler* compiler; @@ -590,10 +601,6 @@ class IndirectCallTransformer checkBlock = CreateAndInsertBasicBlock(BBJ_ALWAYS, thenBlock); checkFallsThrough = false; - // prevCheckBlock is expected to jump to this new check (if its type check doesn't succeed) - prevCheckBlock->SetCond(checkBlock, prevCheckBlock->Next()); - compiler->fgAddRefPred(checkBlock, prevCheckBlock); - // Calculate the total likelihood for this check as a sum of likelihoods // of all previous candidates (thenBlocks) unsigned checkLikelihood = 100; @@ -604,7 +611,12 @@ class IndirectCallTransformer // Make sure we didn't overflow assert(checkLikelihood <= 100); + weight_t checkLikelihoodWt = ((weight_t)checkLikelihood) / 100.0; + // prevCheckBlock is expected to jump to this new check (if its type check doesn't succeed) + prevCheckBlock->SetCond(checkBlock, prevCheckBlock->Next()); + FlowEdge* const checkEdge = compiler->fgAddRefPred(checkBlock, prevCheckBlock); + checkEdge->setLikelihood(checkLikelihoodWt); checkBlock->inheritWeightPercentage(currBlock, checkLikelihood); } @@ -1002,18 +1014,26 @@ class IndirectCallTransformer // virtual void CreateThen(uint8_t checkIdx) { + // Compute likelihoods + // + unsigned const thenLikelihood = origCall->GetGDVCandidateInfo(checkIdx)->likelihood; + weight_t thenLikelihoodWt = min(((weight_t)thenLikelihood) / 100.0, 100.0); + weight_t elseLikelihoodWt = max(1.0 - thenLikelihoodWt, 0.0); + // thenBlock always jumps to remainderBlock thenBlock = CreateAndInsertBasicBlock(BBJ_ALWAYS, checkBlock, remainderBlock); thenBlock->CopyFlags(currBlock, BBF_SPLIT_GAINED); - thenBlock->inheritWeightPercentage(currBlock, origCall->GetGDVCandidateInfo(checkIdx)->likelihood); + thenBlock->inheritWeightPercentage(currBlock, thenLikelihood); // Also, thenBlock has a single pred - last checkBlock assert(checkBlock->KindIs(BBJ_ALWAYS)); checkBlock->SetTarget(thenBlock); checkBlock->SetFlags(BBF_NONE_QUIRK); assert(checkBlock->JumpsToNext()); - compiler->fgAddRefPred(thenBlock, checkBlock); - compiler->fgAddRefPred(remainderBlock, thenBlock); + FlowEdge* const thenEdge = compiler->fgAddRefPred(thenBlock, checkBlock); + thenEdge->setLikelihood(thenLikelihoodWt); + FlowEdge* const elseEdge = compiler->fgAddRefPred(remainderBlock, thenBlock); + elseEdge->setLikelihood(elseLikelihoodWt); DevirtualizeCall(thenBlock, checkIdx); } @@ -1023,6 +1043,17 @@ class IndirectCallTransformer // virtual void CreateElse() { + // 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++) + { + elseLikelihood -= origCall->GetGDVCandidateInfo(i)->likelihood; + } + // Make sure it didn't overflow + assert(elseLikelihood <= 100); + weight_t elseLikelihoodDbl = ((weight_t)elseLikelihood) / 100.0; + elseBlock = CreateAndInsertBasicBlock(BBJ_ALWAYS, thenBlock, thenBlock->Next()); elseBlock->CopyFlags(currBlock, BBF_SPLIT_GAINED); elseBlock->SetFlags(BBF_NONE_QUIRK); @@ -1032,7 +1063,8 @@ class IndirectCallTransformer if (!checkFallsThrough) { checkBlock->SetCond(elseBlock, checkBlock->Next()); - compiler->fgAddRefPred(elseBlock, checkBlock); + FlowEdge* const checkEdge = compiler->fgAddRefPred(elseBlock, checkBlock); + checkEdge->setLikelihood(elseLikelihoodDbl); } else { @@ -1043,17 +1075,8 @@ class IndirectCallTransformer } // elseBlock always flows into remainderBlock - compiler->fgAddRefPred(remainderBlock, elseBlock); - - // 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++) - { - elseLikelihood -= origCall->GetGDVCandidateInfo(i)->likelihood; - } - // Make sure it didn't overflow - assert(elseLikelihood <= 100); + FlowEdge* const elseEdge = compiler->fgAddRefPred(remainderBlock, elseBlock); + elseEdge->setLikelihood(1.0); // Remove everything related to inlining from the original call origCall->ClearInlineInfo(); @@ -1153,9 +1176,9 @@ class IndirectCallTransformer // Finally, rewire the cold block to jump to the else block, // not fall through to the check block. // - compiler->fgRemoveRefPred(checkBlock, coldBlock); + FlowEdge* const oldEdge = compiler->fgRemoveRefPred(checkBlock, coldBlock); coldBlock->SetKindAndTarget(BBJ_ALWAYS, elseBlock); - compiler->fgAddRefPred(elseBlock, coldBlock); + compiler->fgAddRefPred(elseBlock, coldBlock, oldEdge); } // When the current candidate hads sufficiently high likelihood, scan diff --git a/src/coreclr/jit/jitconfigvalues.h b/src/coreclr/jit/jitconfigvalues.h index bdcace7841f47..abc510d967a80 100644 --- a/src/coreclr/jit/jitconfigvalues.h +++ b/src/coreclr/jit/jitconfigvalues.h @@ -144,8 +144,9 @@ CONFIG_INTEGER(JitPrintInlinedMethodsVerbose, W("JitPrintInlinedMethodsVerboseLe CONFIG_METHODSET(JitPrintInlinedMethods, W("JitPrintInlinedMethods")) CONFIG_METHODSET(JitPrintDevirtualizedMethods, W("JitPrintDevirtualizedMethods")) -CONFIG_INTEGER(JitProfileChecks, W("JitProfileChecks"), 0) // Bitflag: 0x1 check classic, 0x2 check likely, 0x4 enable - // asserts +// -1: just do internal checks +// Else bitflag: 0x1 check classic, 0x2 check likely, 0x4 enable asserts +CONFIG_INTEGER(JitProfileChecks, W("JitProfileChecks"), -1) CONFIG_INTEGER(JitRequired, W("JITRequired"), -1) CONFIG_INTEGER(JitRoundFloat, W("JITRoundFloat"), DEFAULT_ROUND_LEVEL) CONFIG_INTEGER(JitStackAllocToLocalSize, W("JitStackAllocToLocalSize"), DEFAULT_MAX_LOCALLOC_TO_LOCAL_SIZE) diff --git a/src/coreclr/jit/jiteh.cpp b/src/coreclr/jit/jiteh.cpp index 50ed909a570cc..a4eacd9069db4 100644 --- a/src/coreclr/jit/jiteh.cpp +++ b/src/coreclr/jit/jiteh.cpp @@ -1986,7 +1986,8 @@ bool Compiler::fgNormalizeEHCase1() // handler. BasicBlock* newHndStart = BasicBlock::New(this, BBJ_ALWAYS, handlerStart); fgInsertBBbefore(handlerStart, newHndStart); - fgAddRefPred(handlerStart, newHndStart); + FlowEdge* newEdge = fgAddRefPred(handlerStart, newHndStart); + newEdge->setLikelihood(1.0); // Handler begins have an extra implicit ref count. // BasicBlock::New has already handled this for newHndStart. @@ -2156,7 +2157,8 @@ bool Compiler::fgNormalizeEHCase2() BasicBlock* newTryStart = BasicBlock::New(this, BBJ_ALWAYS, insertBeforeBlk); newTryStart->bbRefs = 0; fgInsertBBbefore(insertBeforeBlk, newTryStart); - fgAddRefPred(insertBeforeBlk, newTryStart); + FlowEdge* const newEdge = fgAddRefPred(insertBeforeBlk, newTryStart); + newEdge->setLikelihood(1.0); // It's possible for a try to start at the beginning of a method. If so, we need // to adjust the implicit ref counts as we've just created a new first bb @@ -2372,7 +2374,8 @@ bool Compiler::fgCreateFiltersForGenericExceptions() // Insert it right before the handler (and make it a pred of the handler) fgInsertBBbefore(handlerBb, filterBb); - fgAddRefPred(handlerBb, filterBb); + FlowEdge* const newEdge = fgAddRefPred(handlerBb, filterBb); + newEdge->setLikelihood(1.0); fgNewStmtAtEnd(filterBb, retFilt, handlerBb->firstStmt()->GetDebugInfo()); filterBb->bbCatchTyp = BBCT_FILTER; @@ -2678,7 +2681,8 @@ bool Compiler::fgNormalizeEHCase3() newLast->bbCodeOffsEnd = newLast->bbCodeOffs; // code size = 0. TODO: use BAD_IL_OFFSET instead? newLast->inheritWeight(insertAfterBlk); newLast->SetFlags(BBF_INTERNAL | BBF_NONE_QUIRK); - fgAddRefPred(newLast, insertAfterBlk); + FlowEdge* const newEdge = fgAddRefPred(newLast, insertAfterBlk); + newEdge->setLikelihood(1.0); // Move the insert pointer. More enclosing equivalent 'last' blocks will be inserted after this. insertAfterBlk = newLast; @@ -4340,7 +4344,8 @@ void Compiler::fgExtendEHRegionBefore(BasicBlock* block) // Change the bbTarget for bFilterLast from the old first 'block' to the new first 'bPrev' fgRemoveRefPred(bFilterLast->GetTarget(), bFilterLast); bFilterLast->SetTarget(bPrev); - fgAddRefPred(bPrev, bFilterLast); + FlowEdge* const newEdge = fgAddRefPred(bPrev, bFilterLast); + newEdge->setLikelihood(1.0); } }