Skip to content

[DSE] Introduce eliminateRedundantStoresViaDominatingConditions #181709

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
antoniofrighetto merged 1 commit into from
Apr 4, 2026
+121 −65
Merged

[DSE] Introduce eliminateRedundantStoresViaDominatingConditions #181709

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
183 changes: 121 additions & 62 deletions llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -174,6 +175,11 @@ static cl::opt<bool> EnableInitializesImprovement(
"enable-dse-initializes-attr-improvement", cl::init(true), cl::Hidden,
cl::desc("Enable the initializes attr improvement in DSE"));

static cl::opt<unsigned> MaxDepthRecursion(

@dtcxzyw dtcxzyw Apr 2, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it is a good threshold. The dominator tree may contain a long chain.
See the ir diff for report/cpython/flowgraph.ll in dtcxzyw/llvm-opt-benchmark-nightly@6a71f1d.
The function _PyCfg_OptimizeCodeUnit is very huge. As the compile-time overhead has been addressed by ScopedHashTable, can we drop this depth limit? If the limit is to avoid stack overflow, we can emulate the recursion with a stack.

@antoniofrighetto antoniofrighetto Apr 3, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've been giving this quite of a thought and I think the main challenge, if we were to go iteratively, is that not only would we need to take care that the conditions would be pushed for some children (those that are dominated by the equality edge), but the recursive version now relies on ScopedHashTable, w/ scopes being RAII-based, and the conditions nicely popped for us automatically when leaving a subtree.

I think replacing the recursion with a worklist would also imply managing the scopes lifetime explicitly (e.g., maintaining another stack of ScopeTy objects manually pushing/destroying scopes, or maybe directly using a map instead with manual insert/erase?). So, while there's no reason in theory not to go with an iterative DFS, I think the recursive version is considerably easier to read and reason about (on top of mapping directly to how the algorithm is described). Not sure if I'm missing anything while thinking this over (maybe also cc/ @nikic who initially suggested it above); not too strong on this either though.

Wrt the threshold heuristic, I agree it was a bit too conservatively chosen, so raised to 1024, which should be well safe to avoid stack overflows and possibly to get triggered only on pathological cases.

"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
//===----------------------------------------------------------------------===//
Expand Down Expand Up @@ -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);
Expand All @@ -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
Expand Down Expand Up @@ -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<Value *, Value *>;
using ScopedHTType = ScopedHashTable<ConditionInfo, Instruction *>;

// We maintain a scoped hash table of the active dominating conditions for a
// given node.
ScopedHTType ActiveConditions;
auto GetDominatingCondition = [&](BasicBlock *BB)
-> std::optional<std::tuple<ConditionInfo, Instruction *, BasicBlock *>> {
auto *BI = dyn_cast<CondBrInst>(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<MemoryDef>(&Access);
if (!Def)
continue;

auto *SI = dyn_cast<StoreInst>(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<MemSetInst>(DefI);
Expand Down Expand Up @@ -2237,61 +2353,6 @@ bool DSEState::tryFoldIntoCalloc(MemoryDef *Def, const Value *DefUO) {
return true;
}

bool DSEState::dominatingConditionImpliesValue(MemoryDef *Def) {
auto *StoreI = cast<StoreInst>(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<CondBrInst>(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<StoreInst>(DefI);
Expand Down Expand Up @@ -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<LoadInst>(Store->getOperand(0))) {
if (LoadI->getPointerOperand() == Store->getOperand(1)) {
// Get the defining access for the load.
Expand Down Expand Up @@ -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();
Expand Down
3 changes: 0 additions & 3 deletions llvm/test/Transforms/DeadStoreElimination/dead-stores-via-dom-conditions.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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]]
Expand Down
Loading