From d925b7de74f04843bc0391da8720075126cb4ef0 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Wed, 11 Dec 2024 16:52:20 -0800 Subject: [PATCH 1/6] JIT: split loop headers that are also try entries when there are backedges that have exited the try. This allows the preheader and header to be in the same EH region and may unblock other optimizations. This is done by splitting the header block and making the trailing portion be the try entry. Perform a mid (or end) block split if the initial (or all) statements in the header cannot throw. Fixes #96887. Enable cloning of loops with EH. --- src/coreclr/jit/compiler.h | 2 +- src/coreclr/jit/jitconfigvalues.h | 2 +- src/coreclr/jit/optimizer.cpp | 121 ++++++++++++++++++++++++++++-- 3 files changed, 115 insertions(+), 10 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 72f2a5d4df848..107bb08d6edf3 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -7134,7 +7134,7 @@ class Compiler void optCompactLoops(); void optCompactLoop(FlowGraphNaturalLoop* loop); - bool optCreatePreheader(FlowGraphNaturalLoop* loop); + bool optCreatePreheader(FlowGraphNaturalLoop* loop, bool* splitHeader); void optSetWeightForPreheaderOrExit(FlowGraphNaturalLoop* loop, BasicBlock* block); bool optCanonicalizeExits(FlowGraphNaturalLoop* loop); diff --git a/src/coreclr/jit/jitconfigvalues.h b/src/coreclr/jit/jitconfigvalues.h index 4c5fc2e8d5328..7b82ac1afc22c 100644 --- a/src/coreclr/jit/jitconfigvalues.h +++ b/src/coreclr/jit/jitconfigvalues.h @@ -59,7 +59,7 @@ CONFIG_INTEGER(JitBreakMorphTree, "JitBreakMorphTree", 0xffffffff) CONFIG_INTEGER(JitBreakOnBadCode, "JitBreakOnBadCode", 0) CONFIG_INTEGER(JitBreakOnMinOpts, "JITBreakOnMinOpts", 0) // Halt if jit switches to MinOpts CONFIG_INTEGER(JitCloneLoops, "JitCloneLoops", 1) // If 0, don't clone. Otherwise clone loops for optimizations. -CONFIG_INTEGER(JitCloneLoopsWithEH, "JitCloneLoopsWithEH", 0) // If 0, don't clone loops containing EH regions +CONFIG_INTEGER(JitCloneLoopsWithEH, "JitCloneLoopsWithEH", 1) // If 0, don't clone loops containing EH regions CONFIG_INTEGER(JitCloneLoopsWithGdvTests, "JitCloneLoopsWithGdvTests", 1) // If 0, don't clone loops based on // invariant type/method address tests RELEASE_CONFIG_INTEGER(JitCloneLoopsSizeLimit, "JitCloneLoopsSizeLimit", 400) // limit cloning to loops with no more diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp index c734c1c8a7a3a..046b70e831216 100644 --- a/src/coreclr/jit/optimizer.cpp +++ b/src/coreclr/jit/optimizer.cpp @@ -2749,11 +2749,24 @@ void Compiler::optFindLoops() // bool Compiler::optCanonicalizeLoops() { - bool changed = false; + bool changed = false; + bool splitHeaders = false; for (FlowGraphNaturalLoop* loop : m_loops->InReversePostOrder()) { - changed |= optCreatePreheader(loop); + bool splitHeader = false; + changed |= optCreatePreheader(loop, &splitHeader); + splitHeaders |= splitHeader; + } + + // If we split any headers we must rebuild DFS/loops. + // This should be relatively uncommon. + // + if (splitHeaders) + { + fgInvalidateDfsTree(); + m_dfsTree = fgComputeDfs(); + m_loops = FlowGraphNaturalLoops::Find(m_dfsTree); } // At this point we've created preheaders. That means we are working with @@ -2864,13 +2877,15 @@ void Compiler::optCompactLoop(FlowGraphNaturalLoop* loop) // // Parameters: // loop - The loop to create the preheader for +// splitHeader - [out] set true if the loop header was split // // Returns: -// True if a new preheader block had to be created. +// True if a new preheader block had to be created or if the header was split // -bool Compiler::optCreatePreheader(FlowGraphNaturalLoop* loop) +bool Compiler::optCreatePreheader(FlowGraphNaturalLoop* loop, bool* splitHeader) { - BasicBlock* header = loop->GetHeader(); + BasicBlock* const header = loop->GetHeader(); + *splitHeader = false; // If all loop backedges sources are within the same try region as the loop header, // then the preheader can be in the same try region as the header. @@ -2879,8 +2894,8 @@ bool Compiler::optCreatePreheader(FlowGraphNaturalLoop* loop) // unsigned preheaderEHRegion = EHblkDsc::NO_ENCLOSING_INDEX; bool inSameRegionAsHeader = true; - if (header->hasTryIndex()) - { + + auto checkBackedges = [&]() { preheaderEHRegion = header->getTryIndex(); for (FlowEdge* backEdge : loop->BackEdges()) { @@ -2894,6 +2909,96 @@ bool Compiler::optCreatePreheader(FlowGraphNaturalLoop* loop) break; } } + }; + + if (header->hasTryIndex()) + { + checkBackedges(); + } + + // If we couldn't place the preheader in the same EH region as the + // header, and the header is also a try entry, split the header before + // the first statement that can raise and exception, and so push the try + // entry down inside the loop. + // + if (!inSameRegionAsHeader && bbIsTryBeg(header)) + { + JITDUMP("Create preheader: splitting loop header / try entry " FMT_BB "\n", header->bbNum); + *splitHeader = true; + + Statement* const firstStmt = header->firstStmt(); + BasicBlock* newTryEntry = nullptr; + + if (firstStmt == nullptr) + { + // Empty header + // + newTryEntry = fgSplitBlockAtEnd(header); + } + else + { + // Non-empty header. + // + Statement* const lastStmt = header->lastStmt(); + bool const hasTerminator = header->HasTerminator(); + Statement* const stopStmt = hasTerminator ? lastStmt : nullptr; + Statement* splitBefore = firstStmt; + + while ((splitBefore != stopStmt) && (splitBefore->GetRootNode()->gtFlags & GTF_EXCEPT) == 0) + { + splitBefore = splitBefore->GetNextStmt(); + } + + // If no statement can throw, split at the end, as long as there's no terminator + // + if (splitBefore == nullptr) + { + assert(!hasTerminator); + newTryEntry = fgSplitBlockAtEnd(header); + } + // If the header has a single statement and needs a terminator, or the first statement + // can throw, split at the beginning + // + else if (splitBefore == firstStmt) + { + newTryEntry = fgSplitBlockAtBeginning(header); + } + // Else split in the middle + // + else + { + newTryEntry = fgSplitBlockAfterStatement(header, splitBefore->GetPrevStmt()); + } + } + + // update EH table, and keep track of the outermost enclosing try + // + EHblkDsc* outermostHBtab = nullptr; + for (EHblkDsc* const HBtab : EHClauses(this)) + { + if (HBtab->ebdTryBeg == header) + { + fgSetTryBeg(HBtab, newTryEntry); + outermostHBtab = HBtab; + } + } + assert(outermostHBtab != nullptr); + + // Recompute preheader placement + // + const unsigned enclosingTryIndex = outermostHBtab->ebdEnclosingTryIndex; + preheaderEHRegion = enclosingTryIndex; + + if (enclosingTryIndex == EHblkDsc::NO_ENCLOSING_INDEX) + { + header->clearTryIndex(); + inSameRegionAsHeader = true; + } + else + { + header->setTryIndex(enclosingTryIndex); + checkBackedges(); + } } if (!bbIsHandlerBeg(header) && (loop->EntryEdges().size() == 1)) @@ -2906,7 +3011,7 @@ bool Compiler::optCreatePreheader(FlowGraphNaturalLoop* loop) { JITDUMP("Natural loop " FMT_LP " already has preheader " FMT_BB "\n", loop->GetIndex(), preheaderCandidate->bbNum); - return false; + return *splitHeader; } } From e492001762a65af7e76547c200f2f3fc0bd132db Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Sat, 21 Dec 2024 14:29:50 -0800 Subject: [PATCH 2/6] need to consistently use the same traits --- src/coreclr/jit/compiler.h | 2 +- src/coreclr/jit/fgehopt.cpp | 11 ++++++----- src/coreclr/jit/flowgraph.cpp | 2 +- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 107bb08d6edf3..a831cfdab9306 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -2566,7 +2566,7 @@ struct RelopImplicationInfo // struct CloneTryInfo { - CloneTryInfo(Compiler* comp); + CloneTryInfo(BitVecTraits& traits); // bbID based traits and vector // diff --git a/src/coreclr/jit/fgehopt.cpp b/src/coreclr/jit/fgehopt.cpp index d619e501e3b56..06537ad1fbbd7 100644 --- a/src/coreclr/jit/fgehopt.cpp +++ b/src/coreclr/jit/fgehopt.cpp @@ -3155,19 +3155,20 @@ bool Compiler::fgCanCloneTryRegion(BasicBlock* tryEntry) { assert(bbIsTryBeg(tryEntry)); - CloneTryInfo info(this); + BitVecTraits traits(compBasicBlockID, this); + CloneTryInfo info(traits); BasicBlock* const result = fgCloneTryRegion(tryEntry, info); return result != nullptr; } //------------------------------------------------------------------------ -// CloneTryInfo::CloneTryInfo +// CloneTryInfo::CloneTryInfo: construct an object for cloning a try region // // Arguments: -// construct an object for cloning a try region +// traits - bbID based traits to use for the Visited set // -CloneTryInfo::CloneTryInfo(Compiler* comp) - : Traits(comp->compBasicBlockID, comp) +CloneTryInfo::CloneTryInfo(BitVecTraits& traits) + : Traits(traits) , Visited(BitVecOps::MakeEmpty(&Traits)) { } diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp index 127b76e63e848..c648e988a2e22 100644 --- a/src/coreclr/jit/flowgraph.cpp +++ b/src/coreclr/jit/flowgraph.cpp @@ -6212,7 +6212,7 @@ void FlowGraphNaturalLoop::DuplicateWithEH(BasicBlock** insertAfter, BlockToBloc // if (comp->bbIsTryBeg(blk)) { - CloneTryInfo info(comp); + CloneTryInfo info(traits); info.Map = map; info.AddEdges = false; info.ProfileScale = weightScale; From 43b14ad0171798449e243f6aaebbf1e55e3dac5a Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Thu, 26 Dec 2024 13:49:40 -0800 Subject: [PATCH 3/6] avoid odd x86 case; fix if conversion dumping --- src/coreclr/jit/fgehopt.cpp | 14 ++++++++++++-- src/coreclr/jit/ifconversion.cpp | 2 +- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/coreclr/jit/fgehopt.cpp b/src/coreclr/jit/fgehopt.cpp index 06537ad1fbbd7..a305f5c44437f 100644 --- a/src/coreclr/jit/fgehopt.cpp +++ b/src/coreclr/jit/fgehopt.cpp @@ -2633,11 +2633,21 @@ BasicBlock* Compiler::fgCloneTryRegion(BasicBlock* tryEntry, CloneTryInfo& info, #if defined(FEATURE_EH_WINDOWS_X86) // For non-funclet X86 we must also clone the next block after the callfinallyret. - // (it will contain an END_LFIN) + // (it will contain an END_LFIN). But if this block is also a CALLFINALLY we + // bail out, since we can't clone it in isolation, but we need to clone it. + // (a proper fix would be to split the block, perhaps). // if (!UsesFunclets()) { - addBlockToClone(block->GetTarget(), "lfin-continuation"); + BasicBlock* const lfin = block->GetTarget(); + + if (lfin->KindIs(BBJ_CALLFINALLY)) + { + JITDUMP("Can't clone, as an END_LFIN is contained in CALLFINALLY block " FMT_BB "\n", + lfin->bbNum); + return nullptr; + } + addBlockToClone(lfin, "lfin-continuation"); } #endif } diff --git a/src/coreclr/jit/ifconversion.cpp b/src/coreclr/jit/ifconversion.cpp index 6008bb432550b..570ae1ef2bdda 100644 --- a/src/coreclr/jit/ifconversion.cpp +++ b/src/coreclr/jit/ifconversion.cpp @@ -383,7 +383,7 @@ void OptIfConversionDsc::IfConvertDump() { assert(m_startBlock != nullptr); m_comp->fgDumpBlock(m_startBlock); - BasicBlock* dumpBlock = m_startBlock->KindIs(BBJ_COND) ? m_startBlock->GetFalseTarget() : m_startBlock->Next(); + BasicBlock* dumpBlock = m_startBlock->KindIs(BBJ_COND) ? m_startBlock->GetFalseTarget() : m_startBlock->GetTarget(); for (; dumpBlock != m_finalBlock; dumpBlock = dumpBlock->GetUniqueSucc()) { m_comp->fgDumpBlock(dumpBlock); From bfe577d3e07134ada2d4704897e37cf19232f981 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Tue, 7 Jan 2025 11:06:15 -0800 Subject: [PATCH 4/6] split headers after exit canonicalization --- src/coreclr/jit/compiler.h | 3 +- src/coreclr/jit/optimizer.cpp | 231 +++++++++++++++++----------------- 2 files changed, 119 insertions(+), 115 deletions(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index d3b5478e08955..053d1517af787 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -7131,8 +7131,9 @@ class Compiler void optCompactLoops(); void optCompactLoop(FlowGraphNaturalLoop* loop); - bool optCreatePreheader(FlowGraphNaturalLoop* loop, bool* splitHeader); + bool optCreatePreheader(FlowGraphNaturalLoop* loop); void optSetWeightForPreheaderOrExit(FlowGraphNaturalLoop* loop, BasicBlock* block); + bool optSplitHeaderIfNecessary(FlowGraphNaturalLoop* loop); bool optCanonicalizeExits(FlowGraphNaturalLoop* loop); bool optCanonicalizeExit(FlowGraphNaturalLoop* loop, BasicBlock* exit); diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp index 046b70e831216..b886c11103484 100644 --- a/src/coreclr/jit/optimizer.cpp +++ b/src/coreclr/jit/optimizer.cpp @@ -2749,24 +2749,11 @@ void Compiler::optFindLoops() // bool Compiler::optCanonicalizeLoops() { - bool changed = false; - bool splitHeaders = false; + bool changed = false; for (FlowGraphNaturalLoop* loop : m_loops->InReversePostOrder()) { - bool splitHeader = false; - changed |= optCreatePreheader(loop, &splitHeader); - splitHeaders |= splitHeader; - } - - // If we split any headers we must rebuild DFS/loops. - // This should be relatively uncommon. - // - if (splitHeaders) - { - fgInvalidateDfsTree(); - m_dfsTree = fgComputeDfs(); - m_loops = FlowGraphNaturalLoops::Find(m_dfsTree); + changed |= optCreatePreheader(loop); } // At this point we've created preheaders. That means we are working with @@ -2775,7 +2762,7 @@ bool Compiler::optCanonicalizeLoops() // 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. + // canonicalization still 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 @@ -2785,6 +2772,13 @@ bool Compiler::optCanonicalizeLoops() changed |= optCanonicalizeExits(loop); } + // We may have created preheaders in different EH regions than the loop + // header. If so, split the header to put it into the same region as the preheader. + for (FlowGraphNaturalLoop* loop : m_loops->InReversePostOrder()) + { + changed |= optSplitHeaderIfNecessary(loop); + } + return changed; } @@ -2877,15 +2871,13 @@ void Compiler::optCompactLoop(FlowGraphNaturalLoop* loop) // // Parameters: // loop - The loop to create the preheader for -// splitHeader - [out] set true if the loop header was split // // Returns: -// True if a new preheader block had to be created or if the header was split +// True if a new preheader block had to be created. // -bool Compiler::optCreatePreheader(FlowGraphNaturalLoop* loop, bool* splitHeader) +bool Compiler::optCreatePreheader(FlowGraphNaturalLoop* loop) { - BasicBlock* const header = loop->GetHeader(); - *splitHeader = false; + BasicBlock* header = loop->GetHeader(); // If all loop backedges sources are within the same try region as the loop header, // then the preheader can be in the same try region as the header. @@ -2894,8 +2886,8 @@ bool Compiler::optCreatePreheader(FlowGraphNaturalLoop* loop, bool* splitHeader) // unsigned preheaderEHRegion = EHblkDsc::NO_ENCLOSING_INDEX; bool inSameRegionAsHeader = true; - - auto checkBackedges = [&]() { + if (header->hasTryIndex()) + { preheaderEHRegion = header->getTryIndex(); for (FlowEdge* backEdge : loop->BackEdges()) { @@ -2909,96 +2901,6 @@ bool Compiler::optCreatePreheader(FlowGraphNaturalLoop* loop, bool* splitHeader) break; } } - }; - - if (header->hasTryIndex()) - { - checkBackedges(); - } - - // If we couldn't place the preheader in the same EH region as the - // header, and the header is also a try entry, split the header before - // the first statement that can raise and exception, and so push the try - // entry down inside the loop. - // - if (!inSameRegionAsHeader && bbIsTryBeg(header)) - { - JITDUMP("Create preheader: splitting loop header / try entry " FMT_BB "\n", header->bbNum); - *splitHeader = true; - - Statement* const firstStmt = header->firstStmt(); - BasicBlock* newTryEntry = nullptr; - - if (firstStmt == nullptr) - { - // Empty header - // - newTryEntry = fgSplitBlockAtEnd(header); - } - else - { - // Non-empty header. - // - Statement* const lastStmt = header->lastStmt(); - bool const hasTerminator = header->HasTerminator(); - Statement* const stopStmt = hasTerminator ? lastStmt : nullptr; - Statement* splitBefore = firstStmt; - - while ((splitBefore != stopStmt) && (splitBefore->GetRootNode()->gtFlags & GTF_EXCEPT) == 0) - { - splitBefore = splitBefore->GetNextStmt(); - } - - // If no statement can throw, split at the end, as long as there's no terminator - // - if (splitBefore == nullptr) - { - assert(!hasTerminator); - newTryEntry = fgSplitBlockAtEnd(header); - } - // If the header has a single statement and needs a terminator, or the first statement - // can throw, split at the beginning - // - else if (splitBefore == firstStmt) - { - newTryEntry = fgSplitBlockAtBeginning(header); - } - // Else split in the middle - // - else - { - newTryEntry = fgSplitBlockAfterStatement(header, splitBefore->GetPrevStmt()); - } - } - - // update EH table, and keep track of the outermost enclosing try - // - EHblkDsc* outermostHBtab = nullptr; - for (EHblkDsc* const HBtab : EHClauses(this)) - { - if (HBtab->ebdTryBeg == header) - { - fgSetTryBeg(HBtab, newTryEntry); - outermostHBtab = HBtab; - } - } - assert(outermostHBtab != nullptr); - - // Recompute preheader placement - // - const unsigned enclosingTryIndex = outermostHBtab->ebdEnclosingTryIndex; - preheaderEHRegion = enclosingTryIndex; - - if (enclosingTryIndex == EHblkDsc::NO_ENCLOSING_INDEX) - { - header->clearTryIndex(); - inSameRegionAsHeader = true; - } - else - { - header->setTryIndex(enclosingTryIndex); - checkBackedges(); - } } if (!bbIsHandlerBeg(header) && (loop->EntryEdges().size() == 1)) @@ -3011,7 +2913,7 @@ bool Compiler::optCreatePreheader(FlowGraphNaturalLoop* loop, bool* splitHeader) { JITDUMP("Natural loop " FMT_LP " already has preheader " FMT_BB "\n", loop->GetIndex(), preheaderCandidate->bbNum); - return *splitHeader; + return false; } } @@ -3048,6 +2950,107 @@ bool Compiler::optCreatePreheader(FlowGraphNaturalLoop* loop, bool* splitHeader) return true; } +//----------------------------------------------------------------------------- +// optSplitHeaderIfNecessary: If preheader and header are in different try +// regions, split the header to put it into the same try region as the preheader +// +// Parameters: +// loop - loop that may need header splitting +// +// Returns: +// True if the header was split +// +bool Compiler::optSplitHeaderIfNecessary(FlowGraphNaturalLoop* loop) +{ + BasicBlock* header = loop->GetHeader(); + BasicBlock* preheader = loop->GetPreheader(); + + if (BasicBlock::sameTryRegion(header, preheader)) + { + return false; + } + + // If the preheader and header are in different try regions, + // // the header should be a try entry. + // + assert(bbIsTryBeg(header)); + + JITDUMP("Splitting " FMT_LP " header / try entry " FMT_BB "\n", loop->GetIndex(), header->bbNum); + + Statement* const firstStmt = header->firstStmt(); + BasicBlock* newTryEntry = nullptr; + + if (firstStmt == nullptr) + { + // Empty header + // + newTryEntry = fgSplitBlockAtEnd(header); + } + else + { + // Non-empty header. + // + Statement* const lastStmt = header->lastStmt(); + bool const hasTerminator = header->HasTerminator(); + Statement* const stopStmt = hasTerminator ? lastStmt : nullptr; + Statement* splitBefore = firstStmt; + + while ((splitBefore != stopStmt) && (splitBefore->GetRootNode()->gtFlags & GTF_EXCEPT) == 0) + { + splitBefore = splitBefore->GetNextStmt(); + } + + // If no statement can throw, split at the end, as long as there's no terminator + // + if (splitBefore == nullptr) + { + assert(!hasTerminator); + newTryEntry = fgSplitBlockAtEnd(header); + } + // If the header has a single statement and needs a terminator, or the first statement + // can throw, split at the beginning + // + else if (splitBefore == firstStmt) + { + newTryEntry = fgSplitBlockAtBeginning(header); + } + // Else split in the middle + // + else + { + newTryEntry = fgSplitBlockAfterStatement(header, splitBefore->GetPrevStmt()); + } + } + + // update EH table, and keep track of the outermost enclosing try + // + EHblkDsc* outermostHBtab = nullptr; + for (EHblkDsc* const HBtab : EHClauses(this)) + { + if (HBtab->ebdTryBeg == header) + { + fgSetTryBeg(HBtab, newTryEntry); + outermostHBtab = HBtab; + } + } + assert(outermostHBtab != nullptr); + + // Recompute preheader placement + // + const unsigned enclosingTryIndex = outermostHBtab->ebdEnclosingTryIndex; + + if (enclosingTryIndex == EHblkDsc::NO_ENCLOSING_INDEX) + { + header->clearTryIndex(); + } + else + { + header->setTryIndex(enclosingTryIndex); + } + + return true; +} + //----------------------------------------------------------------------------- // optCanonicalizeExits: Canonicalize all regular exits of the loop so that // they have only loop predecessors. From 923e0e6d8d55ba696ee934ec1f44253c746a9c45 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Tue, 7 Jan 2025 13:03:55 -0800 Subject: [PATCH 5/6] add diagnostic assert that loop headers are not try entries --- src/coreclr/jit/compiler.h | 2 ++ src/coreclr/jit/fgdiagnostic.cpp | 1 + src/coreclr/jit/flowgraph.cpp | 12 ++++++++++++ src/coreclr/jit/optimizer.cpp | 9 ++++++++- 4 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 053d1517af787..33c0f3fe7bed2 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -2240,6 +2240,8 @@ class FlowGraphNaturalLoop bool IsPostDominatedOnLoopIteration(BasicBlock* block, BasicBlock* postDominator); + void SetEntryEdge(FlowEdge* newEdge); + #ifdef DEBUG static void Dump(FlowGraphNaturalLoop* loop); #endif // DEBUG diff --git a/src/coreclr/jit/fgdiagnostic.cpp b/src/coreclr/jit/fgdiagnostic.cpp index 642d2d70bb1fb..7c5e7638ada5b 100644 --- a/src/coreclr/jit/fgdiagnostic.cpp +++ b/src/coreclr/jit/fgdiagnostic.cpp @@ -4704,6 +4704,7 @@ void Compiler::fgDebugCheckLoops() { assert(loop->EntryEdges().size() == 1); assert(loop->EntryEdge(0)->getSourceBlock()->KindIs(BBJ_ALWAYS)); + assert(!bbIsTryBeg(loop->GetHeader())); loop->VisitRegularExitBlocks([=](BasicBlock* exit) { for (BasicBlock* pred : exit->PredBlocks()) diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp index c648e988a2e22..6a41de3c7ea34 100644 --- a/src/coreclr/jit/flowgraph.cpp +++ b/src/coreclr/jit/flowgraph.cpp @@ -4393,6 +4393,18 @@ BasicBlock* FlowGraphNaturalLoop::GetPreheader() const return preheader; } +//------------------------------------------------------------------------ +// SetEntryEdge: Set the entry edge of a loop +// +// Arguments: +// entryEdge - The new entry edge +// +void FlowGraphNaturalLoop::SetEntryEdge(FlowEdge* entryEdge) +{ + m_entryEdges.clear(); + m_entryEdges.push_back(entryEdge); +} + //------------------------------------------------------------------------ // GetDepth: Get the depth of the loop. // diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp index b886c11103484..2db1ee4112f38 100644 --- a/src/coreclr/jit/optimizer.cpp +++ b/src/coreclr/jit/optimizer.cpp @@ -2945,6 +2945,8 @@ bool Compiler::optCreatePreheader(FlowGraphNaturalLoop* loop) fgReplaceJumpTarget(enterBlock, header, preheader); } + loop->SetEntryEdge(newEdge); + optSetWeightForPreheaderOrExit(loop, preheader); return true; @@ -2960,6 +2962,9 @@ bool Compiler::optCreatePreheader(FlowGraphNaturalLoop* loop) // Returns: // True if the header was split // +// Notes: +// Ensures that no loop header is also a try entry. +// bool Compiler::optSplitHeaderIfNecessary(FlowGraphNaturalLoop* loop) { BasicBlock* header = loop->GetHeader(); @@ -2967,11 +2972,12 @@ bool Compiler::optSplitHeaderIfNecessary(FlowGraphNaturalLoop* loop) if (BasicBlock::sameTryRegion(header, preheader)) { + assert(!bbIsTryBeg(header)); return false; } // If the preheader and header are in different try regions, - // // the header should be a try entry. + // the header should be a try entry. // assert(bbIsTryBeg(header)); @@ -3048,6 +3054,7 @@ bool Compiler::optSplitHeaderIfNecessary(FlowGraphNaturalLoop* loop) header->setTryIndex(enclosingTryIndex); } + assert(!bbIsTryBeg(header)); return true; } From 3948da89f6295b22957525c7455a5befd49c4a54 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Tue, 7 Jan 2025 17:42:35 -0800 Subject: [PATCH 6/6] more cautious header splitting --- src/coreclr/jit/optimizer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp index 2db1ee4112f38..fedebf2890c6f 100644 --- a/src/coreclr/jit/optimizer.cpp +++ b/src/coreclr/jit/optimizer.cpp @@ -3001,7 +3001,7 @@ bool Compiler::optSplitHeaderIfNecessary(FlowGraphNaturalLoop* loop) Statement* const stopStmt = hasTerminator ? lastStmt : nullptr; Statement* splitBefore = firstStmt; - while ((splitBefore != stopStmt) && (splitBefore->GetRootNode()->gtFlags & GTF_EXCEPT) == 0) + while ((splitBefore != stopStmt) && (splitBefore->GetRootNode()->gtFlags & (GTF_EXCEPT | GTF_CALL)) == 0) { splitBefore = splitBefore->GetNextStmt(); }