Revert "[MLIR] Change getBackwardSlice to return a logicalresult rather than crash (llvm#140961)"

This reverts commit 6a8dde0.

Author: hanhanW

mlir/include/mlir/Analysis/SliceAnalysis.h

@@ -138,17 +138,13 @@ void getForwardSlice(Value root, SetVector<Operation *> *forwardSlice,
 /// Assuming all local orders match the numbering order:
 ///    {1, 2, 5, 3, 4, 6}
 ///
-/// This function returns whether the backwards slice was able to be
-/// successfully computed, and failure if it was unable to determine the slice.
-LogicalResult getBackwardSlice(Operation *op,
-                               SetVector<Operation *> *backwardSlice,
-                               const BackwardSliceOptions &options = {});
+void getBackwardSlice(Operation *op, SetVector<Operation *> *backwardSlice,
+                      const BackwardSliceOptions &options = {});
 
 /// Value-rooted version of `getBackwardSlice`. Return the union of all backward
 /// slices for the op defining or owning the value `root`.
-LogicalResult getBackwardSlice(Value root,
-                               SetVector<Operation *> *backwardSlice,
-                               const BackwardSliceOptions &options = {});
+void getBackwardSlice(Value root, SetVector<Operation *> *backwardSlice,
+                      const BackwardSliceOptions &options = {});
 
 /// Iteratively computes backward slices and forward slices until
 /// a fixed point is reached. Returns an `SetVector<Operation *>` which

mlir/include/mlir/Query/Matcher/SliceMatchers.h

@@ -112,8 +112,7 @@ bool BackwardSliceMatcher<Matcher>::matches(
     }
     return true;
   };
-  LogicalResult result = getBackwardSlice(rootOp, &backwardSlice, options);
-  assert(result.succeeded() && "expected backward slice to succeed");
+  getBackwardSlice(rootOp, &backwardSlice, options);
   return options.inclusive ? backwardSlice.size() > 1
                            : backwardSlice.size() >= 1;
 }

mlir/lib/Analysis/SliceAnalysis.cpp

@@ -80,43 +80,41 @@ void mlir::getForwardSlice(Value root, SetVector<Operation *> *forwardSlice,
   forwardSlice->insert(v.rbegin(), v.rend());
 }
 
-static LogicalResult getBackwardSliceImpl(Operation *op,
-                                          SetVector<Operation *> *backwardSlice,
-                                          const BackwardSliceOptions &options) {
+static void getBackwardSliceImpl(Operation *op,
+                                 SetVector<Operation *> *backwardSlice,
+                                 const BackwardSliceOptions &options) {
   if (!op || op->hasTrait<OpTrait::IsIsolatedFromAbove>())
-    return success();
+    return;
 
   // Evaluate whether we should keep this def.
   // This is useful in particular to implement scoping; i.e. return the
   // transitive backwardSlice in the current scope.
   if (options.filter && !options.filter(op))
-    return success();
+    return;
 
   auto processValue = [&](Value value) {
     if (auto *definingOp = value.getDefiningOp()) {
       if (backwardSlice->count(definingOp) == 0)
-        return getBackwardSliceImpl(definingOp, backwardSlice, options);
+        getBackwardSliceImpl(definingOp, backwardSlice, options);
     } else if (auto blockArg = dyn_cast<BlockArgument>(value)) {
       if (options.omitBlockArguments)
-        return success();
+        return;
 
       Block *block = blockArg.getOwner();
       Operation *parentOp = block->getParentOp();
       // TODO: determine whether we want to recurse backward into the other
       // 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->getNumRegions() == 1 &&
-            llvm::hasSingleElement(parentOp->getRegion(0).getBlocks())) {
-          return getBackwardSliceImpl(parentOp, backwardSlice, options);
-        }
+        assert(parentOp->getNumRegions() == 1 &&
+               llvm::hasSingleElement(parentOp->getRegion(0).getBlocks()));
+        getBackwardSliceImpl(parentOp, backwardSlice, options);
       }
+    } else {
+      llvm_unreachable("No definingOp and not a block argument.");
     }
-    return failure();
   };
 
