Skip to content

[MLIR][IR] Rename Block::hasTerminator to mightHaveTerminator #66870

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Sep 21, 2023

Conversation

victor-eds
Copy link
Contributor

@victor-eds victor-eds commented Sep 20, 2023

This Block member function introduced in 87d77d3 may be misleading to users as the last operation in the block might have not been registered, so there would be no way to ensure that is a terminator.

@victor-eds victor-eds self-assigned this Sep 20, 2023
@victor-eds victor-eds changed the title [MLIR][IR] Drop bool Block::hasTerminator() [MLIR][IR] Drop Block::hasTerminator() Sep 20, 2023
@llvmbot llvmbot added mlir:core MLIR Core Infrastructure mlir labels Sep 20, 2023
@llvmbot
Copy link
Member

llvmbot commented Sep 20, 2023

@llvm/pr-subscribers-mlir-core

@llvm/pr-subscribers-mlir

Changes

This Block member function introduced in 87d77d3 may be misleading to users as the last operation in the block might have not been registered, so there would be no way to ensure that is a terminator. As this also adds little to no value, this commit removes that function.


Full diff: https://github.com/llvm/llvm-project/pull/66870.diff

2 Files Affected:

  • (modified) mlir/include/mlir/IR/Block.h (-3)
  • (modified) mlir/lib/IR/Block.cpp (+1-6)
diff --git a/mlir/include/mlir/IR/Block.h b/mlir/include/mlir/IR/Block.h
index 4b50a0aec945c65..eb56290fbe77180 100644
--- a/mlir/include/mlir/IR/Block.h
+++ b/mlir/include/mlir/IR/Block.h
@@ -214,9 +214,6 @@ 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/lib/IR/Block.cpp b/mlir/lib/IR/Block.cpp
index 029864d9ea47b98..fbbba96468a2a19 100644
--- a/mlir/lib/IR/Block.cpp
+++ b/mlir/lib/IR/Block.cpp
@@ -236,15 +236,10 @@ 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(hasTerminator());
+  assert(!empty() && back().mightHaveTrait<OpTrait::IsTerminator>());
   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();

This `Block` member function introduced in
87d77d3 may be misleading to users as
the last operation in the block might have not been registered, so
there would be no way to ensure that is a terminator.

Signed-off-by: Victor Perez <[email protected]>
@victor-eds victor-eds changed the title [MLIR][IR] Drop Block::hasTerminator() [MLIR][IR] Rename Block::hasTerminator to mightHaveTerminator Sep 20, 2023
@victor-eds victor-eds requested a review from ftynse September 20, 2023 15:04
@victor-eds victor-eds merged commit 02981c9 into llvm:main Sep 21, 2023
@victor-eds victor-eds deleted the drop-has-term branch September 21, 2023 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mlir:core MLIR Core Infrastructure mlir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants