From 496992f0c5220810fb025eee660d9a07c7a1f055 Mon Sep 17 00:00:00 2001 From: Bruce Forstall Date: Wed, 13 Mar 2024 21:56:40 -0700 Subject: [PATCH] More fixes This includes various fixes that are being separately PR'ed: 1. https://github.com/dotnet/runtime/pull/99744: Introduce HandleKindDataIsInvariant helper 2. https://github.com/dotnet/runtime/pull/99743: Add basic support for TYP_MASK constants 3. https://github.com/dotnet/runtime/pull/99742: Fix problem with scaling general loop blocks; add dumpers 4. https://github.com/dotnet/runtime/pull/99740: Add edge likelihood dumping; fix one edge likelihood update case Also: 1. Add support for running JitOptRepeat under JitStress. This is still over-written by JitOptRepeat being forced on at 4 iterations. --- src/coreclr/jit/block.cpp | 65 +++++++++++++++-------- src/coreclr/jit/block.h | 29 ++++++++++- src/coreclr/jit/clrjit.natvis | 10 ++-- src/coreclr/jit/compiler.cpp | 44 ++++++++++++---- src/coreclr/jit/compiler.h | 25 +++++++-- src/coreclr/jit/fgbasic.cpp | 46 +++++++++++++++-- src/coreclr/jit/fgdiagnostic.cpp | 85 +++++++++++++++++++------------ src/coreclr/jit/fgopt.cpp | 23 ++++++++- src/coreclr/jit/fgprofile.cpp | 19 +++++++ src/coreclr/jit/flowgraph.cpp | 51 +++++++++++++++++++ src/coreclr/jit/jitconfigvalues.h | 15 ++++-- src/coreclr/jit/optimizer.cpp | 22 +++++--- 12 files changed, 341 insertions(+), 93 deletions(-) diff --git a/src/coreclr/jit/block.cpp b/src/coreclr/jit/block.cpp index ec81d028c43a52..1e7750997a103a 100644 --- a/src/coreclr/jit/block.cpp +++ b/src/coreclr/jit/block.cpp @@ -79,11 +79,20 @@ void FlowEdge::setLikelihood(weight_t likelihood) { assert(likelihood >= 0.0); assert(likelihood <= 1.0); + + if (m_likelihoodSet) + { + JITDUMP("setting likelihood of " FMT_BB " -> " FMT_BB " from " FMT_WT " to " FMT_WT "\n", m_sourceBlock->bbNum, + m_destBlock->bbNum, m_likelihood, likelihood); + } + else + { + JITDUMP("setting likelihood of " FMT_BB " -> " FMT_BB " to " FMT_WT "\n", m_sourceBlock->bbNum, + m_destBlock->bbNum, likelihood); + } + m_likelihoodSet = true; m_likelihood = likelihood; - - JITDUMP("setting likelihood of " FMT_BB " -> " FMT_BB " to " FMT_WT "\n", m_sourceBlock->bbNum, m_destBlock->bbNum, - m_likelihood); } //------------------------------------------------------------------------ @@ -114,10 +123,11 @@ void FlowEdge::addLikelihood(weight_t addedLikelihood) 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); + JITDUMP("updating likelihood of " FMT_BB " -> " FMT_BB " from " FMT_WT " to " FMT_WT "\n", m_sourceBlock->bbNum, + m_destBlock->bbNum, m_likelihood, newLikelihood); + + m_likelihood = newLikelihood; } //------------------------------------------------------------------------ @@ -678,20 +688,33 @@ void BasicBlock::dspSuccs(Compiler* compiler) // things strictly. void BasicBlock::dspKind() const { - auto dspBlockNum = [](const BasicBlock* b) -> const char* { + auto dspBlockNum = [](const FlowEdge* e) -> const char* { static char buffers[3][64]; // static array of 3 to allow 3 concurrent calls in one printf() static int nextBufferIndex = 0; - auto& buffer = buffers[nextBufferIndex]; - nextBufferIndex = (nextBufferIndex + 1) % ArrLen(buffers); + auto& buffer = buffers[nextBufferIndex]; + nextBufferIndex = (nextBufferIndex + 1) % ArrLen(buffers); + const size_t sizeOfBuffer = ArrLen(buffer); + int written; + const BasicBlock* b = e->getDestinationBlock(); if (b == nullptr) { - _snprintf_s(buffer, ArrLen(buffer), ArrLen(buffer), "NULL"); + written = _snprintf_s(buffer, sizeOfBuffer, sizeOfBuffer, "NULL"); } else { - _snprintf_s(buffer, ArrLen(buffer), ArrLen(buffer), FMT_BB, b->bbNum); + written = _snprintf_s(buffer, sizeOfBuffer, sizeOfBuffer, FMT_BB, b->bbNum); + } + + const bool printEdgeLikelihoods = true; // TODO: parameterize this? + if (printEdgeLikelihoods) + { + if (e->hasLikelihood()) + { + written = _snprintf_s(buffer + written, sizeOfBuffer - written, sizeOfBuffer - written, "(" FMT_WT ")", + e->getLikelihood()); + } } return buffer; @@ -715,7 +738,7 @@ void BasicBlock::dspKind() const for (unsigned i = 0; i < jumpCnt; i++) { - printf("%c%s", (i == 0) ? ' ' : ',', dspBlockNum(jumpTab[i]->getDestinationBlock())); + printf("%c%s", (i == 0) ? ' ' : ',', dspBlockNum(jumpTab[i])); } } @@ -728,11 +751,11 @@ void BasicBlock::dspKind() const break; case BBJ_EHFILTERRET: - printf(" -> %s (fltret)", dspBlockNum(GetTarget())); + printf(" -> %s (fltret)", dspBlockNum(GetTargetEdge())); break; case BBJ_EHCATCHRET: - printf(" -> %s (cret)", dspBlockNum(GetTarget())); + printf(" -> %s (cret)", dspBlockNum(GetTargetEdge())); break; case BBJ_THROW: @@ -746,28 +769,28 @@ void BasicBlock::dspKind() const case BBJ_ALWAYS: if (HasFlag(BBF_KEEP_BBJ_ALWAYS)) { - printf(" -> %s (ALWAYS)", dspBlockNum(GetTarget())); + printf(" -> %s (ALWAYS)", dspBlockNum(GetTargetEdge())); } else { - printf(" -> %s (always)", dspBlockNum(GetTarget())); + printf(" -> %s (always)", dspBlockNum(GetTargetEdge())); } break; case BBJ_LEAVE: - printf(" -> %s (leave)", dspBlockNum(GetTarget())); + printf(" -> %s (leave)", dspBlockNum(GetTargetEdge())); break; case BBJ_CALLFINALLY: - printf(" -> %s (callf)", dspBlockNum(GetTarget())); + printf(" -> %s (callf)", dspBlockNum(GetTargetEdge())); break; case BBJ_CALLFINALLYRET: - printf(" -> %s (callfr)", dspBlockNum(GetTarget())); + printf(" -> %s (callfr)", dspBlockNum(GetTargetEdge())); break; case BBJ_COND: - printf(" -> %s,%s (cond)", dspBlockNum(GetTrueTarget()), dspBlockNum(GetFalseTarget())); + printf(" -> %s,%s (cond)", dspBlockNum(GetTrueEdge()), dspBlockNum(GetFalseEdge())); break; case BBJ_SWITCH: @@ -779,7 +802,7 @@ void BasicBlock::dspKind() const for (unsigned i = 0; i < jumpCnt; i++) { - printf("%c%s", (i == 0) ? ' ' : ',', dspBlockNum(jumpTab[i]->getDestinationBlock())); + printf("%c%s", (i == 0) ? ' ' : ',', dspBlockNum(jumpTab[i])); const bool isDefault = bbSwtTargets->bbsHasDefault && (i == jumpCnt - 1); if (isDefault) diff --git a/src/coreclr/jit/block.h b/src/coreclr/jit/block.h index cd2a79fd098a5b..ae881d99f7361b 100644 --- a/src/coreclr/jit/block.h +++ b/src/coreclr/jit/block.h @@ -46,9 +46,13 @@ typedef BitVec_ValRet_T ASSERT_VALRET_TP; // Use this format for loop indices #define FMT_LP "L%02u" -// And this format for profile weights +// Use this format for profile weights #define FMT_WT "%.7g" +// Use this format for profile weights where we want to conserve horizontal space, at the expense of displaying +// less precision. +#define FMT_WT_NARROW "%.3g" + /***************************************************************************** * * Each basic block ends with a jump which is described as a value @@ -1037,6 +1041,27 @@ struct BasicBlock : private LIR::Range return (bbFalseEdge == nullptr) ? nullptr : bbFalseEdge->getDestinationBlock(); } + // Return the target edge; it might be null. Only used during dumping. + FlowEdge* GetTargetEdgeRaw() const + { + assert(HasTarget()); + return bbTargetEdge; + } + + // Return the BBJ_COND true target edge; it might be null. Only used during dumping. + FlowEdge* GetTrueEdgeRaw() const + { + assert(KindIs(BBJ_COND)); + return bbTrueEdge; + } + + // Return the BBJ_COND false target edge; it might be null. Only used during dumping. + FlowEdge* GetFalseEdgeRaw() const + { + assert(KindIs(BBJ_COND)); + return bbFalseEdge; + } + #endif // DEBUG private: @@ -1537,7 +1562,7 @@ struct BasicBlock : private LIR::Range } // PredBlocksEditing: convenience method for enabling range-based `for` iteration over predecessor blocks, e.g.: - // for (BasicBlock* const predBlock : block->PredBlocksList()) ... + // for (BasicBlock* const predBlock : block->PredBlocksEditing()) ... // This iterator tolerates modifications to bbPreds. // PredBlockList PredBlocksEditing() const diff --git a/src/coreclr/jit/clrjit.natvis b/src/coreclr/jit/clrjit.natvis index a15bf3cf6e73b2..cfbc6a181e9743 100644 --- a/src/coreclr/jit/clrjit.natvis +++ b/src/coreclr/jit/clrjit.natvis @@ -8,9 +8,8 @@ The .NET Foundation licenses this file to you under the MIT license. @@ -27,6 +26,11 @@ Documentation for VS debugger format specifiers: https://docs.microsoft.com/en-u BB{bbNum,d}; {bbKind,en} + + BB{m_sourceBlock->bbNum,d}->BB{m_destBlock->bbNum,d} ({m_likelihood,g}) (dup {m_dupCount,d}) + BB{m_sourceBlock->bbNum,d}->BB{m_destBlock->bbNum,d} ({m_likelihood,g}) + + REMOVED [BB{lpTop->bbNum,d}..BB{lpBottom->bbNum,d}] pre-h:BB{lpHead->bbNum,d} e:BB{lpEntry->bbNum,d} {lpFlags,en} diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index 4888d94c9ace0d..b4800978390c6a 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/src/coreclr/jit/compiler.cpp @@ -2872,6 +2872,7 @@ void Compiler::compInitOptions(JitFlags* jitFlags) opts.dspUnwind = false; opts.compLongAddress = false; opts.optRepeat = false; + opts.optRepeatCount = 1; #ifdef LATE_DISASM opts.doLateDisasm = false; @@ -2955,13 +2956,30 @@ void Compiler::compInitOptions(JitFlags* jitFlags) if (JitConfig.JitOptRepeat().contains(info.compMethodHnd, info.compClassHnd, &info.compMethodInfo->args)) { - opts.optRepeat = true; + opts.optRepeat = true; + opts.optRepeatCount = JitConfig.JitOptRepeatCount(); + + JITDUMP("\n*************** JitOptRepeat enabled; repetition count: %d\n\n", opts.optRepeatCount); } - opts.dspMetrics = (JitConfig.JitMetrics() != 0); + if (!opts.optRepeat && compStressCompile(STRESS_OPT_REPEAT, 10)) + { + // Turn on optRepeat as part of JitStress. In this case, decide how many iterations to do, from 2 to 5, + // based on a random number seeded by the method hash. + opts.optRepeat = true; + + CLRRandom rng; + rng.Init(info.compMethodHash()); + opts.optRepeatCount = rng.Next(4) + 2; // generates [2..5] + + JITDUMP("\n*************** JitOptRepeat under stress; repetition count: %d\n\n", opts.optRepeatCount); + } ////////////////// TESTING - opts.optRepeat = true; + opts.optRepeat = true; + opts.optRepeatCount = 4; + + opts.dspMetrics = (JitConfig.JitMetrics() != 0); } if (verboseDump) @@ -4916,7 +4934,7 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl if (opts.optRepeat) { - iterations = JitConfig.JitOptRepeatCount(); + iterations = opts.optRepeatCount; } #endif // defined(OPT_CONFIG) @@ -5033,6 +5051,13 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl DoPhase(this, PHASE_COMPUTE_EDGE_WEIGHTS2, &Compiler::fgComputeEdgeWeights); } +#ifdef DEBUG + if (verbose && opts.optRepeat) + { + printf("\n*************** JitOptRepeat: iterations remaining: %d\n\n", iterations - 1); + } +#endif // DEBUG + // Iterate if requested, resetting annotations first. if (--iterations == 0) { @@ -5045,13 +5070,6 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl // DoPhase(this, PHASE_CANONICALIZE_ENTRY, &Compiler::fgCanonicalizeFirstBB); -#ifdef DEBUG - if (verbose) - { - printf("\n*************** JitOptRepeat: iterations remaining: %d\n\n", iterations); - } -#endif - ResetOptAnnotations(); RecomputeFlowGraphAnnotations(); } @@ -5866,6 +5884,10 @@ void Compiler::RecomputeFlowGraphAnnotations() fgInvalidateDfsTree(); fgDfsBlocksAndRemove(); optFindLoops(); + + // Should we call this using the phase method: + // DoPhase(this, PHASE_SET_BLOCK_WEIGHTS, &Compiler::optSetBlockWeights); + // ? It could be called multiple times. optSetBlockWeights(); if (m_domTree == nullptr) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 3d0b64cf204182..4dfd2817237d8a 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -1584,7 +1584,7 @@ enum class ProfileChecks : unsigned int CHECK_NONE = 0, CHECK_CLASSIC = 1 << 0, // check "classic" jit weights CHECK_HASLIKELIHOOD = 1 << 1, // check all FlowEdges for hasLikelihood - CHECK_LIKELIHOODSUM = 1 << 2, // check block succesor likelihoods sum to 1 + CHECK_LIKELIHOODSUM = 1 << 2, // check block successor likelihoods sum to 1 CHECK_LIKELY = 1 << 3, // fully check likelihood based weights RAISE_ASSERT = 1 << 4, // assert on check failure CHECK_ALL_BLOCKS = 1 << 5, // check blocks even if bbHasProfileWeight is false @@ -1951,6 +1951,10 @@ class FlowGraphDfsTree return m_hasCycle; } +#ifdef DEBUG + void Dump() const; +#endif // DEBUG + bool Contains(BasicBlock* block) const; bool IsAncestor(BasicBlock* ancestor, BasicBlock* descendant) const; }; @@ -2230,7 +2234,7 @@ class FlowGraphNaturalLoops return m_dfsTree; } - size_t NumLoops() + size_t NumLoops() const { return m_loops.size(); } @@ -2322,6 +2326,7 @@ class FlowGraphDominatorTree } static BasicBlock* IntersectDom(BasicBlock* block1, BasicBlock* block2); + public: const FlowGraphDfsTree* GetDfsTree() { @@ -2355,6 +2360,10 @@ class BlockToNaturalLoopMap FlowGraphNaturalLoop* GetLoop(BasicBlock* block); static BlockToNaturalLoopMap* Build(FlowGraphNaturalLoops* loops); + +#ifdef DEBUG + void Dump() const; +#endif // DEBUG }; // Represents a data structure that can answer A -> B reachability queries in @@ -6076,7 +6085,11 @@ class Compiler void fgDispBBLiveness(BasicBlock* block); void fgDispBBLiveness(); - void fgTableDispBasicBlock(const BasicBlock* block, const BasicBlock* nextBlock = nullptr, int blockTargetFieldWidth = 21, int ibcColWidth = 0); + void fgTableDispBasicBlock(const BasicBlock* block, + const BasicBlock* nextBlock = nullptr, + bool printEdgeLikelihoods = true, + int blockTargetFieldWidth = 21, + int ibcColWidth = 0); void fgDispBasicBlocks(BasicBlock* firstBlock, BasicBlock* lastBlock, bool dumpTrees); void fgDispBasicBlocks(bool dumpTrees = false); void fgDumpStmtTree(const BasicBlock* block, Statement* stmt); @@ -9816,7 +9829,8 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX bool altJit; // True if we are an altjit and are compiling this method #ifdef OPT_CONFIG - bool optRepeat; // Repeat optimizer phases k times + bool optRepeat; // Repeat optimizer phases k times + int optRepeatCount; // How many times to repeat. By default, comes from JitConfig.JitOptRepeatCount(). #endif bool disAsm; // Display native code as it is generated @@ -10061,13 +10075,14 @@ XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX STRESS_MODE(PHYSICAL_PROMOTION) /* Use physical promotion */ \ STRESS_MODE(PHYSICAL_PROMOTION_COST) \ STRESS_MODE(UNWIND) /* stress unwind info; e.g., create function fragments */ \ + STRESS_MODE(OPT_REPEAT) /* stress JitOptRepeat */ \ \ /* After COUNT_VARN, stress level 2 does all of these all the time */ \ \ STRESS_MODE(COUNT_VARN) \ \ /* "Check" stress areas that can be exhaustively used if we */ \ - /* dont care about performance at all */ \ + /* don't care about performance at all */ \ \ STRESS_MODE(FORCE_INLINE) /* Treat every method as AggressiveInlining */ \ STRESS_MODE(CHK_FLOW_UPDATE) \ diff --git a/src/coreclr/jit/fgbasic.cpp b/src/coreclr/jit/fgbasic.cpp index 200c72a4d4e1c8..f389e7cbe9fbd8 100644 --- a/src/coreclr/jit/fgbasic.cpp +++ b/src/coreclr/jit/fgbasic.cpp @@ -686,20 +686,56 @@ void Compiler::fgReplaceJumpTarget(BasicBlock* block, BasicBlock* oldTarget, Bas case BBJ_SWITCH: { - unsigned const jumpCnt = block->GetSwitchTargets()->bbsCount; - FlowEdge** const jumpTab = block->GetSwitchTargets()->bbsDstTab; - bool changed = false; + unsigned const jumpCnt = block->GetSwitchTargets()->bbsCount; + FlowEdge** const jumpTab = block->GetSwitchTargets()->bbsDstTab; + bool existingEdge = false; + FlowEdge* oldEdge = nullptr; + FlowEdge* newEdge = nullptr; + bool changed = false; for (unsigned i = 0; i < jumpCnt; i++) { + if (jumpTab[i]->getDestinationBlock() == newTarget) + { + // The new target already has an edge from this switch statement. + // We'll need to add the likelihood from the edge we're redirecting + // to the existing edge. Note that if there is no existing edge, + // then we'll copy the likelihood from the existing edge we pass to + // `fgAddRefPred`. Note also that we can visit the same edge multiple + // times if there are multiple switch cases with the same target. The + // edge has a dup count and a single likelihood for all the possible + // paths to the target, so we only want to add the likelihood once + // despite visiting the duplicated edges in the `jumpTab` array + // multiple times. + existingEdge = true; + } + if (jumpTab[i]->getDestinationBlock() == oldTarget) { - fgRemoveRefPred(jumpTab[i]); - jumpTab[i] = fgAddRefPred(newTarget, block, jumpTab[i]); + assert((oldEdge == nullptr) || (oldEdge == jumpTab[i])); + oldEdge = jumpTab[i]; + fgRemoveRefPred(oldEdge); + newEdge = fgAddRefPred(newTarget, block, oldEdge); + jumpTab[i] = newEdge; changed = true; } } + if (existingEdge) + { + assert(oldEdge != nullptr); + assert(oldEdge->getSourceBlock() == block); + assert(oldEdge->getDestinationBlock() == oldTarget); + assert(newEdge != nullptr); + assert(newEdge->getSourceBlock() == block); + assert(newEdge->getDestinationBlock() == newTarget); + + if (newEdge->hasLikelihood() && oldEdge->hasLikelihood()) + { + newEdge->addLikelihood(oldEdge->getLikelihood()); + } + } + assert(changed); InvalidateUniqueSwitchSuccMap(); break; diff --git a/src/coreclr/jit/fgdiagnostic.cpp b/src/coreclr/jit/fgdiagnostic.cpp index 10124c44b2cdf5..f49f863b16dc57 100644 --- a/src/coreclr/jit/fgdiagnostic.cpp +++ b/src/coreclr/jit/fgdiagnostic.cpp @@ -1806,6 +1806,7 @@ void Compiler::fgDumpFlowGraphLoops(FILE* file) void Compiler::fgTableDispBasicBlock(const BasicBlock* block, const BasicBlock* nextBlock /* = nullptr */, + bool printEdgeLikelihoods /* = true */, int blockTargetFieldWidth /* = 21 */, int ibcColWidth /* = 0 */) { @@ -1931,27 +1932,41 @@ void Compiler::fgTableDispBasicBlock(const BasicBlock* block, // Call `dspBlockNum()` to get the block number to print, and update `printedBlockWidth` with the width // of the generated string. Note that any computation using `printedBlockWidth` must be done after all // calls to this function. - auto dspBlockNum = [terseNext, nextBlock, &printedBlockWidth](const BasicBlock* b) -> const char* { + auto dspBlockNum = [printEdgeLikelihoods, terseNext, nextBlock, + &printedBlockWidth](const FlowEdge* e) -> const char* { static char buffers[3][64]; // static array of 3 to allow 3 concurrent calls in one printf() static int nextBufferIndex = 0; - auto& buffer = buffers[nextBufferIndex]; - nextBufferIndex = (nextBufferIndex + 1) % ArrLen(buffers); + auto& buffer = buffers[nextBufferIndex]; + nextBufferIndex = (nextBufferIndex + 1) % ArrLen(buffers); + const size_t sizeOfBuffer = ArrLen(buffer); + int written; + const BasicBlock* b = e->getDestinationBlock(); if (b == nullptr) { - _snprintf_s(buffer, ArrLen(buffer), ArrLen(buffer), "NULL"); - printedBlockWidth += 4; + written = _snprintf_s(buffer, sizeOfBuffer, sizeOfBuffer, "NULL"); + printedBlockWidth += written; } else if (terseNext && (b == nextBlock)) { - _snprintf_s(buffer, ArrLen(buffer), ArrLen(buffer), "*"); - printedBlockWidth += 1; + written = _snprintf_s(buffer, sizeOfBuffer, sizeOfBuffer, "*"); + printedBlockWidth += written; } else { - _snprintf_s(buffer, ArrLen(buffer), ArrLen(buffer), FMT_BB, b->bbNum); - printedBlockWidth += 2 /* BB */ + max(CountDigits(b->bbNum), 2); + written = _snprintf_s(buffer, sizeOfBuffer, sizeOfBuffer, FMT_BB, b->bbNum); + printedBlockWidth += written; + } + + if (printEdgeLikelihoods) + { + if (e->hasLikelihood()) + { + written = _snprintf_s(buffer + written, sizeOfBuffer - written, sizeOfBuffer - written, + "(" FMT_WT_NARROW ")", e->getLikelihood()); + printedBlockWidth += written; + } } return buffer; @@ -1968,19 +1983,19 @@ void Compiler::fgTableDispBasicBlock(const BasicBlock* block, { case BBJ_COND: printedBlockWidth = 3 /* "-> " */ + 1 /* comma */ + 9 /* kind */; - printf("-> %s,%s", dspBlockNum(block->GetTrueTargetRaw()), dspBlockNum(block->GetFalseTargetRaw())); + printf("-> %s,%s", dspBlockNum(block->GetTrueEdgeRaw()), dspBlockNum(block->GetFalseEdgeRaw())); printf("%*s ( cond )", blockTargetFieldWidth - printedBlockWidth, ""); break; case BBJ_CALLFINALLY: printedBlockWidth = 3 /* "-> " */ + 9 /* kind */; - printf("-> %s", dspBlockNum(block->GetTargetRaw())); + printf("-> %s", dspBlockNum(block->GetTargetEdgeRaw())); printf("%*s (callf )", blockTargetFieldWidth - printedBlockWidth, ""); break; case BBJ_CALLFINALLYRET: printedBlockWidth = 3 /* "-> " */ + 9 /* kind */; - printf("-> %s", dspBlockNum(block->GetFinallyContinuation())); + printf("-> %s", dspBlockNum(block->GetTargetEdgeRaw())); printf("%*s (callfr)", blockTargetFieldWidth - printedBlockWidth, ""); break; @@ -1988,13 +2003,13 @@ void Compiler::fgTableDispBasicBlock(const BasicBlock* block, const char* label; label = (flags & BBF_KEEP_BBJ_ALWAYS) ? "ALWAYS" : "always"; printedBlockWidth = 3 /* "-> " */ + 9 /* kind */; - printf("-> %s", dspBlockNum(block->GetTargetRaw())); + printf("-> %s", dspBlockNum(block->GetTargetEdgeRaw())); printf("%*s (%s)", blockTargetFieldWidth - printedBlockWidth, "", label); break; case BBJ_LEAVE: printedBlockWidth = 3 /* "-> " */ + 9 /* kind */; - printf("-> %s", dspBlockNum(block->GetTargetRaw())); + printf("-> %s", dspBlockNum(block->GetTargetEdgeRaw())); printf("%*s (leave )", blockTargetFieldWidth - printedBlockWidth, ""); break; @@ -2019,7 +2034,7 @@ void Compiler::fgTableDispBasicBlock(const BasicBlock* block, for (unsigned i = 0; i < jumpCnt; i++) { printedBlockWidth += 1 /* space/comma */; - printf("%c%s", (i == 0) ? ' ' : ',', dspBlockNum(jumpTab[i]->getDestinationBlock())); + printf("%c%s", (i == 0) ? ' ' : ',', dspBlockNum(jumpTab[i])); } } @@ -2039,13 +2054,13 @@ void Compiler::fgTableDispBasicBlock(const BasicBlock* block, case BBJ_EHFILTERRET: printedBlockWidth = 3 /* "-> " */ + 9 /* kind */; - printf("-> %s", dspBlockNum(block->GetTargetRaw())); + printf("-> %s", dspBlockNum(block->GetTargetEdgeRaw())); printf("%*s (fltret)", blockTargetFieldWidth - printedBlockWidth, ""); break; case BBJ_EHCATCHRET: printedBlockWidth = 3 /* "-> " */ + 9 /* kind */; - printf("-> %s", dspBlockNum(block->GetTargetRaw())); + printf("-> %s", dspBlockNum(block->GetTargetEdgeRaw())); printf("%*s ( cret )", blockTargetFieldWidth - printedBlockWidth, ""); break; @@ -2071,7 +2086,7 @@ void Compiler::fgTableDispBasicBlock(const BasicBlock* block, for (unsigned i = 0; i < jumpCnt; i++) { printedBlockWidth += 1 /* space/comma */; - printf("%c%s", (i == 0) ? ' ' : ',', dspBlockNum(jumpTab[i]->getDestinationBlock())); + printf("%c%s", (i == 0) ? ' ' : ',', dspBlockNum(jumpTab[i])); const bool isDefault = jumpSwt->bbsHasDefault && (i == jumpCnt - 1); if (isDefault) @@ -2321,10 +2336,16 @@ void Compiler::fgDispBasicBlocks(BasicBlock* firstBlock, BasicBlock* lastBlock, maxBlockNumWidth = max(maxBlockNumWidth, 2); int padWidth = maxBlockNumWidth - 2; // Account for functions with a large number of blocks. + const bool printEdgeLikelihoods = true; // TODO: parameterize? + + // Edge likelihoods are printed as "(0.123)", so take 7 characters maxmimum. + int edgeLikelihoodsWidth = printEdgeLikelihoods ? 7 : 0; + // Calculate the field width allocated for the block target. The field width is allocated to allow for two blocks // for BBJ_COND. It does not include any extra space for variable-sized BBJ_EHFINALLYRET and BBJ_SWITCH. - int blockTargetFieldWidth = 3 /* "-> " */ + 2 /* BB */ + maxBlockNumWidth + 1 /* comma */ + 2 /* BB */ + - maxBlockNumWidth + 1 /* space */ + 8 /* kind: "(xxxxxx)" */; + int blockTargetFieldWidth = 3 /* "-> " */ + 2 /* BB */ + maxBlockNumWidth + edgeLikelihoodsWidth + 1 /* comma */ + + 2 /* BB */ + maxBlockNumWidth + edgeLikelihoodsWidth + 1 /* space */ + + 8 /* kind: "(xxxxxx)" */; // clang-format off @@ -2332,7 +2353,7 @@ void Compiler::fgDispBasicBlocks(BasicBlock* firstBlock, BasicBlock* lastBlock, printf("------%*s-------------------------------------%*s--------------------------%*s--------------------------\n", padWidth, "------------", // ibcColWidth, "------------", // - blockTargetFieldWidth, "-----------------------"); // + blockTargetFieldWidth, "----------------------------------------------"); // printf("BBnum %*sBBid ref try hnd %s weight %*s%s [IL range] [jump]%*s [EH region] [flags]\n", padWidth, "", (fgPredsComputed ? "preds " @@ -2345,7 +2366,7 @@ void Compiler::fgDispBasicBlocks(BasicBlock* firstBlock, BasicBlock* lastBlock, printf("------%*s-------------------------------------%*s--------------------------%*s--------------------------\n", padWidth, "------------", // ibcColWidth, "------------", // - blockTargetFieldWidth, "-----------------------"); // + blockTargetFieldWidth, "----------------------------------------------"); // // clang-format on @@ -2377,9 +2398,9 @@ void Compiler::fgDispBasicBlocks(BasicBlock* firstBlock, BasicBlock* lastBlock, { printf("~~~~~~%*s~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~%*s~~~~~~~~~~~~~~~~~~~~~~~~~~%*s~~~~~~~~~~" "~~~~~~~~~~~~~~~~\n", - padWidth, "~~~~~~~~~~~~", // - ibcColWidth, "~~~~~~~~~~~~", // - blockTargetFieldWidth, "~~~~~~~~~~~~~~~~~~~~~~~"); // + padWidth, "~~~~~~~~~~~~", // + ibcColWidth, "~~~~~~~~~~~~", // + blockTargetFieldWidth, "~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~"); // } #if defined(FEATURE_EH_FUNCLETS) @@ -2387,13 +2408,13 @@ void Compiler::fgDispBasicBlocks(BasicBlock* firstBlock, BasicBlock* lastBlock, { printf("++++++%*s+++++++++++++++++++++++++++++++++++++%*s++++++++++++++++++++++++++%*s++++++++++" "++++++++++++++++ funclets follow\n", - padWidth, "++++++++++++", // - ibcColWidth, "++++++++++++", // - blockTargetFieldWidth, "+++++++++++++++++++++++"); // + padWidth, "++++++++++++", // + ibcColWidth, "++++++++++++", // + blockTargetFieldWidth, "++++++++++++++++++++++++++++++++++++++++++++++"); // } #endif // FEATURE_EH_FUNCLETS - fgTableDispBasicBlock(block, nextBlock, blockTargetFieldWidth, ibcColWidth); + fgTableDispBasicBlock(block, nextBlock, printEdgeLikelihoods, blockTargetFieldWidth, ibcColWidth); if (block == lastBlock) { @@ -2403,9 +2424,9 @@ void Compiler::fgDispBasicBlocks(BasicBlock* firstBlock, BasicBlock* lastBlock, printf("------%*s-------------------------------------%*s--------------------------%*s------------------" "--------\n", - padWidth, "------------", // - ibcColWidth, "------------", // - blockTargetFieldWidth, "-----------------------"); // + padWidth, "------------", // + ibcColWidth, "------------", // + blockTargetFieldWidth, "----------------------------------------------"); // if (dumpTrees) { diff --git a/src/coreclr/jit/fgopt.cpp b/src/coreclr/jit/fgopt.cpp index 3052d2fca63eaf..45340a647da82b 100644 --- a/src/coreclr/jit/fgopt.cpp +++ b/src/coreclr/jit/fgopt.cpp @@ -1928,6 +1928,7 @@ bool Compiler::fgOptimizeSwitchBranches(BasicBlock* block) { printf("\nRemoving a switch jump with a single target (" FMT_BB ")\n", block->bbNum); printf("BEFORE:\n"); + fgDispBasicBlocks(); } #endif // DEBUG @@ -5271,7 +5272,7 @@ PhaseStatus Compiler::fgDfsBlocksAndRemove() #ifdef DEBUG if (verbose) { - printf("%u/%u blocks are unreachable and will be removed\n", fgBBcount - m_dfsTree->GetPostOrderCount(), + printf("%u/%u blocks are unreachable and will be removed:\n", fgBBcount - m_dfsTree->GetPostOrderCount(), fgBBcount); for (BasicBlock* block : Blocks()) { @@ -5281,7 +5282,7 @@ PhaseStatus Compiler::fgDfsBlocksAndRemove() } } } -#endif +#endif // DEBUG // The DFS we run is not precise around call-finally, so // `fgRemoveUnreachableBlocks` can expose newly unreachable blocks @@ -5309,6 +5310,24 @@ PhaseStatus Compiler::fgDfsBlocksAndRemove() m_dfsTree = fgComputeDfs(); } +#ifdef DEBUG + // Did we actually remove all the blocks we said we were going to? + if (verbose) + { + if (m_dfsTree->GetPostOrderCount() != fgBBcount) + { + printf("%u unreachable blocks were not removed:\n", fgBBcount - m_dfsTree->GetPostOrderCount()); + for (BasicBlock* block : Blocks()) + { + if (!m_dfsTree->Contains(block)) + { + printf(" " FMT_BB "\n", block->bbNum); + } + } + } + } +#endif // DEBUG + status = PhaseStatus::MODIFIED_EVERYTHING; } diff --git a/src/coreclr/jit/fgprofile.cpp b/src/coreclr/jit/fgprofile.cpp index 82192c9f28ca63..4693107168ba61 100644 --- a/src/coreclr/jit/fgprofile.cpp +++ b/src/coreclr/jit/fgprofile.cpp @@ -5664,6 +5664,25 @@ bool Compiler::fgDebugCheckOutgoingProfileData(BasicBlock* block, ProfileChecks else { likelyWeightsValid = false; + +#ifdef DEBUG + if (verbose) + { + for (const FlowEdge* succEdge : block->SuccEdges(this)) + { + const BasicBlock* succBlock = succEdge->getDestinationBlock(); + if (succEdge->hasLikelihood()) + { + printf(" " FMT_BB " -> " FMT_BB ": " FMT_WT "\n", block->bbNum, succBlock->bbNum, + succEdge->getLikelihood()); + } + else + { + printf(" " FMT_BB " -> " FMT_BB ": no likelihood\n", block->bbNum, succBlock->bbNum); + } + } + } +#endif // DEBUG } } } diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp index 6c6edd7485694a..9cc121919acdf8 100644 --- a/src/coreclr/jit/flowgraph.cpp +++ b/src/coreclr/jit/flowgraph.cpp @@ -3907,6 +3907,26 @@ void Compiler::fgLclFldAssign(unsigned lclNum) } } +#ifdef DEBUG + +//------------------------------------------------------------------------ +// FlowGraphDfsTree::Dump: Dump a textual representation of the DFS tree. +// +void FlowGraphDfsTree::Dump() const +{ + printf("DFS tree. %s.\n", HasCycle() ? "Has cycle" : "No cycle"); + printf("PO RPO -> BB [pre, post]\n"); + for (unsigned i = 0; i < GetPostOrderCount(); i++) + { + unsigned rpoNum = GetPostOrderCount() - i - 1; + BasicBlock* const block = GetPostOrder(i); + printf("%02u %02u -> " FMT_BB "[%u, %u]\n", i, rpoNum, block->bbNum, block->bbPreorderNum, + block->bbPostorderNum); + } +} + +#endif // DEBUG + //------------------------------------------------------------------------ // FlowGraphDfsTree::Contains: Check if a block is contained in the DFS tree; // i.e., if it is reachable. @@ -6131,6 +6151,37 @@ BlockToNaturalLoopMap* BlockToNaturalLoopMap::Build(FlowGraphNaturalLoops* loops return new (comp, CMK_Loops) BlockToNaturalLoopMap(loops, indices); } +#ifdef DEBUG + +//------------------------------------------------------------------------ +// BlockToNaturalLoopMap::Dump: Dump a textual representation of the map. +// +void BlockToNaturalLoopMap::Dump() const +{ + const FlowGraphDfsTree* dfs = m_loops->GetDfsTree(); + unsigned blockCount = dfs->GetPostOrderCount(); + + printf("Block -> natural loop map: %u blocks\n", blockCount); + if (blockCount > 0) + { + printf("PO : loop index\n"); + for (unsigned i = 0; i < blockCount; i++) + { + if (m_indices[i] == UINT_MAX) + { + // Just leave the loop space empty if there is no enclosing loop + printf("%02u : \n", i); + } + else + { + printf("%02u : %02u\n", i, m_indices[i]); + } + } + } +} + +#endif // DEBUG + //------------------------------------------------------------------------ // BlockReachabilitySets::Build: Build the reachability sets. // diff --git a/src/coreclr/jit/jitconfigvalues.h b/src/coreclr/jit/jitconfigvalues.h index 9791d6e79495ff..d1765aaf4b596a 100644 --- a/src/coreclr/jit/jitconfigvalues.h +++ b/src/coreclr/jit/jitconfigvalues.h @@ -144,9 +144,11 @@ CONFIG_INTEGER(JitPrintInlinedMethodsVerbose, W("JitPrintInlinedMethodsVerboseLe CONFIG_METHODSET(JitPrintInlinedMethods, W("JitPrintInlinedMethods")) CONFIG_METHODSET(JitPrintDevirtualizedMethods, W("JitPrintDevirtualizedMethods")) -// -1: just do internal checks -// Else bitflag: 0x1 check classic, 0x2 check likely, 0x4 enable asserts + +// -1: just do internal checks (CHECK_HASLIKELIHOOD | CHECK_LIKELIHOODSUM | RAISE_ASSERT) +// Else bitflag of ProfileChecks enum. 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) @@ -386,6 +388,7 @@ CONFIG_INTEGER(JitRLCSEGreedy, W("JitRLCSEGreedy"), 0) CONFIG_INTEGER(JitRLCSEVerbose, W("JitRLCSEVerbose"), 0) #if defined(DEBUG) + // Allow fine-grained controls of CSEs done in a particular method // // Specify method that will respond to the CSEMask. @@ -437,7 +440,7 @@ CONFIG_STRING(JitRLCSEAlpha, W("JitRLCSEAlpha")) // If nonzero, dump candidate feature values CONFIG_INTEGER(JitRLCSECandidateFeatures, W("JitRLCSECandidateFeatures"), 0) -#endif +#endif // DEBUG /// /// JIT @@ -480,6 +483,7 @@ CONFIG_INTEGER(JitNoRngChks, W("JitNoRngChks"), 0) // If 1, don't generate range #endif // defined(FEATURE_ENABLE_NO_RANGE_CHECKS) #if defined(OPT_CONFIG) + CONFIG_INTEGER(JitDoAssertionProp, W("JitDoAssertionProp"), 1) // Perform assertion propagation optimization CONFIG_INTEGER(JitDoCopyProp, W("JitDoCopyProp"), 1) // Perform copy propagation on variables that appear redundant CONFIG_INTEGER(JitDoOptimizeIVs, W("JitDoOptimizeIVs"), 1) // Perform optimization of induction variables @@ -504,9 +508,10 @@ CONFIG_INTEGER(JitDoSsa, W("JitDoSsa"), 1) // Perform Static Single Assignment ( CONFIG_INTEGER(JitDoValueNumber, W("JitDoValueNumber"), 1) // Perform value numbering on method expressions CONFIG_METHODSET(JitOptRepeat, W("JitOptRepeat")) // Runs optimizer multiple times on the method -CONFIG_INTEGER(JitOptRepeatCount, W("JitOptRepeatCount"), 4) // Number of times to repeat opts when repeating +CONFIG_INTEGER(JitOptRepeatCount, W("JitOptRepeatCount"), 2) // Number of times to repeat opts when repeating CONFIG_INTEGER(JitDoIfConversion, W("JitDoIfConversion"), 1) // Perform If conversion -#endif // defined(OPT_CONFIG) + +#endif // defined(OPT_CONFIG) // Max # of MapSelect's considered for a particular top-level invocation. CONFIG_INTEGER(JitVNMapSelBudget, W("JitVNMapSelBudget"), DEFAULT_MAP_SELECT_BUDGET) diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp index e837dbfe72d21f..cabdfb29f61fa9 100644 --- a/src/coreclr/jit/optimizer.cpp +++ b/src/coreclr/jit/optimizer.cpp @@ -246,6 +246,13 @@ void Compiler::optScaleLoopBlocks(BasicBlock* begBlk, BasicBlock* endBlk) continue; } + // Don't change the block weight if it's unreachable. + if (!m_reachabilitySets->GetDfsTree()->Contains(curBlk)) + { + reportBlockWeight(curBlk, "; unchanged: unreachable"); + continue; + } + // For curBlk to be part of a loop that starts at begBlk, curBlk must be reachable from begBlk and // (since this is a loop) begBlk must likewise be reachable from curBlk. @@ -496,14 +503,12 @@ bool Compiler::optExtractInitTestIncr( if (initStmt->GetRootNode()->OperIs(GT_JTRUE)) { bool doGetPrev = true; -#ifdef DEBUG if (opts.optRepeat) { // Previous optimization passes may have inserted compiler-generated // statements other than duplicated loop conditions. doGetPrev = (initStmt->GetPrevStmt() != nullptr); } -#endif // DEBUG if (doGetPrev) { initStmt = initStmt->GetPrevStmt(); @@ -2681,7 +2686,7 @@ void Compiler::optFindAndScaleGeneralLoopBlocks() } //----------------------------------------------------------------------------- -// optFindLoops: find loops in the function. +// optFindLoopsPhase: find loops in the function. // // The JIT recognizes two types of loops in a function: natural loops and "general" (or "unnatural") loops. // Natural loops are those which get added to Compiler::m_loops. Most downstream optimizations require @@ -3118,6 +3123,9 @@ bool Compiler::optCanonicalizeExit(FlowGraphNaturalLoop* loop, BasicBlock* exit) BasicBlock* newExit; + JITDUMP("Canonicalize exit " FMT_BB " for " FMT_LP " to have only loop predecessors\n", exit->bbNum, + loop->GetIndex()); + #if FEATURE_EH_CALLFINALLY_THUNKS if (exit->KindIs(BBJ_CALLFINALLY)) { @@ -3140,7 +3148,7 @@ bool Compiler::optCanonicalizeExit(FlowGraphNaturalLoop* loop, BasicBlock* exit) } } else -#endif +#endif // FEATURE_EH_CALLFINALLY_THUNKS { newExit = fgNewBBbefore(BBJ_ALWAYS, exit, false); newExit->SetFlags(BBF_NONE_QUIRK); @@ -3154,9 +3162,6 @@ bool Compiler::optCanonicalizeExit(FlowGraphNaturalLoop* loop, BasicBlock* exit) newExit->bbCodeOffs = exit->bbCodeOffs; - JITDUMP("Created new exit " FMT_BB " to replace " FMT_BB " for " FMT_LP "\n", newExit->bbNum, exit->bbNum, - loop->GetIndex()); - for (BasicBlock* pred : exit->PredBlocksEditing()) { if (loop->ContainsBlock(pred)) @@ -3166,6 +3171,9 @@ bool Compiler::optCanonicalizeExit(FlowGraphNaturalLoop* loop, BasicBlock* exit) } optSetWeightForPreheaderOrExit(loop, newExit); + + JITDUMP("Created new exit " FMT_BB " to replace " FMT_BB " exit for " FMT_LP "\n", newExit->bbNum, exit->bbNum, + loop->GetIndex()); return true; }