https://github.com/xlauko updated https://github.com/llvm/llvm-project/pull/231449
>From 4d9e6d7b5d25f1a7bc0bcfa7843e2e9d1f0277ad Mon Sep 17 00:00:00 2001 From: Henrich Lauko <[email protected]> Date: Tue, 13 Oct 2026 10:40:32 +0000 Subject: [PATCH] [CIR] Fix cir.try successor regions `TryOp::getSuccessorRegions` listed the try region and every handler as entries from the parent, so a handler could run without the try body. Now the parent enters the try region, a `cir.resume` ending the try region unwinds into the handlers, and every other exit leaves the op. This matches `CIRTryOpFlattening`, which branches such resumes to the handler dispatch block. A `cir.try` with no handlers has nowhere to unwind to, so a `cir.resume` in its body leaves the op. The interface only describes exits through a region terminator. A throw from a call inside the try body is not one, so it gets no edge, and a handler reached only that way has no predecessor. The catch-all unit test therefore runs `verifyControlFlowInterfaceConsistency` with `requireAllReachable` set to false, which replaces its TODO. Co-Authored-By: Claude Opus 5.5 <[email protected]> --- clang/lib/CIR/Dialect/IR/CIRDialect.cpp | 28 +++-- clang/unittests/CIR/ControlFlowTest.cpp | 140 +++++++++++++++++++++--- 2 files changed, 147 insertions(+), 21 deletions(-) diff --git a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp index f564eee053705..fb00f640dae22 100644 --- a/clang/lib/CIR/Dialect/IR/CIRDialect.cpp +++ b/clang/lib/CIR/Dialect/IR/CIRDialect.cpp @@ -777,7 +777,7 @@ cir::ConditionOp::getMutableSuccessorOperands(RegionSuccessor point) { MutableOperandRange cir::ResumeOp::getMutableSuccessorOperands(RegionSuccessor point) { - // The eh_token operand is not forwarded to the parent region. + // The eh_token operand is not modeled as forwarded to any successor. return MutableOperandRange(getOperation(), 0, 0); } @@ -5107,18 +5107,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..1da98b88fce58 100644 --- a/clang/unittests/CIR/ControlFlowTest.cpp +++ b/clang/unittests/CIR/ControlFlowTest.cpp @@ -67,6 +67,19 @@ static RegionBranchTerminatorOpInterface getTerminator(Region ®ion) { return branchTerm; } +/// Return the RegionBranchTerminatorOpInterface terminators of `region`'s +/// blocks, in block order. These are the region's branch points. A block that +/// ends in a plain branch such as cir.brcond is not one and is skipped. +static SmallVector<RegionBranchTerminatorOpInterface> +getExitTerminators(Region ®ion) { + SmallVector<RegionBranchTerminatorOpInterface> terms; + for (Block &block : region) + if (auto term = + dyn_cast<RegionBranchTerminatorOpInterface>(block.getTerminator())) + terms.push_back(term); + return terms; +} + /// Check that the terminator of `region` has the given successor regions. static void expectTerminatorSuccessors(Region ®ion, ArrayRef<Region *> expected) { @@ -640,15 +653,8 @@ TEST_F(CIRControlFlowTest, CleanupScopeOpMultiBlockBodyNormal) { expectSuccessors(cleanupScopeOp, RegionBranchPoint::parent(), {&cleanupScopeOp.getBodyRegion()}); - // 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(cleanupScopeOp).getAllRegionBranchPoints()) - if (!point.isParent() && - point.getTerminatorPredecessorOrNull()->getParentRegion() == - &cleanupScopeOp.getBodyRegion()) - bodyTerms.push_back(point.getTerminatorPredecessorOrNull()); + SmallVector<RegionBranchTerminatorOpInterface> bodyTerms = + getExitTerminators(cleanupScopeOp.getBodyRegion()); ASSERT_EQ(bodyTerms.size(), 2u); EXPECT_TRUE(isa<cir::YieldOp>(*bodyTerms[0])); EXPECT_TRUE(isa<cir::ResumeOp>(*bodyTerms[1])); @@ -727,15 +733,123 @@ 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]; + + SmallVector<RegionBranchTerminatorOpInterface> bodyTerms = + getExitTerminators(tryRegion); + 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) { _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
