diff --git a/mlir/include/mlir/Analysis/SliceAnalysis.h b/mlir/include/mlir/Analysis/SliceAnalysis.h index 18349d071bb2e..89f570a7c7115 100644 --- a/mlir/include/mlir/Analysis/SliceAnalysis.h +++ b/mlir/include/mlir/Analysis/SliceAnalysis.h @@ -140,17 +140,13 @@ void getForwardSlice(Value root, SetVector *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 *backwardSlice, - const BackwardSliceOptions &options = {}); +void getBackwardSlice(Operation *op, SetVector *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 *backwardSlice, - const BackwardSliceOptions &options = {}); +void getBackwardSlice(Value root, SetVector *backwardSlice, + const BackwardSliceOptions &options = {}); /// Iteratively computes backward slices and forward slices until /// a fixed point is reached. Returns an `SetVector` which diff --git a/mlir/include/mlir/Query/Matcher/SliceMatchers.h b/mlir/include/mlir/Query/Matcher/SliceMatchers.h index 43b699817927c..58ca187eec360 100644 --- a/mlir/include/mlir/Query/Matcher/SliceMatchers.h +++ b/mlir/include/mlir/Query/Matcher/SliceMatchers.h @@ -113,9 +113,7 @@ bool BackwardSliceMatcher::matches( } return true; }; - LogicalResult result = getBackwardSlice(rootOp, &backwardSlice, options); - assert(result.succeeded() && "expected backward slice to succeed"); - (void)result; + getBackwardSlice(rootOp, &backwardSlice, options); return options.inclusive ? backwardSlice.size() > 1 : backwardSlice.size() >= 1; } @@ -143,9 +141,7 @@ class PredicateBackwardSliceMatcher { options.filter = [&](Operation *subOp) { return !filterMatcher.match(subOp); }; - LogicalResult result = getBackwardSlice(rootOp, &backwardSlice, options); - assert(result.succeeded() && "expected backward slice to succeed"); - (void)result; + getBackwardSlice(rootOp, &backwardSlice, options); return options.inclusive ? backwardSlice.size() > 1 : backwardSlice.size() >= 1; } diff --git a/mlir/lib/Analysis/SliceAnalysis.cpp b/mlir/lib/Analysis/SliceAnalysis.cpp index 12dff19ed31d3..ac9c96e5dd855 100644 --- a/mlir/lib/Analysis/SliceAnalysis.cpp +++ b/mlir/lib/Analysis/SliceAnalysis.cpp @@ -105,52 +105,47 @@ void mlir::getForwardSlice(Value root, SetVector *forwardSlice, forwardSlice->insert(v.rbegin(), v.rend()); } -static LogicalResult getBackwardSliceImpl(Operation *op, - DenseSet &visited, - SetVector *backwardSlice, - const BackwardSliceOptions &options) { +static void getBackwardSliceImpl(Operation *op, DenseSet &visited, + SetVector *backwardSlice, + const BackwardSliceOptions &options) { if (!op) - 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 && - visited.insert(definingOp).second) - return getBackwardSliceImpl(definingOp, visited, backwardSlice, - options); + visited.insert(definingOp).second) { + getBackwardSliceImpl(definingOp, visited, backwardSlice, options); + return; + } visited.erase(definingOp); - } else if (auto blockArg = dyn_cast(value)) { - if (options.omitBlockArguments) - return success(); - - 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->hasTrait() && - parentOp->getNumRegions() == 1 && - parentOp->getRegion(0).hasOneBlock()) { - return getBackwardSliceImpl(parentOp, visited, backwardSlice, - options); - } + return; + } + auto blockArg = cast(value); + if (options.omitBlockArguments) + 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->hasTrait() && + parentOp->getNumRegions() == 1 && + parentOp->getRegion(0).hasOneBlock()) { + getBackwardSliceImpl(parentOp, visited, backwardSlice, options); } - } else { - return failure(); } - return success(); }; - bool succeeded = true; - if (!options.omitUsesFromAbove && !op->hasTrait()) { llvm::for_each(op->getRegions(), [&](Region ®ion) { @@ -162,44 +157,38 @@ 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 *backwardSlice, - const BackwardSliceOptions &options) { +void mlir::getBackwardSlice(Operation *op, + SetVector *backwardSlice, + const BackwardSliceOptions &options) { DenseSet visited; visited.insert(op); - LogicalResult result = - getBackwardSliceImpl(op, visited, backwardSlice, options); + getBackwardSliceImpl(op, visited, 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 *backwardSlice, - const BackwardSliceOptions &options) { +void mlir::getBackwardSlice(Value root, SetVector *backwardSlice, + const BackwardSliceOptions &options) { if (Operation *definingOp = root.getDefiningOp()) { - return getBackwardSlice(definingOp, backwardSlice, options); + getBackwardSlice(definingOp, backwardSlice, options); + return; } - Operation *bbAargOwner = cast(root).getOwner()->getParentOp(); - return getBackwardSlice(bbAargOwner, backwardSlice, options); + Operation *bbArgOwner = cast(root).getOwner()->getParentOp(); + getBackwardSlice(bbArgOwner, backwardSlice, options); } SetVector @@ -215,10 +204,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()); - (void)result; + getBackwardSlice(currentOp, &backwardSlice, backwardSliceOptions); slice.insert_range(backwardSlice); // Compute and insert the forwardSlice starting from currentOp. @@ -241,9 +227,7 @@ static bool dependsOnCarriedVals(Value value, sliceOptions.filter = [&](Operation *op) { return !ancestorOp->isAncestor(op); }; - LogicalResult result = getBackwardSlice(value, &slice, sliceOptions); - assert(result.succeeded()); - (void)result; + 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. diff --git a/mlir/lib/Conversion/VectorToGPU/VectorToGPU.cpp b/mlir/lib/Conversion/VectorToGPU/VectorToGPU.cpp index 98434357f826f..417b6bb3030d6 100644 --- a/mlir/lib/Conversion/VectorToGPU/VectorToGPU.cpp +++ b/mlir/lib/Conversion/VectorToGPU/VectorToGPU.cpp @@ -312,10 +312,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"); - (void)result; + getBackwardSlice(currentOp, &backwardSlice, backwardSliceOptions); 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 9436f1c6cd9b0..5e28306e00935 100644 --- a/mlir/lib/Dialect/Linalg/Transforms/HoistPadding.cpp +++ b/mlir/lib/Dialect/Linalg/Transforms/HoistPadding.cpp @@ -122,16 +122,10 @@ static void computeBackwardSlice(tensor::PadOp padOp, SetVector valuesDefinedAbove; getUsedValuesDefinedAbove(padOp.getRegion(), padOp.getRegion(), valuesDefinedAbove); - for (Value v : valuesDefinedAbove) { - LogicalResult result = getBackwardSlice(v, &backwardSlice, sliceOptions); - assert(result.succeeded() && "expected a backward slice"); - (void)result; - } + for (Value v : valuesDefinedAbove) + 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"); - (void)result; + getBackwardSlice(padOp.getOperation(), &backwardSlice, sliceOptions); } //===----------------------------------------------------------------------===// diff --git a/mlir/lib/Dialect/NVGPU/TransformOps/NVGPUTransformOps.cpp b/mlir/lib/Dialect/NVGPU/TransformOps/NVGPUTransformOps.cpp index 2a857eddbb932..efba956990697 100644 --- a/mlir/lib/Dialect/NVGPU/TransformOps/NVGPUTransformOps.cpp +++ b/mlir/lib/Dialect/NVGPU/TransformOps/NVGPUTransformOps.cpp @@ -281,13 +281,9 @@ static void getPipelineStages( return visited->getBlock() == forOp.getBody(); }); 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"); - (void)result; - } - } + for (Operation &op : forOp.getBody()->getOperations()) + if (stage0Ops.contains(&op)) + getBackwardSlice(&op, &dependencies, options); for (Operation &op : forOp.getBody()->getOperations()) { if (!dependencies.contains(&op) && !isa(op)) diff --git a/mlir/lib/Dialect/SCF/Transforms/TileUsingInterface.cpp b/mlir/lib/Dialect/SCF/Transforms/TileUsingInterface.cpp index 29b770fb4b279..44bdfc28ef8ee 100644 --- a/mlir/lib/Dialect/SCF/Transforms/TileUsingInterface.cpp +++ b/mlir/lib/Dialect/SCF/Transforms/TileUsingInterface.cpp @@ -1970,11 +1970,8 @@ checkAssumptionForLoop(Operation *loopOp, Operation *consumerOp, return !dominanceInfo.properlyDominates(op, *firstUserOfLoop); }; llvm::SetVector slice; - for (auto operand : consumerOp->getOperands()) { - LogicalResult result = getBackwardSlice(operand, &slice, options); - assert(result.succeeded() && "expected a backward slice"); - (void)result; - } + for (auto operand : consumerOp->getOperands()) + getBackwardSlice(operand, &slice, options); if (!slice.empty()) { // If consumerOp has one producer, which is also the user of loopOp. diff --git a/mlir/lib/Transforms/Utils/RegionUtils.cpp b/mlir/lib/Transforms/Utils/RegionUtils.cpp index 31ae1d1895b81..d00e137124a4e 100644 --- a/mlir/lib/Transforms/Utils/RegionUtils.cpp +++ b/mlir/lib/Transforms/Utils/RegionUtils.cpp @@ -1116,9 +1116,7 @@ LogicalResult mlir::moveOperationDependencies(RewriterBase &rewriter, return !dominance.properlyDominates(sliceBoundaryOp, insertionPoint); }; llvm::SetVector slice; - LogicalResult result = getBackwardSlice(op, &slice, options); - assert(result.succeeded() && "expected a backward slice"); - (void)result; + getBackwardSlice(op, &slice, options); // If the slice contains `insertionPoint` cannot move the dependencies. if (slice.contains(insertionPoint)) { @@ -1182,11 +1180,8 @@ LogicalResult mlir::moveValueDefinitions(RewriterBase &rewriter, return !dominance.properlyDominates(sliceBoundaryOp, insertionPoint); }; llvm::SetVector slice; - for (auto value : prunedValues) { - LogicalResult result = getBackwardSlice(value, &slice, options); - assert(result.succeeded() && "expected a backward slice"); - (void)result; - } + for (auto value : prunedValues) + getBackwardSlice(value, &slice, options); // If the slice contains `insertionPoint` cannot move the dependencies. if (slice.contains(insertionPoint)) { diff --git a/mlir/test/lib/Dialect/Affine/TestVectorizationUtils.cpp b/mlir/test/lib/Dialect/Affine/TestVectorizationUtils.cpp index 2c93991386675..f26058f30ad7b 100644 --- a/mlir/test/lib/Dialect/Affine/TestVectorizationUtils.cpp +++ b/mlir/test/lib/Dialect/Affine/TestVectorizationUtils.cpp @@ -154,10 +154,7 @@ void VectorizerTestPass::testBackwardSlicing(llvm::raw_ostream &outs) { patternTestSlicingOps().match(f, &matches); for (auto m : matches) { SetVector backwardSlice; - LogicalResult result = - getBackwardSlice(m.getMatchedOperation(), &backwardSlice); - assert(result.succeeded() && "expected a backward slice"); - (void)result; + getBackwardSlice(m.getMatchedOperation(), &backwardSlice); 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 5a5ac450f91fb..95db70ec7a815 100644 --- a/mlir/test/lib/IR/TestSlicing.cpp +++ b/mlir/test/lib/IR/TestSlicing.cpp @@ -41,9 +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"); - (void)result; + getBackwardSlice(op, &slice, options); for (Operation *slicedOp : slice) builder.clone(*slicedOp, mapper); func::ReturnOp::create(builder, loc);