[Mlir-commits] [mlir] [MLIR] Fix infinite loop in tryFold (PR #189232)

[Mehdi Amini]

https://github.com/joker-eph updated https://github.com/llvm/llvm-project/pull/189232

>From 5393ef7d6debf5d7b25ed9aed3e8a71c3b4bf343 Mon Sep 17 00:00:00 2001
From: Mehdi Amini <joker.eph at gmail.com>
Date: Sat, 28 Mar 2026 11:28:13 -0700
Subject: [PATCH] [MLIR] Fix infinite loop in tryFold for circular SSA uses in
 graph regions

In a graph region (e.g. the module body), an op may use its own result as
an operand, creating a circular SSA dependency.  When such an op was passed
to OpBuilder::tryFold, the do-while loop could spin forever:

  1. foldCommutative moved the constant operand to the RHS (in-place fold,
     foldResults still empty).
  2. The next fold call returned the op's own result, signalling an in-place
     fold, but foldSingleResultHook re-ran trait folds, found nothing to swap,
     and returned success with empty foldResults.
  3. Step 2 repeated without end.

Fix: cap the number of in-place fold iterations at 64. Legitimate chains are
very short in practice (foldCommutative swaps once, then the op folds to a
value or constant), so the cap is never reached for correct IR. For the
circular-SSA case the loop now terminates and tryFold returns failure, letting
dialect conversion fall through to a conversion pattern instead of looping.

Add a regression test in Conversion/ArithToLLVM/ that previously hung.

Fixes #159675

Assisted-by: Claude Code
---
 mlir/lib/IR/Builders.cpp                      |  8 +++++-
 .../graph-region-circular-ssa.mlir            | 28 +++++++++++++++++++
 2 files changed, 35 insertions(+), 1 deletion(-)
 create mode 100644 mlir/test/Conversion/ArithToLLVM/graph-region-circular-ssa.mlir

diff --git a/mlir/lib/IR/Builders.cpp b/mlir/lib/IR/Builders.cpp
index cf64954751d5e..43b2f7d992a43 100644
--- a/mlir/lib/IR/Builders.cpp
+++ b/mlir/lib/IR/Builders.cpp
@@ -496,11 +496,17 @@ OpBuilder::tryFold(Operation *op, SmallVectorImpl<Value> &results,
   if (failed(op->fold(foldResults)))
     return cleanupFailure();
 
+  // Bound the number of in-place fold iterations. Legitimate chains are very
+  // short (e.g. foldCommutative swaps once, then the op folds to a value).
+  // Without a bound, circular SSA uses in graph regions can cause an infinite
+  // loop (e.g. addi(x, 0) where x is the op's own result).
+  constexpr int kMaxInPlaceFolds = 64;
   int count = 0;
   do {
     LDBG() << "Folded in place #" << count
            << " times: " << OpWithFlags(op, OpPrintingFlags().skipRegions());
-    count++;
+    if (++count >= kMaxInPlaceFolds)
+      return cleanupFailure();
   } while (foldResults.empty() && succeeded(op->fold(foldResults)));
 
   // An in-place fold does not require generation of any constants.
diff --git a/mlir/test/Conversion/ArithToLLVM/graph-region-circular-ssa.mlir b/mlir/test/Conversion/ArithToLLVM/graph-region-circular-ssa.mlir
new file mode 100644
index 0000000000000..49568a07535f2
--- /dev/null
+++ b/mlir/test/Conversion/ArithToLLVM/graph-region-circular-ssa.mlir
@@ -0,0 +1,28 @@
+// RUN: mlir-opt --convert-arith-to-llvm %s | FileCheck %s
+
+// Regression test for https://github.com/llvm/llvm-project/issues/159675
+//
+// In a graph region (e.g. the module body), SSA values may be used before
+// they are defined.  The arith.addi below uses %1 (a constant zero) as its
+// LHS operand, but %1 is defined *after* %0 in the block, creating a
+// circular SSA dependency from %0's perspective.
+//
+// During dialect conversion, OpBuilder::tryFold attempted to fold arith.addi:
+//   1. foldCommutative swapped operands so the constant moved to RHS.
+//   2. On the next iteration, addi.fold() returned %0 (the op's own result)
+//      because the RHS is now a zero constant, signalling an in-place fold.
+//   3. foldSingleResultHook called trait folds again; foldCommutative found
+//      nothing to swap and returned success with empty fold-results.
+//   4. The do-while loop condition was still true, causing an infinite loop.
+//
+// The fix detects this fixpoint: if a fold returns empty results but leaves
+// the op state (operands, attributes, and properties) unchanged, the loop
+// stops and tryFold returns failure so that the normal conversion pattern is
+// applied instead.
+
+// CHECK: module {
+// CHECK: llvm.add
+"builtin.module"() ({
+  %0 = "arith.addi"(%1, %0) <{overflowFlags = #arith.overflow<none>}> : (index, index) -> index
+  %1 = "arith.constant"() <{value = 0 : index}> : () -> index
+}) : () -> ()