[Mlir-commits] [mlir] [MLIR][RemoveDeadValues] Fix affine.for induction variable incorrectly removed (PR #172612)

[Francisco Geiman Thiesen]

================
@@ -327,6 +328,32 @@ RunLivenessAnalysis::RunLivenessAnalysis(Operation *op) {
   solver.load<LivenessAnalysis>(symbolTable);
   LDBG() << "Initializing and running solver";
   (void)solver.initializeAndRun(op);
+
+  // Mark block arguments of RegionBranchOpInterface ops that are NOT successor
+  // inputs as live. These include induction variables (IVs) like those in
+  // affine.for or scf.for. The fix in visitBranchOperand() only handles this
----------------
FranciscoThiesen wrote:

Sorry for that.

For any loop (scf.for, affine.for), the IV is in the entry block and is NOT a successor input in BOTH transition types. So both approaches would identify and mark the same IV.

The key insight: My fix runs unconditionally after the analysis completes. So:

1. For scf.for with variable bounds: #161117 marks IV live during analysis, then my fix marks it live again after
(redundant but harmless)
2. For affine.for with constant bounds: visitBranchOperand() never called, so #161117 never runs. My fix marks IV live after analysis (essential)


For the specific purpose of marking IVs live, the #161117 fix is now redundant - my fix will mark IVs live for ALL
RegionBranchOpInterface ops regardless of whether visitBranchOperand() runs. The #161117 code could theoretically be removed, but it's harmless to keep (marking something live twice is idempotent) and provides defense-in-depth.

The only theoretical case where #161117 would still matter is if there's a RegionBranchOpInterface op with an implicit block argument that appears ONLY in internal region-to-region transitions (not in the initial parent-to-region entry). I'm not aware of any such op in MLIR today.

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