[Mlir-commits] [mlir] e51652f - [mlir] Simplify LoopLikeOpInterface

llvmlistbot at llvm.org llvmlistbot at llvm.org
Mon Mar 28 11:13:03 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 Mlir-commits mailing list