[Mlir-commits] [mlir] Revert "[MLIR] getBackwardSlice: don't bail on ops that are IsolatedFromAbove" (PR #160160)

[Ian Wood]

https://github.com/IanWood1 created https://github.com/llvm/llvm-project/pull/160160

Reverts llvm/llvm-project#158135

>From 6116c6813f1343faf3bb7946be2c6d86ca30356a Mon Sep 17 00:00:00 2001
From: Ian Wood <ianwood2024 at u.northwestern.edu>
Date: Mon, 22 Sep 2025 10:56:14 -0700
Subject: [PATCH] =?UTF-8?q?Revert=20"[MLIR]=20getBackwardSlice:=20don't=20?=
 =?UTF-8?q?bail=20on=20ops=20that=20are=20IsolatedFromAbove=E2=80=A6"?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This reverts commit 2dd3d3852d16cab2c3a032223fc751db750a78f2.
---
 mlir/lib/Analysis/SliceAnalysis.cpp           |  8 ++----
 mlir/test/Transforms/move-operation-deps.mlir | 28 -------------------
 mlir/test/lib/Dialect/Test/TestOps.td         |  3 +-
 .../Transforms/TestMakeIsolatedFromAbove.cpp  |  4 +--
 4 files changed, 6 insertions(+), 37 deletions(-)

diff --git a/mlir/lib/Analysis/SliceAnalysis.cpp b/mlir/lib/Analysis/SliceAnalysis.cpp
index 12dff19ed31d3..7037fa644c7be 100644
--- a/mlir/lib/Analysis/SliceAnalysis.cpp
+++ b/mlir/lib/Analysis/SliceAnalysis.cpp
@@ -109,7 +109,7 @@ static LogicalResult getBackwardSliceImpl(Operation *op,
                                           DenseSet<Operation *> &visited,
                                           SetVector<Operation *> *backwardSlice,
                                           const BackwardSliceOptions &options) {
-  if (!op)
+  if (!op || op->hasTrait<OpTrait::IsIsolatedFromAbove>())
     return success();
 
   // Evaluate whether we should keep this def.
@@ -136,8 +136,7 @@ static LogicalResult getBackwardSliceImpl(Operation *op,
       // blocks of parentOp, which are not technically backward unless they flow
       // into us. For now, just bail.
       if (parentOp && backwardSlice->count(parentOp) == 0) {
-        if (!parentOp->hasTrait<OpTrait::IsIsolatedFromAbove>() &&
-            parentOp->getNumRegions() == 1 &&
+        if (parentOp->getNumRegions() == 1 &&
             parentOp->getRegion(0).hasOneBlock()) {
           return getBackwardSliceImpl(parentOp, visited, backwardSlice,
                                       options);
@@ -151,8 +150,7 @@ static LogicalResult getBackwardSliceImpl(Operation *op,
 
   bool succeeded = true;
 
-  if (!options.omitUsesFromAbove &&
-      !op->hasTrait<OpTrait::IsIsolatedFromAbove>()) {
+  if (!options.omitUsesFromAbove) {
     llvm::for_each(op->getRegions(), [&](Region &region) {
       // Walk this region recursively to collect the regions that descend from
       // this op's nested regions (inclusive).
diff --git a/mlir/test/Transforms/move-operation-deps.mlir b/mlir/test/Transforms/move-operation-deps.mlir
index 75d8386d520ee..aa7b5dc2a240a 100644
--- a/mlir/test/Transforms/move-operation-deps.mlir
+++ b/mlir/test/Transforms/move-operation-deps.mlir
@@ -460,31 +460,3 @@ module attributes {transform.with_named_sequence} {
     transform.yield
   }
 }
-
-// -----
-
-func.func @move_isolated_from_above() -> () {
-  %1 = "before"() : () -> (f32)
-  %2 = "moved0"() : () -> (f32)
-  %3 = test.isolated_one_region_op %2 {} : f32 -> f32
-  %4 = "moved1"(%3) : (f32) -> (f32)
-  return
-}
-// CHECK-LABEL: func @move_isolated_from_above()
-//       CHECK:   %[[MOVED0:.+]] = "moved0"
-//       CHECK:   %[[ISOLATED:.+]] = test.isolated_one_region_op %[[MOVED0]]
-//       CHECK:   %[[MOVED1:.+]] = "moved1"(%[[ISOLATED]])
-//       CHECK:   %[[BEFORE:.+]] = "before"
-
-module attributes {transform.with_named_sequence} {
-  transform.named_sequence @__transform_main(%arg0 : !transform.any_op {transform.readonly}) {
-    %op1 = transform.structured.match ops{["before"]} in %arg0
-        : (!transform.any_op) -> !transform.any_op
-    %op2 = transform.structured.match ops{["moved1"]} in %arg0
-        : (!transform.any_op) -> !transform.any_op
-    %v1 = transform.get_result %op2[0] : (!transform.any_op) -> !transform.any_value
-    transform.test.move_value_defns %v1 before %op1
-        : (!transform.any_value), !transform.any_op
-    transform.yield
-  }
-}
diff --git a/mlir/test/lib/Dialect/Test/TestOps.td b/mlir/test/lib/Dialect/Test/TestOps.td
index d9bbb3261febc..5564264ed8b0b 100644
--- a/mlir/test/lib/Dialect/Test/TestOps.td
+++ b/mlir/test/lib/Dialect/Test/TestOps.td
@@ -552,10 +552,9 @@ def OneRegionWithOperandsOp : TEST_Op<"one_region_with_operands_op", []> {
 
 def IsolatedOneRegionOp : TEST_Op<"isolated_one_region_op", [IsolatedFromAbove]> {
   let arguments = (ins Variadic<AnyType>:$operands);
-  let results = (outs Variadic<AnyType>:$results);
   let regions = (region AnyRegion:$my_region);
   let assemblyFormat = [{
-    attr-dict-with-keyword $operands $my_region `:` type($operands) `->` type($results)
+    attr-dict-with-keyword $operands $my_region `:` type($operands)
   }];
 }
 
diff --git a/mlir/test/lib/Transforms/TestMakeIsolatedFromAbove.cpp b/mlir/test/lib/Transforms/TestMakeIsolatedFromAbove.cpp
index f7bde79274e91..c1fb70605ab46 100644
--- a/mlir/test/lib/Transforms/TestMakeIsolatedFromAbove.cpp
+++ b/mlir/test/lib/Transforms/TestMakeIsolatedFromAbove.cpp
@@ -27,8 +27,8 @@ makeIsolatedFromAboveImpl(RewriterBase &rewriter,
       makeRegionIsolatedFromAbove(rewriter, region, callBack);
   SmallVector<Value> operands = regionOp.getOperands();
   operands.append(capturedValues);
-  auto isolatedRegionOp = test::IsolatedOneRegionOp::create(
-      rewriter, regionOp.getLoc(), TypeRange(), operands);
+  auto isolatedRegionOp =
+      test::IsolatedOneRegionOp::create(rewriter, regionOp.getLoc(), operands);
   rewriter.inlineRegionBefore(region, isolatedRegionOp.getRegion(),
                               isolatedRegionOp.getRegion().begin());
   rewriter.eraseOp(regionOp);