diff --git a/src/coreclr/jit/block.h b/src/coreclr/jit/block.h index 054edae23113b..1c4674c83826a 100644 --- a/src/coreclr/jit/block.h +++ b/src/coreclr/jit/block.h @@ -468,7 +468,7 @@ enum BasicBlockFlags : uint64_t // Flags to update when two blocks are compacted BBF_COMPACT_UPD = BBF_GC_SAFE_POINT | BBF_NEEDS_GCPOLL | BBF_HAS_JMP | BBF_HAS_IDX_LEN | BBF_HAS_MD_IDX_LEN | BBF_BACKWARD_JUMP | \ - BBF_HAS_NEWOBJ | BBF_HAS_NULLCHECK | BBF_HAS_MDARRAYREF, + BBF_HAS_NEWOBJ | BBF_HAS_NULLCHECK | BBF_HAS_MDARRAYREF | BBF_LOOP_HEAD, // Flags a block should not have had before it is split. diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 6136c97d5d47b..7a4306878950d 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -6088,9 +6088,9 @@ class Compiler void fgPrepareCallFinallyRetForRemoval(BasicBlock* block); - bool fgCanCompactBlocks(BasicBlock* block, BasicBlock* bNext); + bool fgCanCompactBlock(BasicBlock* block); - void fgCompactBlocks(BasicBlock* block, BasicBlock* bNext DEBUGARG(bool doDebugCheck = true)); + void fgCompactBlock(BasicBlock* block); BasicBlock* fgConnectFallThrough(BasicBlock* bSrc, BasicBlock* bDst); @@ -6159,7 +6159,7 @@ class Compiler bool fgIsForwardBranch(BasicBlock* bJump, BasicBlock* bDest, BasicBlock* bSrc = nullptr); - bool fgUpdateFlowGraph(bool doTailDup = false, bool isPhase = false); + bool fgUpdateFlowGraph(bool doTailDup = false, bool isPhase = false, bool doAggressiveCompaction = true); PhaseStatus fgUpdateFlowGraphPhase(); PhaseStatus fgDfsBlocksAndRemove(); diff --git a/src/coreclr/jit/fgdiagnostic.cpp b/src/coreclr/jit/fgdiagnostic.cpp index 1c5f204d7ed78..48c7d685ed32a 100644 --- a/src/coreclr/jit/fgdiagnostic.cpp +++ b/src/coreclr/jit/fgdiagnostic.cpp @@ -139,7 +139,7 @@ void Compiler::fgDebugCheckUpdate() /* no un-compacted blocks */ - if (fgCanCompactBlocks(block, block->Next())) + if (fgCanCompactBlock(block)) { noway_assert(!"Found un-compacted blocks!"); } diff --git a/src/coreclr/jit/fgopt.cpp b/src/coreclr/jit/fgopt.cpp index 7f0c0f2e342a5..975ad0aaa546e 100644 --- a/src/coreclr/jit/fgopt.cpp +++ b/src/coreclr/jit/fgopt.cpp @@ -886,64 +886,81 @@ PhaseStatus Compiler::fgPostImportationCleanup() } //------------------------------------------------------------- -// fgCanCompactBlocks: Determine if a block and its bbNext successor can be compacted. +// fgCanCompactBlock: Determine if a BBJ_ALWAYS block and its target can be compacted. // // Arguments: -// block - block to check. If nullptr, return false. -// bNext - bbNext of `block`. If nullptr, return false. +// block - BBJ_ALWAYS block to check // // Returns: // true if compaction is allowed // -bool Compiler::fgCanCompactBlocks(BasicBlock* block, BasicBlock* bNext) +bool Compiler::fgCanCompactBlock(BasicBlock* block) { assert(block != nullptr); - assert(block->NextIs(bNext)); - if (!block->KindIs(BBJ_ALWAYS) || !block->TargetIs(bNext) || block->HasFlag(BBF_KEEP_BBJ_ALWAYS)) + if (!block->KindIs(BBJ_ALWAYS) || block->HasFlag(BBF_KEEP_BBJ_ALWAYS)) { return false; } - // If the next block has multiple incoming edges, we can still compact if the first block is empty. + BasicBlock* const target = block->GetTarget(); + + if (block == target) + { + return false; + } + + if (target->IsFirst() || (target == fgEntryBB) || (target == fgOSREntryBB)) + { + return false; + } + + // Don't bother compacting a call-finally pair if it doesn't succeed block + // + if (target->isBBCallFinallyPair() && !block->NextIs(target)) + { + return false; + } + + // If target has multiple incoming edges, we can still compact if block is empty. // However, not if it is the beginning of a handler. - if (bNext->countOfInEdges() != 1 && + // + if (target->countOfInEdges() != 1 && (!block->isEmpty() || block->HasFlag(BBF_FUNCLET_BEG) || (block->bbCatchTyp != BBCT_NONE))) { return false; } - if (bNext->HasFlag(BBF_DONT_REMOVE)) + if (target->HasFlag(BBF_DONT_REMOVE)) { return false; } // Don't compact the first block if it was specially created as a scratch block. + // if (fgBBisScratch(block)) { return false; } - // We don't want to compact blocks that are in different Hot/Cold regions + // We don't want to compact blocks that are in different hot/cold regions // - if (fgInDifferentRegions(block, bNext)) + if (fgInDifferentRegions(block, target)) { return false; } // We cannot compact two blocks in different EH regions. // - if (fgCanRelocateEHRegions) + if (!BasicBlock::sameEHRegion(block, target)) { - if (!BasicBlock::sameEHRegion(block, bNext)) - { - return false; - } + return false; } // If there is a switch predecessor don't bother because we'd have to update the uniquesuccs as well // (if they are valid). - for (BasicBlock* const predBlock : bNext->PredBlocks()) + // + for (BasicBlock* const predBlock : target->PredBlocks()) { if (predBlock->KindIs(BBJ_SWITCH)) { @@ -955,179 +972,126 @@ bool Compiler::fgCanCompactBlocks(BasicBlock* block, BasicBlock* bNext) } //------------------------------------------------------------- -// fgCompactBlocks: Compact two blocks into one. +// fgCompactBlock: Compact BBJ_ALWAYS block and its target into one. // -// Assumes that all necessary checks have been performed, i.e. fgCanCompactBlocks returns true. +// Requires that all necessary checks have been performed, i.e. fgCanCompactBlock returns true. // // Uses for this function - whenever we change links, insert blocks, ... // It will keep the flowgraph data in synch - bbNum, bbRefs, bbPreds // // Arguments: -// block - move all code into this block. -// bNext - bbNext of `block`. This block will be removed. -// doDebugCheck - in Debug builds, check flowgraph for correctness after compaction -// (some callers might compact blocks during destructive flowgraph changes, and thus should skip checks) +// block - move all code into this block from its target. // -void Compiler::fgCompactBlocks(BasicBlock* block, BasicBlock* bNext DEBUGARG(bool doDebugCheck /* = true */)) +void Compiler::fgCompactBlock(BasicBlock* block) { - noway_assert(block != nullptr); - noway_assert(bNext != nullptr); - noway_assert(!block->HasFlag(BBF_REMOVED)); - noway_assert(!bNext->HasFlag(BBF_REMOVED)); - noway_assert(block->NextIs(bNext)); - noway_assert(bNext->countOfInEdges() == 1 || block->isEmpty()); - noway_assert(bNext->bbPreds != nullptr); - - assert(block->KindIs(BBJ_ALWAYS)); - assert(block->TargetIs(bNext)); - assert(!fgInDifferentRegions(block, bNext)); - - // Make sure the second block is not the start of a TRY block or an exception handler + assert(fgCanCompactBlock(block)); + BasicBlock* const target = block->GetTarget(); - noway_assert(!bbIsTryBeg(bNext)); - noway_assert(bNext->bbCatchTyp == BBCT_NONE); - noway_assert(!bNext->HasFlag(BBF_DONT_REMOVE)); - - /* both or none must have an exception handler */ - noway_assert(block->hasTryIndex() == bNext->hasTryIndex()); - - JITDUMP("\nCompacting " FMT_BB " into " FMT_BB ":\n", bNext->bbNum, block->bbNum); + JITDUMP("\nCompacting " FMT_BB " into " FMT_BB ":\n", target->bbNum, block->bbNum); fgRemoveRefPred(block->GetTargetEdge()); - if (bNext->countOfInEdges() > 0) + if (target->countOfInEdges() > 0) { - JITDUMP("Second block has %u other incoming edges\n", bNext->countOfInEdges()); + JITDUMP("Second block has %u other incoming edges\n", target->countOfInEdges()); assert(block->isEmpty()); - // Retarget all the other edges incident on bNext. Do this - // in two passes as we can't both walk and modify the pred list. - // - ArrayStack preds(getAllocator(CMK_BasicBlock), bNext->countOfInEdges()); - for (BasicBlock* const predBlock : bNext->PredBlocks()) - { - preds.Push(predBlock); - } - while (preds.Height() > 0) + // Retarget all the other edges incident on target + for (BasicBlock* const predBlock : target->PredBlocksEditing()) { - BasicBlock* const predBlock = preds.Pop(); - fgReplaceJumpTarget(predBlock, bNext, block); + fgReplaceJumpTarget(predBlock, target, block); } } - assert(bNext->countOfInEdges() == 0); - assert(bNext->bbPreds == nullptr); + assert(target->countOfInEdges() == 0); + assert(target->bbPreds == nullptr); /* Start compacting - move all the statements in the second block to the first block */ // First move any phi definitions of the second block after the phi defs of the first. - // TODO-CQ: This may be the wrong thing to do. If we're compacting blocks, it's because a - // control-flow choice was constant-folded away. So probably phi's need to go away, - // as well, in favor of one of the incoming branches. Or at least be modified. + // TODO-CQ: This may be the wrong thing to do. If we're compacting blocks, it's because a + // control-flow choice was constant-folded away. So probably phi's need to go away, + // as well, in favor of one of the incoming branches. Or at least be modified. - assert(block->IsLIR() == bNext->IsLIR()); + assert(block->IsLIR() == target->IsLIR()); if (block->IsLIR()) { - LIR::Range& blockRange = LIR::AsRange(block); - LIR::Range& nextRange = LIR::AsRange(bNext); + LIR::Range& blockRange = LIR::AsRange(block); + LIR::Range& targetRange = LIR::AsRange(target); - // Does the next block have any phis? - GenTree* nextNode = nextRange.FirstNode(); + // Does target have any phis? + GenTree* targetNode = targetRange.FirstNode(); // Does the block have any code? - if (nextNode != nullptr) + if (targetNode != nullptr) { - LIR::Range nextNodes = nextRange.Remove(nextNode, nextRange.LastNode()); - blockRange.InsertAtEnd(std::move(nextNodes)); + LIR::Range targetNodes = targetRange.Remove(targetNode, targetRange.LastNode()); + blockRange.InsertAtEnd(std::move(targetNodes)); } } else { - Statement* blkNonPhi1 = block->FirstNonPhiDef(); - Statement* bNextNonPhi1 = bNext->FirstNonPhiDef(); - Statement* blkFirst = block->firstStmt(); - Statement* bNextFirst = bNext->firstStmt(); + Statement* blkNonPhi1 = block->FirstNonPhiDef(); + Statement* targetNonPhi1 = target->FirstNonPhiDef(); + Statement* blkFirst = block->firstStmt(); + Statement* targetFirst = target->firstStmt(); // Does the second have any phis? - if (bNextFirst != nullptr && bNextFirst != bNextNonPhi1) + if ((targetFirst != nullptr) && (targetFirst != targetNonPhi1)) { - Statement* bNextLast = bNextFirst->GetPrevStmt(); - assert(bNextLast->GetNextStmt() == nullptr); + Statement* targetLast = targetFirst->GetPrevStmt(); + assert(targetLast->GetNextStmt() == nullptr); // Does "blk" have phis? if (blkNonPhi1 != blkFirst) { // Yes, has phis. // Insert after the last phi of "block." - // First, bNextPhis after last phi of block. - Statement* blkLastPhi; - if (blkNonPhi1 != nullptr) - { - blkLastPhi = blkNonPhi1->GetPrevStmt(); - } - else - { - blkLastPhi = blkFirst->GetPrevStmt(); - } + // First, targetPhis after last phi of block. + Statement* blkLastPhi = (blkNonPhi1 != nullptr) ? blkNonPhi1->GetPrevStmt() : blkFirst->GetPrevStmt(); + blkLastPhi->SetNextStmt(targetFirst); + targetFirst->SetPrevStmt(blkLastPhi); - blkLastPhi->SetNextStmt(bNextFirst); - bNextFirst->SetPrevStmt(blkLastPhi); + // Now, rest of "block" after last phi of "target". + Statement* targetLastPhi = + (targetNonPhi1 != nullptr) ? targetNonPhi1->GetPrevStmt() : targetFirst->GetPrevStmt(); + targetLastPhi->SetNextStmt(blkNonPhi1); - // Now, rest of "block" after last phi of "bNext". - Statement* bNextLastPhi = nullptr; - if (bNextNonPhi1 != nullptr) - { - bNextLastPhi = bNextNonPhi1->GetPrevStmt(); - } - else - { - bNextLastPhi = bNextFirst->GetPrevStmt(); - } - - bNextLastPhi->SetNextStmt(blkNonPhi1); if (blkNonPhi1 != nullptr) { - blkNonPhi1->SetPrevStmt(bNextLastPhi); + blkNonPhi1->SetPrevStmt(targetLastPhi); } else { // block has no non phis, so make the last statement be the last added phi. - blkFirst->SetPrevStmt(bNextLastPhi); + blkFirst->SetPrevStmt(targetLastPhi); } - // Now update the bbStmtList of "bNext". - bNext->bbStmtList = bNextNonPhi1; - if (bNextNonPhi1 != nullptr) + // Now update the bbStmtList of "target". + target->bbStmtList = targetNonPhi1; + if (targetNonPhi1 != nullptr) { - bNextNonPhi1->SetPrevStmt(bNextLast); + targetNonPhi1->SetPrevStmt(targetLast); } } else { if (blkFirst != nullptr) // If "block" has no statements, fusion will work fine... { - // First, bNextPhis at start of block. + // First, targetPhis at start of block. Statement* blkLast = blkFirst->GetPrevStmt(); - block->bbStmtList = bNextFirst; - // Now, rest of "block" (if it exists) after last phi of "bNext". - Statement* bNextLastPhi = nullptr; - if (bNextNonPhi1 != nullptr) - { - // There is a first non phi, so the last phi is before it. - bNextLastPhi = bNextNonPhi1->GetPrevStmt(); - } - else - { - // All the statements are phi defns, so the last one is the prev of the first. - bNextLastPhi = bNextFirst->GetPrevStmt(); - } - bNextFirst->SetPrevStmt(blkLast); - bNextLastPhi->SetNextStmt(blkFirst); - blkFirst->SetPrevStmt(bNextLastPhi); - // Now update the bbStmtList of "bNext" - bNext->bbStmtList = bNextNonPhi1; - if (bNextNonPhi1 != nullptr) + block->bbStmtList = targetFirst; + // Now, rest of "block" (if it exists) after last phi of "target". + Statement* targetLastPhi = + (targetNonPhi1 != nullptr) ? targetNonPhi1->GetPrevStmt() : targetFirst->GetPrevStmt(); + + targetFirst->SetPrevStmt(blkLast); + targetLastPhi->SetNextStmt(blkFirst); + blkFirst->SetPrevStmt(targetLastPhi); + // Now update the bbStmtList of "target" + target->bbStmtList = targetNonPhi1; + if (targetNonPhi1 != nullptr) { - bNextNonPhi1->SetPrevStmt(bNextLast); + targetNonPhi1->SetPrevStmt(targetLast); } } } @@ -1135,7 +1099,7 @@ void Compiler::fgCompactBlocks(BasicBlock* block, BasicBlock* bNext DEBUGARG(boo // Now proceed with the updated bbTreeLists. Statement* stmtList1 = block->firstStmt(); - Statement* stmtList2 = bNext->firstStmt(); + Statement* stmtList2 = target->firstStmt(); /* the block may have an empty list */ @@ -1146,7 +1110,7 @@ void Compiler::fgCompactBlocks(BasicBlock* block, BasicBlock* bNext DEBUGARG(boo /* The second block may be a GOTO statement or something with an empty bbStmtList */ if (stmtList2 != nullptr) { - Statement* stmtLast2 = bNext->lastStmt(); + Statement* stmtLast2 = target->lastStmt(); /* append list2 to list 1 */ @@ -1157,50 +1121,23 @@ void Compiler::fgCompactBlocks(BasicBlock* block, BasicBlock* bNext DEBUGARG(boo } else { - /* block was formerly empty and now has bNext's statements */ + /* block was formerly empty and now has target's statements */ block->bbStmtList = stmtList2; } } - // If bNext is BBJ_THROW, block will become run rarely. - // - // Otherwise, if either block or bNext has a profile weight - // or if both block and bNext have non-zero weights - // then we will use the max weight for the block. - // - if (bNext->KindIs(BBJ_THROW)) - { - block->bbSetRunRarely(); - } - else - { - const bool hasProfileWeight = block->hasProfileWeight() || bNext->hasProfileWeight(); - const bool hasNonZeroWeight = (block->bbWeight > BB_ZERO_WEIGHT) || (bNext->bbWeight > BB_ZERO_WEIGHT); - - if (hasProfileWeight || hasNonZeroWeight) - { - weight_t const newWeight = max(block->bbWeight, bNext->bbWeight); + // Transfer target's weight to block + // (target's weight should include block's weight, + // plus the weights of target's preds, which now flow into block) + const bool hasProfileWeight = block->hasProfileWeight(); + block->inheritWeight(target); - if (hasProfileWeight) - { - block->setBBProfileWeight(newWeight); - } - else - { - assert(newWeight != BB_ZERO_WEIGHT); - block->bbWeight = newWeight; - block->RemoveFlags(BBF_RUN_RARELY); - } - } - // otherwise if either block has a zero weight we select the zero weight - else - { - noway_assert((block->bbWeight == BB_ZERO_WEIGHT) || (bNext->bbWeight == BB_ZERO_WEIGHT)); - block->bbSetRunRarely(); - } + if (hasProfileWeight) + { + block->SetFlags(BBF_PROF_WEIGHT); } - VarSetOps::AssignAllowUninitRhs(this, block->bbLiveOut, bNext->bbLiveOut); + VarSetOps::AssignAllowUninitRhs(this, block->bbLiveOut, target->bbLiveOut); // Update the beginning and ending IL offsets (bbCodeOffs and bbCodeOffsEnd). // Set the beginning IL offset to the minimum, and the ending offset to the maximum, of the respective blocks. @@ -1210,62 +1147,62 @@ void Compiler::fgCompactBlocks(BasicBlock* block, BasicBlock* bNext DEBUGARG(boo if (block->bbCodeOffs == BAD_IL_OFFSET) { - block->bbCodeOffs = bNext->bbCodeOffs; // If they are both BAD_IL_OFFSET, this doesn't change anything. + block->bbCodeOffs = target->bbCodeOffs; // If they are both BAD_IL_OFFSET, this doesn't change anything. } - else if (bNext->bbCodeOffs != BAD_IL_OFFSET) + else if (target->bbCodeOffs != BAD_IL_OFFSET) { // The are both valid offsets; compare them. - if (block->bbCodeOffs > bNext->bbCodeOffs) + if (block->bbCodeOffs > target->bbCodeOffs) { - block->bbCodeOffs = bNext->bbCodeOffs; + block->bbCodeOffs = target->bbCodeOffs; } } if (block->bbCodeOffsEnd == BAD_IL_OFFSET) { - block->bbCodeOffsEnd = bNext->bbCodeOffsEnd; // If they are both BAD_IL_OFFSET, this doesn't change anything. + block->bbCodeOffsEnd = target->bbCodeOffsEnd; // If they are both BAD_IL_OFFSET, this doesn't change anything. } - else if (bNext->bbCodeOffsEnd != BAD_IL_OFFSET) + else if (target->bbCodeOffsEnd != BAD_IL_OFFSET) { // The are both valid offsets; compare them. - if (block->bbCodeOffsEnd < bNext->bbCodeOffsEnd) + if (block->bbCodeOffsEnd < target->bbCodeOffsEnd) { - block->bbCodeOffsEnd = bNext->bbCodeOffsEnd; + block->bbCodeOffsEnd = target->bbCodeOffsEnd; } } - if (block->HasFlag(BBF_INTERNAL) && !bNext->HasFlag(BBF_INTERNAL)) + if (block->HasFlag(BBF_INTERNAL) && !target->HasFlag(BBF_INTERNAL)) { - // If 'block' is an internal block and 'bNext' isn't, then adjust the flags set on 'block'. + // If 'block' is an internal block and 'target' isn't, then adjust the flags set on 'block'. block->RemoveFlags(BBF_INTERNAL); // Clear the BBF_INTERNAL flag block->SetFlags(BBF_IMPORTED); // Set the BBF_IMPORTED flag } - /* Update the flags for block with those found in bNext */ + /* Update the flags for block with those found in target */ - block->CopyFlags(bNext, BBF_COMPACT_UPD); + block->CopyFlags(target, BBF_COMPACT_UPD); - /* mark bNext as removed */ + /* mark target as removed */ - bNext->SetFlags(BBF_REMOVED); + target->SetFlags(BBF_REMOVED); - /* Unlink bNext and update all the marker pointers if necessary */ + /* Unlink target and update all the marker pointers if necessary */ - fgUnlinkRange(bNext, bNext); + fgUnlinkRange(target, target); fgBBcount--; - // If bNext was the last block of a try or handler, update the EH table. + // If target was the last block of a try or handler, update the EH table. - ehUpdateForDeletedBlock(bNext); + ehUpdateForDeletedBlock(target); /* Set the jump targets */ - switch (bNext->GetKind()) + switch (target->GetKind()) { case BBJ_CALLFINALLY: // Propagate RETLESS property - block->CopyFlags(bNext, BBF_RETLESS_CALL); + block->CopyFlags(target, BBF_RETLESS_CALL); FALLTHROUGH; @@ -1273,22 +1210,22 @@ void Compiler::fgCompactBlocks(BasicBlock* block, BasicBlock* bNext DEBUGARG(boo case BBJ_EHCATCHRET: case BBJ_EHFILTERRET: { - /* Update the predecessor list for bNext's target */ - FlowEdge* const targetEdge = bNext->GetTargetEdge(); + /* Update the predecessor list for target's target */ + FlowEdge* const targetEdge = target->GetTargetEdge(); fgReplacePred(targetEdge, block); - block->SetKindAndTargetEdge(bNext->GetKind(), targetEdge); + block->SetKindAndTargetEdge(target->GetKind(), targetEdge); break; } case BBJ_COND: { - /* Update the predecessor list for bNext's true target */ - FlowEdge* const trueEdge = bNext->GetTrueEdge(); - FlowEdge* const falseEdge = bNext->GetFalseEdge(); + /* Update the predecessor list for target's true target */ + FlowEdge* const trueEdge = target->GetTrueEdge(); + FlowEdge* const falseEdge = target->GetFalseEdge(); fgReplacePred(trueEdge, block); - /* Update the predecessor list for bNext's false target if it is different from the true target */ + /* Update the predecessor list for target's false target if it is different from the true target */ if (trueEdge != falseEdge) { fgReplacePred(falseEdge, block); @@ -1299,22 +1236,22 @@ void Compiler::fgCompactBlocks(BasicBlock* block, BasicBlock* bNext DEBUGARG(boo } case BBJ_EHFINALLYRET: - block->SetEhf(bNext->GetEhfTargets()); - fgChangeEhfBlock(bNext, block); + block->SetEhf(target->GetEhfTargets()); + fgChangeEhfBlock(target, block); break; case BBJ_EHFAULTRET: case BBJ_THROW: case BBJ_RETURN: /* no jumps or fall through blocks to set here */ - block->SetKind(bNext->GetKind()); + block->SetKind(target->GetKind()); break; case BBJ_SWITCH: - block->SetSwitch(bNext->GetSwitchTargets()); - // We are moving the switch jump from bNext to block. Examine the jump targets - // of the BBJ_SWITCH at bNext and replace the predecessor to 'bNext' with ones to 'block' - fgChangeSwitchBlock(bNext, block); + block->SetSwitch(target->GetSwitchTargets()); + // We are moving the switch jump from target to block. Examine the jump targets + // of the BBJ_SWITCH at target and replace the predecessor to 'target' with ones to 'block' + fgChangeSwitchBlock(target, block); break; default: @@ -1322,7 +1259,7 @@ void Compiler::fgCompactBlocks(BasicBlock* block, BasicBlock* bNext DEBUGARG(boo break; } - assert(block->KindIs(bNext->GetKind())); + assert(block->KindIs(target->GetKind())); #if DEBUG if (verbose && 0) @@ -1330,10 +1267,8 @@ void Compiler::fgCompactBlocks(BasicBlock* block, BasicBlock* bNext DEBUGARG(boo printf("\nAfter compacting:\n"); fgDispBasicBlocks(false); } -#endif -#if DEBUG - if (doDebugCheck && (JitConfig.JitSlowDebugChecksEnabled() != 0)) + if (JitConfig.JitSlowDebugChecksEnabled() != 0) { // Make sure that the predecessor lists are accurate fgDebugCheckBBlist(); @@ -3295,9 +3230,9 @@ bool Compiler::fgExpandRarelyRunBlocks() } /* COMPACT blocks if possible */ - if (fgCanCompactBlocks(bPrev, block)) + if (fgCanCompactBlock(bPrev)) { - fgCompactBlocks(bPrev, block); + fgCompactBlock(bPrev); block = bPrev; continue; @@ -4566,17 +4501,6 @@ void Compiler::fgMoveHotJumps() BasicBlock* next; for (BasicBlock* block = fgFirstBB; block != fgFirstFuncletBB; block = next) { - if (fgCanCompactBlocks(block, block->Next())) - { - // Compact blocks before trying to move any jumps. - // This can unlock more opportunities for fallthrough behavior. - // We haven't fixed EH information yet, so don't do any correctness checks here. - // - fgCompactBlocks(block, block->Next() DEBUGARG(/* doDebugCheck */ false)); - next = block; - continue; - } - next = block->Next(); BlockSetOps::AddElemD(this, visitedBlocks, block->bbNum); @@ -5262,6 +5186,8 @@ PhaseStatus Compiler::fgUpdateFlowGraphPhase() // Arguments: // doTailDuplication - true to attempt tail duplication optimization // isPhase - true if being run as the only thing in a phase +// doAggressiveCompaction - if false, only compact blocks that jump to the next block +// to prevent modifying the flowgraph; else, compact as much as possible // // Returns: true if the flowgraph has been modified // @@ -5269,7 +5195,9 @@ PhaseStatus Compiler::fgUpdateFlowGraphPhase() // Debuggable code and Min Optimization JIT also introduces basic blocks // but we do not optimize those! // -bool Compiler::fgUpdateFlowGraph(bool doTailDuplication /* = false */, bool isPhase /* = false */) +bool Compiler::fgUpdateFlowGraph(bool doTailDuplication /* = false */, + bool isPhase /* = false */, + bool doAggressiveCompaction /* = true */) { #ifdef DEBUG if (verbose && !isPhase) @@ -5629,13 +5557,14 @@ bool Compiler::fgUpdateFlowGraph(bool doTailDuplication /* = false */, bool isPh /* COMPACT blocks if possible */ - if (fgCanCompactBlocks(block, bNext)) + if (fgCanCompactBlock(block) && (doAggressiveCompaction || block->JumpsToNext())) { - fgCompactBlocks(block, bNext); + fgCompactBlock(block); /* we compacted two blocks - goto REPEAT to catch similar cases */ change = true; modified = true; + bPrev = block->Prev(); goto REPEAT; } diff --git a/src/coreclr/jit/helperexpansion.cpp b/src/coreclr/jit/helperexpansion.cpp index e62660aaab3af..908d9fb129fc1 100644 --- a/src/coreclr/jit/helperexpansion.cpp +++ b/src/coreclr/jit/helperexpansion.cpp @@ -1533,9 +1533,9 @@ bool Compiler::fgExpandStaticInitForCall(BasicBlock** pBlock, Statement* stmt, G assert(BasicBlock::sameEHRegion(prevBb, isInitedBb)); // Extra step: merge prevBb with isInitedBb if possible - if (fgCanCompactBlocks(prevBb, isInitedBb)) + if (fgCanCompactBlock(prevBb)) { - fgCompactBlocks(prevBb, isInitedBb); + fgCompactBlock(prevBb); } // Clear gtInitClsHnd as a mark that we've already visited this call @@ -1862,9 +1862,9 @@ bool Compiler::fgVNBasedIntrinsicExpansionForCall_ReadUtf8(BasicBlock** pBlock, assert(BasicBlock::sameEHRegion(prevBb, fastpathBb)); // Extra step: merge prevBb with lengthCheckBb if possible - if (fgCanCompactBlocks(prevBb, lengthCheckBb)) + if (fgCanCompactBlock(prevBb)) { - fgCompactBlocks(prevBb, lengthCheckBb); + fgCompactBlock(prevBb); } JITDUMP("ReadUtf8: succesfully expanded!\n") @@ -2686,9 +2686,9 @@ bool Compiler::fgLateCastExpansionForCall(BasicBlock** pBlock, Statement* stmt, } // Bonus step: merge prevBb with nullcheckBb as they are likely to be mergeable - if (fgCanCompactBlocks(firstBb, nullcheckBb)) + if (fgCanCompactBlock(firstBb)) { - fgCompactBlocks(firstBb, nullcheckBb); + fgCompactBlock(firstBb); } return true; diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index a966813f04754..3ba309102e1ca 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -7554,7 +7554,9 @@ PhaseStatus Lowering::DoPhase() // local var liveness can delete code, which may create empty blocks if (comp->opts.OptimizationEnabled()) { - bool modified = comp->fgUpdateFlowGraph(/* doTailDuplication */ false, /* isPhase */ false); + // Don't churn the flowgraph with aggressive compaction since we've already run block layout + bool modified = comp->fgUpdateFlowGraph(/* doTailDuplication */ false, /* isPhase */ false, + /* doAggressiveCompaction */ false); modified |= comp->fgRemoveDeadBlocks(); if (modified) diff --git a/src/coreclr/jit/optimizebools.cpp b/src/coreclr/jit/optimizebools.cpp index f915cf7c34948..9bf2875432bce 100644 --- a/src/coreclr/jit/optimizebools.cpp +++ b/src/coreclr/jit/optimizebools.cpp @@ -1062,9 +1062,9 @@ bool OptBoolsDsc::optOptimizeCompareChainCondBlock() m_b2->CopyFlags(m_b1, BBF_COPY_PROPAGATE); // Join the two blocks. This is done now to ensure that additional conditions can be chained. - if (m_b1->NextIs(m_b2) && m_comp->fgCanCompactBlocks(m_b1, m_b2)) + if (m_comp->fgCanCompactBlock(m_b1)) { - m_comp->fgCompactBlocks(m_b1, m_b2); + m_comp->fgCompactBlock(m_b1); } #ifdef DEBUG