diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index a2676c57c78bb3..ab3a71537b579d 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/src/coreclr/jit/compiler.cpp @@ -4944,6 +4944,9 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl } } + // Remove dead blocks + DoPhase(this, PHASE_REMOVE_DEAD_BLOCKS, &Compiler::fgRemoveDeadBlocks); + // Insert GC Polls DoPhase(this, PHASE_INSERT_GC_POLLS, &Compiler::fgInsertGCPolls); diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 44651689b3bc23..4b2515778e16c4 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -5196,7 +5196,6 @@ class Compiler bool fgHaveValidEdgeWeights; // true if we were successful in computing all of the edge weights bool fgSlopUsedInEdgeWeights; // true if their was some slop used when computing the edge weights bool fgRangeUsedInEdgeWeights; // true if some of the edgeWeight are expressed in Min..Max form - bool fgNeedsUpdateFlowGraph; // true if we need to run fgUpdateFlowGraph weight_t fgCalledCount; // count of the number of times this method was called // This is derived from the profile data // or is BB_UNITY_WEIGHT when we don't have profile data @@ -5774,10 +5773,14 @@ class Compiler void fgComputeEnterBlocksSet(); // Compute the set of entry blocks, 'fgEnterBlks'. - bool fgRemoveUnreachableBlocks(); // Remove blocks determined to be unreachable by the bbReach sets. + // Remove blocks determined to be unreachable by the 'canRemoveBlock'. + template + bool fgRemoveUnreachableBlocks(CanRemoveBlockBody canRemoveBlock); void fgComputeReachability(); // Perform flow graph node reachability analysis. + void fgRemoveDeadBlocks(); // Identify and remove dead blocks. + BasicBlock* fgIntersectDom(BasicBlock* a, BasicBlock* b); // Intersect two immediate dominator sets. void fgDfsInvPostOrder(); // In order to compute dominance using fgIntersectDom, the flow graph nodes must be diff --git a/src/coreclr/jit/compphases.h b/src/coreclr/jit/compphases.h index 3b1907e86fde0b..05ec93a76e34bd 100644 --- a/src/coreclr/jit/compphases.h +++ b/src/coreclr/jit/compphases.h @@ -84,6 +84,7 @@ CompPhaseNameMacro(PHASE_OPTIMIZE_BRANCHES, "Redundant branch opts", CompPhaseNameMacro(PHASE_ASSERTION_PROP_MAIN, "Assertion prop", "AST-PROP", false, -1, false) CompPhaseNameMacro(PHASE_OPT_UPDATE_FLOW_GRAPH, "Update flow graph opt pass", "UPD-FG-O", false, -1, false) CompPhaseNameMacro(PHASE_COMPUTE_EDGE_WEIGHTS2, "Compute edge weights (2, false)","EDG-WGT2", false, -1, false) +CompPhaseNameMacro(PHASE_REMOVE_DEAD_BLOCKS, "Remove dead blocks", "DEAD-BLK", false, -1, false) CompPhaseNameMacro(PHASE_INSERT_GC_POLLS, "Insert GC Polls", "GC-POLLS", false, -1, true) CompPhaseNameMacro(PHASE_DETERMINE_FIRST_COLD_BLOCK, "Determine first cold block", "COLD-BLK", false, -1, true) CompPhaseNameMacro(PHASE_RATIONALIZE, "Rationalize IR", "RAT", false, -1, false) diff --git a/src/coreclr/jit/emit.cpp b/src/coreclr/jit/emit.cpp index 462b9876f97169..a507cadf36aed2 100644 --- a/src/coreclr/jit/emit.cpp +++ b/src/coreclr/jit/emit.cpp @@ -6556,7 +6556,8 @@ unsigned emitter::emitEndCodeGen(Compiler* comp, #ifdef DEBUG if (emitComp->opts.disAsm || emitComp->verbose) { - printf("\t\t\t\t\t\t;; bbWeight=%s PerfScore %.2f", refCntWtd2str(ig->igWeight), ig->igPerfScore); + printf("\t\t\t\t\t\t;; size=%d bbWeight=%s PerfScore %.2f", ig->igSize, refCntWtd2str(ig->igWeight), + ig->igPerfScore); } *instrCount += ig->igInsCnt; #endif // DEBUG diff --git a/src/coreclr/jit/fgbasic.cpp b/src/coreclr/jit/fgbasic.cpp index bf6eba0ca07c7f..8151dff192377a 100644 --- a/src/coreclr/jit/fgbasic.cpp +++ b/src/coreclr/jit/fgbasic.cpp @@ -31,7 +31,6 @@ void Compiler::fgInit() fgHaveValidEdgeWeights = false; fgSlopUsedInEdgeWeights = false; fgRangeUsedInEdgeWeights = true; - fgNeedsUpdateFlowGraph = false; fgCalledCount = BB_ZERO_WEIGHT; /* We haven't yet computed the dominator sets */ diff --git a/src/coreclr/jit/fgopt.cpp b/src/coreclr/jit/fgopt.cpp index 58f89625f57e81..1700f35815a939 100644 --- a/src/coreclr/jit/fgopt.cpp +++ b/src/coreclr/jit/fgopt.cpp @@ -367,17 +367,14 @@ void Compiler::fgComputeEnterBlocksSet() BlockSetOps::AddElemD(this, fgEnterBlks, fgFirstBB->bbNum); assert(fgFirstBB->bbNum == 1); - if (compHndBBtabCount > 0) + /* Also 'or' in the handler basic blocks */ + for (EHblkDsc* const HBtab : EHClauses(this)) { - /* Also 'or' in the handler basic blocks */ - for (EHblkDsc* const HBtab : EHClauses(this)) + if (HBtab->HasFilter()) { - if (HBtab->HasFilter()) - { - BlockSetOps::AddElemD(this, fgEnterBlks, HBtab->ebdFilter->bbNum); - } - BlockSetOps::AddElemD(this, fgEnterBlks, HBtab->ebdHndBeg->bbNum); + BlockSetOps::AddElemD(this, fgEnterBlks, HBtab->ebdFilter->bbNum); } + BlockSetOps::AddElemD(this, fgEnterBlks, HBtab->ebdHndBeg->bbNum); } #if defined(FEATURE_EH_FUNCLETS) && defined(TARGET_ARM) @@ -420,16 +417,28 @@ void Compiler::fgComputeEnterBlocksSet() // are converted to `throw` blocks. Internal throw helper blocks and the single return block (if any) // are never considered unreachable. // +// Arguments: +// canRemoveBlock - Method that determines if a block can be removed or not. In earlier phases, it +// relies on the reachability set. During final phase, it depends on the DFS walk of the flowgraph +// and considering blocks that are not visited as unreachable. +// // Return Value: // Return true if changes were made that may cause additional blocks to be removable. // -// Assumptions: -// The reachability sets must be computed and valid. +// Notes: +// Unreachable blocks removal phase happens twice. +// +// During early phases RecomputeLoopInfo, the logic to determine if a block is reachable +// or not is based on the reachability sets, and hence it must be computed and valid. +// +// During late phase, all the reachable blocks from fgFirstBB are traversed and everything +// else are marked as unreachable (with exceptions of handler/filter blocks and BBJ_ALWAYS +// blocks in Arm). As such, it is not dependent on the validity of reachability sets. // -bool Compiler::fgRemoveUnreachableBlocks() +template +bool Compiler::fgRemoveUnreachableBlocks(CanRemoveBlockBody canRemoveBlock) { assert(!fgCheapPredsValid); - assert(fgReachabilitySetsValid); bool hasUnreachableBlocks = false; bool changed = false; @@ -451,18 +460,10 @@ bool Compiler::fgRemoveUnreachableBlocks() } else { - // If any of the entry blocks can reach this block, then we skip it. - if (!BlockSetOps::IsEmptyIntersection(this, fgEnterBlks, block->bbReach)) - { - continue; - } - -#if defined(FEATURE_EH_FUNCLETS) && defined(TARGET_ARM) - if (!BlockSetOps::IsEmptyIntersection(this, fgAlwaysBlks, block->bbReach)) + if (!canRemoveBlock(block)) { continue; } -#endif // defined(FEATURE_EH_FUNCLETS) && defined(TARGET_ARM) } // Remove all the code for the block @@ -572,6 +573,24 @@ void Compiler::fgComputeReachability() // The dominator algorithm expects that all blocks can be reached from the fgEnterBlks set. unsigned passNum = 1; bool changed; + + auto canRemoveBlock = [&](BasicBlock* block) -> bool { + // If any of the entry blocks can reach this block, then we skip it. + if (!BlockSetOps::IsEmptyIntersection(this, fgEnterBlks, block->bbReach)) + { + return false; + } + +#if defined(FEATURE_EH_FUNCLETS) && defined(TARGET_ARM) + if (!BlockSetOps::IsEmptyIntersection(this, fgAlwaysBlks, block->bbReach)) + { + return false; + } +#endif // defined(FEATURE_EH_FUNCLETS) && defined(TARGET_ARM) + + return true; + }; + do { // Just to be paranoid, avoid infinite loops; fall back to minopts. @@ -601,7 +620,7 @@ void Compiler::fgComputeReachability() // Use reachability information to delete unreachable blocks. // - changed = fgRemoveUnreachableBlocks(); + changed = fgRemoveUnreachableBlocks(canRemoveBlock); } while (changed); @@ -624,6 +643,95 @@ void Compiler::fgComputeReachability() fgComputeDoms(); } +//------------------------------------------------------------------------ +// fgRemoveDeadBlocks: Identify all the unreachable blocks and remove them. +// Handler and filter blocks are considered as reachable and hence won't +// be removed. For Arm32, do not remove BBJ_ALWAYS block of +// BBJ_CALLFINALLY/BBJ_ALWAYS pair. +// +void Compiler::fgRemoveDeadBlocks() +{ + jitstd::list worklist(jitstd::allocator(getAllocator(CMK_Reachability))); + worklist.push_back(fgFirstBB); + + // Do not remove handler blocks + for (EHblkDsc* const HBtab : EHClauses(this)) + { + if (HBtab->HasFilter()) + { + worklist.push_back(HBtab->ebdFilter); + } + worklist.push_back(HBtab->ebdHndBeg); + } + +#if defined(FEATURE_EH_FUNCLETS) && defined(TARGET_ARM) + // For ARM code, prevent creating retless calls by adding the BBJ_ALWAYS to the "fgAlwaysBlks" list. + for (BasicBlock* const block : Blocks()) + { + if (block->bbJumpKind == BBJ_CALLFINALLY) + { + assert(block->isBBCallAlwaysPair()); + + // Don't remove the BBJ_ALWAYS block that is only here for the unwinder. + worklist.push_back(block->bbNext); + } + } +#endif // defined(FEATURE_EH_FUNCLETS) && defined(TARGET_ARM) + + unsigned prevFgCurBBEpoch = fgCurBBEpoch; + EnsureBasicBlockEpoch(); + + if (prevFgCurBBEpoch != fgCurBBEpoch) + { + // If Epoch has changed, reset the doms computed as well because + // in future, during insert gc polls or lowering, when we compact + // blocks during flowgraph update, it might propagate the invalid + // bbReach as well (although Epoch adjustment resets fgReachabilitySetsValid). + fgDomsComputed = false; + } + + BlockSet visitedBlocks(BlockSetOps::MakeEmpty(this)); + + // Visit all the reachable blocks, everything else can be removed + while (!worklist.empty()) + { + BasicBlock* block = *(worklist.begin()); + worklist.pop_front(); + + if (BlockSetOps::IsMember(this, visitedBlocks, block->bbNum)) + { + continue; + } + + BlockSetOps::AddElemD(this, visitedBlocks, block->bbNum); + + for (BasicBlock* succ : block->Succs(this)) + { + worklist.push_back(succ); + } + } + + // A block is unreachable if no path was found from + // any of the fgFirstBB, handler, filter or BBJ_ALWAYS (Arm) blocks. + auto isBlockRemovable = [&](BasicBlock* block) -> bool { + return !BlockSetOps::IsMember(this, visitedBlocks, block->bbNum); + }; + + bool changed = fgRemoveUnreachableBlocks(isBlockRemovable); + +#ifdef DEBUG + if (verbose && changed) + { + printf("\nAfter dead block removal:\n"); + fgDispBasicBlocks(verboseTrees); + printf("\n"); + } + + fgVerifyHandlerTab(); + fgDebugCheckBBlist(false); +#endif // DEBUG +} + //------------------------------------------------------------- // fgDfsInvPostOrder: Helper function for computing dominance information. // @@ -2163,6 +2271,7 @@ void Compiler::fgCompactBlocks(BasicBlock* block, BasicBlock* bNext) // if (fgDomsComputed && (block->bbNum > fgDomBBcount)) { + assert(fgReachabilitySetsValid); BlockSetOps::Assign(this, block->bbReach, bNext->bbReach); BlockSetOps::ClearD(this, bNext->bbReach); @@ -2512,8 +2621,7 @@ bool Compiler::fgOptimizeBranchToEmptyUnconditional(BasicBlock* block, BasicBloc // if (fgIsUsingProfileWeights() && !fgEdgeWeightsComputed) { - fgNeedsUpdateFlowGraph = true; - optimizeJump = false; + optimizeJump = false; } if (optimizeJump) @@ -2875,8 +2983,7 @@ bool Compiler::fgOptimizeSwitchBranches(BasicBlock* block) // if (fgIsUsingProfileWeights() && !fgEdgeWeightsComputed) { - fgNeedsUpdateFlowGraph = true; - optimizeJump = false; + optimizeJump = false; } if (optimizeJump) @@ -5683,7 +5790,6 @@ bool Compiler::fgReorderBlocks() if (changed) { - fgNeedsUpdateFlowGraph = true; #if DEBUG // Make sure that the predecessor lists are accurate if (expensiveDebugCheckLevel >= 2) @@ -5908,8 +6014,7 @@ bool Compiler::fgUpdateFlowGraph(bool doTailDuplication) // because we can't allow fall-through into the cold region. if (!fgEdgeWeightsComputed || fgInDifferentRegions(block, bDest)) { - fgNeedsUpdateFlowGraph = true; - optimizeJump = false; + optimizeJump = false; } } @@ -6194,8 +6299,6 @@ bool Compiler::fgUpdateFlowGraph(bool doTailDuplication) } } while (change); - fgNeedsUpdateFlowGraph = false; - #ifdef DEBUG if (verbose && modified) { diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_66578/Runtime_66578.cs b/src/tests/JIT/Regression/JitBlue/Runtime_66578/Runtime_66578.cs new file mode 100644 index 00000000000000..757950b39d800a --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_66578/Runtime_66578.cs @@ -0,0 +1,84 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// +// Note: In below test case, would keep around unreachable blocks which would wrongly keep the +// variables alive and we end up generating false refpositions. This leads to not marking +// an interval as spilled. +using System; +using System.Runtime.CompilerServices; + +public interface I0 +{ +} + +public interface I1 +{ +} + +public class C0 : I0, I1 +{ + public sbyte F0; + public bool F1; + public uint F2; + public ushort F5; + public C0(sbyte f0, bool f1, uint f2, ushort f5) + { + F0 = f0; + F1 = f1; + F2 = f2; + F5 = f5; + } +} + +public class Program2 +{ + public static I1[][] s_18; + //= { new I1[] { null }, }; + + public static ushort[][] s_43; + public static I1 s_64; + public static I0 s_88; + + public static int Main() + { + var vr6 = new C0(0, false, 0, 0); + try + { + M52(vr6); + } catch (NullReferenceException ex) + { + return 100; + } + return 1; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + public static void M52(C0 argThis) + { + I1 vr9 = s_18[0][0]; + if (argThis.F1) + { + return; + } + + if (argThis.F1) + { + try + { + s_43 = new ushort[][] { new ushort[] { 0 } }; + } + finally + { + for (int var8 = 0; var8 < 2; var8++) + { + s_64 = argThis; + } + + C0 vr10 = new C0(0, true, 0, 0); + s_88 = new C0(-1, true, 0, 0); + } + + I1 vr12 = s_18[0][0]; + } + } +} \ No newline at end of file diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_66578/Runtime_66578.csproj b/src/tests/JIT/Regression/JitBlue/Runtime_66578/Runtime_66578.csproj new file mode 100644 index 00000000000000..e822a8b10a5a6f --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_66578/Runtime_66578.csproj @@ -0,0 +1,13 @@ + + + Exe + True + + + None + True + + + + + \ No newline at end of file