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

[Mehdi Amini]

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

In a graph region (e.g. the module body), or in unreachable blocks in SSACFG region, 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: before each iteration of the loop, snapshot the operand list.  If a fold returns empty results but the operands are unchanged, we have reached a fixpoint.  Stop the loop and return failure from tryFold so that dialect conversion falls through to a conversion pattern instead of looping.

Fixes #159675

Assisted-by: Claude Code

>From 102ef2798704853d8c6fa3e039d2b870a9c18d1c 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: before each iteration of the loop, snapshot the operand list.  If a fold
returns empty results but the operands are unchanged, we have reached a
fixpoint.  Stop the loop and return failure from tryFold so that dialect
conversion falls 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                      | 28 +++++++++++++++++--
 .../graph-region-circular-ssa.mlir            | 28 +++++++++++++++++++
 2 files changed, 53 insertions(+), 3 deletions(-)
 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..f68abf056cc21 100644
--- a/mlir/lib/IR/Builders.cpp
+++ b/mlir/lib/IR/Builders.cpp
@@ -497,15 +497,37 @@ OpBuilder::tryFold(Operation *op, SmallVectorImpl<Value> &results,
     return cleanupFailure();
 
   int count = 0;
+  bool fixpoint = false;
   do {
     LDBG() << "Folded in place #" << count
            << " times: " << OpWithFlags(op, OpPrintingFlags().skipRegions());
     count++;
-  } while (foldResults.empty() && succeeded(op->fold(foldResults)));
+  } while (foldResults.empty() && [&]() {
+    // Snapshot the op's observable state before the next fold attempt to detect
+    // fixpoints. An in-place fold may modify operands, discardable attributes,
+    // or properties (inherent attributes) without producing fold results. If
+    // none of these change, no further progress is possible and we stop to
+    // avoid an infinite loop.
+    // Note: getRawDictionaryAttrs() and operand Values are interned pointers,
+    // so equality is O(1). hashProperties() covers the properties blob.
+    SmallVector<Value> operandsBefore(op->getOperands());
+    DictionaryAttr attrsBefore = op->getRawDictionaryAttrs();
+    llvm::hash_code propHashBefore = op->hashProperties();
+    if (!succeeded(op->fold(foldResults)))
+      return false;
+    if (foldResults.empty() && llvm::equal(op->getOperands(), operandsBefore) &&
+        op->getRawDictionaryAttrs() == attrsBefore &&
+        op->hashProperties() == propHashBefore) {
+      fixpoint = true;
+      return false;
+    }
+    return true;
+  }());
 
-  // An in-place fold does not require generation of any constants.
+  // An in-place fold does not require generation of any constants. Return
+  // failure if no actual progress was made (fixpoint reached).
   if (foldResults.empty())
-    return success();
+    return fixpoint ? cleanupFailure() : success();
 
   // A temporary builder used for creating constants during folding.
   OpBuilder cstBuilder(context);
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
+}) : () -> ()