[Mlir-commits] [mlir] [mlir][CSE] Eliminate redundant reads across dominating blocks (PR #218146)
llvmlistbot at llvm.org
llvmlistbot at llvm.org
Sat Aug 22 09:35:01 PDT 2026
https://github.com/prometheusfma-llvm created https://github.com/llvm/llvm-project/pull/218146
CSE only removed a duplicated read when both reads lived in the same block. Extend the read-elimination path so a read can be replaced by an equivalent read that dominates it from another block of the same region, provided no conflicting write may execute on any path between them.
The intervening-write check now scans every block on a path from the dominating read to the redundant one, scanning endpoint blocks in full when they sit on a cycle so a write reached through a back edge still blocks the elimination.
Fixes t#218117.
>From a016ba312c9c852426392849beb783075f4f5268 Mon Sep 17 00:00:00 2001
From: Prometheus <prometheus.f.ma at gmail.com>
Date: Sat, 22 Aug 2026 09:23:22 -0700
Subject: [PATCH] [mlir][CSE] Eliminate redundant reads across dominating
blocks
CSE only removed a duplicated read when both reads lived in the same
block. Extend the read-elimination path so a read can be replaced by an
equivalent read that dominates it from another block of the same region,
provided no conflicting write may execute on any path between them.
The intervening-write check now scans every block on a path from the
dominating read to the redundant one, scanning endpoint blocks in full
when they sit on a cycle so a write reached through a back edge still
blocks the elimination.
Fixes llvm/llvm-project#218117.
---
mlir/lib/Transforms/Utils/CSE.cpp | 196 ++++++++++++++++++++++--------
mlir/test/Transforms/cse.mlir | 60 +++++++++
2 files changed, 207 insertions(+), 49 deletions(-)
diff --git a/mlir/lib/Transforms/Utils/CSE.cpp b/mlir/lib/Transforms/Utils/CSE.cpp
index 5af76bc99e956..45f1389849f19 100644
--- a/mlir/lib/Transforms/Utils/CSE.cpp
+++ b/mlir/lib/Transforms/Utils/CSE.cpp
@@ -18,6 +18,7 @@
#include "mlir/Interfaces/SideEffectInterfaces.h"
#include "llvm/ADT/DenseMapInfo.h"
#include "llvm/ADT/ScopedHashTable.h"
+#include "llvm/ADT/SmallPtrSet.h"
#include "llvm/Support/Allocator.h"
#include "llvm/Support/RecyclingAllocator.h"
#include <deque>
@@ -45,6 +46,57 @@ struct SimpleOperationInfo : public llvm::DenseMapInfo<Operation *> {
};
} // namespace
+/// Collect the read effects of `op`. A write can only block CSE of a read if
+/// it can conflict with one of these effects.
+static SmallVector<MemoryEffects::EffectInstance>
+getReadEffects(Operation *op) {
+ SmallVector<MemoryEffects::EffectInstance> readEffects;
+ if (auto memOp = dyn_cast<MemoryEffectOpInterface>(op)) {
+ SmallVector<MemoryEffects::EffectInstance> effects;
+ memOp.getEffects(effects);
+ for (MemoryEffects::EffectInstance &e : effects)
+ if (isa<MemoryEffects::Read>(e.getEffect()))
+ readEffects.push_back(e);
+ }
+ return readEffects;
+}
+
+/// Return true if `op` may perform a write that conflicts with `readEffects`,
+/// or if its effects are unknown (conservatively treated as a write).
+static bool
+mayConflictWithReads(Operation *op,
+ ArrayRef<MemoryEffects::EffectInstance> readEffects) {
+ std::optional<SmallVector<MemoryEffects::EffectInstance>> effects =
+ getEffectsRecursively(op);
+ // If the operation does not implement the MemoryEffectOpInterface we
+ // conservatively assume it writes.
+ if (!effects)
+ return true;
+
+ for (const MemoryEffects::EffectInstance &effect : *effects) {
+ if (!isa<MemoryEffects::Write>(effect.getEffect()))
+ continue;
+ // A write on a resource disjoint from all read resources cannot conflict
+ // with the reads being CSE'd.
+ SideEffects::Resource *writeResource = effect.getResource();
+ bool canConflict = llvm::any_of(readEffects, [&](const auto &readEffect) {
+ SideEffects::Resource *readResource = readEffect.getResource();
+ if (writeResource->isDisjointFrom(readResource))
+ return false;
+ // A pointer-based access to an addressable resource cannot conflict
+ // with a non-addressable resource.
+ if (readEffect.getValue() && !writeResource->isAddressable())
+ return false;
+ if (effect.getValue() && !readResource->isAddressable())
+ return false;
+ return true;
+ });
+ if (canConflict)
+ return true;
+ }
+ return false;
+}
+
namespace {
/// Simple common sub-expression elimination.
class CSEDriver {
@@ -107,9 +159,16 @@ class CSEDriver {
Operation *existing, bool hasSSADominance);
/// Check if there is side-effecting operations other than the given effect
- /// between the two operations.
+ /// between the two operations in the same block.
bool hasOtherSideEffectingOpInBetween(Operation *fromOp, Operation *toOp);
+ /// Check if a write conflicting with the reads of `existing` may execute on
+ /// any path from `existing` to `op`, where the two operations live in
+ /// different blocks of the same region and `existing` dominates `op`.
+ bool hasConflictingWriteAcrossBlocks(
+ Operation *existing, Operation *op,
+ ArrayRef<MemoryEffects::EffectInstance> readEffects);
+
/// A rewriter for modifying the IR.
RewriterBase &rewriter;
@@ -173,16 +232,8 @@ bool CSEDriver::hasOtherSideEffectingOpInBetween(Operation *fromOp,
assert(hasEffect<MemoryEffects::Read>(toOp) &&
"expected read effect on toOp");
- // Collect the read effects of fromOp. A write can only block CSE if it
- // can conflict with one of these reads.
- SmallVector<MemoryEffects::EffectInstance> readEffects;
- if (auto memOp = dyn_cast<MemoryEffectOpInterface>(fromOp)) {
- SmallVector<MemoryEffects::EffectInstance> fromEffects;
- memOp.getEffects(fromEffects);
- for (MemoryEffects::EffectInstance &e : fromEffects)
- if (isa<MemoryEffects::Read>(e.getEffect()))
- readEffects.push_back(e);
- }
+ SmallVector<MemoryEffects::EffectInstance> readEffects =
+ getReadEffects(fromOp);
Operation *nextOp = fromOp->getNextNode();
auto result =
@@ -200,41 +251,10 @@ bool CSEDriver::hasOtherSideEffectingOpInBetween(Operation *fromOp,
}
}
while (nextOp && nextOp != toOp) {
- std::optional<SmallVector<MemoryEffects::EffectInstance>> effects =
- getEffectsRecursively(nextOp);
- if (!effects) {
- // TODO: Do we need to handle other effects generically?
- // If the operation does not implement the MemoryEffectOpInterface we
- // conservatively assume it writes.
- result.first->second =
- std::make_pair(nextOp, MemoryEffects::Write::get());
+ if (mayConflictWithReads(nextOp, readEffects)) {
+ result.first->second = {nextOp, MemoryEffects::Write::get()};
return true;
}
-
- for (const MemoryEffects::EffectInstance &effect : *effects) {
- if (isa<MemoryEffects::Write>(effect.getEffect())) {
- // A write on a resource disjoint from all read resources cannot
- // conflict with the reads being CSE'd.
- SideEffects::Resource *writeResource = effect.getResource();
- bool canConflict =
- llvm::any_of(readEffects, [&](const auto &readEffect) {
- SideEffects::Resource *readResource = readEffect.getResource();
- if (writeResource->isDisjointFrom(readResource))
- return false;
- // A pointer-based access to an addressable resource cannot
- // conflict with a non-addressable resource.
- if (readEffect.getValue() && !writeResource->isAddressable())
- return false;
- if (effect.getValue() && !readResource->isAddressable())
- return false;
- return true;
- });
- if (canConflict) {
- result.first->second = {nextOp, MemoryEffects::Write::get()};
- return true;
- }
- }
- }
nextOp = nextOp->getNextNode();
}
// Record the previous op of `toOp` as the insertion point, since `toOp`
@@ -245,6 +265,76 @@ bool CSEDriver::hasOtherSideEffectingOpInBetween(Operation *fromOp,
return false;
}
+bool CSEDriver::hasConflictingWriteAcrossBlocks(
+ Operation *existing, Operation *op,
+ ArrayRef<MemoryEffects::EffectInstance> readEffects) {
+ Block *fromBlock = existing->getBlock();
+ Block *toBlock = op->getBlock();
+ assert(fromBlock != toBlock && "expected different blocks");
+ assert(existing->getParentRegion() == op->getParentRegion() &&
+ "expected operations in the same region");
+
+ // Blocks that can reach `toBlock` (predecessor closure, including itself).
+ SmallPtrSet<Block *, 8> canReachTo;
+ SmallVector<Block *, 8> worklist(toBlock->pred_begin(), toBlock->pred_end());
+ canReachTo.insert(toBlock);
+ while (!worklist.empty()) {
+ Block *b = worklist.pop_back_val();
+ if (canReachTo.insert(b).second)
+ worklist.append(b->pred_begin(), b->pred_end());
+ }
+
+ // Blocks on some path from `fromBlock` to `toBlock`: reachable from
+ // `fromBlock` and able to reach `toBlock`. Every op that may execute between
+ // the two reads lives in such a block.
+ SmallPtrSet<Block *, 8> between;
+ worklist.assign(1, fromBlock);
+ while (!worklist.empty()) {
+ Block *b = worklist.pop_back_val();
+ if (canReachTo.contains(b) && between.insert(b).second)
+ worklist.append(b->succ_begin(), b->succ_end());
+ }
+
+ // Bail if the region between the reads contains a cycle: a back edge could
+ // carry a write to a later occurrence of the read. Kahn's algorithm reduces
+ // an acyclic graph completely by repeatedly removing sources.
+ DenseMap<Block *, unsigned> numPreds;
+ for (Block *b : between)
+ for (Block *s : b->getSuccessors())
+ if (between.contains(s))
+ ++numPreds[s];
+ SmallVector<Block *, 8> sources;
+ for (Block *b : between)
+ if (!numPreds.contains(b))
+ sources.push_back(b);
+ unsigned reduced = 0;
+ while (!sources.empty()) {
+ Block *b = sources.pop_back_val();
+ ++reduced;
+ for (Block *s : b->getSuccessors())
+ if (between.contains(s) && --numPreds[s] == 0)
+ sources.push_back(s);
+ }
+ if (reduced != between.size())
+ return true;
+
+ // Scan every op that may lie between the two reads: after `existing` in its
+ // block, before `op` in its block, and all of every intermediate block.
+ auto rangeHasConflict = [&](Block::iterator begin, Block::iterator end) {
+ return llvm::any_of(llvm::make_range(begin, end), [&](Operation &cur) {
+ return mayConflictWithReads(&cur, readEffects);
+ });
+ };
+ if (rangeHasConflict(std::next(existing->getIterator()), fromBlock->end()) ||
+ rangeHasConflict(toBlock->begin(), op->getIterator()))
+ return true;
+ for (Block *b : between)
+ if (b != fromBlock && b != toBlock &&
+ rangeHasConflict(b->begin(), b->end()))
+ return true;
+ return false;
+}
+
/// Attempt to eliminate a redundant operation.
LogicalResult CSEDriver::simplifyOperation(ScopedMapTy &knownValues,
Operation *op,
@@ -270,11 +360,19 @@ LogicalResult CSEDriver::simplifyOperation(ScopedMapTy &knownValues,
// Look for an existing definition for the operation.
if (auto *existing = knownValues.lookup(op)) {
- if (existing->getBlock() == op->getBlock() &&
- !hasOtherSideEffectingOpInBetween(existing, op)) {
- // The operation that can be deleted has been reach with no
- // side-effecting operations in between the existing operation and
- // this one so we can remove the duplicate.
+ bool canRemove = false;
+ if (existing->getBlock() == op->getBlock()) {
+ // Both reads live in the same block: no side-effecting op may lie
+ // between them.
+ canRemove = !hasOtherSideEffectingOpInBetween(existing, op);
+ } else if (hasSSADominance &&
+ existing->getParentRegion() == op->getParentRegion()) {
+ // The existing read dominates `op` from another block of the same
+ // region: no conflicting write may occur on any path between them.
+ canRemove = !hasConflictingWriteAcrossBlocks(existing, op,
+ getReadEffects(existing));
+ }
+ if (canRemove) {
replaceUsesAndDelete(knownValues, op, existing, hasSSADominance);
return success();
}
diff --git a/mlir/test/Transforms/cse.mlir b/mlir/test/Transforms/cse.mlir
index 4b2907287d89e..be05fd19f646e 100644
--- a/mlir/test/Transforms/cse.mlir
+++ b/mlir/test/Transforms/cse.mlir
@@ -683,3 +683,63 @@ func.func @cse_pointer_write_does_not_block_non_addressable_read() -> i32 {
%2 = arith.addi %0, %1 : i32
return %2 : i32
}
+
+// -----
+
+/// A read whose result is used again in a dominated block can be CSE'd even
+/// though the two reads live in different blocks, as long as no write occurs
+/// on the path between them.
+// CHECK-LABEL: @cross_block_dominating_read
+func.func @cross_block_dominating_read(%arg0: memref<?xi32>, %arg1: index)
+ -> (i32, i32) {
+ // CHECK: %[[V:.*]] = memref.load
+ %0 = memref.load %arg0[%arg1] : memref<?xi32>
+ cf.br ^bb1
+^bb1:
+ // CHECK-NOT: memref.load
+ %1 = memref.load %arg0[%arg1] : memref<?xi32>
+ // CHECK: return %[[V]], %[[V]]
+ return %0, %1 : i32, i32
+}
+
+// -----
+
+/// A write on the path between the two reads blocks cross-block CSE.
+// CHECK-LABEL: @cross_block_write_blocks
+func.func @cross_block_write_blocks(%arg0: memref<?xi32>, %arg1: index,
+ %v: i32) -> (i32, i32) {
+ // CHECK: %[[V0:.*]] = memref.load
+ %0 = memref.load %arg0[%arg1] : memref<?xi32>
+ cf.br ^bb1
+^bb1:
+ memref.store %v, %arg0[%arg1] : memref<?xi32>
+ // CHECK: %[[V1:.*]] = memref.load
+ %1 = memref.load %arg0[%arg1] : memref<?xi32>
+ // CHECK: return %[[V0]], %[[V1]]
+ return %0, %1 : i32, i32
+}
+
+// -----
+
+/// A write inside the loop after the read reaches the next occurrence of the
+/// read through the back edge, so the loop read is not CSE'd with the read
+/// dominating the loop.
+// CHECK-LABEL: @cross_block_loop_write_blocks
+func.func @cross_block_loop_write_blocks(%arg0: memref<?xi32>, %arg1: index,
+ %v: i32, %n: index) -> (i32, i32) {
+ %c0 = arith.constant 0 : index
+ %c1 = arith.constant 1 : index
+ // CHECK: %[[V0:.*]] = memref.load
+ %0 = memref.load %arg0[%arg1] : memref<?xi32>
+ cf.br ^bb1(%c0 : index)
+^bb1(%i: index):
+ // CHECK: %[[V1:.*]] = memref.load
+ %1 = memref.load %arg0[%arg1] : memref<?xi32>
+ memref.store %v, %arg0[%arg1] : memref<?xi32>
+ %next = arith.addi %i, %c1 : index
+ %cmp = arith.cmpi slt, %next, %n : index
+ cf.cond_br %cmp, ^bb1(%next : index), ^bb2
+^bb2:
+ // CHECK: return %[[V0]], %[[V1]]
+ return %0, %1 : i32, i32
+}
More information about the Mlir-commits
mailing list