[Mlir-commits] [mlir] 61bab7c - [MLIR] Replace OpBuilder(Block) with atBlockBegin and atBlockEnd

[Tres Popp]

Author: Tres Popp
Date: 2020-04-01T10:06:41+02:00
New Revision: 61bab7c31a1b42d4e8be5060877843eb20d94644

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

LOG: [MLIR] Replace OpBuilder(Block) with atBlockBegin and atBlockEnd

Summary:
OpBuilder(Block) is specifically replaced with
OpBuilder::atBlockEnd(Block);

This is to make insertion behavior clear due to there being no one
correct answer for which location in a block the default insertion
point should be.

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

Added: 
    

Modified: 
    mlir/include/mlir/IR/Builders.h
    mlir/lib/Dialect/SPIRV/SPIRVOps.cpp

Removed: 
    


################################################################################
diff  --git a/mlir/include/mlir/IR/Builders.h b/mlir/include/mlir/IR/Builders.h
index 5b42132d463a..1c6b16f22989 100644
--- a/mlir/include/mlir/IR/Builders.h
+++ b/mlir/include/mlir/IR/Builders.h
@@ -188,13 +188,23 @@ class OpBuilder : public Builder {
     setInsertionPoint(op);
   }
 
-  explicit OpBuilder(Block *block) : OpBuilder(block, block->end()) {}
-
   OpBuilder(Block *block, Block::iterator insertPoint)
       : OpBuilder(block->getParent()) {
     setInsertionPoint(block, insertPoint);
   }
 
+  /// Create a builder and set the insertion point to before the first operation
+  /// in the block but still inside th block.
+  static OpBuilder atBlockBegin(Block *block) {
+    return OpBuilder(block, block->begin());
+  }
+
+  /// Create a builder and set the insertion point to after the last operation
+  /// in the block but still inside the block.
+  static OpBuilder atBlockEnd(Block *block) {
+    return OpBuilder(block, block->end());
+  }
+
   /// This class represents a saved insertion point.
   class InsertPoint {
   public:

diff  --git a/mlir/lib/Dialect/SPIRV/SPIRVOps.cpp b/mlir/lib/Dialect/SPIRV/SPIRVOps.cpp
index f6b862156c49..841b5e0ace64 100644
--- a/mlir/lib/Dialect/SPIRV/SPIRVOps.cpp
+++ b/mlir/lib/Dialect/SPIRV/SPIRVOps.cpp
@@ -2076,7 +2076,7 @@ void spirv::LoopOp::addEntryAndMergeBlock() {
   body().push_back(new Block());
   auto *mergeBlock = new Block();
   body().push_back(mergeBlock);
-  OpBuilder builder(mergeBlock);
+  OpBuilder builder = OpBuilder::atBlockEnd(mergeBlock);
 
   // Add a spv._merge op into the merge block.
   builder.create<spirv::MergeOp>(getLoc());
@@ -2373,7 +2373,7 @@ void spirv::SelectionOp::addMergeBlock() {
   assert(body().empty() && "entry and merge block already exist");
   auto *mergeBlock = new Block();
   body().push_back(mergeBlock);
-  OpBuilder builder(mergeBlock);
+  OpBuilder builder = OpBuilder::atBlockEnd(mergeBlock);
 
   // Add a spv._merge op into the merge block.
   builder.create<spirv::MergeOp>(getLoc());