[Mlir-commits] [mlir] [MLIR] Fix broken 'Successor<CPred<...>>' functionality in mlir-tblgen (PR #205375)
Christopher Bate
llvmlistbot at llvm.org
Tue Jun 23 09:26:41 PDT 2026
https://github.com/christopherbate created https://github.com/llvm/llvm-project/pull/205375
Previously, mlir-tblgen would generate incorrect C++ code when passing a custom constraint to `Successor<...>`. The code would fail to compile. We lacked end-to-end tests for this functionality and all in-tree dialects only use `AnySuccessor` for their successors.
Custom C++ predicates for an Operation's successor(s) can be useful in order to generate verification for certain conditions (e.g. that the successor is the "next" block). This commit fixes the mlir-tblgen functionality and adds end-to-end tests in the Test dialect for single and variadic successors.
Assisted-by: Claude
>From 87cf14daa76787df2c35351798fd089e34026f86 Mon Sep 17 00:00:00 2001
From: Christopher Bate <cbate at nvidia.com>
Date: Tue, 23 Jun 2026 16:24:21 +0000
Subject: [PATCH] [MLIR] Fix broken 'Successor<CPred<...>>' functionality in
mlir-tblgen
Previously, mlir-tblgen would generate incorrect C++ code when
passing a custom constraint to `Successor<...>`. The code would fail
to compile. We lacked end-to-end tests for this functionality and all
in-tree dialects only use `AnySuccessor` for their successors.
Custom C++ predicates for an Operation's successor(s) can be useful
in order to generate verification for certain conditions (e.g. that the
successor is the "next" block). This commit fixes the mlir-tblgen
functionality and adds end-to-end tests in the Test dialect for single
and variadic successors.
Assisted-by: Claude
---
mlir/lib/TableGen/CodeGenHelpers.cpp | 2 +-
mlir/test/IR/test-successor-verifier.mlir | 49 +++++++++++++++++++++
mlir/test/lib/Dialect/Test/TestOps.td | 27 ++++++++++++
mlir/test/mlir-tblgen/constraint-unique.td | 12 +++--
mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp | 31 ++++++++-----
5 files changed, 103 insertions(+), 18 deletions(-)
create mode 100644 mlir/test/IR/test-successor-verifier.mlir
diff --git a/mlir/lib/TableGen/CodeGenHelpers.cpp b/mlir/lib/TableGen/CodeGenHelpers.cpp
index 9ad031eb701ad..c8c419669f3d4 100644
--- a/mlir/lib/TableGen/CodeGenHelpers.cpp
+++ b/mlir/lib/TableGen/CodeGenHelpers.cpp
@@ -244,7 +244,7 @@ static ::llvm::LogicalResult {0}(
::llvm::StringRef successorName, unsigned successorIndex) {
if (!({1})) {
return op->emitOpError("successor #") << successorIndex << " ('"
- << successorName << ")' failed to verify constraint: {2}";
+ << successorName << "') failed to verify constraint: {2}";
}
return ::mlir::success();
}
diff --git a/mlir/test/IR/test-successor-verifier.mlir b/mlir/test/IR/test-successor-verifier.mlir
new file mode 100644
index 0000000000000..1eb5b1f396991
--- /dev/null
+++ b/mlir/test/IR/test-successor-verifier.mlir
@@ -0,0 +1,49 @@
+// RUN: mlir-opt %s -split-input-file -verify-diagnostics | FileCheck %s
+
+// Tests verification of successors with a non-trivial predicate.
+
+// CHECK-LABEL: func @fallthrough_ok
+func.func @fallthrough_ok() {
+ // CHECK: test.fallthrough_br ^bb1 forward[]
+ test.fallthrough_br ^bb1 forward []
+^bb1:
+ return
+}
+
+// -----
+
+// CHECK-LABEL: func @forward_successors_ok
+func.func @forward_successors_ok() {
+ // CHECK: test.fallthrough_br ^bb1 forward[^bb2, ^bb3]
+ test.fallthrough_br ^bb1 forward [^bb2, ^bb3]
+^bb1:
+ return
+^bb2:
+ return
+^bb3:
+ return
+}
+
+// -----
+
+func.func @fallthrough_not_next_block() {
+ // expected-error @+1 {{successor #0 ('target') failed to verify constraint: the fallthrough block (the block immediately following the op's block)}}
+ test.fallthrough_br ^bb2 forward []
+^bb1:
+ return
+^bb2:
+ return
+}
+
+// -----
+
+func.func @forward_successor_is_backward() {
+ cf.br ^bb1
+^bb1:
+ cf.br ^bb2
+^bb2:
+ // expected-error @+1 {{successor #1 ('forwardTargets') failed to verify constraint: a forward block (a block listed after the op's block)}}
+ test.fallthrough_br ^bb3 forward [^bb1]
+^bb3:
+ return
+}
diff --git a/mlir/test/lib/Dialect/Test/TestOps.td b/mlir/test/lib/Dialect/Test/TestOps.td
index 31c19487075d8..29f456d1ec7eb 100644
--- a/mlir/test/lib/Dialect/Test/TestOps.td
+++ b/mlir/test/lib/Dialect/Test/TestOps.td
@@ -1130,6 +1130,33 @@ def TestInternalBranchOp : TEST_Op<"internal_br",
let successors = (successor AnySuccessor:$successPath, AnySuccessor:$errorPath);
}
+// A successor that must be the block immediately following the op's block.
+def FallthroughSuccessor : Successor<
+ CPred<"$_self == ($_op).getBlock()->getNextNode()">,
+ "the fallthrough block (the block immediately following the op's block)">;
+
+// A successor that must be a block listed after the op's block.
+def ForwardSuccessor : Successor<
+ CPred<[{
+ [&] {
+ for (::mlir::Block *block = ($_op).getBlock()->getNextNode(); block;
+ block = block->getNextNode())
+ if (block == $_self)
+ return true;
+ return false;
+ }()
+ }]>,
+ "a forward block (a block listed after the op's block)">;
+
+def TestFallthroughBranchOp : TEST_Op<"fallthrough_br", [Terminator]> {
+ let summary = "branch whose successors are constrained by predicates";
+ let successors = (successor FallthroughSuccessor:$target,
+ VariadicSuccessor<ForwardSuccessor>:$forwardTargets);
+ let assemblyFormat = [{
+ $target `forward` `[` $forwardTargets `]` attr-dict
+ }];
+}
+
def AttrSizedOperandOp : TEST_Op<"attr_sized_operands",
[AttrSizedOperandSegments]> {
let arguments = (ins
diff --git a/mlir/test/mlir-tblgen/constraint-unique.td b/mlir/test/mlir-tblgen/constraint-unique.td
index 3f2e5cd4bfad4..5fdaf368c323a 100644
--- a/mlir/test/mlir-tblgen/constraint-unique.td
+++ b/mlir/test/mlir-tblgen/constraint-unique.td
@@ -87,16 +87,16 @@ def OpC : NS_Op<"op_c"> {
// CHECK: static ::llvm::LogicalResult [[$A_SUCCESSOR_CONSTRAINT:__mlir_ods_local_successor_constraint.*]](
// CHECK: if (!((successorPred(successor, *op)))) {
// CHECK-NEXT: return op->emitOpError("successor #") << successorIndex << " ('"
-// CHECK-NEXT: << successorName << ")' failed to verify constraint: a successor";
+// CHECK-NEXT: << successorName << "') failed to verify constraint: a successor";
/// Test that duplicate successor constraint was not generated.
-// CHECK-NOT: << successorName << ")' failed to verify constraint: a successor";
+// CHECK-NOT: << successorName << "') failed to verify constraint: a successor";
/// Test that a successor constraint with a different description was generated.
// CHECK: static ::llvm::LogicalResult [[$O_SUCCESSOR_CONSTRAINT:__mlir_ods_local_successor_constraint.*]](
// CHECK: if (!((successorPred(successor, *op)))) {
// CHECK-NEXT: return op->emitOpError("successor #") << successorIndex << " ('"
-// CHECK-NEXT: << successorName << ")' failed to verify constraint: another successor";
+// CHECK-NEXT: << successorName << "') failed to verify constraint: another successor";
/// Test that a region contraint was generated.
// CHECK: static ::llvm::LogicalResult [[$A_REGION_CONSTRAINT:__mlir_ods_local_region_constraint.*]](
@@ -131,8 +131,7 @@ def OpC : NS_Op<"op_c"> {
// CHECK: for (auto ®ion : ::llvm::MutableArrayRef((*this)->getRegion(0)))
// CHECK-NEXT: if (::mlir::failed([[$A_REGION_CONSTRAINT]](*this, region, "d", index++)))
// CHECK-NEXT: return ::mlir::failure();
-// CHECK: for (auto *successor : ::llvm::MutableArrayRef(c()))
-// CHECK-NEXT: if (::mlir::failed([[$A_SUCCESSOR_CONSTRAINT]](*this, successor, "c", index++)))
+// CHECK: if (::mlir::failed([[$A_SUCCESSOR_CONSTRAINT]](*this, getC(), "c", index++)))
// CHECK-NEXT: return ::mlir::failure();
/// Test that the op with the same predicates but different with descriptions
@@ -152,6 +151,5 @@ def OpC : NS_Op<"op_c"> {
// CHECK: for (auto ®ion : ::llvm::MutableArrayRef((*this)->getRegion(0)))
// CHECK-NEXT: if (::mlir::failed([[$O_REGION_CONSTRAINT]](*this, region, "d", index++)))
// CHECK-NEXT: return ::mlir::failure();
-// CHECK: for (auto *successor : ::llvm::MutableArrayRef(c()))
-// CHECK-NEXT: if (::mlir::failed([[$O_SUCCESSOR_CONSTRAINT]](*this, successor, "c", index++)))
+// CHECK: if (::mlir::failed([[$O_SUCCESSOR_CONSTRAINT]](*this, getC(), "c", index++)))
// CHECK-NEXT: return ::mlir::failure();
diff --git a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
index 90ada40302296..38f4f9bcc1dbc 100644
--- a/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
+++ b/mlir/tools/mlir-tblgen/OpDefinitionsGen.cpp
@@ -4037,15 +4037,27 @@ void OpEmitter::genRegionVerifier(MethodBody &body) {
}
void OpEmitter::genSuccessorVerifier(MethodBody &body) {
- const char *const verifySuccessor = R"(
+ // Code to verify a variadic successor.
+ //
+ // {0}: The accessor for the successor range.
+ // {1}: The successor constraint.
+ // {2}: The successor's name.
+ const char *const verifyVariadicSuccessor = R"(
for (auto *successor : {0})
if (::mlir::failed({1}(*this, successor, "{2}", index++)))
return ::mlir::failure();
)";
- /// Get a single successor.
- ///
- /// {0}: The successor's name.
- const char *const getSingleSuccessor = "::llvm::MutableArrayRef({0}())";
+ // Code to verify a single successor. The accessor returns a `Block *` by
+ // value, which can't be wrapped in a `MutableArrayRef`, so verify it
+ // directly.
+ //
+ // {0}: The accessor for the successor.
+ // {1}: The successor constraint.
+ // {2}: The successor's name.
+ const char *const verifySingleSuccessor = R"(
+ if (::mlir::failed({1}(*this, {0}, "{2}", index++)))
+ return ::mlir::failure();
+)";
// If we have no successors, there is nothing more to do.
const auto canSkip = [](const NamedSuccessor &successor) {
@@ -4063,13 +4075,12 @@ void OpEmitter::genSuccessorVerifier(MethodBody &body) {
continue;
auto getSuccessor =
- formatv(successor.isVariadic() ? "{0}()" : getSingleSuccessor,
- successor.name)
- .str();
+ formatv("{0}()", op.getGetterName(successor.name)).str();
auto constraintFn =
staticVerifierEmitter.getSuccessorConstraintFn(successor.constraint);
- body << formatv(verifySuccessor, getSuccessor, constraintFn,
- successor.name);
+ body << formatv(successor.isVariadic() ? verifyVariadicSuccessor
+ : verifySingleSuccessor,
+ getSuccessor, constraintFn, successor.name);
}
body << " }\n";
}
More information about the Mlir-commits
mailing list