[Mlir-commits] [mlir] ed5363a - [MLIR] Add getBody() method to SingleImplicitBlockTerminator op trait.

Alexander Belyaev llvmlistbot at llvm.org
Mon Apr 27 12:49:22 PDT 2020


Author: Alexander Belyaev
Date: 2020-04-27T21:48:52+02:00
New Revision: ed5363a6747b7e7914efa216b82438124587c862

URL: https://github.com/llvm/llvm-project/commit/ed5363a6747b7e7914efa216b82438124587c862
DIFF: https://github.com/llvm/llvm-project/commit/ed5363a6747b7e7914efa216b82438124587c862.diff

LOG: [MLIR] Add getBody() method to SingleImplicitBlockTerminator op trait.

Many ops with this trait have `getBody()` and `getBodyBuilder()` methods defined in `extraClassDeclaration` in tablegen. `getBody()` implementation is the same accross all these ops, but `getBodyBuilder()` can return builders with varying insertion points set. In this PR, `getBody()` is moved into `SingleImplicitBlockTerminator` struct and `getBodyBuilder()` is replaced with `OpBuilder::atBlock(End|Terminator)(op.getBody);`.

Differential Revision: https://reviews.llvm.org/D78864

Added: 
    

Modified: 
    flang/include/flang/Optimizer/Dialect/FIROps.td
    mlir/include/mlir/Dialect/Affine/IR/AffineOps.td
    mlir/include/mlir/Dialect/LoopOps/LoopOps.td
    mlir/include/mlir/Dialect/StandardOps/IR/Ops.td
    mlir/include/mlir/IR/Builders.h
    mlir/include/mlir/IR/OpDefinition.h
    mlir/lib/Transforms/Utils/LoopUtils.cpp

Removed: 
    


