[Mlir-commits] [mlir] 87d77d3 - [mlir][IR] Insert operations before `SingleBlock`'s terminator (#65959)

[llvmlistbot at llvm.org]

Author: vic
Date: 2023-09-15T16:17:35+02:00
New Revision: 87d77d3cfb5049b3b3714f95b2e48bbc78d8c5f9

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

LOG: [mlir][IR] Insert operations before `SingleBlock`'s terminator  (#65959)

Change `SingleBlock::{insert,push_back}` to avoid inserting the argument
operation after the block's terminator. This allows removing
`SingleBlockImplicitTerminator`'s functions with the same name.

Define `Block::hasTerminator` checking whether the block has a
terminator operation.

Signed-off-by: Victor Perez <victor.perez at codeplay.com>

Added: 
    

Modified: 
    mlir/include/mlir/IR/Block.h
    mlir/include/mlir/IR/OpDefinition.h
    mlir/lib/IR/Block.cpp

Removed: 
    


################################################################################
diff  --git a/mlir/include/mlir/IR/Block.h b/mlir/include/mlir/IR/Block.h
index eb56290fbe77180..4b50a0aec945c65 100644
--- a/mlir/include/mlir/IR/Block.h
+++ b/mlir/include/mlir/IR/Block.h
@@ -214,6 +214,9 @@ class Block : public IRObjectWithUseList<BlockOperand>,
   /// the block has a valid terminator operation.
   Operation *getTerminator();
 
+  /// Check whether this block has a terminator.
+  bool hasTerminator();
+
   //===--------------------------------------------------------------------===//
   // Predecessors and successors.
   //===--------------------------------------------------------------------===//

diff  --git a/mlir/include/mlir/IR/OpDefinition.h b/mlir/include/mlir/IR/OpDefinition.h
index e00f1ebd1fb2678..306b3789a044f83 100644
--- a/mlir/include/mlir/IR/OpDefinition.h
+++ b/mlir/include/mlir/IR/OpDefinition.h
@@ -932,6 +932,10 @@ struct SingleBlock : public TraitBase<ConcreteType, SingleBlock> {
   }
   template <typename OpT = ConcreteType>
   enable_if_single_region<OpT> insert(Block::iterator insertPt, Operation *op) {
+    Block *body = getBody();
+    // Insert op before the block's terminator if it has one
+    if (insertPt == body->end() && body->hasTerminator())
+      insertPt = Block::iterator(body->getTerminator());
     getBody()->getOperations().insert(insertPt, op);
   }
 };
@@ -997,37 +1001,6 @@ struct SingleBlockImplicitTerminator {
       ::mlir::impl::ensureRegionTerminator(region, builder, loc,
                                            buildTerminator);
     }
-
-    //===------------------------------------------------------------------===//
-    // Single Region Utilities
-    //===------------------------------------------------------------------===//
-
-    template <typename OpT, typename T = void>
-    using enable_if_single_region =
-        std::enable_if_t<OpT::template hasTrait<OneRegion>(), T>;
-
-    /// Insert the operation into the back of the body, before the terminator.
-    template <typename OpT = ConcreteType>
-    enable_if_single_region<OpT> push_back(Operation *op) {
-      Block *body = static_cast<SingleBlock<ConcreteType> *>(this)->getBody();
-      insert(Block::iterator(body->getTerminator()), op);
-    }
-
-    /// Insert the operation at the given insertion point. Note: The operation
-    /// is never inserted after the terminator, even if the insertion point is
-    /// end().
-    template <typename OpT = ConcreteType>
-    enable_if_single_region<OpT> insert(Operation *insertPt, Operation *op) {
-      insert(Block::iterator(insertPt), op);
-    }
-    template <typename OpT = ConcreteType>
-    enable_if_single_region<OpT> insert(Block::iterator insertPt,
-                                        Operation *op) {
-      Block *body = static_cast<SingleBlock<ConcreteType> *>(this)->getBody();
-      if (insertPt == body->end())
-        insertPt = Block::iterator(body->getTerminator());
-      body->getOperations().insert(insertPt, op);
-    }
   };
 };
 

diff  --git a/mlir/lib/IR/Block.cpp b/mlir/lib/IR/Block.cpp
index fbbba96468a2a19..029864d9ea47b98 100644
--- a/mlir/lib/IR/Block.cpp
+++ b/mlir/lib/IR/Block.cpp
@@ -236,10 +236,15 @@ void Block::eraseArguments(function_ref<bool(BlockArgument)> shouldEraseFn) {
 /// Get the terminator operation of this block. This function asserts that
 /// the block has a valid terminator operation.
 Operation *Block::getTerminator() {
-  assert(!empty() && back().mightHaveTrait<OpTrait::IsTerminator>());
+  assert(hasTerminator());
   return &back();
 }
 
+/// Check whether this block has a terminator.
+bool Block::hasTerminator() {
+  return !empty() && back().mightHaveTrait<OpTrait::IsTerminator>();
+}
+
 // Indexed successor access.
 unsigned Block::getNumSuccessors() {
   return empty() ? 0 : back().getNumSuccessors();