[clang] [CIR] Switch FlattenCFGPass to no longer use greedy manager. (PR #211368)
Erich Keane via cfe-commits
cfe-commits at lists.llvm.org
Thu Jul 23 06:48:08 PDT 2026
https://github.com/erichkeane updated https://github.com/llvm/llvm-project/pull/211368
>From 770775a58491e1b3a387ce772ed0437c53017867 Mon Sep 17 00:00:00 2001
From: erichkeane <ekeane at nvidia.com>
Date: Tue, 21 Jul 2026 16:15:18 -0700
Subject: [PATCH 1/3] [CIR] Switch FlattenCFGPass to no longer use greedy
manager.
This showed up doing a self-build of MLIR's Presburger
IntegerRelation.cpp, which is a bit pathalogical. It resulted in us
doing a lot of rewrite patterns during flatten, taking about 20s. After
this patch, we're down to sub-1s spent doing that.
This is because applyOpPatternsGreedily was re-enqueing every child opops every
time we modified anything nearby. This caused us in cases where there
were operations that were visited TONS of times just because a parent
got modified.
This patch replaces this with a very simple inside-out iteration of
these operations. The recent loop-op 'cleanup' flattening modification
necessitates us re-visiting these sometimes (hence the loop).
This patch is effectively 'NFC' other than build time, so there really
isn't a test I could write.
AI: Note: I've used Claude Opus 4.8 to help me with this patch. I've
read/attempted to comprehend as much of this as possible, but I'm still
pretty inexperienced as to how to manage passes/etc. Claude promises me
this is the best way, and I haven't been able to find anything better
grepping around other transformations in other projects, but please
comment if you have a better idea!
---
.../lib/CIR/Dialect/Transforms/FlattenCFG.cpp | 57 +++++++++++++------
1 file changed, 40 insertions(+), 17 deletions(-)
diff --git a/clang/lib/CIR/Dialect/Transforms/FlattenCFG.cpp b/clang/lib/CIR/Dialect/Transforms/FlattenCFG.cpp
index 160b6418528fc..64bb5c4e5282f 100644
--- a/clang/lib/CIR/Dialect/Transforms/FlattenCFG.cpp
+++ b/clang/lib/CIR/Dialect/Transforms/FlattenCFG.cpp
@@ -17,9 +17,9 @@
#include "mlir/IR/Builders.h"
#include "mlir/IR/PatternMatch.h"
#include "mlir/Interfaces/SideEffectInterfaces.h"
+#include "mlir/Rewrite/PatternApplicator.h"
#include "mlir/Support/LogicalResult.h"
#include "mlir/Transforms/DialectConversion.h"
-#include "mlir/Transforms/GreedyPatternRewriteDriver.h"
#include "clang/CIR/Dialect/IR/CIRDataLayout.h"
#include "clang/CIR/Dialect/IR/CIRDialect.h"
#include "clang/CIR/Dialect/Passes.h"
@@ -682,9 +682,9 @@ class CIRLoopOpInterfaceFlattening
// exceptions that might be thrown from the step region. Rather than trying
// to figure out all of the cleanup routing here, we sink the condition into
// the body region, hoist the step region (if any) and create a new
- // cir.cleanup.scope enclosing the body region. Subsequent passes of the
- // greedy driver will flatten the cir.cleanup.scope and the loop reusing
- // the normal handlers.
+ // cir.cleanup.scope enclosing the body region. A subsequent sweep of the
+ // pass will flatten the cir.cleanup.scope and the loop reusing the normal
+ // handlers.
if (op.maybeGetCleanup())
return rewriteLoopWithCleanup(op, rewriter);
@@ -2001,20 +2001,43 @@ void populateFlattenCFGPatterns(RewritePatternSet &patterns) {
}
void CIRFlattenCFGPass::runOnOperation() {
- RewritePatternSet patterns(&getContext());
- populateFlattenCFGPatterns(patterns);
-
- // Collect operations to apply patterns.
- llvm::SmallVector<Operation *, 16> ops;
- getOperation()->walk<mlir::WalkOrder::PostOrder>([&](Operation *op) {
- if (isa<IfOp, ScopeOp, SwitchOp, LoopOpInterface, TernaryOp, CleanupScopeOp,
- TryOp>(op))
- ops.push_back(op);
- });
+ RewritePatternSet patternList(&getContext());
+ populateFlattenCFGPatterns(patternList);
+ FrozenRewritePatternSet patterns(std::move(patternList));
+
+ PatternApplicator applicator(patterns);
+ // We need _A_ cost model, and everything here is the same cost-model, so this
+ // is effectively a no-op, but necessary to use the PatternApplicator.
+ applicator.applyDefaultCostModel();
+
+ mlir::PatternRewriter rewriter(&getContext());
+
+
+ bool changed;
+ do {
+ changed = false;
+ // Collect flatten candidates post-order so an inner op is handled before
+ // its parent; op pointers stay valid across the block splits / region
+ // inlines the patterns perform (a pattern only erases the matched op and
+ // its descendants, which are visited first), so the list can be iterated
+ // directly.
+ llvm::SmallVector<Operation *, 16> ops;
+ getOperation()->walk<mlir::WalkOrder::PostOrder>([&](Operation *op) {
+ if (isa<IfOp, ScopeOp, SwitchOp, LoopOpInterface, TernaryOp,
+ CleanupScopeOp, TryOp>(op))
+ ops.push_back(op);
+ });
- // Apply patterns.
- if (applyOpPatternsGreedily(ops, std::move(patterns)).failed())
- signalPassFailure();
+ for (mlir::Operation *op : ops) {
+ rewriter.setInsertionPoint(op);
+ if (mlir::succeeded(applicator.matchAndRewrite(op, rewriter))) {
+ // A vast majority of these don't modify the structured ops. However,
+ // if they do, we have to try again. Store whether we've made any
+ // modifications and try again until we stop changing anything.
+ changed = true;
+ }
+ }
+ } while (changed);
}
} // namespace
>From dc63e4a54afcbad0fc6d36b3f313d29609d3f409 Mon Sep 17 00:00:00 2001
From: erichkeane <ekeane at nvidia.com>
Date: Wed, 22 Jul 2026 15:13:03 -0700
Subject: [PATCH 2/3] Remove extraneous newline, per clang format
---
clang/lib/CIR/Dialect/Transforms/FlattenCFG.cpp | 1 -
1 file changed, 1 deletion(-)
diff --git a/clang/lib/CIR/Dialect/Transforms/FlattenCFG.cpp b/clang/lib/CIR/Dialect/Transforms/FlattenCFG.cpp
index 64bb5c4e5282f..fee319c93eb22 100644
--- a/clang/lib/CIR/Dialect/Transforms/FlattenCFG.cpp
+++ b/clang/lib/CIR/Dialect/Transforms/FlattenCFG.cpp
@@ -2012,7 +2012,6 @@ void CIRFlattenCFGPass::runOnOperation() {
mlir::PatternRewriter rewriter(&getContext());
-
bool changed;
do {
changed = false;
>From 59888d7735f1284cab7ef8c8c09f51577112a264 Mon Sep 17 00:00:00 2001
From: erichkeane <ekeane at nvidia.com>
Date: Thu, 23 Jul 2026 06:47:32 -0700
Subject: [PATCH 3/3] Add mutation-listening type to keep an eye on whether
stuff has changed.
---
.../lib/CIR/Dialect/Transforms/FlattenCFG.cpp | 50 +++++++++++++++----
1 file changed, 41 insertions(+), 9 deletions(-)
diff --git a/clang/lib/CIR/Dialect/Transforms/FlattenCFG.cpp b/clang/lib/CIR/Dialect/Transforms/FlattenCFG.cpp
index fee319c93eb22..eda0b59cc2367 100644
--- a/clang/lib/CIR/Dialect/Transforms/FlattenCFG.cpp
+++ b/clang/lib/CIR/Dialect/Transforms/FlattenCFG.cpp
@@ -2000,6 +2000,41 @@ void populateFlattenCFGPatterns(RewritePatternSet &patterns) {
patterns.getContext());
}
+namespace {
+// An implementation of RewriterBase::Listener that determines whether the IR
+// has been modified since the last time it was 'reset'. At the moment, this is
+// the only use for something like this, but we might wish to move this
+// somewhere if someone else needs similar functionality in the future.
+class MLIRChangedListener final : public mlir::RewriterBase::Listener {
+ bool hasChanged = false;
+
+public:
+ void reset() { hasChanged = false; }
+
+ bool changed() const { return hasChanged; }
+
+ void notifyBlockErased(Block *) override { hasChanged = true; }
+ void notifyOperationModified(Operation *) override { hasChanged = true; }
+ void notifyOperationReplaced(Operation *, Operation *) override {
+ hasChanged = true;
+ }
+ void notifyOperationReplaced(Operation *, ValueRange) override {
+ hasChanged = true;
+ }
+ void notifyOperationErased(Operation *) override { hasChanged = true; }
+
+ // notifyPatternBegin, notifyPatternEnd, notifyMatchFailure all skipped, since
+ // they don't modify.
+ void notifyOperationInserted(Operation *,
+ mlir::IRRewriter::InsertPoint) override {
+ hasChanged = true;
+ }
+ void notifyBlockInserted(Block *, Region *, Region::iterator) override {
+ hasChanged = true;
+ }
+};
+} // namespace
+
void CIRFlattenCFGPass::runOnOperation() {
RewritePatternSet patternList(&getContext());
populateFlattenCFGPatterns(patternList);
@@ -2011,10 +2046,12 @@ void CIRFlattenCFGPass::runOnOperation() {
applicator.applyDefaultCostModel();
mlir::PatternRewriter rewriter(&getContext());
+ MLIRChangedListener changedListener;
+ rewriter.setListener(&changedListener);
- bool changed;
do {
- changed = false;
+ changedListener.reset();
+
// Collect flatten candidates post-order so an inner op is handled before
// its parent; op pointers stay valid across the block splits / region
// inlines the patterns perform (a pattern only erases the matched op and
@@ -2029,14 +2066,9 @@ void CIRFlattenCFGPass::runOnOperation() {
for (mlir::Operation *op : ops) {
rewriter.setInsertionPoint(op);
- if (mlir::succeeded(applicator.matchAndRewrite(op, rewriter))) {
- // A vast majority of these don't modify the structured ops. However,
- // if they do, we have to try again. Store whether we've made any
- // modifications and try again until we stop changing anything.
- changed = true;
- }
+ (void)applicator.matchAndRewrite(op, rewriter);
}
- } while (changed);
+ } while (changedListener.changed());
}
} // namespace
More information about the cfe-commits
mailing list