[flang-commits] [flang] e51652f - [mlir] Simplify LoopLikeOpInterface
via flang-commits
flang-commits at lists.llvm.org
Mon Mar 28 11:13:05 PDT 2022
Author: Mogball
Date: 2022-03-28T18:10:04Z
New Revision: e51652f9bf2e63497ed1e007c4a8c31c7ec08930
URL: https://github.com/llvm/llvm-project/commit/e51652f9bf2e63497ed1e007c4a8c31c7ec08930
DIFF: https://github.com/llvm/llvm-project/commit/e51652f9bf2e63497ed1e007c4a8c31c7ec08930.diff
LOG: [mlir] Simplify LoopLikeOpInterface
- Adds default implementations of `isDefinedOutsideOfLoop` and `moveOutOfLoop` since 99% of all implementations of these functions were identical
- `moveOutOfLoop` takes one operation and doesn't return anything anymore. 100% of all implementations of this function would always return `success` and uses would either respond with a pass failure or an `llvm_unreachable`.
Added:
Modified:
flang/lib/Optimizer/Dialect/FIROps.cpp
mlir/include/mlir/Interfaces/LoopLikeInterface.h
mlir/include/mlir/Interfaces/LoopLikeInterface.td
mlir/lib/Dialect/Affine/IR/AffineOps.cpp
mlir/lib/Dialect/Linalg/Transforms/Hoisting.cpp
mlir/lib/Dialect/Linalg/Transforms/LinalgStrategyPasses.cpp
mlir/lib/Dialect/SCF/SCF.cpp
mlir/lib/Dialect/Tosa/IR/TosaOps.cpp
mlir/lib/Interfaces/LoopLikeInterface.cpp
mlir/lib/Transforms/LoopInvariantCodeMotion.cpp
mlir/test/lib/Dialect/SCF/TestSCFUtils.cpp
Removed:
################################################################################
diff --git a/flang/lib/Optimizer/Dialect/FIROps.cpp b/flang/lib/Optimizer/Dialect/FIROps.cpp
index 2c25805bb58f6..6dfc5a90d8fcb 100644
--- a/flang/lib/Optimizer/Dialect/FIROps.cpp
+++ b/flang/lib/Optimizer/Dialect/FIROps.cpp
@@ -1724,17 +1724,6 @@ void IterWhileOp::print(mlir::OpAsmPrinter &p) {
mlir::Region &fir::IterWhileOp::getLoopBody() { return getRegion(); }
-bool fir::IterWhileOp::isDefinedOutsideOfLoop(mlir::Value value) {
- return !getRegion().isAncestor(value.getParentRegion());
-}
-
-mlir::LogicalResult
-fir::IterWhileOp::moveOutOfLoop(llvm::ArrayRef<mlir::Operation *> ops) {
- for (auto *op : ops)
- op->moveBefore(*this);
- return success();
-}
-
mlir::BlockArgument fir::IterWhileOp::iterArgToBlockArg(mlir::Value iterArg) {
for (auto i : llvm::enumerate(getInitArgs()))
if (iterArg == i.value())
@@ -2022,17 +2011,6 @@ void DoLoopOp::print(mlir::OpAsmPrinter &p) {
mlir::Region &fir::DoLoopOp::getLoopBody() { return getRegion(); }
-bool fir::DoLoopOp::isDefinedOutsideOfLoop(mlir::Value value) {
- return !getRegion().isAncestor(value.getParentRegion());
-}
-
-mlir::LogicalResult
-fir::DoLoopOp::moveOutOfLoop(llvm::ArrayRef<mlir::Operation *> ops) {
- for (auto op : ops)
- op->moveBefore(*this);
- return success();
-}
-
/// Translate a value passed as an iter_arg to the corresponding block
/// argument in the body of the loop.
mlir::BlockArgument fir::DoLoopOp::iterArgToBlockArg(mlir::Value iterArg) {
diff --git a/mlir/include/mlir/Interfaces/LoopLikeInterface.h b/mlir/include/mlir/Interfaces/LoopLikeInterface.h
index df4631690a3f9..a1cf065ea5daa 100644
--- a/mlir/include/mlir/Interfaces/LoopLikeInterface.h
+++ b/mlir/include/mlir/Interfaces/LoopLikeInterface.h
@@ -28,7 +28,7 @@
namespace mlir {
/// Move loop invariant code out of a `looplike` operation.
-LogicalResult moveLoopInvariantCode(LoopLikeOpInterface looplike);
+void moveLoopInvariantCode(LoopLikeOpInterface looplike);
} // namespace mlir
#endif // MLIR_INTERFACES_LOOPLIKEINTERFACE_H_
diff --git a/mlir/include/mlir/Interfaces/LoopLikeInterface.td b/mlir/include/mlir/Interfaces/LoopLikeInterface.td
index fd8a8aef70053..18b81760c7ae6 100644
--- a/mlir/include/mlir/Interfaces/LoopLikeInterface.td
+++ b/mlir/include/mlir/Interfaces/LoopLikeInterface.td
@@ -30,7 +30,9 @@ def LoopLikeOpInterface : OpInterface<"LoopLikeOpInterface"> {
explicit capture of dependencies, an implementation could check whether
the value corresponds to a captured dependency.
}],
- "bool", "isDefinedOutsideOfLoop", (ins "::mlir::Value ":$value)
+ "bool", "isDefinedOutsideOfLoop", (ins "::mlir::Value ":$value), [{}], [{
+ return value.getParentRegion()->isProperAncestor(&$_op.getLoopBody());
+ }]
>,
InterfaceMethod<[{
Returns the region that makes up the body of the loop and should be
@@ -39,10 +41,12 @@ def LoopLikeOpInterface : OpInterface<"LoopLikeOpInterface"> {
"::mlir::Region &", "getLoopBody"
>,
InterfaceMethod<[{
- Moves the given vector of operations out of the loop. The vector is
- sorted topologically.
+ Moves the given loop-invariant operation out of the loop.
}],
- "::mlir::LogicalResult", "moveOutOfLoop", (ins "::mlir::ArrayRef<::mlir::Operation *>":$ops)
+ "void", "moveOutOfLoop",
+ (ins "::mlir::Operation *":$op), [{}], [{
+ op->moveBefore($_op);
+ }]
>,
InterfaceMethod<[{
If there is a single induction variable return it, otherwise return
diff --git a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp b/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
index 6d4d041594ac9..cf4299af6d770 100644
--- a/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
+++ b/mlir/lib/Dialect/Affine/IR/AffineOps.cpp
@@ -1859,10 +1859,6 @@ bool AffineForOp::matchingBoundOperandList() {
Region &AffineForOp::getLoopBody() { return region(); }
-bool AffineForOp::isDefinedOutsideOfLoop(Value value) {
- return !region().isAncestor(value.getParentRegion());
-}
-
Optional<Value> AffineForOp::getSingleInductionVar() {
return getInductionVar();
}
@@ -1879,12 +1875,6 @@ Optional<OpFoldResult> AffineForOp::getSingleStep() {
return OpFoldResult(b.getI64IntegerAttr(getStep()));
}
-LogicalResult AffineForOp::moveOutOfLoop(ArrayRef<Operation *> ops) {
- for (auto *op : ops)
- op->moveBefore(*this);
- return success();
-}
-
/// Returns true if the provided value is the induction variable of a
/// AffineForOp.
bool mlir::isForInductionVar(Value val) {
@@ -3062,16 +3052,6 @@ void AffineParallelOp::build(OpBuilder &builder, OperationState &result,
Region &AffineParallelOp::getLoopBody() { return region(); }
-bool AffineParallelOp::isDefinedOutsideOfLoop(Value value) {
- return !region().isAncestor(value.getParentRegion());
-}
-
-LogicalResult AffineParallelOp::moveOutOfLoop(ArrayRef<Operation *> ops) {
- for (Operation *op : ops)
- op->moveBefore(*this);
- return success();
-}
-
unsigned AffineParallelOp::getNumDims() { return steps().size(); }
AffineParallelOp::operand_range AffineParallelOp::getLowerBoundsOperands() {
diff --git a/mlir/lib/Dialect/Linalg/Transforms/Hoisting.cpp b/mlir/lib/Dialect/Linalg/Transforms/Hoisting.cpp
index 4e3c7c8497d5a..578d956665b6f 100644
--- a/mlir/lib/Dialect/Linalg/Transforms/Hoisting.cpp
+++ b/mlir/lib/Dialect/Linalg/Transforms/Hoisting.cpp
@@ -271,12 +271,11 @@ static void hoistReadWrite(HoistableRead read, HoistableWrite write,
<< "\nInvolving: " << tensorBBArg << "\n");
// If a read slice is present, hoist it.
- if (read.extractSliceOp && failed(forOp.moveOutOfLoop({read.extractSliceOp})))
- llvm_unreachable("Unexpected failure moving extract_slice out of loop");
+ if (read.extractSliceOp)
+ forOp.moveOutOfLoop(read.extractSliceOp);
// Hoist the transfer_read op.
- if (failed(forOp.moveOutOfLoop({read.transferReadOp})))
- llvm_unreachable("Unexpected failure moving transfer read out of loop");
+ forOp.moveOutOfLoop(read.transferReadOp);
// TODO: don't hardcode /*numIvs=*/1.
assert(tensorBBArg.getArgNumber() >= /*numIvs=*/1);
@@ -397,9 +396,7 @@ void mlir::linalg::hoistRedundantVectorTransfers(FuncOp func) {
// First move loop invariant ops outside of their loop. This needs to be
// done before as we cannot move ops without interputing the function walk.
func.walk([&](LoopLikeOpInterface loopLike) {
- if (failed(moveLoopInvariantCode(loopLike)))
- llvm_unreachable(
- "Unexpected failure to move invariant code out of loop");
+ moveLoopInvariantCode(loopLike);
});
func.walk([&](vector::TransferReadOp transferRead) {
@@ -484,9 +481,7 @@ void mlir::linalg::hoistRedundantVectorTransfers(FuncOp func) {
}
// Hoist read before.
- if (failed(loop.moveOutOfLoop({transferRead})))
- llvm_unreachable(
- "Unexpected failure to move transfer read out of loop");
+ loop.moveOutOfLoop(transferRead);
// Hoist write after.
transferWrite->moveAfter(loop);
diff --git a/mlir/lib/Dialect/Linalg/Transforms/LinalgStrategyPasses.cpp b/mlir/lib/Dialect/Linalg/Transforms/LinalgStrategyPasses.cpp
index 17e0cd79ff8e5..585bc2d990147 100644
--- a/mlir/lib/Dialect/Linalg/Transforms/LinalgStrategyPasses.cpp
+++ b/mlir/lib/Dialect/Linalg/Transforms/LinalgStrategyPasses.cpp
@@ -338,14 +338,9 @@ struct LinalgStrategyEnablePass
return signalPassFailure();
if (options.licm) {
- if (funcOp
- ->walk([&](LoopLikeOpInterface loopLike) {
- if (failed(moveLoopInvariantCode(loopLike)))
- return WalkResult::interrupt();
- return WalkResult::advance();
- })
- .wasInterrupted())
- return signalPassFailure();
+ funcOp->walk([&](LoopLikeOpInterface loopLike) {
+ moveLoopInvariantCode(loopLike);
+ });
}
// Gathers all innermost loops through a post order pruned walk.
diff --git a/mlir/lib/Dialect/SCF/SCF.cpp b/mlir/lib/Dialect/SCF/SCF.cpp
index 8410f897c3daf..3e5028f247a09 100644
--- a/mlir/lib/Dialect/SCF/SCF.cpp
+++ b/mlir/lib/Dialect/SCF/SCF.cpp
@@ -449,16 +449,6 @@ ParseResult ForOp::parse(OpAsmParser &parser, OperationState &result) {
Region &ForOp::getLoopBody() { return getRegion(); }
-bool ForOp::isDefinedOutsideOfLoop(Value value) {
- return !getRegion().isAncestor(value.getParentRegion());
-}
-
-LogicalResult ForOp::moveOutOfLoop(ArrayRef<Operation *> ops) {
- for (auto *op : ops)
- op->moveBefore(*this);
- return success();
-}
-
ForOp mlir::scf::getForInductionVarOwner(Value val) {
auto ivArg = val.dyn_cast<BlockArgument>();
if (!ivArg)
@@ -2061,16 +2051,6 @@ void ParallelOp::print(OpAsmPrinter &p) {
Region &ParallelOp::getLoopBody() { return getRegion(); }
-bool ParallelOp::isDefinedOutsideOfLoop(Value value) {
- return !getRegion().isAncestor(value.getParentRegion());
-}
-
-LogicalResult ParallelOp::moveOutOfLoop(ArrayRef<Operation *> ops) {
- for (auto *op : ops)
- op->moveBefore(*this);
- return success();
-}
-
ParallelOp mlir::scf::getParallelForInductionVarOwner(Value val) {
auto ivArg = val.dyn_cast<BlockArgument>();
if (!ivArg)
diff --git a/mlir/lib/Dialect/Tosa/IR/TosaOps.cpp b/mlir/lib/Dialect/Tosa/IR/TosaOps.cpp
index 9fc35eaacce11..5fdd8b46835b6 100644
--- a/mlir/lib/Dialect/Tosa/IR/TosaOps.cpp
+++ b/mlir/lib/Dialect/Tosa/IR/TosaOps.cpp
@@ -68,21 +68,6 @@ struct TosaInlinerInterface : public DialectInlinerInterface {
/// Returns the while loop body.
Region &tosa::WhileOp::getLoopBody() { return body(); }
-bool tosa::WhileOp::isDefinedOutsideOfLoop(Value value) {
- return !body().isAncestor(value.getParentRegion());
-}
-
-LogicalResult WhileOp::moveOutOfLoop(ArrayRef<mlir::Operation *> ops) {
- if (ops.empty())
- return success();
-
- Operation *tosaWhileOp = this->getOperation();
- for (auto *op : ops)
- op->moveBefore(tosaWhileOp);
-
- return success();
-}
-
//===----------------------------------------------------------------------===//
// Tosa dialect initialization.
//===----------------------------------------------------------------------===//
diff --git a/mlir/lib/Interfaces/LoopLikeInterface.cpp b/mlir/lib/Interfaces/LoopLikeInterface.cpp
index 8ec4b51aa757b..554bea1e61677 100644
--- a/mlir/lib/Interfaces/LoopLikeInterface.cpp
+++ b/mlir/lib/Interfaces/LoopLikeInterface.cpp
@@ -66,7 +66,7 @@ static bool canBeHoisted(Operation *op,
return true;
}
-LogicalResult mlir::moveLoopInvariantCode(LoopLikeOpInterface looplike) {
+void mlir::moveLoopInvariantCode(LoopLikeOpInterface looplike) {
auto &loopBody = looplike.getLoopBody();
// We use two collections here as we need to preserve the order for insertion
@@ -95,7 +95,6 @@ LogicalResult mlir::moveLoopInvariantCode(LoopLikeOpInterface looplike) {
// For all instructions that we found to be invariant, move outside of the
// loop.
- LogicalResult result = looplike.moveOutOfLoop(opsToMove);
- LLVM_DEBUG(looplike.print(llvm::dbgs() << "\n\nModified loop:\n"));
- return result;
+ for (Operation *op : opsToMove)
+ looplike.moveOutOfLoop(op);
}
diff --git a/mlir/lib/Transforms/LoopInvariantCodeMotion.cpp b/mlir/lib/Transforms/LoopInvariantCodeMotion.cpp
index e4e3c16a14656..14761a84f5c40 100644
--- a/mlir/lib/Transforms/LoopInvariantCodeMotion.cpp
+++ b/mlir/lib/Transforms/LoopInvariantCodeMotion.cpp
@@ -37,8 +37,7 @@ void LoopInvariantCodeMotion::runOnOperation() {
// the outer loop, which in turn can be further LICM'ed.
getOperation()->walk([&](LoopLikeOpInterface loopLike) {
LLVM_DEBUG(loopLike.print(llvm::dbgs() << "\nOriginal loop:\n"));
- if (failed(moveLoopInvariantCode(loopLike)))
- signalPassFailure();
+ moveLoopInvariantCode(loopLike);
});
}
diff --git a/mlir/test/lib/Dialect/SCF/TestSCFUtils.cpp b/mlir/test/lib/Dialect/SCF/TestSCFUtils.cpp
index 2e4647a2482ad..1cec4b2386922 100644
--- a/mlir/test/lib/Dialect/SCF/TestSCFUtils.cpp
+++ b/mlir/test/lib/Dialect/SCF/TestSCFUtils.cpp
@@ -44,7 +44,7 @@ class TestSCFForUtilsPass
auto loop = fakeRead->getParentOfType<scf::ForOp>();
OpBuilder b(loop);
- (void)loop.moveOutOfLoop({fakeRead});
+ loop.moveOutOfLoop(fakeRead);
fakeWrite->moveAfter(loop);
auto newLoop = cloneWithNewYields(b, loop, fakeRead->getResult(0),
fakeCompute->getResult(0));
More information about the flang-commits
mailing list