[Mlir-commits] [mlir] [mlir][IR] Fix `Block::without_terminator` for blocks without terminator (PR #154498)

[Matthias Springer]

https://github.com/matthias-springer updated https://github.com/llvm/llvm-project/pull/154498

>From cff3a6ab6c7f52c795be453584a699c060090292 Mon Sep 17 00:00:00 2001
From: Matthias Springer <me at m-sp.org>
Date: Wed, 20 Aug 2025 09:40:16 +0000
Subject: [PATCH] [mlir][IR] Fix `Block::without_terminator` for `NoTerminator`
 blocks

---
 mlir/include/mlir/IR/Block.h               | 15 +++++++--------
 mlir/lib/Analysis/TopologicalSortUtils.cpp |  7 +------
 mlir/lib/IR/Block.cpp                      | 12 ++++++++++++
 3 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/mlir/include/mlir/IR/Block.h b/mlir/include/mlir/IR/Block.h
index e486bb627474d..8c2d2d02800e8 100644
--- a/mlir/include/mlir/IR/Block.h
+++ b/mlir/include/mlir/IR/Block.h
@@ -205,13 +205,11 @@ class alignas(8) Block : public IRObjectWithUseList<BlockOperand>,
   }
 
   /// Return an iterator range over the operation within this block excluding
-  /// the terminator operation at the end.
-  iterator_range<iterator> without_terminator() {
-    if (begin() == end())
-      return {begin(), end()};
-    auto endIt = --end();
-    return {begin(), endIt};
-  }
+  /// the terminator operation at the end. If the block has no terminator,
+  /// return an iterator range over the entire block. If it is unknown if the
+  /// block has a terminator (i.e., last block operation is unregistered), also
+  /// return an iterator range over the entire block.
+  iterator_range<iterator> without_terminator();
 
   //===--------------------------------------------------------------------===//
   // Terminator management
@@ -221,7 +219,8 @@ class alignas(8) Block : public IRObjectWithUseList<BlockOperand>,
   /// the block might have a valid terminator operation.
   Operation *getTerminator();
 
-  /// Check whether this block might have a terminator.
+  /// Return "true" if this block might have a terminator. Return "true" if
+  /// the last operation is unregistered.
   bool mightHaveTerminator();
 
   //===--------------------------------------------------------------------===//
diff --git a/mlir/lib/Analysis/TopologicalSortUtils.cpp b/mlir/lib/Analysis/TopologicalSortUtils.cpp
index a2fd14910892a..99546e71533ae 100644
--- a/mlir/lib/Analysis/TopologicalSortUtils.cpp
+++ b/mlir/lib/Analysis/TopologicalSortUtils.cpp
@@ -101,12 +101,7 @@ bool mlir::sortTopologically(
 
 bool mlir::sortTopologically(
     Block *block, function_ref<bool(Value, Operation *)> isOperandReady) {
-  if (block->empty())
-    return true;
-  if (block->back().hasTrait<OpTrait::IsTerminator>())
-    return sortTopologically(block, block->without_terminator(),
-                             isOperandReady);
-  return sortTopologically(block, *block, isOperandReady);
+  return sortTopologically(block, block->without_terminator(), isOperandReady);
 }
 
 bool mlir::computeTopologicalSorting(
diff --git a/mlir/lib/IR/Block.cpp b/mlir/lib/IR/Block.cpp
index 57825d9b42178..6ce7d8d1b4682 100644
--- a/mlir/lib/IR/Block.cpp
+++ b/mlir/lib/IR/Block.cpp
@@ -251,6 +251,18 @@ bool Block::mightHaveTerminator() {
   return !empty() && back().mightHaveTrait<OpTrait::IsTerminator>();
 }
 
+iterator_range<Block::iterator> Block::without_terminator() {
+  if (begin() == end())
+    return {begin(), end()};
+  // Note: When the op is unregistered, we do not know for sure if the last
+  // op is a terminator. In that case, we include it in `without_terminator`,
+  // but that decision is somewhat arbitrary.
+  if (!back().hasTrait<OpTrait::IsTerminator>())
+    return {begin(), end()};
+  auto endIt = --end();
+  return {begin(), endIt};
+}
+
 // Indexed successor access.
 unsigned Block::getNumSuccessors() {
   return empty() ? 0 : back().getNumSuccessors();