diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index 5fac5c8e0704c6..d2e98f4afaa315 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/src/coreclr/jit/compiler.cpp @@ -5035,7 +5035,7 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl } } - optLoopsRequirePreHeaders = false; + optLoopsCanonical = false; #ifdef DEBUG DoPhase(this, PHASE_STRESS_SPLIT_TREE, &Compiler::StressSplitTree); diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 48bd2523cd03eb..7a1e8e29dff1c2 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -2184,6 +2184,9 @@ class FlowGraphNaturalLoop template BasicBlockVisit VisitLoopBlocksLexical(TFunc func); + template + BasicBlockVisit VisitRegularExitBlocks(TFunc func); + BasicBlock* GetLexicallyTopMostBlock(); BasicBlock* GetLexicallyBottomMostBlock(); @@ -4971,7 +4974,11 @@ class Compiler FlowGraphDominatorTree* m_domTree; BlockReachabilitySets* m_reachabilitySets; - bool optLoopsRequirePreHeaders; // Do we require that all loops (in m_loops) have pre-headers? + // Do we require loops to be in canonical form? The canonical form ensures that: + // 1. All loops have preheaders (single entry blocks that always enter the loop) + // 2. All loop exits where bbIsHandlerBeg(exit) is false have only loop predecessors. + // + bool optLoopsCanonical; unsigned optNumNaturalLoopsFound; // Number of natural loops found in the loop finding phase bool fgBBVarSetsInited; @@ -5906,7 +5913,7 @@ class Compiler PhaseStatus fgCanonicalizeFirstBB(); - void fgSetEHRegionForNewPreheader(BasicBlock* preheader); + void fgSetEHRegionForNewPreheaderOrExit(BasicBlock* preheader); void fgUnreachableBlock(BasicBlock* block); @@ -6785,6 +6792,7 @@ class Compiler void optFindLoops(); bool optCanonicalizeLoops(); + void optCompactLoops(); void optCompactLoop(FlowGraphNaturalLoop* loop); BasicBlock* optFindLoopCompactionInsertionPoint(FlowGraphNaturalLoop* loop, BasicBlock* top); @@ -6792,6 +6800,11 @@ class Compiler bool optCreatePreheader(FlowGraphNaturalLoop* loop); void optSetPreheaderWeight(FlowGraphNaturalLoop* loop, BasicBlock* preheader); + bool optCanonicalizeExits(FlowGraphNaturalLoop* loop); + bool optCanonicalizeExit(FlowGraphNaturalLoop* loop, BasicBlock* exit); + weight_t optEstimateEdgeLikelihood(BasicBlock* from, BasicBlock* to, bool* fromProfile); + void optSetExitWeight(FlowGraphNaturalLoop* loop, BasicBlock* exit); + PhaseStatus optCloneLoops(); void optCloneLoop(FlowGraphNaturalLoop* loop, LoopCloneContext* context); PhaseStatus optUnrollLoops(); // Unrolls loops (needs to have cost info) diff --git a/src/coreclr/jit/compiler.hpp b/src/coreclr/jit/compiler.hpp index d664a57bd24143..7a979c94ed3bbf 100644 --- a/src/coreclr/jit/compiler.hpp +++ b/src/coreclr/jit/compiler.hpp @@ -4969,6 +4969,60 @@ BasicBlockVisit FlowGraphNaturalLoop::VisitLoopBlocksLexical(TFunc func) return BasicBlockVisit::Continue; } +//------------------------------------------------------------------------------ +// FlowGraphNaturalLoop::VisitRegularExitBlocks: Visit non-handler blocks that +// are outside the loop but that may have regular predecessors inside the loop. +// +// Type parameters: +// TFunc - Callback functor type +// +// Arguments: +// func - Callback functor that takes a BasicBlock* and returns a +// BasicBlockVisit. +// +// Returns: +// BasicBlockVisit that indicated whether the visit was aborted by the +// callback or whether all blocks were visited. +// +// Remarks: +// Note that no handler begins are visited by this function, even if they +// have regular predecessors inside the loop (for example, finally handlers +// can have regular BBJ_CALLFINALLY predecessors inside the loop). This +// choice is motivated by the fact that such handlers will also show up as +// exceptional exit blocks that must always be handled specially by client +// code regardless. +// +template +BasicBlockVisit FlowGraphNaturalLoop::VisitRegularExitBlocks(TFunc func) +{ + Compiler* comp = m_dfsTree->GetCompiler(); + + BitVecTraits traits = m_dfsTree->PostOrderTraits(); + BitVec visited(BitVecOps::MakeEmpty(&traits)); + + for (FlowEdge* edge : ExitEdges()) + { + BasicBlockVisit result = edge->getSourceBlock()->VisitRegularSuccs(comp, [&](BasicBlock* succ) { + assert(m_dfsTree->Contains(succ)); + if (!comp->bbIsHandlerBeg(succ) && !ContainsBlock(succ) && + BitVecOps::TryAddElemD(&traits, visited, succ->bbPostorderNum) && + (func(succ) == BasicBlockVisit::Abort)) + { + return BasicBlockVisit::Abort; + } + + return BasicBlockVisit::Continue; + }); + + if (result == BasicBlockVisit::Abort) + { + return BasicBlockVisit::Abort; + } + } + + return BasicBlockVisit::Continue; +} + /*****************************************************************************/ #endif //_COMPILER_HPP_ /*****************************************************************************/ diff --git a/src/coreclr/jit/fgdiagnostic.cpp b/src/coreclr/jit/fgdiagnostic.cpp index ed3f4252bed5ab..ff5cf82ae4456e 100644 --- a/src/coreclr/jit/fgdiagnostic.cpp +++ b/src/coreclr/jit/fgdiagnostic.cpp @@ -4719,12 +4719,20 @@ void Compiler::fgDebugCheckLoops() { return; } - if (optLoopsRequirePreHeaders) + if (optLoopsCanonical) { for (FlowGraphNaturalLoop* loop : m_loops->InReversePostOrder()) { assert(loop->EntryEdges().size() == 1); assert(loop->EntryEdge(0)->getSourceBlock()->KindIs(BBJ_ALWAYS)); + + loop->VisitRegularExitBlocks([=](BasicBlock* exit) { + for (BasicBlock* pred : exit->PredBlocks()) + { + assert(loop->ContainsBlock(pred)); + } + return BasicBlockVisit::Continue; + }); } } } diff --git a/src/coreclr/jit/loopcloning.cpp b/src/coreclr/jit/loopcloning.cpp index 23848154bcff15..ca4c2572fa41d9 100644 --- a/src/coreclr/jit/loopcloning.cpp +++ b/src/coreclr/jit/loopcloning.cpp @@ -2969,11 +2969,18 @@ PhaseStatus Compiler::optCloneLoops() if (optLoopsCloned > 0) { - fgRenumberBlocks(); - fgInvalidateDfsTree(); m_dfsTree = fgComputeDfs(); m_loops = FlowGraphNaturalLoops::Find(m_dfsTree); + + if (optCanonicalizeLoops()) + { + fgInvalidateDfsTree(); + m_dfsTree = fgComputeDfs(); + m_loops = FlowGraphNaturalLoops::Find(m_dfsTree); + } + + fgRenumberBlocks(); } #ifdef DEBUG diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp index 05eb5c7d3b9fb1..180d8ecb1f0034 100644 --- a/src/coreclr/jit/optimizer.cpp +++ b/src/coreclr/jit/optimizer.cpp @@ -21,8 +21,8 @@ void Compiler::optInit() { fgHasLoops = false; - optLoopsRequirePreHeaders = false; - optNumNaturalLoopsFound = 0; + optLoopsCanonical = false; + optNumNaturalLoopsFound = 0; #ifdef DEBUG loopAlignCandidates = 0; @@ -1319,6 +1319,13 @@ PhaseStatus Compiler::optUnrollLoops() fgDfsBlocksAndRemove(); m_loops = FlowGraphNaturalLoops::Find(m_dfsTree); + if (optCanonicalizeLoops()) + { + fgInvalidateDfsTree(); + m_dfsTree = fgComputeDfs(); + m_loops = FlowGraphNaturalLoops::Find(m_dfsTree); + } + fgRenumberBlocks(); DBEXEC(verbose, fgDispBasicBlocks()); @@ -2680,8 +2687,8 @@ void Compiler::optFindLoops() fgRenumberBlocks(); - // Starting now, we require all loops to have pre-headers. - optLoopsRequirePreHeaders = true; + // Starting now we require all loops to be in canonical form. + optLoopsCanonical = true; // Leave a bread crumb for future phases like loop alignment about whether // looking for loops makes sense. We generally do not expect phases to @@ -2709,11 +2716,28 @@ void Compiler::optFindLoops() bool Compiler::optCanonicalizeLoops() { bool changed = false; + for (FlowGraphNaturalLoop* loop : m_loops->InReversePostOrder()) { changed |= optCreatePreheader(loop); } + // At this point we've created preheaders. That means we are working with + // stale loop and DFS data. However, we can do exit canonicalization even + // on the stale data; this relies on the fact that exiting blocks do not + // change as a result of creating preheaders. On the other hand the exit + // blocks themselves may have changed (previously it may have been another + // loop's header, now it might be its preheader instead). Exit + // canonicalization stil works even with this. + // + // The exit canonicalization needs to be done in post order (inner -> outer + // loops) so that inner exits that also exit outer loops have proper exit + // blocks created for each loop. + for (FlowGraphNaturalLoop* loop : m_loops->InPostOrder()) + { + changed |= optCanonicalizeExits(loop); + } + return changed; } @@ -2948,7 +2972,7 @@ bool Compiler::optCreatePreheader(FlowGraphNaturalLoop* loop) BasicBlock* preheader = fgNewBBbefore(BBJ_ALWAYS, insertBefore, false, header); preheader->SetFlags(BBF_INTERNAL); - fgSetEHRegionForNewPreheader(preheader); + fgSetEHRegionForNewPreheaderOrExit(preheader); if (preheader->NextIs(header)) { @@ -3103,6 +3127,271 @@ void Compiler::optSetPreheaderWeight(FlowGraphNaturalLoop* loop, BasicBlock* pre edgeFromPreheader->setEdgeWeights(preheader->bbWeight, preheader->bbWeight, loop->GetHeader()); } +//----------------------------------------------------------------------------- +// optCanonicalizeExits: Canonicalize all regular exits of the loop so that +// they have only loop predecessors. +// +// Parameters: +// loop - The loop +// +// Returns: +// True if any flow graph modifications were made. +// +bool Compiler::optCanonicalizeExits(FlowGraphNaturalLoop* loop) +{ + bool changed = false; + + for (FlowEdge* edge : loop->ExitEdges()) + { + // Find all blocks outside the loop from this exiting block. Those + // blocks are exits. Note that we may see preheaders created by + // previous canonicalization here, which are not part of the DFS tree + // or properly maintained in a parent loop. The canonicalization here + // works despite this. + edge->getSourceBlock()->VisitRegularSuccs(this, [=, &changed](BasicBlock* succ) { + if (!loop->ContainsBlock(succ)) + { + changed |= optCanonicalizeExit(loop, succ); + } + + return BasicBlockVisit::Continue; + }); + } + + return changed; +} + +//----------------------------------------------------------------------------- +// optCanonicalizeExit: Canonicalize a single exit block to have only loop +// predecessors. +// +// Parameters: +// loop - The loop +// +// Returns: +// True if any flow graph modifications were made. +// +bool Compiler::optCanonicalizeExit(FlowGraphNaturalLoop* loop, BasicBlock* exit) +{ + assert(!loop->ContainsBlock(exit)); + + if (bbIsHandlerBeg(exit)) + { + return false; + } + + bool allLoopPreds = true; + for (BasicBlock* pred : exit->PredBlocks()) + { + if (!loop->ContainsBlock(pred)) + { + allLoopPreds = false; + break; + } + } + + if (allLoopPreds) + { + // Already canonical + JITDUMP("All preds of exit " FMT_BB " of " FMT_LP " are already in the loop, no exit canonicalization needed\n", + exit->bbNum, loop->GetIndex()); + return false; + } + + BasicBlock* newExit; + +#if FEATURE_EH_CALLFINALLY_THUNKS + if (exit->KindIs(BBJ_CALLFINALLY)) + { + // Branches to a BBJ_CALLFINALLY _must_ come from inside its associated + // try region, and when we have callfinally thunks the BBJ_CALLFINALLY + // is outside it. First try to see if the lexically bottom most block + // is part of the try; if so, inserting after that is a good choice. + BasicBlock* finallyBlock = exit->GetTarget(); + assert(finallyBlock->hasHndIndex()); + BasicBlock* bottom = loop->GetLexicallyBottomMostBlock(); + if (bottom->hasTryIndex() && (bottom->getTryIndex() == finallyBlock->getHndIndex()) && !bottom->hasHndIndex()) + { + newExit = fgNewBBafter(BBJ_ALWAYS, bottom, true, exit); + } + else + { + // Otherwise just do the heavy-handed thing and insert it anywhere in the right region. + newExit = fgNewBBinRegion(BBJ_ALWAYS, finallyBlock->bbHndIndex, 0, nullptr, exit, /* putInFilter */ false, + /* runRarely */ false, /* insertAtEnd */ true); + } + } + else +#endif + { + newExit = fgNewBBbefore(BBJ_ALWAYS, exit, false, exit); + newExit->SetFlags(BBF_NONE_QUIRK); + fgSetEHRegionForNewPreheaderOrExit(newExit); + } + + newExit->SetFlags(BBF_INTERNAL); + + fgAddRefPred(exit, newExit); + + 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->PredBlocks()) + { + if (loop->ContainsBlock(pred)) + { + fgReplaceJumpTarget(pred, exit, newExit); + } + } + + optSetExitWeight(loop, newExit); + return true; +} + +//----------------------------------------------------------------------------- +// optEstimateEdgeLikelihood: Given a block "from" that may transfer control to +// "to", estimate the likelihood that this will happen taking profile into +// account if available. +// +// Parameters: +// from - From block +// to - To block +// fromProfile - [out] Whether or not the estimate is based on profile data +// +// Returns: +// Estimated likelihood of the edge being taken. +// +weight_t Compiler::optEstimateEdgeLikelihood(BasicBlock* from, BasicBlock* to, bool* fromProfile) +{ + *fromProfile = (from->HasFlag(BBF_PROF_WEIGHT) != BBF_EMPTY) && (to->HasFlag(BBF_PROF_WEIGHT) != BBF_EMPTY); + if (!fgIsUsingProfileWeights() || !from->HasFlag(BBF_PROF_WEIGHT) || !to->HasFlag(BBF_PROF_WEIGHT) || + from->KindIs(BBJ_ALWAYS)) + { + return 1.0 / from->NumSucc(this); + } + + bool useEdgeWeights = fgHaveValidEdgeWeights; + + weight_t takenCount = 0; + weight_t notTakenCount = 0; + + if (useEdgeWeights) + { + from->VisitRegularSuccs(this, [&, to](BasicBlock* succ) { + *fromProfile &= succ->hasProfileWeight(); + FlowEdge* edge = fgGetPredForBlock(succ, from); + weight_t edgeWeight = (edge->edgeWeightMin() + edge->edgeWeightMax()) / 2.0; + + if (succ == to) + { + takenCount += edgeWeight; + } + else + { + notTakenCount += edgeWeight; + } + return BasicBlockVisit::Continue; + }); + + // Watch out for cases where edge weights were not properly maintained + // so that it appears no profile flow goes to 'to'. + // + useEdgeWeights = !fgProfileWeightsConsistent(takenCount, BB_ZERO_WEIGHT); + } + + if (!useEdgeWeights) + { + takenCount = 0; + notTakenCount = 0; + + from->VisitRegularSuccs(this, [&, to](BasicBlock* succ) { + *fromProfile &= succ->hasProfileWeight(); + if (succ == to) + { + takenCount += succ->bbWeight; + } + else + { + notTakenCount += succ->bbWeight; + } + + return BasicBlockVisit::Continue; + }); + } + + if (!*fromProfile) + { + return 1.0 / from->NumSucc(this); + } + + if (fgProfileWeightsConsistent(takenCount, BB_ZERO_WEIGHT)) + { + return 0; + } + + weight_t likelihood = takenCount / (takenCount + notTakenCount); + return likelihood; +} + +//----------------------------------------------------------------------------- +// optSetExitWeight: Set the weight of a newly created exit, after it +// has been added to the flowgraph. +// +// Parameters: +// loop - The loop +// preheader - The new exit block +// +void Compiler::optSetExitWeight(FlowGraphNaturalLoop* loop, BasicBlock* exit) +{ + bool hasProfWeight = true; + + // Inherit first estimate from the exit target; optEstimateEdgeLikelihood + // may use it in its estimate if we do not have edge weights to estimate + // from (we also assume the exiting -> exit edges already inherited their + // edge weights from the previous edge). + exit->inheritWeight(exit->GetTarget()); + + weight_t exitWeight = BB_ZERO_WEIGHT; + for (FlowEdge* exitEdge : exit->PredEdges()) + { + BasicBlock* exiting = exitEdge->getSourceBlock(); + + bool fromProfile = false; + weight_t likelihood = optEstimateEdgeLikelihood(exiting, exit, &fromProfile); + hasProfWeight &= fromProfile; + + weight_t contribution = exiting->bbWeight * likelihood; + JITDUMP(" Estimated likelihood " FMT_BB " -> " FMT_BB " to be " FMT_WT " (contribution: " FMT_WT ")\n", + exiting->bbNum, exit->bbNum, likelihood, contribution); + + exitWeight += contribution; + + // Normalize exiting -> new exit weight + exitEdge->setEdgeWeights(contribution, contribution, exit); + } + + exit->RemoveFlags(BBF_PROF_WEIGHT | BBF_RUN_RARELY); + + exit->bbWeight = exitWeight; + if (hasProfWeight) + { + exit->SetFlags(BBF_PROF_WEIGHT); + } + + if (exitWeight == BB_ZERO_WEIGHT) + { + exit->SetFlags(BBF_RUN_RARELY); + return; + } + + // Normalize new exit -> old exit weight + FlowEdge* const edgeFromNewExit = fgGetPredForBlock(exit->GetTarget(), exit); + assert(edgeFromNewExit != nullptr); + edgeFromNewExit->setEdgeWeights(exit->bbWeight, exit->bbWeight, exit->GetTarget()); +} + /***************************************************************************** * * See if the given tree can be computed in the given precision (which must @@ -5083,43 +5372,39 @@ bool Compiler::optVNIsLoopInvariant(ValueNum vn, FlowGraphNaturalLoop* loop, VNS } //------------------------------------------------------------------------------ -// fgSetEHRegionForNewPreheader: Set the EH region for a newly inserted -// preheader. -// -// In which EH region should the header live? +// fgSetEHRegionForNewPreheaderOrExit: Set the EH region for a newly inserted +// preheader or exit block. // -// The preheader block is expected to have been added immediately before a -// block `next` in the loop that is also in the same EH region as the header. -// This is usually the lexically first block of the loop, but may also be the -// header itself. +// In which EH region should the block live? // -// If the `next` block is NOT the first block of a `try` region, the preheader -// can simply extend the header block's EH region. +// If the `next` block is NOT the first block of a `try` region, the new block +// can simply extend the next block's EH region. // // If the `next` block IS the first block of a `try`, we find its parent region // and use that. For mutual-protect regions, we need to find the actual parent, // as the block stores the most "nested" mutual region. For non-mutual-protect // regions, due to EH canonicalization, we are guaranteed that no other EH // regions begin on the same block, so looking to just the parent is -// sufficient. Note that we can't just extend the EH region of the header to -// the preheader, because the header will still be the target of backward -// branches from within the loop. If those backward branches come from outside -// the `try` (say, only the top half of the loop is a `try` region), then we -// can't branch to a non-first `try` region block (you always must enter the -// `try` in the first block). +// sufficient. +// Note that we can't just extend the EH region of the next block to the new +// block, because it may still be the target of other branches. If those +// branches come from outside the `try` then we can't branch to a non-first +// `try` region block (you always must enter the `try` in the first block). For +// example, for the preheader we can have backedges that come from outside the +// `try` (if, say, only the top half of the loop is a `try` region). For exits, +// we could similarly have branches to the old exit block from outside the `try`. // // Note that hoisting any code out of a try region, for example, to a preheader // block in a different EH region, needs to ensure that no exceptions will be -// thrown. +// thrown. Similar considerations are required for exits. // // Arguments: -// preheader - the new preheader block, which has already been added to the -// block list before a block inside the loop that shares EH -// region with the header. +// block - the new block, which has already been added to the +// block list. // -void Compiler::fgSetEHRegionForNewPreheader(BasicBlock* preheader) +void Compiler::fgSetEHRegionForNewPreheaderOrExit(BasicBlock* block) { - BasicBlock* next = preheader->Next(); + BasicBlock* next = block->Next(); if (bbIsTryBeg(next)) { @@ -5129,15 +5414,15 @@ void Compiler::fgSetEHRegionForNewPreheader(BasicBlock* preheader) if (newTryIndex == EHblkDsc::NO_ENCLOSING_INDEX) { // No EH try index. - preheader->clearTryIndex(); + block->clearTryIndex(); } else { - preheader->setTryIndex(newTryIndex); + block->setTryIndex(newTryIndex); } // What handler region to use? Use the same handler region as `next`. - preheader->copyHndIndex(next); + block->copyHndIndex(next); } else {