[Mlir-commits] [mlir] [mlir] Remove duplicated `SingleBlockImplicitTerminator` member functions (PR #65959)

[llvmlistbot at llvm.org]

https://github.com/victor-eds updated https://github.com/llvm/llvm-project/pull/65959:

>From 6229d53be4906aa6d31f5d94bc9374feb278a90c Mon Sep 17 00:00:00 2001
From: Victor Perez <victor.perez at codeplay.com>
Date: Wed, 13 Sep 2023 12:24:56 +0100
Subject: [PATCH] [mlir][IR] Insert operations before `SingleBlock`'s
 terminator

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>
---
 mlir/include/mlir/IR/Block.h        |  3 +++
 mlir/include/mlir/IR/OpDefinition.h | 35 ++++-------------------------
 mlir/lib/IR/Block.cpp               |  7 +++++-
 3 files changed, 13 insertions(+), 32 deletions(-)

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 84ba46f4d6f3ec1..8dbc3b5b7845f22 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);
   }
 };
@@ -996,37 +1000,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();