From 0a9a2b8e7c427d4e962752c8b639eb9e288eb6ff Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Thu, 14 Mar 2024 16:17:24 -0400 Subject: [PATCH 1/2] Remove fallthrough quirk in fgFindInsertPoint --- src/coreclr/jit/fgbasic.cpp | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/coreclr/jit/fgbasic.cpp b/src/coreclr/jit/fgbasic.cpp index 92a6a58f21347..167602b2c97e8 100644 --- a/src/coreclr/jit/fgbasic.cpp +++ b/src/coreclr/jit/fgbasic.cpp @@ -6390,12 +6390,9 @@ BasicBlock* Compiler::fgFindInsertPoint(unsigned regionIndex, } } - // Look for an insert location. We want blocks that don't end with a fall through. - // Quirk: Manually check for BBJ_COND fallthrough behavior - const bool blkFallsThrough = - blk->bbFallsThrough() && (!blk->KindIs(BBJ_COND) || blk->NextIs(blk->GetFalseTarget())); - const bool blkJumpsToNext = blk->KindIs(BBJ_ALWAYS) && blk->HasFlag(BBF_NONE_QUIRK) && blk->JumpsToNext(); - if (!blkFallsThrough && !blkJumpsToNext) + // Look for an insert location. + // Avoid splitting up call-finally pairs, or BBJ_COND blocks that precede their false targets. + if (!blk->isBBCallFinallyPair() && (!blk->KindIs(BBJ_COND) || !blk->NextIs(blk->GetFalseTarget()))) { bool updateBestBlk = true; // We will probably update the bestBlk From 70c573c07a53e8ef758ca5dfc24f0b620bbb3895 Mon Sep 17 00:00:00 2001 From: "Aman Khalid (from Dev Box)" Date: Fri, 15 Mar 2024 13:28:36 -0400 Subject: [PATCH 2/2] Add BBJ_ALWAYS check back in --- src/coreclr/jit/fgbasic.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/fgbasic.cpp b/src/coreclr/jit/fgbasic.cpp index 167602b2c97e8..c98c7a1281994 100644 --- a/src/coreclr/jit/fgbasic.cpp +++ b/src/coreclr/jit/fgbasic.cpp @@ -6391,8 +6391,12 @@ BasicBlock* Compiler::fgFindInsertPoint(unsigned regionIndex, } // Look for an insert location. - // Avoid splitting up call-finally pairs, or BBJ_COND blocks that precede their false targets. - if (!blk->isBBCallFinallyPair() && (!blk->KindIs(BBJ_COND) || !blk->NextIs(blk->GetFalseTarget()))) + // Avoid splitting up call-finally pairs, or jumps/false branches to the next block. + // (We need the HasInitializedTarget() call because fgFindInsertPoint can be called during importation, + // before targets are set) + const bool jumpsToNext = blk->KindIs(BBJ_ALWAYS) && blk->HasInitializedTarget() && blk->JumpsToNext(); + const bool falseBranchToNext = blk->KindIs(BBJ_COND) && blk->NextIs(blk->GetFalseTarget()); + if (!blk->isBBCallFinallyPair() && !jumpsToNext && !falseBranchToNext) { bool updateBestBlk = true; // We will probably update the bestBlk