Skip to content

[MLIR] Fix walk() after PostOrderTraversal change#191357

Merged
aengelke merged 1 commit into
mainfrom
users/aengelke/spr/mlir-fix-walk-after-postordertraversal-change
Apr 10, 2026
Merged

[MLIR] Fix walk() after PostOrderTraversal change#191357
aengelke merged 1 commit into
mainfrom
users/aengelke/spr/mlir-fix-walk-after-postordertraversal-change

Conversation

@aengelke
Copy link
Copy Markdown
Contributor

make_early_inc_range doesn't keep the range alive, only the iterators.
This breaks with the recent PostOrderTraversal change, which no longer
stores the state in the iterators. Store the range in a variable to keep
it alive for the entire loop.

Fixup of #191047 / 691a130.

Created using spr 1.3.8-wip
@aengelke aengelke requested a review from nikic April 10, 2026 08:00
@llvmbot llvmbot added mlir:core MLIR Core Infrastructure mlir labels Apr 10, 2026
@llvmbot
Copy link
Copy Markdown
Member

llvmbot commented Apr 10, 2026

@llvm/pr-subscribers-mlir

@llvm/pr-subscribers-mlir-core

Author: Alexis Engelke (aengelke)

Changes

make_early_inc_range doesn't keep the range alive, only the iterators.
This breaks with the recent PostOrderTraversal change, which no longer
stores the state in the iterators. Store the range in a variable to keep
it alive for the entire loop.

Fixup of #191047 / 691a130.


Full diff: https://github.com/llvm/llvm-project/pull/191357.diff

1 Files Affected:

  • (modified) mlir/include/mlir/IR/Visitors.h (+6-4)
diff --git a/mlir/include/mlir/IR/Visitors.h b/mlir/include/mlir/IR/Visitors.h
index 893f66ae33deb..907f470c0d248 100644
--- a/mlir/include/mlir/IR/Visitors.h
+++ b/mlir/include/mlir/IR/Visitors.h
@@ -120,8 +120,9 @@ void walk(Operation *op, function_ref<void(Block *)> callback,
           WalkOrder order) {
   for (auto &region : Iterator::makeIterable(*op)) {
     // Early increment here in the case where the block is erased.
-    for (auto &block :
-         llvm::make_early_inc_range(Iterator::makeIterable(region))) {
+    // PostOrderTraversal keeps state outside of iterators, so store it here.
+    auto &&It = Iterator::makeIterable(region);
+    for (auto &block : llvm::make_early_inc_range(It)) {
       if (order == WalkOrder::PreOrder)
         callback(&block);
       for (auto &nestedOp : Iterator::makeIterable(block))
@@ -195,8 +196,9 @@ WalkResult walk(Operation *op, function_ref<WalkResult(Block *)> callback,
                 WalkOrder order) {
   for (auto &region : Iterator::makeIterable(*op)) {
     // Early increment here in the case where the block is erased.
-    for (auto &block :
-         llvm::make_early_inc_range(Iterator::makeIterable(region))) {
+    // PostOrderTraversal keeps state outside of iterators, so store it here.
+    auto &&It = Iterator::makeIterable(region);
+    for (auto &block : llvm::make_early_inc_range(It)) {
       if (order == WalkOrder::PreOrder) {
         WalkResult result = callback(&block);
         if (result.wasSkipped())

@aengelke aengelke enabled auto-merge (squash) April 10, 2026 08:10
@aengelke aengelke merged commit 9f9f0dc into main Apr 10, 2026
13 checks passed
@aengelke aengelke deleted the users/aengelke/spr/mlir-fix-walk-after-postordertraversal-change branch April 10, 2026 08:20
@aengelke
Copy link
Copy Markdown
Contributor Author

(For reference: using C++23 (P2718R0) would fix this problem as well, but it'll be a long time until we can use C++23...)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

mlir:core MLIR Core Infrastructure mlir

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants