[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`.

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) {

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_

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

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() {

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);

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.

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)

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.
 //===----------------------------------------------------------------------===//

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);
 }

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);
   });
 }
 

mlir/test/lib/Dialect/SCF/TestSCFUtils.cpp

@@ -44,7 +44,7 @@ public:
       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));