################################################################################
diff  --git a/flang/include/flang/Optimizer/Dialect/FIROps.td b/flang/include/flang/Optimizer/Dialect/FIROps.td
index 2011ca9aaa6f..034534861b80 100644
--- a/flang/include/flang/Optimizer/Dialect/FIROps.td
+++ b/flang/include/flang/Optimizer/Dialect/FIROps.td
@@ -1805,17 +1805,9 @@ def fir_LoopOp : fir_Op<"loop", [ImplicitFirTerminator]> {
     /// Does loop set (and return) the final value of the control variable?
     bool hasLastValue() { return lastVal().size(); }
 
-    /// Get the body of the loop
-    mlir::Block *getBody() { return &region().front(); }
-
     /// Get the block argument corresponding to the loop control value (PHI)
     mlir::Value getInductionVar() { return getBody()->getArgument(0); }
 
-    /// Get a builder to insert operations into the LoopOp
-    mlir::OpBuilder getBodyBuilder() {
-      return mlir::OpBuilder(getBody(), std::prev(getBody()->end()));
-    }
-
     void setLowerBound(mlir::Value bound) {
       getOperation()->setOperand(0, bound);
     }

diff  --git a/mlir/include/mlir/Dialect/Affine/IR/AffineOps.td b/mlir/include/mlir/Dialect/Affine/IR/AffineOps.td
index 75aaff9468e9..a49413fca27d 100644
--- a/mlir/include/mlir/Dialect/Affine/IR/AffineOps.td
+++ b/mlir/include/mlir/Dialect/Affine/IR/AffineOps.td
@@ -180,11 +180,7 @@ def AffineForOp : Affine_Op<"for",
     static StringRef getLowerBoundAttrName() { return "lower_bound"; }
     static StringRef getUpperBoundAttrName() { return "upper_bound"; }
 
-    Block *getBody() { return &region().front(); }
     Value getInductionVar() { return getBody()->getArgument(0); }
-    OpBuilder getBodyBuilder() {
-      return OpBuilder(getBody(), std::prev(getBody()->end()));
-    }
 
     // TODO: provide iterators for the lower and upper bound operands
     // if the current access via getLowerBound(), getUpperBound() is too slow.

diff  --git a/mlir/include/mlir/Dialect/LoopOps/LoopOps.td b/mlir/include/mlir/Dialect/LoopOps/LoopOps.td
index 3e0004a9642a..72ba9cab760d 100644
--- a/mlir/include/mlir/Dialect/LoopOps/LoopOps.td
+++ b/mlir/include/mlir/Dialect/LoopOps/LoopOps.td
@@ -141,11 +141,7 @@ def ForOp : Loop_Op<"for",
   ];
 
   let extraClassDeclaration = [{
-    Block *getBody() { return &region().front(); }
     Value getInductionVar() { return getBody()->getArgument(0); }
-    OpBuilder getBodyBuilder() {
-      return OpBuilder(getBody(), std::prev(getBody()->end()));
-    }
     Block::BlockArgListType getRegionIterArgs() {
       return getBody()->getArguments().drop_front();
     }
@@ -242,16 +238,14 @@ def IfOp : Loop_Op<"if",
 
   let extraClassDeclaration = [{
     OpBuilder getThenBodyBuilder() {
-      assert(!thenRegion().empty() && "Unexpected empty 'then' region.");
-      Block &body = thenRegion().front();
-      return OpBuilder(&body,
-                       results().empty() ? std::prev(body.end()) : body.end());
+      Block* body = getBody(0);
+      return results().empty() ? OpBuilder::atBlockTerminator(body)
+                               : OpBuilder::atBlockEnd(body);
     }
     OpBuilder getElseBodyBuilder() {
-      assert(!elseRegion().empty() && "Unexpected empty 'else' region.");
-      Block &body = elseRegion().front();
-      return OpBuilder(&body,
-                       results().empty() ? std::prev(body.end()) : body.end());
+      Block* body = getBody(1);
+      return results().empty() ? OpBuilder::atBlockTerminator(body)
+                               : OpBuilder::atBlockEnd(body);
     }
   }];
 }
@@ -322,7 +316,6 @@ def ParallelOp : Loop_Op<"parallel",
   ];
 
   let extraClassDeclaration = [{
-    Block *getBody() { return &region().front(); }
     ValueRange getInductionVars() {
       return getBody()->getArguments();
     }

diff  --git a/mlir/include/mlir/Dialect/StandardOps/IR/Ops.td b/mlir/include/mlir/Dialect/StandardOps/IR/Ops.td
index 17b959396303..1eac2af639d4 100644
--- a/mlir/include/mlir/Dialect/StandardOps/IR/Ops.td
+++ b/mlir/include/mlir/Dialect/StandardOps/IR/Ops.td
@@ -533,11 +533,6 @@ def GenericAtomicRMWOp : Std_Op<"generic_atomic_rmw", [
   ];
 
   let extraClassDeclaration = [{
-    OpBuilder getBodyBuilder() {
-      assert(!body().empty() && "Unexpected empty 'body' region.");
-      Block &block = body().front();
-      return OpBuilder(&block, block.end());
-    }
     // The value stored in memref[ivs].
     Value getCurrentValue() {
       return body().front().getArgument(0);

diff  --git a/mlir/include/mlir/IR/Builders.h b/mlir/include/mlir/IR/Builders.h
index bceba806024d..b9ef11648e38 100644
--- a/mlir/include/mlir/IR/Builders.h
+++ b/mlir/include/mlir/IR/Builders.h
@@ -195,7 +195,7 @@ class OpBuilder : public Builder {
   }
 
   /// Create a builder and set the insertion point to before the first operation
-  /// in the block but still inside th block.
+  /// in the block but still inside the block.
   static OpBuilder atBlockBegin(Block *block) {
     return OpBuilder(block, block->begin());
   }
@@ -206,6 +206,14 @@ class OpBuilder : public Builder {
     return OpBuilder(block, block->end());
   }
 
+  /// Create a builder and set the insertion point to before the block
+  /// terminator.
+  static OpBuilder atBlockTerminator(Block *block) {
+    auto *terminator = block->getTerminator();
+    assert(terminator != nullptr && "the block has no terminator");
+    return OpBuilder(block, terminator->getIterator());
+  }
+
   /// This class represents a saved insertion point.
   class InsertPoint {
   public:
@@ -322,7 +330,7 @@ class OpBuilder : public Builder {
     OpTy::build(this, state, std::forward<Args>(args)...);
     auto *op = createOperation(state);
     auto result = dyn_cast<OpTy>(op);
-    assert(result && "Builder didn't return the right type");
+    assert(result && "builder didn't return the right type");
     return result;
   }
 

diff  --git a/mlir/include/mlir/IR/OpDefinition.h b/mlir/include/mlir/IR/OpDefinition.h
index f2b174c067ff..c38c593ad6c5 100644
--- a/mlir/include/mlir/IR/OpDefinition.h
+++ b/mlir/include/mlir/IR/OpDefinition.h
@@ -1093,6 +1093,12 @@ template <typename TerminatorOpType> struct SingleBlockImplicitTerminator {
       ::mlir::impl::template ensureRegionTerminator<TerminatorOpType>(
           region, builder, loc);
     }
+
+    Block *getBody(unsigned idx = 0) {
+      Region &region = this->getOperation()->getRegion(idx);
+      assert(!region.empty() && "unexpected empty region");
+      return &region.front();
+    }
   };
 };
 

diff  --git a/mlir/lib/Transforms/Utils/LoopUtils.cpp b/mlir/lib/Transforms/Utils/LoopUtils.cpp
index dec0c4f7c4eb..4b0cd6c8eb1d 100644
--- a/mlir/lib/Transforms/Utils/LoopUtils.cpp
+++ b/mlir/lib/Transforms/Utils/LoopUtils.cpp
@@ -192,7 +192,7 @@ static AffineForOp generateShiftedLoop(
 
   BlockAndValueMapping operandMap;
 
-  OpBuilder bodyBuilder = loopChunk.getBodyBuilder();
+  auto bodyBuilder = OpBuilder::atBlockTerminator(loopChunk.getBody());
   for (auto it = opGroupQueue.begin() + offset, e = opGroupQueue.end(); it != e;
        ++it) {
     uint64_t shift = it->first;
@@ -470,7 +470,7 @@ LogicalResult mlir::loopUnrollByFactor(AffineForOp forOp,
 
   // Builder to insert unrolled bodies just before the terminator of the body of
   // 'forOp'.
-  OpBuilder builder = forOp.getBodyBuilder();
+  auto builder = OpBuilder::atBlockTerminator(forOp.getBody());
 
   // Keep a pointer to the last non-terminator operation in the original block
   // so that we know what to clone (since we are doing this in-place).
@@ -906,7 +906,7 @@ stripmineSink(AffineForOp forOp, uint64_t factor,
   SmallVector<AffineForOp, 8> innerLoops;
   for (auto t : targets) {
     // Insert newForOp before the terminator of `t`.
-    OpBuilder b = t.getBodyBuilder();
+    auto b = OpBuilder::atBlockTerminator(t.getBody());
     auto newForOp = b.create<AffineForOp>(t.getLoc(), lbOperands, lbMap,
                                           ubOperands, ubMap, originalStep);
     auto begin = t.getBody()->begin();
@@ -938,7 +938,7 @@ static Loops stripmineSink(loop::ForOp forOp, Value factor,
     auto nOps = t.getBody()->getOperations().size();
 
     // Insert newForOp before the terminator of `t`.
-    OpBuilder b(t.getBodyBuilder());
+    auto b = OpBuilder::atBlockTerminator((t.getBody()));
     Value stepped = b.create<AddIOp>(t.getLoc(), iv, forOp.step());
     Value less = b.create<CmpIOp>(t.getLoc(), CmpIPredicate::slt,
                                   forOp.upperBound(), stepped);
@@ -1510,7 +1510,7 @@ generatePointWiseCopy(Location loc, Value memref, Value fastMemRef,
     if (d == 0)
       copyNestRoot = forOp;
 
-    b = forOp.getBodyBuilder();
+    b = OpBuilder::atBlockTerminator(forOp.getBody());
 
     auto fastBufOffsetMap =
         AffineMap::get(lbOperands.size(), 0, fastBufOffsets[d]);
@@ -2309,7 +2309,7 @@ createFullTiles(MutableArrayRef<AffineForOp> inputNest,
     AffineForOp fullTileLoop = createCanonicalizedAffineForOp(
         b, loop.getLoc(), lbVmap.getOperands(), lbVmap.getAffineMap(),
         ubVmap.getOperands(), ubVmap.getAffineMap());
-    b = fullTileLoop.getBodyBuilder();
+    b = OpBuilder::atBlockTerminator(fullTileLoop.getBody());
     fullTileLoops.push_back(fullTileLoop);
   }
 
@@ -2318,7 +2318,7 @@ createFullTiles(MutableArrayRef<AffineForOp> inputNest,
   for (auto loopEn : llvm::enumerate(inputNest))
     operandMap.map(loopEn.value().getInductionVar(),
                    fullTileLoops[loopEn.index()].getInductionVar());
-  b = fullTileLoops.back().getBodyBuilder();
+  b = OpBuilder::atBlockTerminator(fullTileLoops.back().getBody());
   for (auto &op : inputNest.back().getBody()->without_terminator())
     b.clone(op, operandMap);
   return success();


        


More information about the Mlir-commits mailing list