diff --git a/src/coreclr/jit/block.cpp b/src/coreclr/jit/block.cpp index 2ac9de809d6f65..122f77d7c61eb4 100644 --- a/src/coreclr/jit/block.cpp +++ b/src/coreclr/jit/block.cpp @@ -591,6 +591,10 @@ void BasicBlock::dspFlags() { printf("gcpoll "); } + if (HasFlag(BBF_OLD_LOOP_HEADER_QUIRK)) + { + printf("loopheader "); + } } /***************************************************************************** diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index 5098359ec6193f..12c6c1c0d5503a 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/src/coreclr/jit/compiler.cpp @@ -4833,9 +4833,9 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl // DoPhase(this, PHASE_UNROLL_LOOPS, &Compiler::optUnrollLoops); - // Clear loop table info that is not used after this point, and might become invalid. - // - DoPhase(this, PHASE_CLEAR_LOOP_INFO, &Compiler::optClearLoopIterInfo); + // The loop table is no longer valid. + optLoopTableValid = false; + optLoopTable = nullptr; } #ifdef DEBUG @@ -4852,6 +4852,10 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl // DoPhase(this, PHASE_MARK_LOCAL_VARS, &Compiler::lvaMarkLocalVars); + // Dominator and reachability sets are no longer valid. + fgDomsComputed = false; + fgCompactRenumberQuirk = true; + // IMPORTANT, after this point, locals are ref counted. // However, ref counts are not kept incrementally up to date. assert(lvaLocalVarRefCounted()); @@ -5023,11 +5027,8 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl } } - // Dominator and reachability sets are no longer valid. - // The loop table is no longer valid. - fgDomsComputed = false; - optLoopTableValid = false; optLoopsRequirePreHeaders = false; + fgCompactRenumberQuirk = false; #ifdef DEBUG DoPhase(this, PHASE_STRESS_SPLIT_TREE, &Compiler::StressSplitTree); @@ -5897,13 +5898,17 @@ void Compiler::RecomputeLoopInfo() fgDomsComputed = false; fgComputeReachability(); optSetBlockWeights(); - // Rebuild the loop tree annotations themselves - // But don't leave the iter info lying around. optFindLoops(); - optClearLoopIterInfo(); m_dfsTree = fgComputeDfs(); optFindNewLoops(); + + // Dominators and the loop table are computed above for old<->new loop + // crossreferencing, but they are not actually used for optimization + // anymore. + optLoopTableValid = false; + optLoopTable = nullptr; + fgDomsComputed = false; } /*****************************************************************************/ diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index b93a69e67c387e..441a3d99a27c76 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -5102,7 +5102,8 @@ class Compiler bool fgDomsComputed; // Have we computed the dominator sets? bool fgReturnBlocksComputed; // Have we computed the return blocks list? bool fgOptimizedFinally; // Did we optimize any try-finallys? - bool fgCanonicalizedFirstBB; // Quirk: did we end up canonicalizing first BB? + bool fgCanonicalizedFirstBB; // TODO-Quirk: did we end up canonicalizing first BB? + bool fgCompactRenumberQuirk; // TODO-Quirk: Should fgCompactBlocks renumber BBs above fgDomBBcount? bool fgHasSwitch; // any BBJ_SWITCH jumps? @@ -6800,10 +6801,6 @@ class Compiler // VNPhi's connect VN's to the SSA definition, so we can know if the SSA def occurs in the loop. bool optVNIsLoopInvariant(ValueNum vn, FlowGraphNaturalLoop* loop, VNSet* recordedVNs); - // If "blk" is the entry block of a natural loop, returns true and sets "*pLnum" to the index of the loop - // in the loop table. - bool optBlockIsLoopEntry(BasicBlock* blk, unsigned* pLnum); - // Records the set of "side effects" of all loops: fields (object instance and static) // written to, and SZ-array element type equivalence classes updated. void optComputeLoopSideEffects(); @@ -7106,8 +7103,6 @@ class Compiler BasicBlock* exit, unsigned char exitCnt); - PhaseStatus optClearLoopIterInfo(); - #ifdef DEBUG void optPrintLoopInfo(unsigned lnum, bool printVerbose = false); void optPrintLoopInfo(const LoopDsc* loop, bool printVerbose = false); diff --git a/src/coreclr/jit/compphases.h b/src/coreclr/jit/compphases.h index 73e0eeac6c9a37..edba437cff884b 100644 --- a/src/coreclr/jit/compphases.h +++ b/src/coreclr/jit/compphases.h @@ -70,7 +70,6 @@ CompPhaseNameMacro(PHASE_ZERO_INITS, "Redundant zero Inits", CompPhaseNameMacro(PHASE_FIND_LOOPS, "Find loops", false, -1, false) CompPhaseNameMacro(PHASE_CLONE_LOOPS, "Clone loops", false, -1, false) CompPhaseNameMacro(PHASE_UNROLL_LOOPS, "Unroll loops", false, -1, false) -CompPhaseNameMacro(PHASE_CLEAR_LOOP_INFO, "Clear loop info", false, -1, false) CompPhaseNameMacro(PHASE_MORPH_MDARR, "Morph array ops", false, -1, false) CompPhaseNameMacro(PHASE_HOIST_LOOP_CODE, "Hoist loop code", false, -1, false) CompPhaseNameMacro(PHASE_MARK_LOCAL_VARS, "Mark local vars", false, -1, false) diff --git a/src/coreclr/jit/fgbasic.cpp b/src/coreclr/jit/fgbasic.cpp index c36bf5656809ab..54202b0cfdf0b1 100644 --- a/src/coreclr/jit/fgbasic.cpp +++ b/src/coreclr/jit/fgbasic.cpp @@ -33,6 +33,7 @@ void Compiler::fgInit() /* We haven't yet computed the dominator sets */ fgDomsComputed = false; fgReturnBlocksComputed = false; + fgCompactRenumberQuirk = false; #ifdef DEBUG fgReachabilitySetsValid = false; diff --git a/src/coreclr/jit/fgdiagnostic.cpp b/src/coreclr/jit/fgdiagnostic.cpp index 78fa18cbe8a1a1..4e3eb7d4e4f3c4 100644 --- a/src/coreclr/jit/fgdiagnostic.cpp +++ b/src/coreclr/jit/fgdiagnostic.cpp @@ -4763,7 +4763,6 @@ void Compiler::fgDebugCheckSsa() } assert(fgSsaPassesCompleted > 0); - assert(fgDomsComputed); // Visit the blocks that SSA initially renamed // diff --git a/src/coreclr/jit/fgopt.cpp b/src/coreclr/jit/fgopt.cpp index 75d9ce7672905d..09b902a12683c1 100644 --- a/src/coreclr/jit/fgopt.cpp +++ b/src/coreclr/jit/fgopt.cpp @@ -1925,7 +1925,7 @@ bool Compiler::fgCanCompactBlocks(BasicBlock* block, BasicBlock* bNext) } // Don't compact away any loop entry blocks that we added in optCanonicalizeLoops - if (optIsLoopEntry(block)) + if (block->HasFlag(BBF_OLD_LOOP_HEADER_QUIRK)) { return false; } @@ -2355,20 +2355,28 @@ void Compiler::fgCompactBlocks(BasicBlock* block, BasicBlock* bNext) // before the updates, we can create pred lists with duplicate m_block->bbNum // values (though different m_blocks). // - if (fgDomsComputed && (block->bbNum > fgDomBBcount)) + if ((fgDomsComputed || fgCompactRenumberQuirk) && (block->bbNum > fgDomBBcount)) { - assert(fgReachabilitySetsValid); - BlockSetOps::Assign(this, block->bbReach, bNext->bbReach); - BlockSetOps::ClearD(this, bNext->bbReach); + if (fgDomsComputed) + { + assert(fgReachabilitySetsValid); + BlockSetOps::Assign(this, block->bbReach, bNext->bbReach); + BlockSetOps::ClearD(this, bNext->bbReach); - block->bbIDom = bNext->bbIDom; - bNext->bbIDom = nullptr; + block->bbIDom = bNext->bbIDom; + bNext->bbIDom = nullptr; - // In this case, there's no need to update the preorder and postorder numbering - // since we're changing the bbNum, this makes the basic block all set. - // - JITDUMP("Renumbering " FMT_BB " to be " FMT_BB " to preserve dominator information\n", block->bbNum, - bNext->bbNum); + // In this case, there's no need to update the preorder and postorder numbering + // since we're changing the bbNum, this makes the basic block all set. + // + JITDUMP("Renumbering " FMT_BB " to be " FMT_BB " to preserve dominator information\n", block->bbNum, + bNext->bbNum); + } + else + { + // TODO-Quirk: Remove + JITDUMP("Renumbering " FMT_BB " to be " FMT_BB " for a quirk\n", block->bbNum, bNext->bbNum); + } block->bbNum = bNext->bbNum; @@ -2387,7 +2395,10 @@ void Compiler::fgCompactBlocks(BasicBlock* block, BasicBlock* bNext) } } - fgUpdateLoopsAfterCompacting(block, bNext); + if (optLoopTableValid) + { + fgUpdateLoopsAfterCompacting(block, bNext); + } #if DEBUG if (verbose && 0) @@ -6227,8 +6238,11 @@ bool Compiler::fgUpdateFlowGraph(bool doTailDuplication, bool isPhase) /* Mark the block as removed */ bNext->SetFlags(BBF_REMOVED); - // Update the loop table if we removed the bottom of a loop, for example. - fgUpdateLoopsAfterCompacting(block, bNext); + if (optLoopTableValid) + { + // Update the loop table if we removed the bottom of a loop, for example. + fgUpdateLoopsAfterCompacting(block, bNext); + } // If this is the first Cold basic block update fgFirstColdBlock if (bNext->IsFirstColdBlock(this)) diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp index 6873aebe8f952b..8e252b8dee4497 100644 --- a/src/coreclr/jit/flowgraph.cpp +++ b/src/coreclr/jit/flowgraph.cpp @@ -306,19 +306,23 @@ BasicBlock* Compiler::fgCreateGCPoll(GCPollType pollType, BasicBlock* block) // Mark Poll as rarely run. poll->bbSetRunRarely(); - poll->bbNatLoopNum = lpIndex; // Set the bbNatLoopNum in case we are in a loop - bottom->bbNatLoopNum = lpIndex; // Set the bbNatLoopNum in case we are in a loop - if (lpIndex != BasicBlock::NOT_IN_LOOP) + if (optLoopTableValid) { - // Set the new lpBottom in the natural loop table - optLoopTable[lpIndex].lpBottom = bottom; - } + poll->bbNatLoopNum = lpIndex; // Set the bbNatLoopNum in case we are in a loop - if (lpIndexFallThrough != BasicBlock::NOT_IN_LOOP) - { - // Set the new lpHead in the natural loop table - optLoopTable[lpIndexFallThrough].lpHead = bottom; + bottom->bbNatLoopNum = lpIndex; // Set the bbNatLoopNum in case we are in a loop + if (lpIndex != BasicBlock::NOT_IN_LOOP) + { + // Set the new lpBottom in the natural loop table + optLoopTable[lpIndex].lpBottom = bottom; + } + + if (lpIndexFallThrough != BasicBlock::NOT_IN_LOOP) + { + // Set the new lpHead in the natural loop table + optLoopTable[lpIndexFallThrough].lpHead = bottom; + } } // Add the GC_CALL node to Poll. @@ -2696,7 +2700,7 @@ bool Compiler::fgSimpleLowerCastOfSmpOp(LIR::Range& range, GenTreeCast* cast) // BasicBlock* Compiler::fgGetDomSpeculatively(const BasicBlock* block) { - assert(fgDomsComputed); + assert(fgSsaDomTree != nullptr); BasicBlock* lastReachablePred = nullptr; // Check if we have unreachable preds @@ -2709,7 +2713,7 @@ BasicBlock* Compiler::fgGetDomSpeculatively(const BasicBlock* block) } // We check pred's count of InEdges - it's quite conservative. - // We, probably, could use fgReachable(fgFirstBb, pred) here to detect unreachable preds + // We, probably, could use optReachable(fgFirstBb, pred) here to detect unreachable preds if (predBlock->countOfInEdges() > 0) { if (lastReachablePred != nullptr) diff --git a/src/coreclr/jit/morph.cpp b/src/coreclr/jit/morph.cpp index b9b7b97d80416b..b5269d746e2227 100644 --- a/src/coreclr/jit/morph.cpp +++ b/src/coreclr/jit/morph.cpp @@ -13193,8 +13193,8 @@ Compiler::FoldResult Compiler::fgFoldConditional(BasicBlock* block) /* Unmark the loop if we are removing a backwards branch */ /* dest block must also be marked as a loop head and */ /* We must be able to reach the backedge block */ - if (block->GetTrueTarget()->isLoopHead() && (block->GetTrueTarget()->bbNum <= block->bbNum) && - fgReachable(block->GetTrueTarget(), block)) + if (optLoopTableValid && block->GetTrueTarget()->isLoopHead() && + (block->GetTrueTarget()->bbNum <= block->bbNum) && fgReachable(block->GetTrueTarget(), block)) { optUnmarkLoopBlocks(block->GetTrueTarget(), block); } diff --git a/src/coreclr/jit/optimizebools.cpp b/src/coreclr/jit/optimizebools.cpp index 76058748e3baf4..e5d366548e53aa 100644 --- a/src/coreclr/jit/optimizebools.cpp +++ b/src/coreclr/jit/optimizebools.cpp @@ -1339,10 +1339,13 @@ void OptBoolsDsc::optOptimizeBoolsUpdateTrees() } // Update loop table - m_comp->fgUpdateLoopsAfterCompacting(m_b1, m_b2); - if (optReturnBlock) + if (m_comp->optLoopTableValid) { - m_comp->fgUpdateLoopsAfterCompacting(m_b1, m_b3); + m_comp->fgUpdateLoopsAfterCompacting(m_b1, m_b2); + if (optReturnBlock) + { + m_comp->fgUpdateLoopsAfterCompacting(m_b1, m_b3); + } } // Update IL range of first block diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp index 145c98c64a88a5..8fb135f82f28a7 100644 --- a/src/coreclr/jit/optimizer.cpp +++ b/src/coreclr/jit/optimizer.cpp @@ -520,29 +520,6 @@ void Compiler::optUpdateLoopsBeforeRemoveBlock(BasicBlock* block, bool skipUnmar } } -//------------------------------------------------------------------------ -// optClearLoopIterInfo: Clear the info related to LPFLG_ITER loops in the loop table. -// The various fields related to iterators is known to be valid for loop cloning and unrolling, -// but becomes invalid afterwards. Clear the info that might be used incorrectly afterwards -// in JitDump or by subsequent phases. -// -PhaseStatus Compiler::optClearLoopIterInfo() -{ - for (unsigned lnum = 0; lnum < optLoopCount; lnum++) - { - LoopDsc& loop = optLoopTable[lnum]; - loop.lpFlags &= ~(LPFLG_ITER | LPFLG_CONST_INIT | LPFLG_SIMD_LIMIT | LPFLG_VAR_LIMIT | LPFLG_CONST_LIMIT | - LPFLG_ARRLEN_LIMIT); - - loop.lpIterTree = nullptr; - loop.lpInitBlock = nullptr; - loop.lpConstInit = -1; - loop.lpTestTree = nullptr; - } - - return PhaseStatus::MODIFIED_NOTHING; -} - #ifdef DEBUG /***************************************************************************** @@ -5543,6 +5520,11 @@ void Compiler::optFindNewLoops() // edges, so be conservative here. fgMightHaveNaturalLoops = (m_loops->NumLoops() > 0) || m_loops->HaveNonNaturalLoopCycles(); + for (BasicBlock* block : Blocks()) + { + block->RemoveFlags(BBF_OLD_LOOP_HEADER_QUIRK); + } + for (FlowGraphNaturalLoop* loop : m_loops->InReversePostOrder()) { BasicBlock* head = loop->GetHeader(); @@ -8349,23 +8331,6 @@ bool Compiler::fgCreateLoopPreHeader(unsigned lnum) return true; } -bool Compiler::optBlockIsLoopEntry(BasicBlock* blk, unsigned* pLnum) -{ - for (unsigned lnum = blk->bbNatLoopNum; lnum != BasicBlock::NOT_IN_LOOP; lnum = optLoopTable[lnum].lpParent) - { - if (optLoopTable[lnum].lpIsRemoved()) - { - continue; - } - if (optLoopTable[lnum].lpEntry == blk) - { - *pLnum = lnum; - return true; - } - } - return false; -} - LoopSideEffects::LoopSideEffects() : VarInOut(VarSetOps::UninitVal()), VarUseDef(VarSetOps::UninitVal()) { for (MemoryKind mk : allMemoryKinds()) diff --git a/src/coreclr/jit/redundantbranchopts.cpp b/src/coreclr/jit/redundantbranchopts.cpp index 46eec126d0495d..59d3eec795b0f6 100644 --- a/src/coreclr/jit/redundantbranchopts.cpp +++ b/src/coreclr/jit/redundantbranchopts.cpp @@ -779,36 +779,39 @@ bool Compiler::optJumpThreadCheck(BasicBlock* const block, BasicBlock* const dom return false; } - // If block is a loop header, skip jump threading. - // - // This is an artificial limitation to ensure that subsequent loop table valididity - // checking can pass. We do not expect a loop entry to have multiple non-loop predecessors. - // - // This only blocks jump threading in a small number of cases. - // Revisit once we can ensure that loop headers are not critical blocks. - // - // Likewise we can't properly update the loop table if we thread the entry block. - // Not clear how much impact this has. - // - for (unsigned loopNum = 0; loopNum < optLoopCount; loopNum++) + if (optLoopTableValid) { - const LoopDsc& loop = optLoopTable[loopNum]; - - if (loop.lpIsRemoved()) + // If block is a loop header, skip jump threading. + // + // This is an artificial limitation to ensure that subsequent loop table valididity + // checking can pass. We do not expect a loop entry to have multiple non-loop predecessors. + // + // This only blocks jump threading in a small number of cases. + // Revisit once we can ensure that loop headers are not critical blocks. + // + // Likewise we can't properly update the loop table if we thread the entry block. + // Not clear how much impact this has. + // + for (unsigned loopNum = 0; loopNum < optLoopCount; loopNum++) { - continue; - } + const LoopDsc& loop = optLoopTable[loopNum]; - if (block == loop.lpHead) - { - JITDUMP(FMT_BB " is the header for " FMT_LP "; no threading\n", block->bbNum, loopNum); - return false; - } + if (loop.lpIsRemoved()) + { + continue; + } - if (block == loop.lpEntry) - { - JITDUMP(FMT_BB " is the entry for " FMT_LP "; no threading\n", block->bbNum, loopNum); - return false; + if (block == loop.lpHead) + { + JITDUMP(FMT_BB " is the header for " FMT_LP "; no threading\n", block->bbNum, loopNum); + return false; + } + + if (block == loop.lpEntry) + { + JITDUMP(FMT_BB " is the entry for " FMT_LP "; no threading\n", block->bbNum, loopNum); + return false; + } } }