[Mlir-commits] [mlir] [mlir] Remove redundant DCE worklist visited set (PR #195662)

[llvmlistbot at llvm.org]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir-core

Author: Mehdi Amini (joker-eph)

<details>
<summary>Changes</summary>

The eliminateTriviallyDeadOps worklist only enqueues operations after checking that they are trivially dead. Dropping an operand before testing the defining operation means a propagated enqueue happens only when that defining operation has no remaining users.

During the simplification in #<!-- -->194041 I didn't simplify it far enough to actually entirely remove the visited set, even though it isn't useful to the algorithm right now.

Assisted-by: Codex

---
Full diff: https://github.com/llvm/llvm-project/pull/195662.diff


1 Files Affected:

- (modified) mlir/lib/Transforms/Utils/RegionUtils.cpp (+3-10) 


``````````diff
diff --git a/mlir/lib/Transforms/Utils/RegionUtils.cpp b/mlir/lib/Transforms/Utils/RegionUtils.cpp
index cee48b0b6b126..ae3ea83758dab 100644
--- a/mlir/lib/Transforms/Utils/RegionUtils.cpp
+++ b/mlir/lib/Transforms/Utils/RegionUtils.cpp
@@ -564,8 +564,8 @@ bool mlir::eliminateTriviallyDeadOps(RewriterBase &rewriter, Region &region,
 
   // Step 2: worklist over ops in this region only.
   //
-  // Worklist invariant: an op is pushed *only* once we have verified it is
-  // trivially dead. No speculative enqueues — every op on the worklist will
+  // Worklist invariant: an op is pushed only once we have verified it is
+  // trivially dead. No speculative enqueues: every op on the worklist will
   // be erased when popped. Two things enforce this:
   //   - the initial seed below calls isOpTriviallyDead before enqueueing,
   //   - the propagation inside the loop drops the erasing op's use of
@@ -573,13 +573,11 @@ bool mlir::eliminateTriviallyDeadOps(RewriterBase &rewriter, Region &region,
   //     sees the post-erase use count and only enqueues when actually dead.
   // Deadness is monotonic within this pass (we never add users, only remove
   // them), so an op that was dead at enqueue time is still dead at pop time.
-  // The `visited` set is just for dedup; no re-check is needed on pop.
   SmallVector<Operation *> worklist;
-  DenseSet<Operation *> visited;
 
   LDBG(2) << "Stage 2: Seeding trivially dead operation worklist";
   for (Operation &op : region.getOps()) {
-    if (isOpTriviallyDead(&op) && visited.insert(&op).second) {
+    if (isOpTriviallyDead(&op)) {
       LDBG(2) << "Seeded worklist with operation: "
               << OpWithFlags(&op, OpPrintingFlags().skipRegions());
       worklist.push_back(&op);
@@ -616,11 +614,6 @@ bool mlir::eliminateTriviallyDeadOps(RewriterBase &rewriter, Region &region,
                   << ": defining operation is outside the current region";
           continue;
         }
-        if (visited.count(defOp)) {
-          LDBG(4) << "Skipping operand #" << opOperand.getOperandNumber()
-                  << ": defining operation was already visited";
-          continue;
-        }
         LDBG(4) << "Dropping operand #" << opOperand.getOperandNumber()
                 << " from defining operation: "
                 << OpWithFlags(defOp, OpPrintingFlags().skipRegions());

``````````

</details>


https://github.com/llvm/llvm-project/pull/195662