[llvm-branch-commits] [clang] [CIR] Fix cir.try successor regions (PR #231449)
Henrich Lauko via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Tue Oct 13 04:01:41 PDT 2026
https://github.com/xlauko updated https://github.com/llvm/llvm-project/pull/231449
>From 2cac3c7812411deb46ad9962b732ffa88e552869 Mon Sep 17 00:00:00 2001
From: Henrich Lauko <hlauko at nvidia.com>
Date: Tue, 13 Oct 2026 10:40:32 +0000
Subject: [PATCH] [CIR] Fix cir.try successor regions
Stacked on #221222, which adds the `requireAllReachable` flag the tests use.
`TryOp::getSuccessorRegions` reported the try region and every handler region as
alternative entries from the parent, and every region exit as going back to the
parent. That lets a handler run without the try body. A handler runs after the
try body unwinds, not instead of it.
This reports what the op executes. The parent enters the try region. A
`cir.resume` ending the try region unwinds into the handlers, and the exception
type decides which one runs. Every other exit, including a handler's own
`cir.resume`, leaves the op.
A try body ends in `cir.resume` once FlattenCFG has flattened an inner cleanup
scope, or an inner `cir.try` with an unwind or filter handler, and
`CIRTryOpFlattening` then sends each such resume to the handler dispatch block,
so this edge matches the lowering. A try with no handlers has nowhere to unwind
to, so its `cir.resume` leaves the op.
`RegionBranchOpInterface` can't describe a throw from the middle of the try
region, only an exit from a terminator. A handler reached only that way, which
is the common case, ends up with no predecessor. PR #221222 documents the same
limitation for EH cleanups. No CIR pass uses these edges to delete regions. Only
generic MLIR passes such as `-sccp` or `-remove-dead-values`, run by hand
through cir-opt, would treat such a handler as dead.
No edge carries values. Handler entry blocks take a `!cir.eh_token` argument
that is not a successor input, and `ResumeOp::getMutableSuccessorOperands`
forwards nothing.
The unit tests cover a body that yields, a body that resumes into a typed catch
and an unwind handler, a body with one yielding and one resuming block, and a
resume with no handlers. The catch-all test now runs
`verifyControlFlowInterfaceConsistency` with `requireAllReachable` set to false,
since its handler has no predecessor. Its old TODO blamed the handler's
`!cir.eh_token` block argument. That argument still isn't a successor input, but
no edge carries operands, so the check passes and the TODO goes.
Co-Authored-By: Claude Opus 5.5 <noreply at anthropic.com>
---
clang/lib/CIR/Dialect/IR/CIRDialect.cpp | 26 +++--
clang/unittests/CIR/ControlFlowTest.cpp | 122 +++++++++++++++++++++++-
2 files changed, 137 insertions(+), 11 deletions(-)
diff --git a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp
index 9679a577a4831..ba2b51009d994 100644
--- a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp
+++ b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp
@@ -5120,18 +5120,30 @@ LogicalResult cir::TypeInfoAttr::verify(
void cir::TryOp::getSuccessorRegions(
mlir::RegionBranchPoint point,
llvm::SmallVectorImpl<mlir::RegionSuccessor> ®ions) {
- // The `try` and the `catchers` region branch back to the parent operation.
- if (!point.isParent()) {
- regions.emplace_back(getOperation());
+ // Execution always starts in the try region.
+ if (point.isParent()) {
+ regions.emplace_back(&getTryRegion());
return;
}
- regions.push_back(mlir::RegionSuccessor(&getTryRegion()));
-
+ // A cir.resume ending the try region unwinds into the handlers, and the
+ // exception type decides which one runs.
+ //
// TODO(CIR): If we know a target function never throws a specific type, we
// can remove the catch handler.
- for (mlir::Region &handlerRegion : this->getHandlerRegions())
- regions.push_back(mlir::RegionSuccessor(&handlerRegion));
+ mlir::Operation *term = point.getTerminatorPredecessorOrNull();
+ assert(term && "expected a terminator predecessor");
+ if (term->getParentRegion() == &getTryRegion() && isa<cir::ResumeOp>(term) &&
+ !getHandlerRegions().empty()) {
+ for (mlir::Region &handlerRegion : getHandlerRegions())
+ regions.emplace_back(&handlerRegion);
+ return;
+ }
+
+ // Every other exit leaves the operation. An unwind out of the middle of the
+ // try region is not a RegionBranchPoint, so a handler reached only that way
+ // has no predecessor here.
+ regions.emplace_back(getOperation());
}
/// Verify that each cir.init_catch_param in \p handlerRegion, outside any
diff --git a/clang/unittests/CIR/ControlFlowTest.cpp b/clang/unittests/CIR/ControlFlowTest.cpp
index e3a6d46ffdbbb..daec4b003a9f7 100644
--- a/clang/unittests/CIR/ControlFlowTest.cpp
+++ b/clang/unittests/CIR/ControlFlowTest.cpp
@@ -727,15 +727,129 @@ TEST_F(CIRControlFlowTest, TryOpWithCatchAll) {
MutableArrayRef<Region> handlerRegions = tryOp.getHandlerRegions();
ASSERT_EQ(handlerRegions.size(), 1u);
- expectSuccessors(tryOp, RegionBranchPoint::parent(),
- {&tryRegion, &handlerRegions[0]});
+ // The body's cir.yield is a normal exit, so it skips the handler.
+ expectSuccessors(tryOp, RegionBranchPoint::parent(), {&tryRegion});
expectTerminatorSuccessors(tryRegion, {nullptr});
expectTerminatorSuccessors(handlerRegions[0], {nullptr});
EXPECT_FALSE(asRegionBranch(tryOp).hasLoop());
- // TODO: TryOp::getSuccessorInputs returns empty for handler regions that
- // have block arguments, so verifyControlFlowInterfaceConsistency fails.
+ // Only a throw from the middle of the body reaches the handler, and that is
+ // not a branch point, so the handler region has no predecessor.
+ verifyControlFlowInterfaceConsistency(tryOp, /*requireAllReachable=*/false);
+}
+
+TEST_F(CIRControlFlowTest, TryOpResumeInBody) {
+ // A cir.resume left behind in the body by an already-flattened inner
+ // cleanup scope unwinds into the handlers.
+ OwningOpRef<ModuleOp> module = parse(R"CIR(
+ !u8i = !cir.int<u, 8>
+ !void = !cir.void
+ cir.func @f() {
+ cir.scope {
+ cir.try {
+ %tok = cir.eh.initiate cleanup : !cir.eh_token
+ cir.resume %tok : !cir.eh_token
+ } catch [type #cir.global_view<@_ZTIi> : !cir.ptr<!u8i>]
+ (%eh : !cir.eh_token) {
+ %ct, %exn = cir.begin_catch %eh
+ : !cir.eh_token -> (!cir.catch_token, !cir.ptr<!void>)
+ cir.end_catch %ct : !cir.catch_token
+ cir.yield
+ } unwind (%eh : !cir.eh_token) {
+ cir.resume %eh : !cir.eh_token
+ }
+ }
+ cir.return
+ }
+ )CIR");
+ auto tryOp = findFirstOp<cir::TryOp>(*module);
+
+ Region &tryRegion = tryOp.getTryRegion();
+ MutableArrayRef<Region> handlerRegions = tryOp.getHandlerRegions();
+ ASSERT_EQ(handlerRegions.size(), 2u);
+
+ expectSuccessors(tryOp, RegionBranchPoint::parent(), {&tryRegion});
+ expectTerminatorSuccessors(tryRegion,
+ {&handlerRegions[0], &handlerRegions[1]});
+ // A cir.resume in the unwind handler leaves the op like any handler exit.
+ expectTerminatorSuccessors(handlerRegions[0], {nullptr});
+ expectTerminatorSuccessors(handlerRegions[1], {nullptr});
+
+ EXPECT_FALSE(asRegionBranch(tryOp).hasLoop());
+
+ verifyControlFlowInterfaceConsistency(tryOp);
+}
+
+TEST_F(CIRControlFlowTest, TryOpMultiBlockBody) {
+ // One block leaves the try body normally and another unwinds, so only the
+ // unwinding exit reaches the handler.
+ OwningOpRef<ModuleOp> module = parse(R"CIR(
+ !void = !cir.void
+ cir.func @f() {
+ cir.scope {
+ cir.try {
+ %cond = cir.const #cir.bool<true> : !cir.bool
+ cir.brcond %cond ^bb_normal, ^bb_unwind
+ ^bb_normal:
+ cir.yield
+ ^bb_unwind:
+ %tok = cir.eh.initiate cleanup : !cir.eh_token
+ cir.resume %tok : !cir.eh_token
+ } catch all (%eh : !cir.eh_token) {
+ %ct, %exn = cir.begin_catch %eh
+ : !cir.eh_token -> (!cir.catch_token, !cir.ptr<!void>)
+ cir.end_catch %ct : !cir.catch_token
+ cir.yield
+ }
+ }
+ cir.return
+ }
+ )CIR");
+ auto tryOp = findFirstOp<cir::TryOp>(*module);
+ Region &tryRegion = tryOp.getTryRegion();
+ Region &handler = tryOp.getHandlerRegions()[0];
+
+ // The cir.brcond block is not a branch point, so only the two exiting
+ // blocks show up here.
+ SmallVector<RegionBranchTerminatorOpInterface> bodyTerms;
+ for (RegionBranchPoint point :
+ asRegionBranch(tryOp).getAllRegionBranchPoints())
+ if (!point.isParent() &&
+ point.getTerminatorPredecessorOrNull()->getParentRegion() == &tryRegion)
+ bodyTerms.push_back(point.getTerminatorPredecessorOrNull());
+ ASSERT_EQ(bodyTerms.size(), 2u);
+ EXPECT_TRUE(isa<cir::YieldOp>(*bodyTerms[0]));
+ EXPECT_TRUE(isa<cir::ResumeOp>(*bodyTerms[1]));
+
+ expectSuccessors(tryOp, RegionBranchPoint(bodyTerms[0]), {nullptr});
+ expectSuccessors(tryOp, RegionBranchPoint(bodyTerms[1]), {&handler});
+ expectTerminatorSuccessors(handler, {nullptr});
+
+ EXPECT_FALSE(asRegionBranch(tryOp).hasLoop());
+
+ verifyControlFlowInterfaceConsistency(tryOp);
+}
+
+TEST_F(CIRControlFlowTest, TryOpResumeInBodyWithoutHandlers) {
+ OwningOpRef<ModuleOp> module = parse(R"CIR(
+ cir.func @f() {
+ cir.scope {
+ cir.try {
+ %tok = cir.eh.initiate cleanup : !cir.eh_token
+ cir.resume %tok : !cir.eh_token
+ }
+ }
+ cir.return
+ }
+ )CIR");
+ auto tryOp = findFirstOp<cir::TryOp>(*module);
+
+ // With no handler to unwind into, the cir.resume leaves the op.
+ expectSuccessors(tryOp, RegionBranchPoint::parent(), {&tryOp.getTryRegion()});
+ expectTerminatorSuccessors(tryOp.getTryRegion(), {nullptr});
+
+ verifyControlFlowInterfaceConsistency(tryOp);
}
TEST_F(CIRControlFlowTest, CoroutineOp) {
More information about the llvm-branch-commits
mailing list