[clang] [CIR] Fix remaining (part 2) FlattenCFG rewriter contract violations (PR #192503)
Bruno Cardoso Lopes via cfe-commits
cfe-commits at lists.llvm.org
Thu Apr 16 11:20:33 PDT 2026
https://github.com/bcardosolopes created https://github.com/llvm/llvm-project/pull/192503
Fix all 17 remaining MLIR expensive pattern check violations in CIRFlattenCFGPass. Found by MLIR_ENABLE_EXPENSIVE_PATTERN_API_CHECKS=ON.
Fingerprint violations (patterns modifying IR without notifying rewriter):
- Use rewriter.replaceAllUsesWith() instead of direct replaceAllUsesWith() in replaceCallWithTryCall (tests: invoke-attrs.cpp, try-catch.cpp, try-catch-all-with-cleanup.cpp)
- Use rewriter.modifyOpInPlace() for removeCleanupAttr() in TryOp flattening (tests: flatten-try-op.cir, flatten-cleanup-scope-eh.cir, flatten-throwing-in-cleanup.cir)
IR verification failures (invalid intermediate IR after pattern application):
- Expand nested op checks in Switch, Loop, CleanupScope, and TryOp flattening to include all structured CIR ops (ScopeOp, IfOp, TernaryOp, etc.), not just CleanupScopeOp. Break/continue/return inside nested structured ops creates invalid cross-region branches when the enclosing op hasn't been flattened yet (tests: switch.cir, loop.cpp, cleanup-scope-return-in-loop.cpp, partial-array-cleanup.cpp, switch.cpp, typeid.cpp, flatten-cleanup-scope-simple.cir, flatten-cleanup-scope-multi-exit.cir)
- Relax CIR_ResumeOp ParentOneOf to include all structured ops, since cir.resume can temporarily appear inside them during flattening (test: new-delete-deactivation.cpp). Feels weird to do that, but extended documentation to account for.
- Change loop region constraints from SizedRegion<1> to MinSizedRegion<1> for cond/step regions, since cleanup scope flattening inside these regions may create multiple blocks (test: loop-cond-cleanup.cpp)
>From 7db3788246991b913e5e8641f5340378586cb9ba Mon Sep 17 00:00:00 2001
From: Bruno Cardoso Lopes <bruno.cardoso at gmail.com>
Date: Wed, 15 Apr 2026 16:11:51 -0700
Subject: [PATCH] [CIR] Fix remaining (part 2) FlattenCFG pattern rewriter
contract violations
Fix all 17 remaining MLIR expensive pattern check violations in
CIRFlattenCFGPass. Found by MLIR_ENABLE_EXPENSIVE_PATTERN_API_CHECKS=ON.
Fingerprint violations (patterns modifying IR without notifying rewriter):
- Use rewriter.replaceAllUsesWith() instead of direct replaceAllUsesWith()
in replaceCallWithTryCall (tests: invoke-attrs.cpp, try-catch.cpp,
try-catch-all-with-cleanup.cpp)
- Use rewriter.modifyOpInPlace() for removeCleanupAttr() in TryOp
flattening (tests: flatten-try-op.cir, flatten-cleanup-scope-eh.cir,
flatten-throwing-in-cleanup.cir)
IR verification failures (invalid intermediate IR after pattern application):
- Expand nested op checks in Switch, Loop, CleanupScope, and TryOp
flattening to include all structured CIR ops (ScopeOp, IfOp,
TernaryOp, etc.), not just CleanupScopeOp. Break/continue/return
inside nested structured ops creates invalid cross-region branches
when the enclosing op hasn't been flattened yet (tests: switch.cir,
loop.cpp, cleanup-scope-return-in-loop.cpp, partial-array-cleanup.cpp,
switch.cpp, typeid.cpp, flatten-cleanup-scope-simple.cir,
flatten-cleanup-scope-multi-exit.cir)
- Relax CIR_ResumeOp ParentOneOf to include all structured ops, since
cir.resume can temporarily appear inside them during flattening
(test: new-delete-deactivation.cpp)
- Change loop region constraints from SizedRegion<1> to MinSizedRegion<1>
for cond/step regions, since cleanup scope flattening inside these
regions may create multiple blocks (test: loop-cond-cleanup.cpp)
---
clang/include/clang/CIR/Dialect/IR/CIROps.td | 23 +++--
.../lib/CIR/Dialect/Transforms/FlattenCFG.cpp | 95 ++++++++++++-------
2 files changed, 73 insertions(+), 45 deletions(-)
diff --git a/clang/include/clang/CIR/Dialect/IR/CIROps.td b/clang/include/clang/CIR/Dialect/IR/CIROps.td
index 6f8db65acccc9..a525b856e5a65 100644
--- a/clang/include/clang/CIR/Dialect/IR/CIROps.td
+++ b/clang/include/clang/CIR/Dialect/IR/CIROps.td
@@ -1060,7 +1060,10 @@ def CIR_ResumeOp : CIR_Op<"resume", [
"getMutableSuccessorOperands"
]>,
Terminator,
- ParentOneOf<["cir::TryOp", "cir::CleanupScopeOp", "cir::FuncOp"]>
+ ParentOneOf<["cir::TryOp", "cir::CleanupScopeOp", "cir::FuncOp",
+ "cir::ScopeOp", "cir::ForOp", "cir::WhileOp",
+ "cir::DoWhileOp", "cir::SwitchOp", "cir::IfOp",
+ "cir::TernaryOp", "cir::CaseOp"]>
]> {
let summary = "Resumes execution after not catching exceptions";
let description = [{
@@ -1070,11 +1073,11 @@ def CIR_ResumeOp : CIR_Op<"resume", [
`CatchUnwind` region of `cir.try`, where it receives an `!cir.eh_token`
argument representing the in-flight exception.
- During CFG flattening, this operation may appear within a `cir.try`
- operation where it indicates that exception unwinding should continue
- from that point. When the `cir.try` operation is flattened, the resume
- operation will be replaced with a branch to the flattened try operation's
- dispatch block.
+ During CFG flattening, this operation may temporarily appear inside any
+ structured CIR operation (scope, loop, switch, etc.) when an inner cleanup
+ scope is flattened before the enclosing structured op. When the enclosing
+ op is subsequently flattened, the resume will end up in a `cir.try`
+ operation or at function level.
After CFG flattening, this operation appears at the function level (inside
`cir.func`) to indicate that the exception should be re-thrown to the
@@ -2066,7 +2069,7 @@ class CIR_WhileOpBase<string mnemonic> : CIR_LoopOpBase<mnemonic> {
}
def CIR_WhileOp : CIR_WhileOpBase<"while"> {
- let regions = (region SizedRegion<1>:$cond, MinSizedRegion<1>:$body);
+ let regions = (region MinSizedRegion<1>:$cond, MinSizedRegion<1>:$body);
let assemblyFormat = "$cond `do` $body attr-dict";
let description = [{
@@ -2093,7 +2096,7 @@ def CIR_WhileOp : CIR_WhileOpBase<"while"> {
}
def CIR_DoWhileOp : CIR_WhileOpBase<"do"> {
- let regions = (region MinSizedRegion<1>:$body, SizedRegion<1>:$cond);
+ let regions = (region MinSizedRegion<1>:$body, MinSizedRegion<1>:$cond);
let assemblyFormat = " $body `while` $cond attr-dict";
let extraClassDeclaration = [{
@@ -2149,9 +2152,9 @@ def CIR_ForOp : CIR_LoopOpBase<"for"> {
```
}];
- let regions = (region SizedRegion<1>:$cond,
+ let regions = (region MinSizedRegion<1>:$cond,
MinSizedRegion<1>:$body,
- SizedRegion<1>:$step);
+ MinSizedRegion<1>:$step);
let assemblyFormat = [{
`:` `cond` $cond
`body` $body
diff --git a/clang/lib/CIR/Dialect/Transforms/FlattenCFG.cpp b/clang/lib/CIR/Dialect/Transforms/FlattenCFG.cpp
index 48c47deb2bc0a..7889cb69055ef 100644
--- a/clang/lib/CIR/Dialect/Transforms/FlattenCFG.cpp
+++ b/clang/lib/CIR/Dialect/Transforms/FlattenCFG.cpp
@@ -228,13 +228,19 @@ class CIRSwitchOpFlattening : public mlir::OpRewritePattern<cir::SwitchOp> {
mlir::LogicalResult
matchAndRewrite(cir::SwitchOp op,
mlir::PatternRewriter &rewriter) const override {
- // Cleanup scopes must be lowered before the enclosing switch so that
- // break inside them is properly routed through cleanup.
- // Fail the match so the pattern rewriter will process cleanup scopes first.
- bool hasNestedCleanup = op->walk([&](cir::CleanupScopeOp) {
- return mlir::WalkResult::interrupt();
- }).wasInterrupted();
- if (hasNestedCleanup)
+ // All nested structured CIR ops must be flattened before the switch.
+ // Break statements inside nested structured ops (scopes, ifs, ternaries,
+ // cleanup scopes) would create branches to blocks outside those ops'
+ // regions, which is invalid. Fail the match so the pattern rewriter will
+ // process them first.
+ bool hasNestedStructuredOps =
+ op->walk([&](mlir::Operation *nestedOp) {
+ if (isa<cir::ScopeOp, cir::IfOp, cir::TernaryOp,
+ cir::CleanupScopeOp, cir::TryOp>(nestedOp))
+ return mlir::WalkResult::interrupt();
+ return mlir::WalkResult::advance();
+ }).wasInterrupted();
+ if (hasNestedStructuredOps)
return mlir::failure();
llvm::SmallVector<CaseOp> cases;
@@ -432,13 +438,19 @@ class CIRLoopOpInterfaceFlattening
mlir::LogicalResult
matchAndRewrite(cir::LoopOpInterface op,
mlir::PatternRewriter &rewriter) const final {
- // Cleanup scopes must be lowered before the enclosing loop so that
- // break/continue inside them are properly routed through cleanup.
- // Fail the match so the pattern rewriter will process cleanup scopes first.
- bool hasNestedCleanup = op->walk([&](cir::CleanupScopeOp) {
- return mlir::WalkResult::interrupt();
- }).wasInterrupted();
- if (hasNestedCleanup)
+ // All nested structured CIR ops must be flattened before the loop.
+ // Break/continue statements inside nested structured ops (switches, scopes,
+ // ifs, ternaries, cleanup scopes) would create branches to blocks outside
+ // those ops' regions, which is invalid. Fail the match so the pattern
+ // rewriter will process them first.
+ bool hasNestedOps =
+ op->walk([&](mlir::Operation *nestedOp) {
+ if (isa<cir::SwitchOp, cir::ScopeOp, cir::IfOp, cir::TernaryOp,
+ cir::CleanupScopeOp, cir::TryOp>(nestedOp))
+ return mlir::WalkResult::interrupt();
+ return mlir::WalkResult::advance();
+ }).wasInterrupted();
+ if (hasNestedOps)
return mlir::failure();
// Setup CFG blocks.
@@ -703,8 +715,10 @@ static void replaceCallWithTryCall(cir::CallOp callOp, mlir::Block *unwindDest,
}
// Replace uses of the call result with the try_call result.
+ // Use the rewriter API so that the pattern rewriter is notified of the
+ // in-place modifications to each user operation.
if (callOp->getNumResults() > 0)
- callOp->getResult(0).replaceAllUsesWith(tryCallOp.getResult());
+ rewriter.replaceAllUsesWith(callOp->getResult(0), tryCallOp.getResult());
rewriter.eraseOp(callOp);
}
@@ -1406,18 +1420,18 @@ class CIRCleanupScopeOpFlattening
mlir::PatternRewriter &rewriter) const override {
mlir::OpBuilder::InsertionGuard guard(rewriter);
- // Nested cleanup scopes and try operations must be flattened before the
- // enclosing cleanup scope so that EH cleanup inside them is properly
- // handled. Fail the match so the pattern rewriter processes them first.
+ // All nested structured CIR ops must be flattened before the cleanup scope.
+ // Operations like loops, switches, scopes, and ifs may contain exits
+ // (return, break, continue) that the cleanup scope will replace with
+ // branches to the cleanup entry. If those exits are inside a structured
+ // op's region, the branch would reference a block outside that region,
+ // which is invalid. Fail the match so they are processed first.
//
// Before checking, erase any trivially dead nested cleanup scopes. These
// arise from deactivated cleanups (e.g. partial-construction guards for
// lambda captures). The greedy rewriter may have already DCE'd them, but
// when a trivially dead nested op is erased first, the parent isn't always
- // re-added to the worklist, so we handle it here. These types of operations
- // will normally be removed by the canonicalizer, but we handle it here
- // also, because DCE can run between pattern matches in the current pass,
- // and if a trivially dead operation makes it this far, we will fail.
+ // re-added to the worklist, so we handle it here.
llvm::SmallVector<cir::CleanupScopeOp> deadNestedOps;
cleanupOp.getBodyRegion().walk([&](cir::CleanupScopeOp nested) {
if (mlir::isOpTriviallyDead(nested))
@@ -1426,13 +1440,16 @@ class CIRCleanupScopeOpFlattening
for (auto op : deadNestedOps)
rewriter.eraseOp(op);
- bool hasNestedOps = cleanupOp.getBodyRegion()
- .walk([&](mlir::Operation *op) {
- if (isa<cir::CleanupScopeOp, cir::TryOp>(op))
- return mlir::WalkResult::interrupt();
- return mlir::WalkResult::advance();
- })
- .wasInterrupted();
+ bool hasNestedOps =
+ cleanupOp.getBodyRegion()
+ .walk([&](mlir::Operation *op) {
+ if (isa<cir::CleanupScopeOp, cir::TryOp, cir::LoopOpInterface,
+ cir::SwitchOp, cir::ScopeOp, cir::IfOp, cir::TernaryOp>(
+ op))
+ return mlir::WalkResult::interrupt();
+ return mlir::WalkResult::advance();
+ })
+ .wasInterrupted();
if (hasNestedOps)
return mlir::failure();
@@ -1616,13 +1633,19 @@ class CIRTryOpFlattening : public mlir::OpRewritePattern<cir::TryOp> {
mlir::LogicalResult
matchAndRewrite(cir::TryOp tryOp,
mlir::PatternRewriter &rewriter) const override {
- // Nested try ops and cleanup scopes must be flattened before the enclosing
- // try so that EH cleanup inside them is properly handled. Fail the match so
- // the pattern rewriter will process nested ops first.
+ // All nested structured CIR ops must be flattened before the try op.
+ // Cleanup scopes and nested try ops need to be flat so EH cleanup is
+ // properly handled. Other structured ops (scopes, ifs, loops, switches,
+ // ternaries) must be flat because replaceCallWithTryCall creates try_call
+ // ops whose unwind destination is outside the structured op's region,
+ // which would be an invalid cross-region reference.
bool hasNestedOps =
tryOp
->walk([&](mlir::Operation *op) {
- if (isa<cir::CleanupScopeOp, cir::TryOp>(op) && op != tryOp)
+ if (isa<cir::CleanupScopeOp, cir::TryOp, cir::ScopeOp, cir::IfOp,
+ cir::TernaryOp, cir::LoopOpInterface, cir::SwitchOp>(
+ op) &&
+ op != tryOp)
return mlir::WalkResult::interrupt();
return mlir::WalkResult::advance();
})
@@ -1743,8 +1766,10 @@ class CIRTryOpFlattening : public mlir::OpRewritePattern<cir::TryOp> {
// cir.eh.initiate that produced this token. With catch-all, the LLVM
// landingpad needs "catch ptr null" instead of "cleanup".
if (hasCatchAll) {
- if (auto ehInitiate = traceToEhInitiate(resumeOp.getEhToken()))
- ehInitiate.removeCleanupAttr();
+ if (auto ehInitiate = traceToEhInitiate(resumeOp.getEhToken())) {
+ rewriter.modifyOpInPlace(ehInitiate,
+ [&] { ehInitiate.removeCleanupAttr(); });
+ }
}
mlir::Value ehToken = resumeOp.getEhToken();
More information about the cfe-commits
mailing list