-  bool succeeded = true;
-
   if (!options.omitUsesFromAbove) {
     llvm::for_each(op->getRegions(), [&](Region &region) {
       // Walk this region recursively to collect the regions that descend from
@@ -127,41 +125,36 @@ static LogicalResult getBackwardSliceImpl(Operation *op,
       region.walk([&](Operation *op) {
         for (OpOperand &operand : op->getOpOperands()) {
           if (!descendents.contains(operand.get().getParentRegion()))
-            if (!processValue(operand.get()).succeeded()) {
-              return WalkResult::interrupt();
-            }
+            processValue(operand.get());
         }
-        return WalkResult::advance();
       });
     });
   }
   llvm::for_each(op->getOperands(), processValue);
 
   backwardSlice->insert(op);
-  return success(succeeded);
 }
 
-LogicalResult mlir::getBackwardSlice(Operation *op,
-                                     SetVector<Operation *> *backwardSlice,
-                                     const BackwardSliceOptions &options) {
-  LogicalResult result = getBackwardSliceImpl(op, backwardSlice, options);
+void mlir::getBackwardSlice(Operation *op,
+                            SetVector<Operation *> *backwardSlice,
+                            const BackwardSliceOptions &options) {
+  getBackwardSliceImpl(op, backwardSlice, options);
 
   if (!options.inclusive) {
     // Don't insert the top level operation, we just queried on it and don't
     // want it in the results.
     backwardSlice->remove(op);
   }
-  return result;
 }
 
-LogicalResult mlir::getBackwardSlice(Value root,
-                                     SetVector<Operation *> *backwardSlice,
-                                     const BackwardSliceOptions &options) {
+void mlir::getBackwardSlice(Value root, SetVector<Operation *> *backwardSlice,
+                            const BackwardSliceOptions &options) {
   if (Operation *definingOp = root.getDefiningOp()) {
-    return getBackwardSlice(definingOp, backwardSlice, options);
+    getBackwardSlice(definingOp, backwardSlice, options);
+    return;
   }
   Operation *bbAargOwner = cast<BlockArgument>(root).getOwner()->getParentOp();
-  return getBackwardSlice(bbAargOwner, backwardSlice, options);
+  getBackwardSlice(bbAargOwner, backwardSlice, options);
 }
 
 SetVector<Operation *>
@@ -177,9 +170,7 @@ mlir::getSlice(Operation *op, const BackwardSliceOptions &backwardSliceOptions,
     auto *currentOp = (slice)[currentIndex];
     // Compute and insert the backwardSlice starting from currentOp.
     backwardSlice.clear();
-    LogicalResult result =
-        getBackwardSlice(currentOp, &backwardSlice, backwardSliceOptions);
-    assert(result.succeeded());
+    getBackwardSlice(currentOp, &backwardSlice, backwardSliceOptions);
     slice.insert_range(backwardSlice);
 
     // Compute and insert the forwardSlice starting from currentOp.
@@ -202,8 +193,7 @@ static bool dependsOnCarriedVals(Value value,
   sliceOptions.filter = [&](Operation *op) {
     return !ancestorOp->isAncestor(op);
   };
-  LogicalResult result = getBackwardSlice(value, &slice, sliceOptions);
-  assert(result.succeeded());
+  getBackwardSlice(value, &slice, sliceOptions);
 
   // Check that none of the operands of the operations in the backward slice are
   // loop iteration arguments, and neither is the value itself.

mlir/lib/Conversion/VectorToGPU/VectorToGPU.cpp

@@ -317,9 +317,7 @@ getSliceContract(Operation *op,
     auto *currentOp = (slice)[currentIndex];
     // Compute and insert the backwardSlice starting from currentOp.
     backwardSlice.clear();
-    LogicalResult result =
-        getBackwardSlice(currentOp, &backwardSlice, backwardSliceOptions);
-    assert(result.succeeded() && "expected a backward slice");
+    getBackwardSlice(currentOp, &backwardSlice, backwardSliceOptions);
     slice.insert_range(backwardSlice);
 
     // Compute and insert the forwardSlice starting from currentOp.

mlir/lib/Dialect/Linalg/Transforms/HoistPadding.cpp

@@ -124,13 +124,10 @@ static void computeBackwardSlice(tensor::PadOp padOp,
   getUsedValuesDefinedAbove(padOp.getRegion(), padOp.getRegion(),
                             valuesDefinedAbove);
   for (Value v : valuesDefinedAbove) {
-    LogicalResult result = getBackwardSlice(v, &backwardSlice, sliceOptions);
-    assert(result.succeeded() && "expected a backward slice");
+    getBackwardSlice(v, &backwardSlice, sliceOptions);
   }
   // Then, add the backward slice from padOp itself.
-  LogicalResult result =
-      getBackwardSlice(padOp.getOperation(), &backwardSlice, sliceOptions);
-  assert(result.succeeded() && "expected a backward slice");
+  getBackwardSlice(padOp.getOperation(), &backwardSlice, sliceOptions);
 }
 
 //===----------------------------------------------------------------------===//

mlir/lib/Dialect/NVGPU/TransformOps/NVGPUTransformOps.cpp

@@ -290,10 +290,8 @@ static void getPipelineStages(
   });
   options.inclusive = true;
   for (Operation &op : forOp.getBody()->getOperations()) {
-    if (stage0Ops.contains(&op)) {
-      LogicalResult result = getBackwardSlice(&op, &dependencies, options);
-      assert(result.succeeded() && "expected a backward slice");
-    }
+    if (stage0Ops.contains(&op))
+      getBackwardSlice(&op, &dependencies, options);
   }
 
   for (Operation &op : forOp.getBody()->getOperations()) {

mlir/lib/Dialect/SCF/Transforms/TileUsingInterface.cpp

@@ -1772,8 +1772,7 @@ checkAssumptionForLoop(Operation *loopOp, Operation *consumerOp,
   };
   llvm::SetVector<Operation *> slice;
   for (auto operand : consumerOp->getOperands()) {
-    LogicalResult result = getBackwardSlice(operand, &slice, options);
-    assert(result.succeeded() && "expected a backward slice");
+    getBackwardSlice(operand, &slice, options);
   }
 
   if (!slice.empty()) {

mlir/lib/Transforms/Utils/RegionUtils.cpp

@@ -1094,8 +1094,7 @@ LogicalResult mlir::moveOperationDependencies(RewriterBase &rewriter,
     return !dominance.properlyDominates(sliceBoundaryOp, insertionPoint);
   };
   llvm::SetVector<Operation *> slice;
-  LogicalResult result = getBackwardSlice(op, &slice, options);
-  assert(result.succeeded() && "expected a backward slice");
+  getBackwardSlice(op, &slice, options);
 
   // If the slice contains `insertionPoint` cannot move the dependencies.
   if (slice.contains(insertionPoint)) {
@@ -1160,8 +1159,7 @@ LogicalResult mlir::moveValueDefinitions(RewriterBase &rewriter,
   };
   llvm::SetVector<Operation *> slice;
   for (auto value : prunedValues) {
-    LogicalResult result = getBackwardSlice(value, &slice, options);
-    assert(result.succeeded() && "expected a backward slice");
+    getBackwardSlice(value, &slice, options);
   }
 
   // If the slice contains `insertionPoint` cannot move the dependencies.

mlir/test/lib/Dialect/Affine/TestVectorizationUtils.cpp

@@ -154,9 +154,7 @@ void VectorizerTestPass::testBackwardSlicing(llvm::raw_ostream &outs) {
   patternTestSlicingOps().match(f, &matches);
   for (auto m : matches) {
     SetVector<Operation *> backwardSlice;
-    LogicalResult result =
-        getBackwardSlice(m.getMatchedOperation(), &backwardSlice);
-    assert(result.succeeded() && "expected a backward slice");
+    getBackwardSlice(m.getMatchedOperation(), &backwardSlice);
     outs << "\nmatched: " << *m.getMatchedOperation()
          << " backward static slice: ";
     for (auto *op : backwardSlice)

mlir/test/lib/IR/TestSlicing.cpp

@@ -41,8 +41,7 @@ static LogicalResult createBackwardSliceFunction(Operation *op,
   options.omitBlockArguments = omitBlockArguments;
   // TODO: Make this default.
   options.omitUsesFromAbove = false;
-  LogicalResult result = getBackwardSlice(op, &slice, options);
-  assert(result.succeeded() && "expected a backward slice");
+  getBackwardSlice(op, &slice, options);
   for (Operation *slicedOp : slice)
     builder.clone(*slicedOp, mapper);
   builder.create<func::ReturnOp>(loc);