[Mlir-commits] [mlir] [MLIR] Change getBackwardSlice to return a logicalresult rather than … (PR #140961)
llvmlistbot at llvm.org
llvmlistbot at llvm.org
Wed May 21 14:08:25 PDT 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-mlir-affine
@llvm/pr-subscribers-mlir-linalg
Author: William Moses (wsmoses)
<details>
<summary>Changes</summary>
…crash
---
Full diff: https://github.com/llvm/llvm-project/pull/140961.diff
10 Files Affected:
- (modified) mlir/include/mlir/Analysis/SliceAnalysis.h (+6-4)
- (modified) mlir/include/mlir/Query/Matcher/SliceMatchers.h (+2-1)
- (modified) mlir/lib/Analysis/SliceAnalysis.cpp (+32-20)
- (modified) mlir/lib/Conversion/VectorToGPU/VectorToGPU.cpp (+3-1)
- (modified) mlir/lib/Dialect/Linalg/Transforms/HoistPadding.cpp (+5-2)
- (modified) mlir/lib/Dialect/NVGPU/TransformOps/NVGPUTransformOps.cpp (+4-2)
- (modified) mlir/lib/Dialect/SCF/Transforms/TileUsingInterface.cpp (+2-1)
- (modified) mlir/lib/Transforms/Utils/RegionUtils.cpp (+4-2)
- (modified) mlir/test/lib/Dialect/Affine/TestVectorizationUtils.cpp (+2-1)
- (modified) mlir/test/lib/IR/TestSlicing.cpp (+2-1)
``````````diff
diff --git a/mlir/include/mlir/Analysis/SliceAnalysis.h b/mlir/include/mlir/Analysis/SliceAnalysis.h
index 3b731e8bb1c22..192b096fd31f6 100644
--- a/mlir/include/mlir/Analysis/SliceAnalysis.h
+++ b/mlir/include/mlir/Analysis/SliceAnalysis.h
@@ -138,13 +138,15 @@ void getForwardSlice(Value root, SetVector<Operation *> *forwardSlice,
/// Assuming all local orders match the numbering order:
/// {1, 2, 5, 3, 4, 6}
///
-void getBackwardSlice(Operation *op, SetVector<Operation *> *backwardSlice,
- const BackwardSliceOptions &options = {});
+LogicalResult 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`.
-void getBackwardSlice(Value root, SetVector<Operation *> *backwardSlice,
- const BackwardSliceOptions &options = {});
+LogicalResult 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
diff --git a/mlir/include/mlir/Query/Matcher/SliceMatchers.h b/mlir/include/mlir/Query/Matcher/SliceMatchers.h
index 1b0e4c32dbe94..6b7d10ccaed64 100644
--- a/mlir/include/mlir/Query/Matcher/SliceMatchers.h
+++ b/mlir/include/mlir/Query/Matcher/SliceMatchers.h
@@ -112,7 +112,8 @@ bool BackwardSliceMatcher<Matcher>::matches(
}
return true;
};
- getBackwardSlice(rootOp, &backwardSlice, options);
+ auto result = getBackwardSlice(rootOp, &backwardSlice, options);
+ assert(result.succeeded());
return options.inclusive ? backwardSlice.size() > 1
: backwardSlice.size() >= 1;
}
diff --git a/mlir/lib/Analysis/SliceAnalysis.cpp b/mlir/lib/Analysis/SliceAnalysis.cpp
index 5aebb19e3a86e..5992aceba82d3 100644
--- a/mlir/lib/Analysis/SliceAnalysis.cpp
+++ b/mlir/lib/Analysis/SliceAnalysis.cpp
@@ -80,22 +80,25 @@ void mlir::getForwardSlice(Value root, SetVector<Operation *> *forwardSlice,
forwardSlice->insert(v.rbegin(), v.rend());
}
-static void getBackwardSliceImpl(Operation *op,
- SetVector<Operation *> *backwardSlice,
- const BackwardSliceOptions &options) {
+static LogicalResult getBackwardSliceImpl(Operation *op,
+ SetVector<Operation *> *backwardSlice,
+ const BackwardSliceOptions &options) {
if (!op || op->hasTrait<OpTrait::IsIsolatedFromAbove>())
- return;
+ return success();
// 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;
+ return success();
+
+ bool succeeded = true;
auto processValue = [&](Value value) {
if (auto *definingOp = value.getDefiningOp()) {
if (backwardSlice->count(definingOp) == 0)
- getBackwardSliceImpl(definingOp, backwardSlice, options);
+ succeeded &= getBackwardSliceImpl(definingOp, backwardSlice, options)
+ .succeeded();
} else if (auto blockArg = dyn_cast<BlockArgument>(value)) {
if (options.omitBlockArguments)
return;
@@ -106,9 +109,13 @@ static void 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) {
- assert(parentOp->getNumRegions() == 1 &&
- llvm::hasSingleElement(parentOp->getRegion(0).getBlocks()));
- getBackwardSliceImpl(parentOp, backwardSlice, options);
+ if (parentOp->getNumRegions() == 1 &&
+ llvm::hasSingleElement(parentOp->getRegion(0).getBlocks())) {
+ succeeded &= getBackwardSliceImpl(parentOp, backwardSlice, options)
+ .succeeded();
+ } else {
+ succeeded = false;
+ }
}
} else {
llvm_unreachable("No definingOp and not a block argument.");
@@ -133,28 +140,30 @@ static void getBackwardSliceImpl(Operation *op,
llvm::for_each(op->getOperands(), processValue);
backwardSlice->insert(op);
+ return succeess(succeeded);
}
-void mlir::getBackwardSlice(Operation *op,
- SetVector<Operation *> *backwardSlice,
- const BackwardSliceOptions &options) {
- getBackwardSliceImpl(op, backwardSlice, options);
+LogicalResult result
+mlir::getBackwardSlice(Operation *op, SetVector<Operation *> *backwardSlice,
+ const BackwardSliceOptions &options) {
+ LogicalResult result = 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;
}
-void mlir::getBackwardSlice(Value root, SetVector<Operation *> *backwardSlice,
- const BackwardSliceOptions &options) {
+LogicalResult mlir::getBackwardSlice(Value root,
+ SetVector<Operation *> *backwardSlice,
+ const BackwardSliceOptions &options) {
if (Operation *definingOp = root.getDefiningOp()) {
- getBackwardSlice(definingOp, backwardSlice, options);
- return;
+ return getBackwardSlice(definingOp, backwardSlice, options);
}
Operation *bbAargOwner = cast<BlockArgument>(root).getOwner()->getParentOp();
- getBackwardSlice(bbAargOwner, backwardSlice, options);
+ return getBackwardSlice(bbAargOwner, backwardSlice, options);
}
SetVector<Operation *>
@@ -170,7 +179,9 @@ mlir::getSlice(Operation *op, const BackwardSliceOptions &backwardSliceOptions,
auto *currentOp = (slice)[currentIndex];
// Compute and insert the backwardSlice starting from currentOp.
backwardSlice.clear();
- getBackwardSlice(currentOp, &backwardSlice, backwardSliceOptions);
+ auto result =
+ getBackwardSlice(currentOp, &backwardSlice, backwardSliceOptions);
+ assert(result.succeeded());
slice.insert_range(backwardSlice);
// Compute and insert the forwardSlice starting from currentOp.
@@ -193,7 +204,8 @@ static bool dependsOnCarriedVals(Value value,
sliceOptions.filter = [&](Operation *op) {
return !ancestorOp->isAncestor(op);
};
- getBackwardSlice(value, &slice, sliceOptions);
+ auto result = getBackwardSlice(value, &slice, sliceOptions);
+ assert(result.succeeded());
// Check that none of the operands of the operations in the backward slice are
// loop iteration arguments, and neither is the value itself.
diff --git a/mlir/lib/Conversion/VectorToGPU/VectorToGPU.cpp b/mlir/lib/Conversion/VectorToGPU/VectorToGPU.cpp
index 8b16da387457d..22df878cfe18d 100644
--- a/mlir/lib/Conversion/VectorToGPU/VectorToGPU.cpp
+++ b/mlir/lib/Conversion/VectorToGPU/VectorToGPU.cpp
@@ -317,7 +317,9 @@ getSliceContract(Operation *op,
auto *currentOp = (slice)[currentIndex];
// Compute and insert the backwardSlice starting from currentOp.
backwardSlice.clear();
- getBackwardSlice(currentOp, &backwardSlice, backwardSliceOptions);
+ auto result =
+ getBackwardSlice(currentOp, &backwardSlice, backwardSliceOptions);
+ assert(result.succeeded());
slice.insert_range(backwardSlice);
// Compute and insert the forwardSlice starting from currentOp.
diff --git a/mlir/lib/Dialect/Linalg/Transforms/HoistPadding.cpp b/mlir/lib/Dialect/Linalg/Transforms/HoistPadding.cpp
index d33a17af63459..ee0d44be60b59 100644
--- a/mlir/lib/Dialect/Linalg/Transforms/HoistPadding.cpp
+++ b/mlir/lib/Dialect/Linalg/Transforms/HoistPadding.cpp
@@ -124,10 +124,13 @@ static void computeBackwardSlice(tensor::PadOp padOp,
getUsedValuesDefinedAbove(padOp.getRegion(), padOp.getRegion(),
valuesDefinedAbove);
for (Value v : valuesDefinedAbove) {
- getBackwardSlice(v, &backwardSlice, sliceOptions);
+ auto result = getBackwardSlice(v, &backwardSlice, sliceOptions);
+ assert(result.succeeded());
}
// Then, add the backward slice from padOp itself.
- getBackwardSlice(padOp.getOperation(), &backwardSlice, sliceOptions);
+ auto result =
+ getBackwardSlice(padOp.getOperation(), &backwardSlice, sliceOptions);
+ assert(result.succeeded());
}
//===----------------------------------------------------------------------===//
diff --git a/mlir/lib/Dialect/NVGPU/TransformOps/NVGPUTransformOps.cpp b/mlir/lib/Dialect/NVGPU/TransformOps/NVGPUTransformOps.cpp
index 75dbe0becf80d..c5b8117283b25 100644
--- a/mlir/lib/Dialect/NVGPU/TransformOps/NVGPUTransformOps.cpp
+++ b/mlir/lib/Dialect/NVGPU/TransformOps/NVGPUTransformOps.cpp
@@ -290,8 +290,10 @@ static void getPipelineStages(
});
options.inclusive = true;
for (Operation &op : forOp.getBody()->getOperations()) {
- if (stage0Ops.contains(&op))
- getBackwardSlice(&op, &dependencies, options);
+ if (stage0Ops.contains(&op)) {
+ auto result = getBackwardSlice(&op, &dependencies, options);
+ assert(result.succeeded());
+ }
}
for (Operation &op : forOp.getBody()->getOperations()) {
diff --git a/mlir/lib/Dialect/SCF/Transforms/TileUsingInterface.cpp b/mlir/lib/Dialect/SCF/Transforms/TileUsingInterface.cpp
index 719e2c6fa459e..273fdbfbc4ffe 100644
--- a/mlir/lib/Dialect/SCF/Transforms/TileUsingInterface.cpp
+++ b/mlir/lib/Dialect/SCF/Transforms/TileUsingInterface.cpp
@@ -1772,7 +1772,8 @@ checkAssumptionForLoop(Operation *loopOp, Operation *consumerOp,
};
llvm::SetVector<Operation *> slice;
for (auto operand : consumerOp->getOperands()) {
- getBackwardSlice(operand, &slice, options);
+ auto result = getBackwardSlice(operand, &slice, options);
+ assert(result.succeeded());
}
if (!slice.empty()) {
diff --git a/mlir/lib/Transforms/Utils/RegionUtils.cpp b/mlir/lib/Transforms/Utils/RegionUtils.cpp
index 4985d718c1780..70ba5e321db6d 100644
--- a/mlir/lib/Transforms/Utils/RegionUtils.cpp
+++ b/mlir/lib/Transforms/Utils/RegionUtils.cpp
@@ -1094,7 +1094,8 @@ LogicalResult mlir::moveOperationDependencies(RewriterBase &rewriter,
return !dominance.properlyDominates(sliceBoundaryOp, insertionPoint);
};
llvm::SetVector<Operation *> slice;
- getBackwardSlice(op, &slice, options);
+ auto result = getBackwardSlice(op, &slice, options);
+ assert(result.succeeded());
// If the slice contains `insertionPoint` cannot move the dependencies.
if (slice.contains(insertionPoint)) {
@@ -1159,7 +1160,8 @@ LogicalResult mlir::moveValueDefinitions(RewriterBase &rewriter,
};
llvm::SetVector<Operation *> slice;
for (auto value : prunedValues) {
- getBackwardSlice(value, &slice, options);
+ auto result = getBackwardSlice(value, &slice, options);
+ assert(result.succeeded());
}
// If the slice contains `insertionPoint` cannot move the dependencies.
diff --git a/mlir/test/lib/Dialect/Affine/TestVectorizationUtils.cpp b/mlir/test/lib/Dialect/Affine/TestVectorizationUtils.cpp
index f26058f30ad7b..041cc25b23cbd 100644
--- a/mlir/test/lib/Dialect/Affine/TestVectorizationUtils.cpp
+++ b/mlir/test/lib/Dialect/Affine/TestVectorizationUtils.cpp
@@ -154,7 +154,8 @@ void VectorizerTestPass::testBackwardSlicing(llvm::raw_ostream &outs) {
patternTestSlicingOps().match(f, &matches);
for (auto m : matches) {
SetVector<Operation *> backwardSlice;
- getBackwardSlice(m.getMatchedOperation(), &backwardSlice);
+ auto result = getBackwardSlice(m.getMatchedOperation(), &backwardSlice);
+ assert(result.succeeded());
outs << "\nmatched: " << *m.getMatchedOperation()
<< " backward static slice: ";
for (auto *op : backwardSlice)
diff --git a/mlir/test/lib/IR/TestSlicing.cpp b/mlir/test/lib/IR/TestSlicing.cpp
index e99d5976d6d9d..53ad9b5819986 100644
--- a/mlir/test/lib/IR/TestSlicing.cpp
+++ b/mlir/test/lib/IR/TestSlicing.cpp
@@ -41,7 +41,8 @@ static LogicalResult createBackwardSliceFunction(Operation *op,
options.omitBlockArguments = omitBlockArguments;
// TODO: Make this default.
options.omitUsesFromAbove = false;
- getBackwardSlice(op, &slice, options);
+ auto result = getBackwardSlice(op, &slice, options);
+ assert(result.succeeded());
for (Operation *slicedOp : slice)
builder.clone(*slicedOp, mapper);
builder.create<func::ReturnOp>(loc);
``````````
</details>
https://github.com/llvm/llvm-project/pull/140961
More information about the Mlir-commits
mailing list