From 576e78fc983bc8d3faa0b03f4eebde0b5cbc2e76 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Tue, 4 Jun 2024 08:34:31 -0700 Subject: [PATCH 1/9] WIP --- src/coreclr/jit/compiler.cpp | 2 + src/coreclr/jit/compiler.h | 22 +++++ src/coreclr/jit/compmemkind.h | 1 + src/coreclr/jit/compphases.h | 1 + src/coreclr/jit/jitmetadatalist.h | 1 + src/coreclr/jit/liveness.cpp | 156 ++++++++++++++++++++++++++++++ 6 files changed, 183 insertions(+) diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index 54be15ed7f20bc..4da0df82c97701 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/src/coreclr/jit/compiler.cpp @@ -5067,6 +5067,8 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl fgLocalVarLiveness(); } + DoPhase(this, PHASE_AGGRESSIVE_SSA_DCE, &Compiler::fgSsaBasedDce); + if (doEarlyProp) { // Propagate array length and rewrite getType() method call diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index daa0348eaccad3..6ca16464d5d25e 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -2417,6 +2417,23 @@ class BlockReachabilitySets bool CanReach(BasicBlock* from, BasicBlock* to); + template + void VisitReachingBlocks(BasicBlock* block, TFunc func) + { + if (!m_dfsTree->Contains(block)) + { + return; + } + + BitVecTraits traits = m_dfsTree->PostOrderTraits(); + BitVecOps::Iter iter(&traits, m_reachabilitySets[block->bbPostorderNum]); + unsigned poNum; + while (iter.NextElem(&poNum)) + { + func(m_dfsTree->GetPostOrder(poNum)); + } + } + #ifdef DEBUG void Dump(); #endif @@ -2520,6 +2537,8 @@ struct RelopImplicationInfo bool reverseSense = false; }; +typedef JitHashTable, bool> TreeSet; + /* XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX @@ -5504,6 +5523,9 @@ class Compiler void fgPerNodeLocalVarLiveness(GenTree* node); void fgPerBlockLocalVarLiveness(); + bool fgIsPreLive(GenTree* tree); + PhaseStatus fgSsaBasedDce(); + #if defined(FEATURE_HW_INTRINSICS) void fgPerNodeLocalVarLiveness(GenTreeHWIntrinsic* hwintrinsic); #endif // FEATURE_HW_INTRINSICS diff --git a/src/coreclr/jit/compmemkind.h b/src/coreclr/jit/compmemkind.h index 0221eadb067492..0c271d55eae13f 100644 --- a/src/coreclr/jit/compmemkind.h +++ b/src/coreclr/jit/compmemkind.h @@ -36,6 +36,7 @@ CompMemKindMacro(bitset) CompMemKindMacro(FixedBitVect) CompMemKindMacro(Generic) CompMemKindMacro(LocalAddressVisitor) +CompMemKindMacro(Liveness) CompMemKindMacro(FieldSeqStore) CompMemKindMacro(MemorySsaMap) CompMemKindMacro(MemoryPhiArg) diff --git a/src/coreclr/jit/compphases.h b/src/coreclr/jit/compphases.h index 6f2783f889721c..d37d3a00128178 100644 --- a/src/coreclr/jit/compphases.h +++ b/src/coreclr/jit/compphases.h @@ -83,6 +83,7 @@ CompPhaseNameMacro(PHASE_BUILD_SSA_LIVENESS, "SSA: liveness", CompPhaseNameMacro(PHASE_BUILD_SSA_DF, "SSA: DF", false, PHASE_BUILD_SSA, false) CompPhaseNameMacro(PHASE_BUILD_SSA_INSERT_PHIS, "SSA: insert phis", false, PHASE_BUILD_SSA, false) CompPhaseNameMacro(PHASE_BUILD_SSA_RENAME, "SSA: rename", false, PHASE_BUILD_SSA, false) +CompPhaseNameMacro(PHASE_AGGRESSIVE_SSA_DCE, "Aggressive SSA-based DCE", false, -1, false) CompPhaseNameMacro(PHASE_EARLY_PROP, "Early Value Propagation", false, -1, false) CompPhaseNameMacro(PHASE_OPTIMIZE_INDUCTION_VARIABLES, "Optimize Induction Variables", false, -1, false) CompPhaseNameMacro(PHASE_VALUE_NUMBER, "Do value numbering", false, -1, false) diff --git a/src/coreclr/jit/jitmetadatalist.h b/src/coreclr/jit/jitmetadatalist.h index 209133f4324b0f..5e28f152c84c3d 100644 --- a/src/coreclr/jit/jitmetadatalist.h +++ b/src/coreclr/jit/jitmetadatalist.h @@ -71,6 +71,7 @@ JITMETADATAMETRIC(ProfileInconsistentInlineeScale, int, 0) JITMETADATAMETRIC(ProfileInconsistentInlinee, int, 0) JITMETADATAMETRIC(ProfileInconsistentNoReturnInlinee, int, 0) JITMETADATAMETRIC(ProfileInconsistentMayThrowInlinee, int, 0) +JITMETADATAMETRIC(AggressiveDceDeadNodes, int, 0) #undef JITMETADATA #undef JITMETADATAINFO diff --git a/src/coreclr/jit/liveness.cpp b/src/coreclr/jit/liveness.cpp index ef3fedf5b31d32..0488b05fb331d4 100644 --- a/src/coreclr/jit/liveness.cpp +++ b/src/coreclr/jit/liveness.cpp @@ -568,6 +568,162 @@ void Compiler::fgPerBlockLocalVarLiveness() #endif // DEBUG } +bool Compiler::fgIsPreLive(GenTree* tree) +{ + if (tree->OperIs(GT_STOREIND, GT_STORE_BLK) || tree->IsCall()) + { + return true; + } + + if (tree->OperIs(GT_RETURN)) + { + return true; + } + + if (tree->OperIsLocalStore()) + { + LclVarDsc* lclDsc = lvaGetDesc(tree->AsLclVarCommon()); + if (!lclDsc->lvInSsa) + { + return true; + } + + if (lvaGetDesc(lclDsc->lvParentLcl)->lvDoNotEnregister) + { + return true; + } + + return false; + } + + if (tree->OperMayThrow(this)) + { + return true; + } + + return false; +} + +static NodeCounts s_nodeCounts; +static DumpOnShutdown d("Removable node types", &s_nodeCounts); + +//------------------------------------------------------------------------ +// fgSsaBasedDce: +// +// +// Return Value: +// +PhaseStatus Compiler::fgSsaBasedDce() +{ + BlockReachabilitySets* reachabilitySets = BlockReachabilitySets::Build(m_dfsTree); + TreeSet* live = new (this, CMK_Liveness) TreeSet(getAllocator(CMK_Liveness)); + + struct TreeWithBlock + { + GenTree* Tree; + BasicBlock* Block; + + TreeWithBlock(GenTree* tree, BasicBlock* block) + : Tree(tree), Block(block) + { + } + }; + + ArrayStack worklist(getAllocator(CMK_Liveness)); + + for (BasicBlock* block : Blocks()) + { + for (Statement* stmt : block->Statements()) + { + for (GenTree* tree : stmt->TreeList()) + { + if (fgIsPreLive(tree)) + { + JITDUMP("Visiting [%06u] as prelive node\n", dspTreeID(tree)); + assert(!live->Lookup(tree)); + live->Set(tree, true); + worklist.Emplace(tree, block); + } + } + } + } + + while (!worklist.Empty()) + { + TreeWithBlock treeAndBlock = worklist.Pop(); + GenTree* node = treeAndBlock.Tree; + BasicBlock* block = treeAndBlock.Block; + + if (node->OperIs(GT_COMMA)) + { + if (!live->Set(node->gtGetOp2(), true, TreeSet::Overwrite)) + { + worklist.Emplace(node->gtGetOp2(), block); + } + } + else + { + node->VisitOperands([=, &worklist](GenTree* op) { + if (!live->Set(op, true, TreeSet::Overwrite)) + { + worklist.Emplace(op, block); + } + + return GenTree::VisitResult::Continue; + }); + } + + if (node->OperIsLocalRead()) + { + GenTreeLclVarCommon* lcl = node->AsLclVarCommon(); + LclVarDsc* lclDsc = lvaGetDesc(lcl); + if (lclDsc->lvInSsa) + { + LclSsaVarDsc* ssaDsc = lclDsc->GetPerSsaData(lcl->GetSsaNum()); + GenTreeLclVarCommon* defNode = ssaDsc->GetDefNode(); + if (defNode != nullptr && !live->Set(defNode, true, TreeSet::Overwrite)) + { + worklist.Emplace(defNode, ssaDsc->GetBlock()); + } + } + } + + JITDUMP("Visiting reaching preds of " FMT_BB "\n", block->bbNum); + reachabilitySets->VisitReachingBlocks(block, [=, &worklist](BasicBlock* pred) { + JITDUMP(" Visiting reaching pred " FMT_BB "\n", pred->bbNum); + if ((pred != block) && pred->KindIs(BBJ_COND, BBJ_SWITCH)) + { + GenTree* terminator = pred->lastStmt()->GetRootNode(); + if (!live->Set(terminator, true, TreeSet::Overwrite)) + { + worklist.Emplace(terminator, pred); + } + } + }); + } + + for (BasicBlock* block : Blocks()) + { + for (Statement* stmt : block->Statements()) + { + for (GenTree* tree : stmt->TreeList()) + { + if (!live->Lookup(tree)) + { + if (!tree->OperIs(GT_NOP, GT_COMMA, GT_PHI_ARG, GT_PHI) && !tree->IsPhiDefn()) + { + Metrics.AggressiveDceDeadNodes++; + JITDUMP("We could delete [%06u]\n", dspTreeID(tree)); + s_nodeCounts.record(tree->gtOper); + } + } + } + } + } + + return PhaseStatus::MODIFIED_NOTHING; +} + // Helper functions to mark variables live over their entire scope void Compiler::fgBeginScopeLife(VARSET_TP* inScope, VarScopeDsc* var) From bb8d5e6791c8e08ff537b313f7387a6433cb5802 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Tue, 4 Jun 2024 19:04:13 -0700 Subject: [PATCH 2/9] Move phase, start deleting IR --- src/coreclr/jit/compiler.cpp | 4 +- src/coreclr/jit/flowgraph.cpp | 8 ++- src/coreclr/jit/liveness.cpp | 119 +++++++++++++++++++++++++++++----- 3 files changed, 111 insertions(+), 20 deletions(-) diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index 4da0df82c97701..23d40b6a1b2779 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/src/coreclr/jit/compiler.cpp @@ -5067,8 +5067,6 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl fgLocalVarLiveness(); } - DoPhase(this, PHASE_AGGRESSIVE_SSA_DCE, &Compiler::fgSsaBasedDce); - if (doEarlyProp) { // Propagate array length and rewrite getType() method call @@ -5138,6 +5136,8 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl DoPhase(this, PHASE_OPTIMIZE_INDEX_CHECKS, &Compiler::rangeCheckPhase); } + DoPhase(this, PHASE_AGGRESSIVE_SSA_DCE, &Compiler::fgSsaBasedDce); + if (doOptimizeIVs) { // Simplify and optimize induction variables used in natural loops diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp index d05a815b12f71a..bab2e53fe4f00c 100644 --- a/src/coreclr/jit/flowgraph.cpp +++ b/src/coreclr/jit/flowgraph.cpp @@ -6351,7 +6351,6 @@ BlockReachabilitySets* BlockReachabilitySets::Build(const FlowGraphDfsTree* dfsT sets[i] = BitVecOps::MakeSingleton(&postOrderTraits, i); } - // Find the reachable blocks. Also, set BBF_GC_SAFE_POINT. bool change; unsigned changedIterCount = 1; do @@ -6364,8 +6363,11 @@ BlockReachabilitySets* BlockReachabilitySets::Build(const FlowGraphDfsTree* dfsT for (BasicBlock* const predBlock : block->PredBlocks()) { - change |= BitVecOps::UnionDChanged(&postOrderTraits, sets[block->bbPostorderNum], - sets[predBlock->bbPostorderNum]); + if (dfsTree->Contains(predBlock)) + { + change |= BitVecOps::UnionDChanged(&postOrderTraits, sets[block->bbPostorderNum], + sets[predBlock->bbPostorderNum]); + } } } diff --git a/src/coreclr/jit/liveness.cpp b/src/coreclr/jit/liveness.cpp index 0488b05fb331d4..92ea3a97a32e43 100644 --- a/src/coreclr/jit/liveness.cpp +++ b/src/coreclr/jit/liveness.cpp @@ -568,14 +568,25 @@ void Compiler::fgPerBlockLocalVarLiveness() #endif // DEBUG } +//------------------------------------------------------------------------ +// fgIsPreLive: +// Check if an IR node should be considered as part of the "prelive" set. +// +// Return Value: +// True if the node has a side-effect. +// +// Remarks: +// The prelive set are nodes that always need to be retained. These are the +// "seeds" to the aggressive DCE algorithm. +// bool Compiler::fgIsPreLive(GenTree* tree) { - if (tree->OperIs(GT_STOREIND, GT_STORE_BLK) || tree->IsCall()) + if (tree->OperIs(GT_STOREIND, GT_STORE_BLK, GT_CALL, GT_NO_OP)) { return true; } - if (tree->OperIs(GT_RETURN)) + if (tree->OperIs(GT_RETURN, GT_RETFILT, GT_SWIFT_ERROR_RET)) { return true; } @@ -588,6 +599,9 @@ bool Compiler::fgIsPreLive(GenTree* tree) return true; } + // We currently cannot go from the use of a promoted struct local to + // its field's definition, so we have to consider these cases + // conservatively. if (lvaGetDesc(lclDsc->lvParentLcl)->lvDoNotEnregister) { return true; @@ -596,7 +610,7 @@ bool Compiler::fgIsPreLive(GenTree* tree) return false; } - if (tree->OperMayThrow(this)) + if (tree->OperRequiresAsgFlag() || tree->OperMayThrow(this) || tree->OperRequiresCallFlag(this)) { return true; } @@ -605,16 +619,24 @@ bool Compiler::fgIsPreLive(GenTree* tree) } static NodeCounts s_nodeCounts; -static DumpOnShutdown d("Removable node types", &s_nodeCounts); +//static DumpOnShutdown d("Removable node types", &s_nodeCounts); //------------------------------------------------------------------------ // fgSsaBasedDce: -// +// Do aggressive SSA-based dead code elimination. // // Return Value: +// Suitable phase status. +// +// Remarks: +// Unlike the liveness based DCE, this DCE pass is able to remove loops that +// do not compute anything that meaningful. The algorithm implemented appears +// in Cytron, Ron, et al. "Efficiently computing static single assignment +// form and the control dependence graph." // PhaseStatus Compiler::fgSsaBasedDce() { + m_dfsTree = fgComputeDfs(); BlockReachabilitySets* reachabilitySets = BlockReachabilitySets::Build(m_dfsTree); TreeSet* live = new (this, CMK_Liveness) TreeSet(getAllocator(CMK_Liveness)); @@ -673,13 +695,18 @@ PhaseStatus Compiler::fgSsaBasedDce() }); } - if (node->OperIsLocalRead()) + if (node->OperIsLocalRead() || node->OperIs(GT_STORE_LCL_FLD)) { GenTreeLclVarCommon* lcl = node->AsLclVarCommon(); LclVarDsc* lclDsc = lvaGetDesc(lcl); if (lclDsc->lvInSsa) { LclSsaVarDsc* ssaDsc = lclDsc->GetPerSsaData(lcl->GetSsaNum()); + if (node->OperIs(GT_STORE_LCL_FLD)) + { + ssaDsc = lclDsc->GetPerSsaData(ssaDsc->GetUseDefSsaNum()); + } + GenTreeLclVarCommon* defNode = ssaDsc->GetDefNode(); if (defNode != nullptr && !live->Set(defNode, true, TreeSet::Overwrite)) { @@ -702,26 +729,88 @@ PhaseStatus Compiler::fgSsaBasedDce() }); } - for (BasicBlock* block : Blocks()) + struct ExtractVisitor : GenTreeVisitor { - for (Statement* stmt : block->Statements()) + private: + TreeSet* m_live; + BasicBlock* m_block; + Statement* m_insertBefore; + + public: + enum { - for (GenTree* tree : stmt->TreeList()) + DoPreOrder = true, + UseExecutionOrder = true, + }; + + ExtractVisitor(Compiler* comp, TreeSet* live, BasicBlock* block, Statement* insertBefore) + : GenTreeVisitor(comp), m_live(live), m_block(block), m_insertBefore(insertBefore) + { + } + + fgWalkResult PreOrderVisit(GenTree** use, GenTree* user) + { + GenTree* node = *use; + if (!m_live->Lookup(node)) { - if (!live->Lookup(tree)) + return WALK_CONTINUE; + } + + Statement* newStmt = m_compiler->fgNewStmtFromTree(node, m_insertBefore->GetDebugInfo()); + m_compiler->fgInsertStmtBefore(m_block, m_insertBefore, newStmt); + JITDUMP("Extracted [%06u]\n", Compiler::dspTreeID(node)); + DISPSTMT(newStmt); + JITDUMP("\n"); + + return WALK_SKIP_SUBTREES; + } + }; + + bool changed = false; + for (unsigned i = m_dfsTree->GetPostOrderCount(); i != 0; i--) + { + BasicBlock* block = m_dfsTree->GetPostOrder(i - 1); + + Statement* stmt = block->firstStmt(); + while (stmt != nullptr) + { + Statement* nextStmt = stmt->GetNextStmt(); + + if (live->Lookup(stmt->GetRootNode())) + { + stmt = nextStmt; + continue; + } + + ExtractVisitor visitor(this, live, block, stmt); + // Extract live children nodes in execution order. + visitor.WalkTree(stmt->GetRootNodePointer(), nullptr); + + if (block->HasTerminator() && (stmt == block->lastStmt())) + { + assert(block->KindIs(BBJ_COND, BBJ_SWITCH)); + FlowEdge* bestSuccEdge = nullptr; + for (FlowEdge* edge : block->SuccEdges()) { - if (!tree->OperIs(GT_NOP, GT_COMMA, GT_PHI_ARG, GT_PHI) && !tree->IsPhiDefn()) + if ((bestSuccEdge == nullptr) || + (edge->getDestinationBlock()->bbPostorderNum > bestSuccEdge->getDestinationBlock()->bbPostorderNum)) { - Metrics.AggressiveDceDeadNodes++; - JITDUMP("We could delete [%06u]\n", dspTreeID(tree)); - s_nodeCounts.record(tree->gtOper); + bestSuccEdge = edge; } } + + block->SetKindAndTargetEdge(BBJ_ALWAYS, bestSuccEdge); } + + fgRemoveStmt(block, stmt); + changed = true; + stmt = nextStmt; } } - return PhaseStatus::MODIFIED_NOTHING; + fgInvalidateDfsTree(); + + return changed ? PhaseStatus::MODIFIED_EVERYTHING : PhaseStatus::MODIFIED_NOTHING; } // Helper functions to mark variables live over their entire scope From d0c2b45e685844a4e38624fed83caefc32dc703c Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Wed, 5 Jun 2024 19:41:26 +0200 Subject: [PATCH 3/9] Fix asserts, remove some spammy JITDUMP --- src/coreclr/jit/liveness.cpp | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/coreclr/jit/liveness.cpp b/src/coreclr/jit/liveness.cpp index 92ea3a97a32e43..0a2056800c5ccf 100644 --- a/src/coreclr/jit/liveness.cpp +++ b/src/coreclr/jit/liveness.cpp @@ -586,7 +586,7 @@ bool Compiler::fgIsPreLive(GenTree* tree) return true; } - if (tree->OperIs(GT_RETURN, GT_RETFILT, GT_SWIFT_ERROR_RET)) + if (tree->OperIs(GT_RETURN, GT_RETFILT, GT_SWIFT_ERROR_RET, GT_JMP)) { return true; } @@ -695,7 +695,7 @@ PhaseStatus Compiler::fgSsaBasedDce() }); } - if (node->OperIsLocalRead() || node->OperIs(GT_STORE_LCL_FLD)) + if (node->OperIsLocalRead() || (node->OperIs(GT_STORE_LCL_FLD) && ((node->gtFlags & GTF_VAR_USEASG) != 0))) { GenTreeLclVarCommon* lcl = node->AsLclVarCommon(); LclVarDsc* lclDsc = lvaGetDesc(lcl); @@ -715,9 +715,7 @@ PhaseStatus Compiler::fgSsaBasedDce() } } - JITDUMP("Visiting reaching preds of " FMT_BB "\n", block->bbNum); reachabilitySets->VisitReachingBlocks(block, [=, &worklist](BasicBlock* pred) { - JITDUMP(" Visiting reaching pred " FMT_BB "\n", pred->bbNum); if ((pred != block) && pred->KindIs(BBJ_COND, BBJ_SWITCH)) { GenTree* terminator = pred->lastStmt()->GetRootNode(); @@ -786,20 +784,22 @@ PhaseStatus Compiler::fgSsaBasedDce() // Extract live children nodes in execution order. visitor.WalkTree(stmt->GetRootNodePointer(), nullptr); - if (block->HasTerminator() && (stmt == block->lastStmt())) + if (block->KindIs(BBJ_COND, BBJ_SWITCH) && (stmt == block->lastStmt())) { - assert(block->KindIs(BBJ_COND, BBJ_SWITCH)); - FlowEdge* bestSuccEdge = nullptr; + BasicBlock* bestSucc = nullptr; for (FlowEdge* edge : block->SuccEdges()) { - if ((bestSuccEdge == nullptr) || - (edge->getDestinationBlock()->bbPostorderNum > bestSuccEdge->getDestinationBlock()->bbPostorderNum)) + if ((bestSucc == nullptr) || + (edge->getDestinationBlock()->bbPostorderNum > bestSucc->bbPostorderNum)) { - bestSuccEdge = edge; + bestSucc = edge->getDestinationBlock(); } + + fgRemoveRefPred(edge); } - block->SetKindAndTargetEdge(BBJ_ALWAYS, bestSuccEdge); + FlowEdge* newEdge = fgAddRefPred(bestSucc, block); + block->SetKindAndTargetEdge(BBJ_ALWAYS, newEdge); } fgRemoveStmt(block, stmt); From afbbf1ecb609917e80f419cead54144ae5760bbb Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Wed, 5 Jun 2024 19:42:51 +0200 Subject: [PATCH 4/9] Run jit-format --- src/coreclr/jit/flowgraph.cpp | 2 +- src/coreclr/jit/liveness.cpp | 37 ++++++++++++++++++----------------- 2 files changed, 20 insertions(+), 19 deletions(-) diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp index bab2e53fe4f00c..cb98b5602a3808 100644 --- a/src/coreclr/jit/flowgraph.cpp +++ b/src/coreclr/jit/flowgraph.cpp @@ -6366,7 +6366,7 @@ BlockReachabilitySets* BlockReachabilitySets::Build(const FlowGraphDfsTree* dfsT if (dfsTree->Contains(predBlock)) { change |= BitVecOps::UnionDChanged(&postOrderTraits, sets[block->bbPostorderNum], - sets[predBlock->bbPostorderNum]); + sets[predBlock->bbPostorderNum]); } } } diff --git a/src/coreclr/jit/liveness.cpp b/src/coreclr/jit/liveness.cpp index 0a2056800c5ccf..0b43d4c6e665fc 100644 --- a/src/coreclr/jit/liveness.cpp +++ b/src/coreclr/jit/liveness.cpp @@ -618,9 +618,6 @@ bool Compiler::fgIsPreLive(GenTree* tree) return false; } -static NodeCounts s_nodeCounts; -//static DumpOnShutdown d("Removable node types", &s_nodeCounts); - //------------------------------------------------------------------------ // fgSsaBasedDce: // Do aggressive SSA-based dead code elimination. @@ -636,17 +633,18 @@ static NodeCounts s_nodeCounts; // PhaseStatus Compiler::fgSsaBasedDce() { - m_dfsTree = fgComputeDfs(); + m_dfsTree = fgComputeDfs(); BlockReachabilitySets* reachabilitySets = BlockReachabilitySets::Build(m_dfsTree); - TreeSet* live = new (this, CMK_Liveness) TreeSet(getAllocator(CMK_Liveness)); + TreeSet* live = new (this, CMK_Liveness) TreeSet(getAllocator(CMK_Liveness)); struct TreeWithBlock { - GenTree* Tree; + GenTree* Tree; BasicBlock* Block; TreeWithBlock(GenTree* tree, BasicBlock* block) - : Tree(tree), Block(block) + : Tree(tree) + , Block(block) { } }; @@ -673,8 +671,8 @@ PhaseStatus Compiler::fgSsaBasedDce() while (!worklist.Empty()) { TreeWithBlock treeAndBlock = worklist.Pop(); - GenTree* node = treeAndBlock.Tree; - BasicBlock* block = treeAndBlock.Block; + GenTree* node = treeAndBlock.Tree; + BasicBlock* block = treeAndBlock.Block; if (node->OperIs(GT_COMMA)) { @@ -692,13 +690,13 @@ PhaseStatus Compiler::fgSsaBasedDce() } return GenTree::VisitResult::Continue; - }); + }); } if (node->OperIsLocalRead() || (node->OperIs(GT_STORE_LCL_FLD) && ((node->gtFlags & GTF_VAR_USEASG) != 0))) { - GenTreeLclVarCommon* lcl = node->AsLclVarCommon(); - LclVarDsc* lclDsc = lvaGetDesc(lcl); + GenTreeLclVarCommon* lcl = node->AsLclVarCommon(); + LclVarDsc* lclDsc = lvaGetDesc(lcl); if (lclDsc->lvInSsa) { LclSsaVarDsc* ssaDsc = lclDsc->GetPerSsaData(lcl->GetSsaNum()); @@ -724,25 +722,28 @@ PhaseStatus Compiler::fgSsaBasedDce() worklist.Emplace(terminator, pred); } } - }); + }); } struct ExtractVisitor : GenTreeVisitor { private: - TreeSet* m_live; + TreeSet* m_live; BasicBlock* m_block; - Statement* m_insertBefore; + Statement* m_insertBefore; public: enum { - DoPreOrder = true, + DoPreOrder = true, UseExecutionOrder = true, }; ExtractVisitor(Compiler* comp, TreeSet* live, BasicBlock* block, Statement* insertBefore) - : GenTreeVisitor(comp), m_live(live), m_block(block), m_insertBefore(insertBefore) + : GenTreeVisitor(comp) + , m_live(live) + , m_block(block) + , m_insertBefore(insertBefore) { } @@ -804,7 +805,7 @@ PhaseStatus Compiler::fgSsaBasedDce() fgRemoveStmt(block, stmt); changed = true; - stmt = nextStmt; + stmt = nextStmt; } } From b571cb8f8b65a45b5644ae65ba2365b94bee8ab6 Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Wed, 5 Jun 2024 19:54:21 +0200 Subject: [PATCH 5/9] Postdominators --- src/coreclr/jit/block.h | 9 +- src/coreclr/jit/compiler.cpp | 4 + src/coreclr/jit/compiler.h | 129 +++++- src/coreclr/jit/compiler.hpp | 135 +++++++ src/coreclr/jit/compphases.h | 1 + src/coreclr/jit/copyprop.cpp | 2 +- src/coreclr/jit/fgopt.cpp | 25 ++ src/coreclr/jit/flowgraph.cpp | 503 +++++++++++++++++++++++- src/coreclr/jit/redundantbranchopts.cpp | 3 +- src/coreclr/jit/ssabuilder.cpp | 2 +- 10 files changed, 800 insertions(+), 13 deletions(-) diff --git a/src/coreclr/jit/block.h b/src/coreclr/jit/block.h index b0baaf941832f4..81bf43b1e3569b 100644 --- a/src/coreclr/jit/block.h +++ b/src/coreclr/jit/block.h @@ -1545,11 +1545,18 @@ struct BasicBlock : private LIR::Range void* bbSparseProbeList; // Used early on by fgInstrument }; - void* bbSparseCountInfo; // Used early on by fgIncorporateEdgeCounts + union + { + void* bbSparseCountInfo; // Used early on by fgIncorporateEdgeCounts + BasicBlock* bbIPDom; // closest postdominator of block + }; unsigned bbPreorderNum; // the block's preorder number in the graph [0...postOrderCount) unsigned bbPostorderNum; // the block's postorder number in the graph [0...postOrderCount) + unsigned bbReversePreorderNum; // the block's preorder number in the reverse graph [0...postOrderCount) + unsigned bbReversePostorderNum; // the block's postorder number in the reverse graph [0...postOrderCount) + IL_OFFSET bbCodeOffs; // IL offset of the beginning of the block IL_OFFSET bbCodeOffsEnd; // IL offset past the end of the block. Thus, the [bbCodeOffs..bbCodeOffsEnd) // range is not inclusive of the end offset. The count of IL bytes in the block diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index 23d40b6a1b2779..2a34478ec3605c 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/src/coreclr/jit/compiler.cpp @@ -4975,6 +4975,10 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl // Compute dominators and exceptional entry blocks // DoPhase(this, PHASE_COMPUTE_DOMINATORS, &Compiler::fgComputeDominators); + + // Compute postdominators + // + DoPhase(this, PHASE_COMPUTE_POSTDOMINATORS, &Compiler::fgComputePostDominators); } #ifdef DEBUG diff --git a/src/coreclr/jit/compiler.h b/src/coreclr/jit/compiler.h index 6ca16464d5d25e..afcf0bb2b6d256 100644 --- a/src/coreclr/jit/compiler.h +++ b/src/coreclr/jit/compiler.h @@ -1969,6 +1969,69 @@ class FlowGraphDfsTree bool IsAncestor(BasicBlock* ancestor, BasicBlock* descendant) const; }; +// Represents a depth-first search tree of the reversed flow graph. +class FlowGraphReverseDfsTree +{ + Compiler* m_comp; + + // Post-order that we saw reverse reachable basic blocks in. This order can be + // particularly useful to iterate in reverse, as reverse post-order ensures + // that all predecessors are visited before successors whenever possible. + BasicBlock** m_postOrder; + unsigned m_postOrderCount; + + // Pseudo exit representing the block that post-dominates all blocks + BasicBlock* m_pseudoExit; + +public: + FlowGraphReverseDfsTree(Compiler* comp, BasicBlock** postOrder, unsigned postOrderCount, BasicBlock* pseudoExit) + : m_comp(comp) + , m_postOrder(postOrder) + , m_postOrderCount(postOrderCount) + , m_pseudoExit(pseudoExit) + { + } + + Compiler* GetCompiler() const + { + return m_comp; + } + + BasicBlock** GetPostOrder() const + { + return m_postOrder; + } + + unsigned GetPostOrderCount() const + { + return m_postOrderCount; + } + + BasicBlock* GetPostOrder(unsigned index) const + { + assert(index < m_postOrderCount); + return m_postOrder[index]; + } + + BitVecTraits PostOrderTraits() const + { + return BitVecTraits(m_postOrderCount, m_comp); + } + + BasicBlock* PseudoExit() const + { + return m_pseudoExit; + } + +#ifdef DEBUG + void Dump() const; +#endif // DEBUG + + bool Contains(BasicBlock* block) const; + bool IsAncestor(BasicBlock* ancestor, BasicBlock* descendant) const; +}; + + // Represents the result of induction variable analysis. See // FlowGraphNaturalLoop::AnalyzeIteration. struct NaturalLoopIterInfo @@ -2338,7 +2401,7 @@ class FlowGraphNaturalLoops // Represents the dominator tree of the flow graph. class FlowGraphDominatorTree { - template + template friend class DomTreeVisitor; const FlowGraphDfsTree* m_dfsTree; @@ -2372,6 +2435,43 @@ class FlowGraphDominatorTree static FlowGraphDominatorTree* Build(const FlowGraphDfsTree* dfsTree); }; +// Represents the postdominator tree of the flow graph. +class FlowGraphPostDominatorTree +{ + template + friend class DomTreeVisitor; + + const FlowGraphReverseDfsTree* m_reverseDfsTree; + const DomTreeNode* m_domTree; + const unsigned* m_preorderNum; + const unsigned* m_postorderNum; + + FlowGraphPostDominatorTree(const FlowGraphReverseDfsTree* reverseDfsTree, const DomTreeNode* domTree, const unsigned* preorderNum, const unsigned* postorderNum) + : m_reverseDfsTree(reverseDfsTree) + , m_domTree(domTree) + , m_preorderNum(preorderNum) + , m_postorderNum(postorderNum) + { + } + + static BasicBlock* IntersectPostdom(BasicBlock* block1, BasicBlock* block2); + +public: + const FlowGraphReverseDfsTree* GetReverseDfsTree() + { + return m_reverseDfsTree; + } + + BasicBlock* Intersect(BasicBlock* block, BasicBlock* block2); + bool PostDominates(BasicBlock* dominator, BasicBlock* dominated); + +#ifdef DEBUG + void Dump(); +#endif + + static FlowGraphPostDominatorTree* Build(const FlowGraphReverseDfsTree* dfsTree); +}; + // Represents a reverse mapping from block back to its (most nested) containing loop. class BlockToNaturalLoopMap { @@ -5133,6 +5233,7 @@ class Compiler BasicBlock** fgBBReversePostorder; // Blocks in reverse postorder FlowGraphDfsTree* m_dfsTree; + FlowGraphReverseDfsTree* m_reverseDfsTree; // The next members are annotations on the flow graph used during the // optimization phases. They are invalidated once RBO runs and modifies the // flow graph. @@ -5144,6 +5245,10 @@ class Compiler FlowGraphDominatorTree* m_domTree; BlockReachabilitySets* m_reachabilitySets; + // Postdominator tree + FlowGraphPostDominatorTree* m_postDomTree; + BlkToBlkVectorMap* m_postDomFrontiers; + // Do we require loops to be in canonical form? The canonical form ensures that: // 1. All loops have preheaders (single entry blocks that always enter the loop) // 2. All loop exits where bbIsHandlerBeg(exit) is false have only loop predecessors. @@ -5922,6 +6027,8 @@ class Compiler bool fgRemoveUnreachableBlocks(CanRemoveBlockBody canRemoveBlock); PhaseStatus fgComputeDominators(); // Compute dominators + PhaseStatus fgComputePostDominators(); // Compute postdominators + void fgComputePostDominanceFrontiers(); bool fgRemoveDeadBlocks(); // Identify and remove dead blocks. @@ -6186,6 +6293,12 @@ class Compiler FlowGraphDfsTree* fgComputeDfs(); void fgInvalidateDfsTree(); + template + unsigned fgRunReverseDfs(VisitPreorder assignPreorder, VisitPostorder assignPostorder, BasicBlock* pseudoExit); + + FlowGraphReverseDfsTree* fgComputeReverseDfs(); + void fgInvalidateReverseDfsTree(); + void fgRemoveReturnBlock(BasicBlock* block); void fgConvertBBToThrowBB(BasicBlock* block); @@ -11863,10 +11976,11 @@ class GenericTreeWalker final }; // A dominator tree visitor implemented using the curiously-recurring-template pattern, similar to GenTreeVisitor. -template +template class DomTreeVisitor { friend class FlowGraphDominatorTree; + friend class FlowGraphPostDominatorTree; protected: Compiler* m_compiler; @@ -11901,11 +12015,11 @@ class DomTreeVisitor { static_cast(this)->PreOrderVisit(block); - next = tree[block->bbPostorderNum].firstChild; + next = tree[postDom ? block->bbReversePostorderNum : block->bbPostorderNum].firstChild; if (next != nullptr) { - assert(next->bbIDom == block); + assert((!postDom && (next->bbIDom == block)) || (postDom && (next->bbIPDom == block))); continue; } @@ -11913,15 +12027,16 @@ class DomTreeVisitor { static_cast(this)->PostOrderVisit(block); - next = tree[block->bbPostorderNum].nextSibling; + next = tree[postDom ? block->bbReversePostorderNum : block->bbPostorderNum].nextSibling; if (next != nullptr) { - assert(next->bbIDom == block->bbIDom); + assert((!postDom && (next->bbIDom == block->bbIDom)) || + (postDom && (next->bbIPDom == block->bbIPDom))); break; } - block = block->bbIDom; + block = postDom ? block->bbIPDom : block->bbIDom; } while (block != nullptr); } diff --git a/src/coreclr/jit/compiler.hpp b/src/coreclr/jit/compiler.hpp index 40458c51df36ce..b35f1357473e03 100644 --- a/src/coreclr/jit/compiler.hpp +++ b/src/coreclr/jit/compiler.hpp @@ -4858,6 +4858,141 @@ unsigned Compiler::fgRunDfs(VisitPreorder visitPreorder, VisitPostorder visitPos return preOrderIndex; } +//------------------------------------------------------------------------ +// fgRunReverseDfs: Run DFS over the reverse flow graph. +// +// Type parameters: +// VisitPreorder - Functor type that takes a BasicBlock* and its preorder number +// VisitPostorder - Functor type that takes a BasicBlock* and its postorder number +// +// Parameters: +// visitPreorder - Functor to visit block in its preorder +// visitPostorder - Functor to visit block in its postorder +// pseudoExit - non-graph basic block to serve as the postdominator of all blocks +// +// Returns: +// Number of blocks visited. +// +// Notes: +// Requires DFS tree to be prebuilt. +// +template +unsigned Compiler::fgRunReverseDfs(VisitPreorder visitPreorder, VisitPostorder visitPostorder, BasicBlock* pseudoExit) +{ + BitVecTraits traits(fgBBNumMax + 1, this); + BitVec visited(BitVecOps::MakeEmpty(&traits)); + + unsigned preOrderIndex = 0; + unsigned postOrderIndex = 0; + + struct PredInfo + { + PredInfo(BasicBlock* block) + : m_block(block) + , m_edge(block->bbPreds) + { + } + BasicBlock* m_block; + FlowEdge* m_edge; + }; + + ArrayStack blocks(getAllocator(CMK_DepthFirstSearch)); + + auto reverseDfsFrom = [&](BasicBlock* firstBB) { + BitVecOps::AddElemD(&traits, visited, firstBB->bbNum); + FlowEdge* const preds = firstBB->bbPreds; + visitPreorder(firstBB, preOrderIndex++); + blocks.Emplace(firstBB); + + while (!blocks.Empty()) + { + BasicBlock* block = blocks.TopRef().m_block; + + FlowEdge* edge = blocks.TopRef().m_edge; + + if (edge != nullptr) + { + BasicBlock* pred = edge->getSourceBlock(); + blocks.TopRef().m_edge = edge->getNextPredEdge(); + + if (!this->m_dfsTree->Contains(pred)) + { + continue; + } + + if (BitVecOps::TryAddElemD(&traits, visited, pred->bbNum)) + { + blocks.Emplace(pred); + visitPreorder(pred, preOrderIndex++); + } + } + else + { + blocks.Pop(); + + // Defer postorder visit to the pseudo exit + // since we may have more preds to uncover + // + if (block != pseudoExit) + { + visitPostorder(block, postOrderIndex++); + } + } + } + }; + + assert(pseudoExit->bbPreds == nullptr); + + for (BasicBlock* block : Blocks()) + { + if (!m_dfsTree->Contains(block)) + { + continue; + } + + if (block->KindIs(BBJ_RETURN, BBJ_THROW, BBJ_EHFAULTRET)) + { + JITDUMP("RDFS: adding pseudo exit-edge for " FMT_BB "\n", block->bbNum); + fgAddRefPred(pseudoExit, block); + } + } + + reverseDfsFrom(pseudoExit); + + const unsigned dfsCount = m_dfsTree->GetPostOrderCount(); + assert(dfsCount >= postOrderIndex); + + if (dfsCount > postOrderIndex) + { + // If some forward reachable block is not reverse reachable, + // we may have an infinite loop... process these now. + // + // We work in postorder here to try and find the "lowest" blocks first + // so as to minimize the number of pseudo-edges. + // + for (unsigned i = 0; i < m_dfsTree->GetPostOrderCount(); i++) + { + BasicBlock* const block = m_dfsTree->GetPostOrder(i); + + if (BitVecOps::TryAddElemD(&traits, visited, block->bbNum)) + { + JITDUMP("RDFS: adding pseudo exit-edge for infinite loop to " FMT_BB "\n", block->bbNum); + fgAddRefPred(pseudoExit, block); + reverseDfsFrom(block); + } + } + } + + // We've now visited all the preds of the pseudoExit, + // and all the blocks that were visited by the dfs. + // + visitPostorder(pseudoExit, postOrderIndex++); + + assert(preOrderIndex == postOrderIndex); + assert(preOrderIndex == dfsCount + 1); + return preOrderIndex; +} + //------------------------------------------------------------------------------ // FlowGraphNaturalLoop::VisitLoopBlocksReversePostOrder: Visit all of the // loop's blocks in reverse post order. diff --git a/src/coreclr/jit/compphases.h b/src/coreclr/jit/compphases.h index d37d3a00128178..3166b10f8c8a21 100644 --- a/src/coreclr/jit/compphases.h +++ b/src/coreclr/jit/compphases.h @@ -65,6 +65,7 @@ CompPhaseNameMacro(PHASE_OPTIMIZE_FLOW, "Optimize control flow", CompPhaseNameMacro(PHASE_OPTIMIZE_LAYOUT, "Optimize layout", false, -1, false) CompPhaseNameMacro(PHASE_OPTIMIZE_POST_LAYOUT, "Optimize post-layout", false, -1, false) CompPhaseNameMacro(PHASE_COMPUTE_DOMINATORS, "Compute dominators", false, -1, false) +CompPhaseNameMacro(PHASE_COMPUTE_POSTDOMINATORS, "Compute postdominators", false, -1, false) CompPhaseNameMacro(PHASE_CANONICALIZE_ENTRY, "Canonicalize entry", false, -1, false) CompPhaseNameMacro(PHASE_SET_BLOCK_WEIGHTS, "Set block weights", false, -1, false) CompPhaseNameMacro(PHASE_ZERO_INITS, "Redundant zero Inits", false, -1, false) diff --git a/src/coreclr/jit/copyprop.cpp b/src/coreclr/jit/copyprop.cpp index 11bce9e358e3d3..0e042d7b8e4329 100644 --- a/src/coreclr/jit/copyprop.cpp +++ b/src/coreclr/jit/copyprop.cpp @@ -452,7 +452,7 @@ PhaseStatus Compiler::optVnCopyProp() VarSetOps::AssignNoCopy(this, compCurLife, VarSetOps::MakeEmpty(this)); - class CopyPropDomTreeVisitor : public DomTreeVisitor + class CopyPropDomTreeVisitor : public DomTreeVisitor { // The map from lclNum to its recently live definitions as a stack. LclNumToLiveDefsMap m_curSsaName; diff --git a/src/coreclr/jit/fgopt.cpp b/src/coreclr/jit/fgopt.cpp index de2b6a8f37c936..dce25c8956eeeb 100644 --- a/src/coreclr/jit/fgopt.cpp +++ b/src/coreclr/jit/fgopt.cpp @@ -323,6 +323,31 @@ PhaseStatus Compiler::fgComputeDominators() return PhaseStatus::MODIFIED_NOTHING; } +//------------------------------------------------------------- +// fgComputePostDominators: Compute postdominators +// +// Returns: +// Suitable phase status. +// +PhaseStatus Compiler::fgComputePostDominators() +{ + assert(m_dfsTree != nullptr); + + m_reverseDfsTree = fgComputeReverseDfs(); + JITDUMPEXEC(m_reverseDfsTree->Dump()); + + m_postDomTree = FlowGraphPostDominatorTree::Build(m_reverseDfsTree); + JITDUMPEXEC(m_postDomTree->Dump()); + + fgComputePostDominanceFrontiers(); + + // TODO: Postdominated by exception is interesting... the + // postdominance frontier would show us a possible transition + // from "hot" to cold" cold. + // + return PhaseStatus::MODIFIED_NOTHING; +} + //------------------------------------------------------------- // fgInitBlockVarSets: Initialize the per-block variable sets (used for liveness analysis). // diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp index cb98b5602a3808..d962d0a79c1883 100644 --- a/src/coreclr/jit/flowgraph.cpp +++ b/src/coreclr/jit/flowgraph.cpp @@ -4124,6 +4124,113 @@ void Compiler::fgInvalidateDfsTree() fgSsaValid = false; } +#ifdef DEBUG + +//------------------------------------------------------------------------ +// FlowGraphReverseDfsTree::Dump: Dump a textual representation of the reverse DFS tree. +// +void FlowGraphReverseDfsTree::Dump() const +{ + printf("RDFS tree.\n"); + printf("PO RPO -> BB [pre, post]\n"); + for (unsigned i = 0; i < GetPostOrderCount(); i++) + { + unsigned rpoNum = GetPostOrderCount() - i - 1; + BasicBlock* const block = GetPostOrder(i); + printf("%02u %02u -> " FMT_BB "[%u, %u]\n", i, rpoNum, block->bbNum, block->bbReversePreorderNum, + block->bbReversePostorderNum); + } +} + +#endif // DEBUG + +//------------------------------------------------------------------------ +// FlowGraphReverseDfsTree::Contains: Check if a block is contained in the reverse DFS tree; +// i.e., if it is reachable. +// +// Arguments: +// block - The block +// +// Return Value: +// True if the block reaches an exit point +// +// Remarks: +// If the block was added after the reverse DFS tree was computed, then this +// function returns false. +// +bool FlowGraphReverseDfsTree::Contains(BasicBlock* block) const +{ + return (block->bbReversePostorderNum < m_postOrderCount) && (m_postOrder[block->bbReversePostorderNum] == block); +} + +//------------------------------------------------------------------------ +// FlowGraphReverseDfsTree::IsAncestor: Check if block `ancestor` is an ancestor of +// block `descendant` in the reverse DFS +// +// Arguments: +// ancestor - block that is possible ancestor +// descendant - block that is possible descendant +// +// Returns: +// True if `ancestor` is ancestor of `descendant` in the depth first spanning +// tree. +// +// Notes: +// If return value is false, then `ancestor` does not dominate `descendant`. +// +bool FlowGraphReverseDfsTree::IsAncestor(BasicBlock* ancestor, BasicBlock* descendant) const +{ + assert(Contains(ancestor) && Contains(descendant)); + return (ancestor->bbReversePreorderNum <= descendant->bbReversePreorderNum) && + (descendant->bbReversePostorderNum <= ancestor->bbReversePostorderNum); +} + +//------------------------------------------------------------------------ +// fgComputeReverseDfs: Compute a depth-first search tree for the reverse flow graph. +// +// Returns: +// The tree. +// +// Notes: +// Reverse preorder and postorder numbers are assigned into the BasicBlock structure. +// The tree returned contains a postorder of the basic blocks. +// +FlowGraphReverseDfsTree* Compiler::fgComputeReverseDfs() +{ + BasicBlock** postOrder = new (this, CMK_DepthFirstSearch) BasicBlock*[fgBBcount + 1]; + bool hasCycle = false; + + BasicBlock* pseudoExit = new (this, CMK_BasicBlock) BasicBlock(); + memset((void*)pseudoExit, 0, sizeof(BasicBlock)); + pseudoExit->SetKind(BBJ_THROW); + INDEBUG(pseudoExit->bbID = 0); + + auto visitPreorder = [](BasicBlock* block, unsigned preorderNum) { + block->bbReversePreorderNum = preorderNum; + block->bbReversePostorderNum = UINT_MAX; + // this is union'd so needs pre-clearing + block->bbIPDom = nullptr; + }; + + auto visitPostorder = [=](BasicBlock* block, unsigned postorderNum) { + block->bbReversePostorderNum = postorderNum; + assert(postorderNum <= fgBBcount); + postOrder[postorderNum] = block; + }; + + unsigned numBlocks = + fgRunReverseDfs(visitPreorder, visitPostorder, pseudoExit); + return new (this, CMK_DepthFirstSearch) FlowGraphReverseDfsTree(this, postOrder, numBlocks, pseudoExit); +} + +//------------------------------------------------------------------------ +// fgInvalidateReverseDfsTree: Invalidate computed revese DFS tree +// +void Compiler::fgInvalidateReverseDfsTree() +{ + m_reverseDfsTree = nullptr; +} + //------------------------------------------------------------------------ // FlowGraphNaturalLoop::FlowGraphNaturalLoop: Initialize a new loop instance. // @@ -6199,8 +6306,8 @@ FlowGraphDominatorTree* FlowGraphDominatorTree::Build(const FlowGraphDfsTree* df } #endif - // Assign preorder/postorder nums for fast "domnates" queries. - class NumberDomTreeVisitor : public DomTreeVisitor + // Assign preorder/postorder nums for fast "dominates" queries. + class NumberDomTreeVisitor : public DomTreeVisitor { unsigned* m_preorderNums; unsigned* m_postorderNums; @@ -6235,6 +6342,398 @@ FlowGraphDominatorTree* FlowGraphDominatorTree::Build(const FlowGraphDfsTree* df return new (comp, CMK_DominatorMemory) FlowGraphDominatorTree(dfsTree, domTree, preorderNums, postorderNums); } +//------------------------------------------------------------------------ +// FlowGraphPostDominatorTree::IntersectPostdom: +// Find common IPDom parent, much like least common ancestor. +// +// Parameters: +// finger1 - A basic block that might share IPDom ancestor with finger2. +// finger2 - A basic block that might share IPDom ancestor with finger1. +// +// Returns: +// A basic block whose IPDom is the postdominator for finger1 and finger2, or else +// nullptr. This may be called while immediate postdominators are being computed, +// and if the input values are members of the same loop (each reachable from +// the other), then one may not yet have its immediate postdominator computed +// when we are attempting to find the immediate postdominator of the other. So a +// nullptr return value means that the the two inputs are in a cycle, not +// that they don't have a common postdominator ancestor. +// +// Remarks: +// See "A simple, fast dominance algorithm" by Keith D. Cooper, Timothy J. +// Harvey, Ken Kennedy. +// +BasicBlock* FlowGraphPostDominatorTree::IntersectPostdom(BasicBlock* finger1, BasicBlock* finger2) +{ + while (finger1 != finger2) + { + if ((finger1 == nullptr) || (finger2 == nullptr)) + { + return nullptr; + } + while ((finger1 != nullptr) && (finger1->bbReversePostorderNum < finger2->bbReversePostorderNum)) + { + finger1 = finger1->bbIPDom; + } + if (finger1 == nullptr) + { + return nullptr; + } + while ((finger2 != nullptr) && (finger2->bbReversePostorderNum < finger1->bbReversePostorderNum)) + { + finger2 = finger2->bbIPDom; + } + } + return finger1; +} + +//------------------------------------------------------------------------ +// FlowGraphPostDominatorTree::Intersect: +// See FlowGraphPostominatorTree::IntersectPostdom +// +BasicBlock* FlowGraphPostDominatorTree::Intersect(BasicBlock* block1, BasicBlock* block2) +{ + return IntersectPostdom(block1, block2); +} + +//------------------------------------------------------------------------ +// FlowGraphPostDominatorTree::PostDominates: +// Check if node "postDominator" is an ancestor of node "postDominated". +// +// Parameters: +// postDominator - Node that may postdominate +// postDominated - Node that may be postdominated +// +// Returns: +// True "postDominator" postdominates "postDominated". +// +bool FlowGraphPostDominatorTree::PostDominates(BasicBlock* postDominator, BasicBlock* postDominated) +{ + assert(m_reverseDfsTree->Contains(postDominator) && m_reverseDfsTree->Contains(postDominated)); + + // What we want to ask here is basically if A is in the middle of the path + // from B to the root (the entry node) in the postdominator tree. Turns out + // that can be translated as: + // + // A dom B <-> preorder(A) <= preorder(B) && postorder(A) >= postorder(B) + // + // where the equality holds when you ask if A postdominates itself. + // + return (m_preorderNum[postDominator->bbReversePostorderNum] <= + m_preorderNum[postDominated->bbReversePostorderNum]) && + (m_postorderNum[postDominator->bbReversePostorderNum] >= + m_postorderNum[postDominated->bbReversePostorderNum]); +} + +#ifdef DEBUG +//------------------------------------------------------------------------ +// FlowGraphPostDominatorTree::Dump: Dump a textual representation of the postdominator +// tree. +// +void FlowGraphPostDominatorTree::Dump() +{ + Compiler* comp = m_reverseDfsTree->GetCompiler(); + + for (BasicBlock* block : comp->Blocks()) + { + if (!m_reverseDfsTree->Contains(block) || (m_domTree[block->bbReversePostorderNum].firstChild == nullptr)) + continue; + + printf(FMT_BB " : ", block->bbNum); + for (BasicBlock* child = m_domTree[block->bbReversePostorderNum].firstChild; child != nullptr; + child = m_domTree[child->bbReversePostorderNum].nextSibling) + { + printf(FMT_BB " ", child->bbNum); + } + printf("\n"); + } + + printf("\n"); +} +#endif + +//------------------------------------------------------------------------ +// FlowGraphPostDominatorTree::Build: Compute the dominator tree for the blocks in +// the DFS tree. +// +// Parameters: +// reverseDfsTree - reverse DFS tree. +// +// Returns: +// Data structure representing postdominator tree. Immediate postdominators are +// marked directly into the BasicBlock structures, in the bbIPDom field, so +// multiple instances cannot be simultaneously used. +// +// Remarks: +// As a precondition it is required that the reverse flow graph has a unique root. +// This is handled by the reverse DFS tree via its pseudo block. +// +FlowGraphPostDominatorTree* FlowGraphPostDominatorTree::Build(const FlowGraphReverseDfsTree* reverseDfsTree) +{ + Compiler* comp = reverseDfsTree->GetCompiler(); + BasicBlock** postOrder = reverseDfsTree->GetPostOrder(); + unsigned count = reverseDfsTree->GetPostOrderCount(); + BasicBlock* const pseudoExit = reverseDfsTree->PseudoExit(); + + pseudoExit->bbIPDom = nullptr; + + // First compute immediate postdominators. + unsigned numIters = 0; + bool changed = true; + while (changed) + { + changed = false; + + // In reverse post order, except for the entry block (count - 1 is entry BB). + for (unsigned i = count - 1; i > 0; i--) + { + unsigned poNum = i - 1; + BasicBlock* block = postOrder[poNum]; + + // Intersect Postdom, if computed, for all successors + BasicBlock* bbIPDom = nullptr; + + auto visitSucc = [=, &bbIPDom](BasicBlock* succ) { + if (!reverseDfsTree->Contains(succ)) + { + // Unreachable succ...? + return; + } + + if ((numIters <= 0) && (succ->bbReversePostorderNum <= poNum)) + { + return; + } + + if (bbIPDom == nullptr) + { + bbIPDom = succ; + } + else + { + bbIPDom = IntersectPostdom(bbIPDom, succ); + } + }; + + // Look for flow graph exit points. These have the pseudo exit as successor. + // Hopefully the set is small and the pred search is cheap. + // + FlowEdge* const pseudoExitEdge = comp->fgGetPredForBlock(pseudoExit, block); + + if (pseudoExitEdge != nullptr) + { + // This is an actual or implicit flow graph exit. + // + visitSucc(reverseDfsTree->PseudoExit()); + } + + // Now process the regular successors. + // + // Note "infinite loop" blocks will have both an exit + // successor and regular successors. + // + for (BasicBlock* const succ : block->Succs(comp)) + { + visitSucc(succ); + + // All blocks in try, or just the first? + // (if we knew where the try exits were, maybe just those) + // + if (comp->bbIsTryBeg(succ)) + { + assert(succ->hasTryIndex()); + unsigned const tryInd = succ->getTryIndex(); + EHblkDsc* const succTry = comp->ehGetDsc(tryInd); + + if (succTry->HasFilter()) + { + visitSucc(succTry->ebdFilter); + } + + visitSucc(succTry->ebdHndBeg); + } + } + + // TODO: any block that can cause an exception should arguably have + // the associated filter/handler (or if not in a try, the pseudo-exit) + // as a successor. For now we are not modelling these edges here or + // in the RDFS. + // + assert(bbIPDom != nullptr); + + // Did we change the bbIPDom value? If so, we go around the outer loop again. + // + if (block->bbIPDom != bbIPDom) + { + changed = true; + block->bbIPDom = bbIPDom; + } + } + + numIters++; + } + + // Now build the postdominator tree. + // + DomTreeNode* domTree = new (comp, CMK_DominatorMemory) DomTreeNode[count]{}; + + // Build the child and sibling links based on the immediate postdominators. + // Running this loop in post-order means we end up with sibling links in + // reverse post-order. Skip the root since it has no siblings. + // + for (unsigned i = 0; i < count - 1; i++) + { + BasicBlock* block = postOrder[i]; + BasicBlock* parent = block->bbIPDom; + assert(parent != nullptr); + assert(reverseDfsTree->Contains(block) && reverseDfsTree->Contains(parent)); + + domTree[i].nextSibling = domTree[parent->bbReversePostorderNum].firstChild; + domTree[parent->bbReversePostorderNum].firstChild = block; + } + +#ifdef DEBUG + if (comp->verbose) + { + printf("After computing the postdominance tree:\n"); + for (unsigned i = count; i > 0; i--) + { + unsigned poNum = i - 1; + if (domTree[poNum].firstChild == nullptr) + { + continue; + } + + printf(FMT_BB " :", postOrder[poNum]->bbNum); + for (BasicBlock* child = domTree[poNum].firstChild; child != nullptr; + child = domTree[child->bbReversePostorderNum].nextSibling) + { + printf(" " FMT_BB, child->bbNum); + } + printf("\n"); + } + printf("\n"); + } +#endif + + // Assign preorder/postorder nums for fast "domnates" queries. + class NumberDomTreeVisitor : public DomTreeVisitor + { + unsigned* m_preorderNums; + unsigned* m_postorderNums; + unsigned m_preNum = 0; + unsigned m_postNum = 0; + + public: + NumberDomTreeVisitor(Compiler* comp, unsigned* preorderNums, unsigned* postorderNums) + : DomTreeVisitor(comp) + , m_preorderNums(preorderNums) + , m_postorderNums(postorderNums) + { + } + + void PreOrderVisit(BasicBlock* block) + { + m_preorderNums[block->bbPostorderNum] = m_preNum++; + } + + void PostOrderVisit(BasicBlock* block) + { + m_postorderNums[block->bbPostorderNum] = m_postNum++; + } + }; + + unsigned* preorderNums = new (comp, CMK_DominatorMemory) unsigned[count]; + unsigned* postorderNums = new (comp, CMK_DominatorMemory) unsigned[count]; + + NumberDomTreeVisitor number(comp, preorderNums, postorderNums); + number.WalkTree(domTree); + + return new (comp, CMK_DominatorMemory) + FlowGraphPostDominatorTree(reverseDfsTree, domTree, preorderNums, postorderNums); +} + +//------------------------------------------------------------------------ +// fgComputePostDominanceFrontiers: Compute flow graph dominance frontiers +// +// Notes: +// Assumes m_postDomTree (and hence m_reverseDfsTree) exist. +// +// Based on "A Simple, Fast Dominance Algorithm" by +// Cooper, Harvey & Kennedy +// +void Compiler::fgComputePostDominanceFrontiers() +{ + CompAllocator alloc = getAllocator(CMK_DominatorMemory); + m_postDomFrontiers = new (alloc) BlkToBlkVectorMap(alloc); + + BasicBlock** postOrder = m_reverseDfsTree->GetPostOrder(); + unsigned count = m_reverseDfsTree->GetPostOrderCount(); + + for (unsigned i = 0; i < count; ++i) + { + BasicBlock* const block = postOrder[i]; + unsigned const numSucc = block->NumSucc(this); + + // A block can't be in some other block's PDF unless it + // has multiple successors. + // + if (numSucc < 2) + { + continue; + } + + // Process successors to figure out which ones + // are not post dominated by block. + // + BasicBlock* const ipDom = block->bbIPDom; + + for (BasicBlock* succ : block->Succs(this)) + { + assert(m_reverseDfsTree->Contains(succ)); + + // Walk the postdom links until we hit the postdom of block + // + while (succ != ipDom) + { + // Add block to succ's PDF + // It's possible to encounter the same PDF multiple times, ensure that we don't add duplicates. + // + BlkVector& succPDF = *m_postDomFrontiers->Emplace(succ, alloc); + + if (succPDF.empty() || (succPDF.back() != block)) + { + succPDF.push_back(block); + } + + succ = succ->bbIPDom; + } + } + } + +#ifdef DEBUG + if (verbose) + { + printf("\nComputed postdominance frontier:\n"); + for (unsigned i = 0; i < count; ++i) + { + BasicBlock* const b = postOrder[i]; + printf("Block " FMT_BB " := {", b->bbNum); + BlkVector* const blockPDF = m_postDomFrontiers->LookupPointer(b); + if (blockPDF != nullptr) + { + int index = 0; + for (BasicBlock* f : *blockPDF) + { + printf("%s" FMT_BB, (index++ == 0) ? "" : ",", f->bbNum); + } + } + printf("}\n"); + } + } +#endif +} + //------------------------------------------------------------------------ // BlockToNaturalLoopMap::GetLoop: Map a block back to its most nested // containing loop. diff --git a/src/coreclr/jit/redundantbranchopts.cpp b/src/coreclr/jit/redundantbranchopts.cpp index 99e3fbba31ce02..c8d14f88713e81 100644 --- a/src/coreclr/jit/redundantbranchopts.cpp +++ b/src/coreclr/jit/redundantbranchopts.cpp @@ -19,7 +19,8 @@ PhaseStatus Compiler::optRedundantBranches() } #endif // DEBUG - class OptRedundantBranchesDomTreeVisitor : public DomTreeVisitor + class OptRedundantBranchesDomTreeVisitor + : public DomTreeVisitor { public: bool madeChanges; diff --git a/src/coreclr/jit/ssabuilder.cpp b/src/coreclr/jit/ssabuilder.cpp index a0b018b3078b48..406001341308db 100644 --- a/src/coreclr/jit/ssabuilder.cpp +++ b/src/coreclr/jit/ssabuilder.cpp @@ -1238,7 +1238,7 @@ void SsaBuilder::RenameVariables() } } - class SsaRenameDomTreeVisitor : public DomTreeVisitor + class SsaRenameDomTreeVisitor : public DomTreeVisitor { SsaBuilder* m_builder; SsaRenameState* m_renameStack; From 531a232ab23f2adef3ce5623bbb30df72d72b877 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Wed, 5 Jun 2024 20:44:40 +0200 Subject: [PATCH 6/9] Fix best succ logic, use post dominance frontiers --- src/coreclr/jit/compiler.cpp | 4 ---- src/coreclr/jit/compphases.h | 1 - src/coreclr/jit/flowgraph.cpp | 3 +++ src/coreclr/jit/liveness.cpp | 21 +++++++++++++-------- 4 files changed, 16 insertions(+), 13 deletions(-) diff --git a/src/coreclr/jit/compiler.cpp b/src/coreclr/jit/compiler.cpp index 2a34478ec3605c..23d40b6a1b2779 100644 --- a/src/coreclr/jit/compiler.cpp +++ b/src/coreclr/jit/compiler.cpp @@ -4975,10 +4975,6 @@ void Compiler::compCompile(void** methodCodePtr, uint32_t* methodCodeSize, JitFl // Compute dominators and exceptional entry blocks // DoPhase(this, PHASE_COMPUTE_DOMINATORS, &Compiler::fgComputeDominators); - - // Compute postdominators - // - DoPhase(this, PHASE_COMPUTE_POSTDOMINATORS, &Compiler::fgComputePostDominators); } #ifdef DEBUG diff --git a/src/coreclr/jit/compphases.h b/src/coreclr/jit/compphases.h index 3166b10f8c8a21..d37d3a00128178 100644 --- a/src/coreclr/jit/compphases.h +++ b/src/coreclr/jit/compphases.h @@ -65,7 +65,6 @@ CompPhaseNameMacro(PHASE_OPTIMIZE_FLOW, "Optimize control flow", CompPhaseNameMacro(PHASE_OPTIMIZE_LAYOUT, "Optimize layout", false, -1, false) CompPhaseNameMacro(PHASE_OPTIMIZE_POST_LAYOUT, "Optimize post-layout", false, -1, false) CompPhaseNameMacro(PHASE_COMPUTE_DOMINATORS, "Compute dominators", false, -1, false) -CompPhaseNameMacro(PHASE_COMPUTE_POSTDOMINATORS, "Compute postdominators", false, -1, false) CompPhaseNameMacro(PHASE_CANONICALIZE_ENTRY, "Canonicalize entry", false, -1, false) CompPhaseNameMacro(PHASE_SET_BLOCK_WEIGHTS, "Set block weights", false, -1, false) CompPhaseNameMacro(PHASE_ZERO_INITS, "Redundant zero Inits", false, -1, false) diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp index d962d0a79c1883..c35f5a1b6505b8 100644 --- a/src/coreclr/jit/flowgraph.cpp +++ b/src/coreclr/jit/flowgraph.cpp @@ -4118,8 +4118,11 @@ template FlowGraphDfsTree* Compiler::fgComputeDfs(); void Compiler::fgInvalidateDfsTree() { m_dfsTree = nullptr; + m_reverseDfsTree = nullptr; m_loops = nullptr; m_domTree = nullptr; + m_postDomTree = nullptr; + m_postDomFrontiers = nullptr; m_reachabilitySets = nullptr; fgSsaValid = false; } diff --git a/src/coreclr/jit/liveness.cpp b/src/coreclr/jit/liveness.cpp index 0b43d4c6e665fc..5969da9f8af32b 100644 --- a/src/coreclr/jit/liveness.cpp +++ b/src/coreclr/jit/liveness.cpp @@ -634,7 +634,7 @@ bool Compiler::fgIsPreLive(GenTree* tree) PhaseStatus Compiler::fgSsaBasedDce() { m_dfsTree = fgComputeDfs(); - BlockReachabilitySets* reachabilitySets = BlockReachabilitySets::Build(m_dfsTree); + fgComputePostDominators(); TreeSet* live = new (this, CMK_Liveness) TreeSet(getAllocator(CMK_Liveness)); struct TreeWithBlock @@ -713,16 +713,21 @@ PhaseStatus Compiler::fgSsaBasedDce() } } - reachabilitySets->VisitReachingBlocks(block, [=, &worklist](BasicBlock* pred) { - if ((pred != block) && pred->KindIs(BBJ_COND, BBJ_SWITCH)) + BlkVector* blockPDF = m_postDomFrontiers->LookupPointer(block); + if (blockPDF != nullptr) + { + for (BasicBlock* pd : *blockPDF) { - GenTree* terminator = pred->lastStmt()->GetRootNode(); - if (!live->Set(terminator, true, TreeSet::Overwrite)) + if (pd->KindIs(BBJ_COND, BBJ_SWITCH)) { - worklist.Emplace(terminator, pred); + GenTree* terminator = pd->lastStmt()->GetRootNode(); + if (!live->Set(terminator, true, TreeSet::Overwrite)) + { + worklist.Emplace(terminator, pd); + } } } - }); + } } struct ExtractVisitor : GenTreeVisitor @@ -791,7 +796,7 @@ PhaseStatus Compiler::fgSsaBasedDce() for (FlowEdge* edge : block->SuccEdges()) { if ((bestSucc == nullptr) || - (edge->getDestinationBlock()->bbPostorderNum > bestSucc->bbPostorderNum)) + (edge->getDestinationBlock()->bbPostorderNum < bestSucc->bbPostorderNum)) { bestSucc = edge->getDestinationBlock(); } From 15b2cb8acb06711a59116bbdfcdec60567e01e65 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Wed, 5 Jun 2024 20:45:31 +0200 Subject: [PATCH 7/9] Run jit-format --- src/coreclr/jit/flowgraph.cpp | 4 ++-- src/coreclr/jit/liveness.cpp | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp index c35f5a1b6505b8..5ea7c9e39c025d 100644 --- a/src/coreclr/jit/flowgraph.cpp +++ b/src/coreclr/jit/flowgraph.cpp @@ -4118,10 +4118,10 @@ template FlowGraphDfsTree* Compiler::fgComputeDfs(); void Compiler::fgInvalidateDfsTree() { m_dfsTree = nullptr; - m_reverseDfsTree = nullptr; + m_reverseDfsTree = nullptr; m_loops = nullptr; m_domTree = nullptr; - m_postDomTree = nullptr; + m_postDomTree = nullptr; m_postDomFrontiers = nullptr; m_reachabilitySets = nullptr; fgSsaValid = false; diff --git a/src/coreclr/jit/liveness.cpp b/src/coreclr/jit/liveness.cpp index 5969da9f8af32b..ee6f1f996bfc3b 100644 --- a/src/coreclr/jit/liveness.cpp +++ b/src/coreclr/jit/liveness.cpp @@ -633,9 +633,9 @@ bool Compiler::fgIsPreLive(GenTree* tree) // PhaseStatus Compiler::fgSsaBasedDce() { - m_dfsTree = fgComputeDfs(); + m_dfsTree = fgComputeDfs(); fgComputePostDominators(); - TreeSet* live = new (this, CMK_Liveness) TreeSet(getAllocator(CMK_Liveness)); + TreeSet* live = new (this, CMK_Liveness) TreeSet(getAllocator(CMK_Liveness)); struct TreeWithBlock { From 01a784c2f5f20d77fab1f9b0143cbd65e426dcf5 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Wed, 5 Jun 2024 22:46:00 +0200 Subject: [PATCH 8/9] Fix GT_JMP uses of parameters --- src/coreclr/jit/liveness.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/coreclr/jit/liveness.cpp b/src/coreclr/jit/liveness.cpp index ee6f1f996bfc3b..b791f8ca6df54f 100644 --- a/src/coreclr/jit/liveness.cpp +++ b/src/coreclr/jit/liveness.cpp @@ -607,6 +607,11 @@ bool Compiler::fgIsPreLive(GenTree* tree) return true; } + if (compJmpOpUsed && lclDsc->lvIsParam) + { + return true; + } + return false; } From d9176df68ee4ef3f2884e96a8d4c2c89ff220ef9 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Thu, 6 Jun 2024 17:18:37 +0200 Subject: [PATCH 9/9] Consider nodes with ordering side effects to be prelive --- src/coreclr/jit/liveness.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/coreclr/jit/liveness.cpp b/src/coreclr/jit/liveness.cpp index b791f8ca6df54f..8084fd2bc51cd8 100644 --- a/src/coreclr/jit/liveness.cpp +++ b/src/coreclr/jit/liveness.cpp @@ -615,7 +615,8 @@ bool Compiler::fgIsPreLive(GenTree* tree) return false; } - if (tree->OperRequiresAsgFlag() || tree->OperMayThrow(this) || tree->OperRequiresCallFlag(this)) + GenTreeFlags flags = tree->OperEffects(this); + if ((flags & (GTF_SIDE_EFFECT | GTF_ORDER_SIDEEFF)) != 0) { return true; }