[Mlir-commits] [mlir] [mlir][Transforms][NFC] `remove-dead-values`: Erase ops at the end (PR #174208)

[llvmlistbot at llvm.org]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir-core

Author: Matthias Springer (matthias-springer)

<details>
<summary>Changes</summary>

`remove-dead-values` performs various cleanups:
1. Erasing block arguments
2. Erasing successor operands
3. Erasing operations
4. Erasing function arguments / results
5. Erasing operands
6. Erasing results

This commit moves Step 3 (erasing operations) to the end. While that does not fix any bugs by itself, it is potentially safer. If an operation is erased, we must be careful that the operation is not accessed in the following steps. That can no longer happen if IR is erased only in the final step and not before.

This commit is prefetching a change from #<!-- -->173505 (to keep that PR shorter). With #<!-- -->173505, it will become necessary to erase IR in the final step.


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


1 Files Affected:

- (modified) mlir/lib/Transforms/RemoveDeadValues.cpp (+19-18) 


``````````diff
diff --git a/mlir/lib/Transforms/RemoveDeadValues.cpp b/mlir/lib/Transforms/RemoveDeadValues.cpp
index 07911c6111043..fc2c2acf8afd3 100644
--- a/mlir/lib/Transforms/RemoveDeadValues.cpp
+++ b/mlir/lib/Transforms/RemoveDeadValues.cpp
@@ -805,22 +805,7 @@ static void cleanUpDeadVals(RDVFinalCleanupList &list) {
     }
   }
 
-  // 3. Operations
-  LDBG() << "Cleaning up " << list.operations.size() << " operations";
-  for (Operation *op : list.operations) {
-    LDBG() << "Erasing operation: "
-           << OpWithFlags(op,
-                          OpPrintingFlags().skipRegions().printGenericOpForm());
-    if (op->hasTrait<OpTrait::IsTerminator>()) {
-      // When erasing a terminator, insert an unreachable op in its place.
-      OpBuilder b(op);
-      ub::UnreachableOp::create(b, op->getLoc());
-    }
-    op->dropAllUses();
-    op->erase();
-  }
-
-  // 4. Functions
+  // 3. Functions
   LDBG() << "Cleaning up " << list.functions.size() << " functions";
   // Record which function arguments were erased so we can shrink call-site
   // argument segments for CallOpInterface operations (e.g. ops using
@@ -851,7 +836,7 @@ static void cleanUpDeadVals(RDVFinalCleanupList &list) {
     (void)f.funcOp.eraseResults(f.nonLiveRets);
   }
 
-  // 5. Operands
+  // 4. Operands
   LDBG() << "Cleaning up " << list.operands.size() << " operand lists";
   for (OperandsToCleanup &o : list.operands) {
     // Handle call-specific cleanup only when we have a cached callee reference.
@@ -900,7 +885,7 @@ static void cleanUpDeadVals(RDVFinalCleanupList &list) {
     }
   }
 
-  // 6. Results
+  // 5. Results
   LDBG() << "Cleaning up " << list.results.size() << " result lists";
   for (auto &r : list.results) {
     LDBG_OS([&](raw_ostream &os) {
@@ -912,6 +897,22 @@ static void cleanUpDeadVals(RDVFinalCleanupList &list) {
     });
     dropUsesAndEraseResults(r.op, r.nonLive);
   }
+
+  // 6. Operations
+  LDBG() << "Cleaning up " << list.operations.size() << " operations";
+  for (Operation *op : list.operations) {
+    LDBG() << "Erasing operation: "
+           << OpWithFlags(op,
+                          OpPrintingFlags().skipRegions().printGenericOpForm());
+    if (op->hasTrait<OpTrait::IsTerminator>()) {
+      // When erasing a terminator, insert an unreachable op in its place.
+      OpBuilder b(op);
+      ub::UnreachableOp::create(b, op->getLoc());
+    }
+    op->dropAllUses();
+    op->erase();
+  }
+
   LDBG() << "Finished cleanup of dead values";
 }
 

``````````

</details>


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