From fd23d60e0fb7dca0b2e9583b0d6a7bae7e7373ca Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Mon, 7 Oct 2024 07:48:39 -0700 Subject: [PATCH 1/3] unblock cloning of loops where the header is a try begin --- src/coreclr/jit/flowgraph.cpp | 4 +-- src/coreclr/jit/loopcloning.cpp | 1 - src/coreclr/jit/optimizer.cpp | 45 ++++++++++++++++++++++++++------- 3 files changed, 38 insertions(+), 12 deletions(-) diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp index 5a90961bc5d01..cbfd3ef9bfac8 100644 --- a/src/coreclr/jit/flowgraph.cpp +++ b/src/coreclr/jit/flowgraph.cpp @@ -5846,9 +5846,9 @@ bool FlowGraphNaturalLoop::CanDuplicate(INDEBUG(const char** reason)) Compiler* comp = m_dfsTree->GetCompiler(); BasicBlockVisit result = VisitLoopBlocks([=](BasicBlock* block) { - if (comp->bbIsTryBeg(block)) + if (!BasicBlock::sameEHRegion(block, GetHeader())) { - INDEBUG(*reason = "Loop has a `try` begin"); + INDEBUG(*reason = "Loop not entirely within one EH region"); return BasicBlockVisit::Abort; } diff --git a/src/coreclr/jit/loopcloning.cpp b/src/coreclr/jit/loopcloning.cpp index 0a74288cb0b7e..bb68f1b4c874a 100644 --- a/src/coreclr/jit/loopcloning.cpp +++ b/src/coreclr/jit/loopcloning.cpp @@ -1841,7 +1841,6 @@ bool Compiler::optIsLoopClonable(FlowGraphNaturalLoop* loop, LoopCloneContext* c loop->GetIndex()); return false; } - assert(!requireIterable || !lvaVarAddrExposed(iterInfo->IterVar)); if (requireIterable) diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp index f9880026eab26..2e88b79838d19 100644 --- a/src/coreclr/jit/optimizer.cpp +++ b/src/coreclr/jit/optimizer.cpp @@ -2866,15 +2866,33 @@ bool Compiler::optCreatePreheader(FlowGraphNaturalLoop* loop) { BasicBlock* header = loop->GetHeader(); - // If the header is already a try entry then we need to keep it as such - // since blocks from within the loop will be jumping back to it after we're - // done. Thus, in that case we insert the preheader in the enclosing try - // region. - unsigned headerEHRegion = header->hasTryIndex() ? header->getTryIndex() : EHblkDsc::NO_ENCLOSING_INDEX; - unsigned preheaderEHRegion = headerEHRegion; - if ((headerEHRegion != EHblkDsc::NO_ENCLOSING_INDEX) && bbIsTryBeg(header)) + // 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. + // + // Otherwise the preheader must be in the enclosing try region. + // + unsigned preheaderEHRegion = EHblkDsc::NO_ENCLOSING_INDEX; + bool inSameRegionAsHeader = true; + if (header->hasTryIndex()) { - preheaderEHRegion = ehTrueEnclosingTryIndexIL(headerEHRegion); + preheaderEHRegion = header->getTryIndex(); + for (FlowEdge* backEdge : loop->BackEdges()) + { + BasicBlock* const backedgeSource = backEdge->getSourceBlock(); + if (!bbInTryRegions(preheaderEHRegion, backedgeSource)) + { + if (backedgeSource->hasTryIndex()) + { + preheaderEHRegion = backedgeSource->getTryIndex(); + } + else + { + preheaderEHRegion = EHblkDsc::NO_ENCLOSING_INDEX; + } + inSameRegionAsHeader = false; + break; + } + } } if (!bbIsHandlerBeg(header) && (loop->EntryEdges().size() == 1)) @@ -2893,7 +2911,16 @@ bool Compiler::optCreatePreheader(FlowGraphNaturalLoop* loop) BasicBlock* preheader = fgNewBBbefore(BBJ_ALWAYS, header, false); preheader->SetFlags(BBF_INTERNAL); - fgSetEHRegionForNewPreheaderOrExit(preheader); + + if (inSameRegionAsHeader) + { + fgExtendEHRegionBefore(header); + } + else + { + fgSetEHRegionForNewPreheaderOrExit(preheader); + } + preheader->bbCodeOffs = header->bbCodeOffs; JITDUMP("Created new preheader " FMT_BB " for " FMT_LP "\n", preheader->bbNum, loop->GetIndex()); From b7320bdd4c4b9c83031a49ab634616c9803b285c Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Tue, 15 Oct 2024 09:23:10 -0700 Subject: [PATCH 2/3] review feedback --- src/coreclr/jit/optimizer.cpp | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/coreclr/jit/optimizer.cpp b/src/coreclr/jit/optimizer.cpp index 2e88b79838d19..c41a5d1d61491 100644 --- a/src/coreclr/jit/optimizer.cpp +++ b/src/coreclr/jit/optimizer.cpp @@ -2881,14 +2881,9 @@ bool Compiler::optCreatePreheader(FlowGraphNaturalLoop* loop) BasicBlock* const backedgeSource = backEdge->getSourceBlock(); if (!bbInTryRegions(preheaderEHRegion, backedgeSource)) { - if (backedgeSource->hasTryIndex()) - { - preheaderEHRegion = backedgeSource->getTryIndex(); - } - else - { - preheaderEHRegion = EHblkDsc::NO_ENCLOSING_INDEX; - } + // Preheader should be in the true enclosing region of the header. + // + preheaderEHRegion = ehTrueEnclosingTryIndexIL(preheaderEHRegion); inSameRegionAsHeader = false; break; } From 78812e6f8921df20c472837b1596a209405fa691 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Tue, 15 Oct 2024 10:02:19 -0700 Subject: [PATCH 3/3] format --- 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 c41a5d1d61491..809d097f563ab 100644 --- a/src/coreclr/jit/optimizer.cpp +++ b/src/coreclr/jit/optimizer.cpp @@ -2883,7 +2883,7 @@ bool Compiler::optCreatePreheader(FlowGraphNaturalLoop* loop) { // Preheader should be in the true enclosing region of the header. // - preheaderEHRegion = ehTrueEnclosingTryIndexIL(preheaderEHRegion); + preheaderEHRegion = ehTrueEnclosingTryIndexIL(preheaderEHRegion); inSameRegionAsHeader = false; break; }