diff --git a/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp b/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp index d3ae9a6cf8d80..6704fd0ef86e0 100644 --- a/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp +++ b/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp @@ -32,6 +32,7 @@ #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/MapVector.h" #include "llvm/ADT/PostOrderIterator.h" +#include "llvm/ADT/ScopedHashTable.h" #include "llvm/ADT/SetVector.h" #include "llvm/ADT/SmallPtrSet.h" #include "llvm/ADT/SmallVector.h" @@ -174,6 +175,11 @@ static cl::opt EnableInitializesImprovement( "enable-dse-initializes-attr-improvement", cl::init(true), cl::Hidden, cl::desc("Enable the initializes attr improvement in DSE")); +static cl::opt MaxDepthRecursion( + "dse-max-dom-cond-depth", cl::init(1024), cl::Hidden, + cl::desc("Max dominator tree recursion depth for eliminating redundant " + "stores via dominating conditions")); + //===----------------------------------------------------------------------===// // Helper functions //===----------------------------------------------------------------------===// @@ -1106,10 +1112,6 @@ struct DSEState { /// try folding it into a call to calloc. bool tryFoldIntoCalloc(MemoryDef *Def, const Value *DefUO); - // Check if there is a dominating condition, that implies that the value - // being stored in a ptr is already present in the ptr. - bool dominatingConditionImpliesValue(MemoryDef *Def); - /// \returns true if \p Def is a no-op store, either because it /// directly stores back a loaded value or stores zero to a calloced object. bool storeIsNoop(MemoryDef *Def, const Value *DefUO); @@ -1120,6 +1122,11 @@ struct DSEState { /// is already stored at the same location. bool eliminateRedundantStoresOfExistingValues(); + /// If there is a dominating condition that implies the value being stored in + /// a pointer, and such a condition appears in a node that dominates the + /// store, then the store may be redundant if no write occurs in between. + bool eliminateRedundantStoresViaDominatingConditions(); + // Return the locations written by the initializes attribute. // Note that this function considers: // 1. Unwind edge: use "initializes" attribute only if the callee has @@ -2133,6 +2140,115 @@ bool DSEState::eliminateDeadWritesAtEndOfFunction() { return MadeChange; } +bool DSEState::eliminateRedundantStoresViaDominatingConditions() { + bool MadeChange = false; + LLVM_DEBUG(dbgs() << "Trying to eliminate MemoryDefs whose value being " + "written is implied by a dominating condition\n"); + + using ConditionInfo = std::pair; + using ScopedHTType = ScopedHashTable; + + // We maintain a scoped hash table of the active dominating conditions for a + // given node. + ScopedHTType ActiveConditions; + auto GetDominatingCondition = [&](BasicBlock *BB) + -> std::optional> { + auto *BI = dyn_cast(BB->getTerminator()); + if (!BI) + return std::nullopt; + + // In case both blocks are the same, it is not possible to determine + // if optimization is possible. (We would not want to optimize a store + // in the FalseBB if condition is true and vice versa.) + if (BI->getSuccessor(0) == BI->getSuccessor(1)) + return std::nullopt; + + Instruction *ICmpL; + CmpPredicate Pred; + Value *StorePtr, *StoreVal; + if (!match(BI->getCondition(), + m_c_ICmp(Pred, m_Instruction(ICmpL, m_Load(m_Value(StorePtr))), + m_Value(StoreVal))) || + !ICmpInst::isEquality(Pred)) + return std::nullopt; + + // Ensure the replacement is allowed when comparing pointers, as + // the equality compares addresses only, not pointers' provenance. + if (StoreVal->getType()->isPointerTy() && + !canReplacePointersIfEqual(StoreVal, ICmpL, DL)) + return std::nullopt; + + unsigned ImpliedSuccIdx = Pred == ICmpInst::ICMP_EQ ? 0 : 1; + BasicBlock *ImpliedSucc = BI->getSuccessor(ImpliedSuccIdx); + return {{ConditionInfo(StorePtr, StoreVal), ICmpL, ImpliedSucc}}; + }; + + auto VisitNode = [&](DomTreeNode *Node, unsigned Depth, auto &Self) -> void { + if (Depth > MaxDepthRecursion) + return; + + BasicBlock *BB = Node->getBlock(); + // Check for redundant stores against active known conditions. + if (auto *Accesses = MSSA.getBlockDefs(BB)) { + for (auto &Access : make_early_inc_range(*Accesses)) { + auto *Def = dyn_cast(&Access); + if (!Def) + continue; + + auto *SI = dyn_cast(Def->getMemoryInst()); + if (!SI || !SI->isUnordered()) + continue; + + Instruction *LI = ActiveConditions.lookup( + {SI->getPointerOperand(), SI->getValueOperand()}); + if (!LI) + continue; + + // Found a dominating condition that may imply the value being stored. + // Make sure there does not exist any clobbering access between the + // load and the potential redundant store. + MemoryAccess *LoadAccess = MSSA.getMemoryAccess(LI); + MemoryAccess *ClobberingAccess = + MSSA.getSkipSelfWalker()->getClobberingMemoryAccess(Def, BatchAA); + if (MSSA.dominates(ClobberingAccess, LoadAccess)) { + LLVM_DEBUG(dbgs() + << "Removing No-Op Store:\n DEAD: " << *SI << '\n'); + deleteDeadInstruction(SI); + NumRedundantStores++; + MadeChange = true; + } + } + } + + // See whether this basic block establishes a dominating condition. + auto MaybeCondition = GetDominatingCondition(BB); + + for (DomTreeNode *Child : Node->children()) { + // RAII scope for the active conditions. + ScopedHTType::ScopeTy Scope(ActiveConditions); + if (MaybeCondition) { + const auto &[Cond, LI, ImpliedSucc] = *MaybeCondition; + if (DT.dominates(BasicBlockEdge(BB, ImpliedSucc), Child->getBlock())) { + // Found a condition that holds for this child, dominated by the + // current node via the equality edge. Propagate the condition to + // the children by pushing it onto the table. + ActiveConditions.insert(Cond, LI); + } + } + + // Recursively visit the children of this node. Upon destruction, the no + // longer active condition before visiting any sibling nodes is popped + // from the active scope. + Self(Child, Depth + 1, Self); + } + }; + + // Do a DFS walk of the dom-tree. + VisitNode(DT.getRootNode(), 0, VisitNode); + + return MadeChange; +} + bool DSEState::tryFoldIntoCalloc(MemoryDef *Def, const Value *DefUO) { Instruction *DefI = Def->getMemoryInst(); MemSetInst *MemSet = dyn_cast(DefI); @@ -2237,61 +2353,6 @@ bool DSEState::tryFoldIntoCalloc(MemoryDef *Def, const Value *DefUO) { return true; } -bool DSEState::dominatingConditionImpliesValue(MemoryDef *Def) { - auto *StoreI = cast(Def->getMemoryInst()); - BasicBlock *StoreBB = StoreI->getParent(); - Value *StorePtr = StoreI->getPointerOperand(); - Value *StoreVal = StoreI->getValueOperand(); - - DomTreeNode *IDom = DT.getNode(StoreBB)->getIDom(); - if (!IDom) - return false; - - auto *BI = dyn_cast(IDom->getBlock()->getTerminator()); - if (!BI) - return false; - - // In case both blocks are the same, it is not possible to determine - // if optimization is possible. (We would not want to optimize a store - // in the FalseBB if condition is true and vice versa.) - if (BI->getSuccessor(0) == BI->getSuccessor(1)) - return false; - - Instruction *ICmpL; - CmpPredicate Pred; - if (!match(BI->getCondition(), - m_c_ICmp(Pred, - m_CombineAnd(m_Load(m_Specific(StorePtr)), - m_Instruction(ICmpL)), - m_Specific(StoreVal))) || - !ICmpInst::isEquality(Pred)) - return false; - - // Ensure the replacement is allowed when comparing pointers, as - // the equality compares addresses only, not pointers' provenance. - if (StoreVal->getType()->isPointerTy() && - !canReplacePointersIfEqual(StoreVal, ICmpL, DL)) - return false; - - // In case the else blocks also branches to the if block or the other way - // around it is not possible to determine if the optimization is possible. - if (Pred == ICmpInst::ICMP_EQ && - !DT.dominates(BasicBlockEdge(BI->getParent(), BI->getSuccessor(0)), - StoreBB)) - return false; - - if (Pred == ICmpInst::ICMP_NE && - !DT.dominates(BasicBlockEdge(BI->getParent(), BI->getSuccessor(1)), - StoreBB)) - return false; - - MemoryAccess *LoadAcc = MSSA.getMemoryAccess(ICmpL); - MemoryAccess *ClobAcc = - MSSA.getSkipSelfWalker()->getClobberingMemoryAccess(Def, BatchAA); - - return MSSA.dominates(ClobAcc, LoadAcc); -} - bool DSEState::storeIsNoop(MemoryDef *Def, const Value *DefUO) { Instruction *DefI = Def->getMemoryInst(); StoreInst *Store = dyn_cast(DefI); @@ -2320,9 +2381,6 @@ bool DSEState::storeIsNoop(MemoryDef *Def, const Value *DefUO) { if (!Store) return false; - if (dominatingConditionImpliesValue(Def)) - return true; - if (auto *LoadI = dyn_cast(Store->getOperand(0))) { if (LoadI->getPointerOperand() == Store->getOperand(1)) { // Get the defining access for the load. @@ -2729,6 +2787,7 @@ static bool eliminateDeadStores(Function &F, AliasAnalysis &AA, MemorySSA &MSSA, MadeChange |= State.eliminateRedundantStoresOfExistingValues(); MadeChange |= State.eliminateDeadWritesAtEndOfFunction(); + MadeChange |= State.eliminateRedundantStoresViaDominatingConditions(); while (!State.ToRemove.empty()) { Instruction *DeadInst = State.ToRemove.pop_back_val(); diff --git a/llvm/test/Transforms/DeadStoreElimination/dead-stores-via-dom-conditions.ll b/llvm/test/Transforms/DeadStoreElimination/dead-stores-via-dom-conditions.ll index c1f07c00d648b..53f570e2b3c53 100644 --- a/llvm/test/Transforms/DeadStoreElimination/dead-stores-via-dom-conditions.ll +++ b/llvm/test/Transforms/DeadStoreElimination/dead-stores-via-dom-conditions.ll @@ -20,7 +20,6 @@ define void @remove_tautological_store_via_dom_condition(ptr %x, i1 %c) { ; CHECK: [[JOIN]]: ; CHECK-NEXT: br label %[[INNER:.*]] ; CHECK: [[INNER]]: -; CHECK-NEXT: store i32 0, ptr [[X]], align 4 ; CHECK-NEXT: br label %[[END]] ; CHECK: [[END]]: ; CHECK-NEXT: ret void @@ -114,8 +113,6 @@ define void @remove_tautological_store_via_dom_condition_3(ptr %x, ptr %y, i1 %c ; CHECK-NEXT: [[CMP_2:%.*]] = icmp eq i32 [[VAL_2]], 0 ; CHECK-NEXT: br i1 [[CMP_2]], label %[[IF_EQ:.*]], label %[[IF_ELSE:.*]] ; CHECK: [[IF_EQ]]: -; CHECK-NEXT: store i32 0, ptr [[X]], align 4 -; CHECK-NEXT: store i32 0, ptr [[Y]], align 4 ; CHECK-NEXT: br label %[[JOIN:.*]] ; CHECK: [[IF_ELSE]]: ; CHECK-NEXT: br label %[[JOIN]]