[Mlir-commits] [mlir] d627924 - [mlir][SliceAnalysis] Fix visited set to avoid infinite recursion (#200008)

[llvmlistbot at llvm.org]

Author: fzi-haxel
Date: 2026-05-27T19:00:33+02:00
New Revision: d6279249920562e28cdd1fe4058451fb1d909241

URL: https://github.com/llvm/llvm-project/commit/d6279249920562e28cdd1fe4058451fb1d909241
DIFF: https://github.com/llvm/llvm-project/commit/d6279249920562e28cdd1fe4058451fb1d909241.diff

LOG: [mlir][SliceAnalysis] Fix visited set to avoid infinite recursion  (#200008)

Fixes #139694, which introduced use-def cycle detection during slice
analysis, but some cycles were still not detected, potentially leading
to infinite recursion.

This PR fixes the handling of the visited set, which tracks the current
DFS path during recursion. Previously, the set could fail to detect
double cycles because entries were erased even when no recursive call
was made. The insert/erase operations are now only performed when
recursion actually occurs, ensuring that cycle detection correctly
reflects the active DFS path.

Added: 
    

Modified: 
    mlir/lib/Analysis/SliceAnalysis.cpp
    mlir/test/Dialect/Affine/slicing-utils.mlir

Removed: 
    


################################################################################
diff  --git a/mlir/lib/Analysis/SliceAnalysis.cpp b/mlir/lib/Analysis/SliceAnalysis.cpp
index f388cd8041d6b..d7a083cef7cc0 100644
--- a/mlir/lib/Analysis/SliceAnalysis.cpp
+++ b/mlir/lib/Analysis/SliceAnalysis.cpp
@@ -62,10 +62,10 @@ getForwardSliceImpl(Operation *op, DenseSet<Operation *> &visited,
       // 'visited' set across regions/blocks as long as we remove operations
       // from the set again when the DFS traverses back from the leaf to the
       // root.
-      if (forwardSlice->count(userOp) == 0 && visited.insert(userOp).second)
+      if (forwardSlice->count(userOp) == 0 && visited.insert(userOp).second) {
         getForwardSliceImpl(userOp, visited, forwardSlice, filter);
-
-      visited.erase(userOp);
+        visited.erase(userOp);
+      }
     }
 
   forwardSlice->insert(op);
@@ -81,6 +81,7 @@ void mlir::getForwardSlice(Operation *op, SetVector<Operation *> *forwardSlice,
     // want it in the results.
     forwardSlice->remove(op);
   }
+  assert(visited.size() == 1 && "visited set should only contain op");
 
   // Reverse to get back the actual topological order.
   // std::reverse does not work out of the box on SetVector and I want an
@@ -97,6 +98,7 @@ void mlir::getForwardSlice(Value root, SetVector<Operation *> *forwardSlice,
     getForwardSliceImpl(user, visited, forwardSlice, options.filter);
     visited.erase(user);
   }
+  assert(visited.empty() && "visited set should be empty");
 
   // Reverse to get back the actual topological order.
   // std::reverse does not work out of the box on SetVector and I want an
@@ -121,11 +123,12 @@ static LogicalResult getBackwardSliceImpl(Operation *op,
   auto processValue = [&](Value value) {
     if (auto *definingOp = value.getDefiningOp()) {
       if (backwardSlice->count(definingOp) == 0 &&
-          visited.insert(definingOp).second)
-        return getBackwardSliceImpl(definingOp, visited, backwardSlice,
-                                    options);
-
-      visited.erase(definingOp);
+          visited.insert(definingOp).second) {
+        LogicalResult result =
+            getBackwardSliceImpl(definingOp, visited, backwardSlice, options);
+        visited.erase(definingOp);
+        return result;
+      }
     } else if (auto blockArg = dyn_cast<BlockArgument>(value)) {
       if (options.omitBlockArguments)
         return success();
@@ -183,6 +186,7 @@ LogicalResult mlir::getBackwardSlice(Operation *op,
   visited.insert(op);
   LogicalResult result =
       getBackwardSliceImpl(op, visited, backwardSlice, options);
+  assert(visited.size() == 1 && "visited set should only contain op");
 
   if (!options.inclusive) {
     // Don't insert the top level operation, we just queried on it and don't

diff  --git a/mlir/test/Dialect/Affine/slicing-utils.mlir b/mlir/test/Dialect/Affine/slicing-utils.mlir
index c53667a98cfbe..e1bfeee23d0d2 100644
--- a/mlir/test/Dialect/Affine/slicing-utils.mlir
+++ b/mlir/test/Dialect/Affine/slicing-utils.mlir
@@ -315,3 +315,26 @@ func.func @graph_region_with_cycle() {
 
   return
 }
+
+// -----
+
+// FWD-LABEL: graph_region_with_double_cycle
+// BWD-LABEL: graph_region_with_double_cycle
+// FWDBWD-LABEL: graph_region_with_double_cycle
+func.func @graph_region_with_double_cycle() {
+  test.isolated_graph_region {
+    // FWD: matched: [[V0:%.+]] = "slicing-test-op"([[V1:%.+]], [[V1]]) : (i1, i1) -> i1 forward static slice:
+    // FWD: [[V1]] = "slicing-test-op"([[V0]], [[V0]]) : (i1, i1) -> i1 
+    // FWD: matched: [[V1]] = "slicing-test-op"([[V0]], [[V0]]) : (i1, i1) -> i1 forward static slice:
+    // FWD: [[V0]] = "slicing-test-op"([[V1]], [[V1]]) : (i1, i1) -> i1 
+    
+    // BWD: matched: [[V0:%.+]] = "slicing-test-op"([[V1:%.+]], [[V1]]) : (i1, i1) -> i1 backward static slice:
+    // BWD: [[V1]] = "slicing-test-op"([[V0]], [[V0]]) : (i1, i1) -> i1 
+    // BWD: matched: [[V1]] = "slicing-test-op"([[V0]], [[V0]]) : (i1, i1) -> i1 backward static slice:
+    // BWD: [[V0]] = "slicing-test-op"([[V1]], [[V1]]) : (i1, i1) -> i1 
+    %0 = "slicing-test-op"(%1, %1) : (i1, i1) -> i1
+    %1 = "slicing-test-op"(%0, %0) : (i1, i1) -> i1
+  }
+
+  return
+}