[mlir][IR][NFC] Move `walk` definitions to header file

[springerm]

Summary

This allows users to provide custom Iterator templates. A new iterator will be added in a subsequent change.

Also rename makeRange to makeIterable and add a test case for the reverse iterator.

[mehdi_amini]

mlir/include/mlir/IR/Visitors.h

190

This makes me think that our walk implementation is recursive right? Seems like a ticking bomb in the compiler :(
How hard will it be to remove the recursion?

[rriddle]

Any idea of how this will affect build times? This code is now going to get instantiated a lot.

[springerm]

In D144887#4156011, @rriddle wrote:

Any idea of how this will affect build times? This code is now going to get instantiated a lot.

It does not affect compile time on my machine. It's a pretty fast machine though (AMD Ryzen Threadripper PRO 3995WX 64-Cores).

BEFORE

➜  llvm-project git:(3f9b0446f9a7) time cmake --build build --target mlir-opt
[3109/3109] Linking CXX executable bin/mlir-opt
cmake --build build --target mlir-opt  13980.53s user 550.07s system 7589% cpu 3:11.46 total

➜  llvm-project git:(3f9b0446f9a7) time cmake --build build --target mlir-opt
[3109/3109] Linking CXX executable bin/mlir-opt
cmake --build build --target mlir-opt  13951.46s user 552.38s system 7599% cpu 3:10.85 total



AFTER

➜  llvm-project git:(269d19604f36) time cmake --build build --target mlir-opt
[3109/3109] Linking CXX executable bin/mlir-opt
cmake --build build --target mlir-opt  13983.18s user 554.72s system 7620% cpu 3:10.78 total

➜  llvm-project git:(269d19604f36) time cmake --build build --target mlir-opt
[3109/3109] Linking CXX executable bin/mlir-opt
cmake --build build --target mlir-opt  13982.50s user 552.88s system 7614% cpu 3:10.90 total

[springerm]

mlir/include/mlir/IR/Visitors.h

190

The post-order traversal makes it quite difficult. I think we would need to maintain a stack of iterators. Actually 3 stacks, one for operation, block and region. Alternatively, one iterator that keeps track of current op, block and region. And with three types of increment: getNextOp, getNextBlock, getNextRegion.

[rriddle]

The failures look real, seems missing clang-format

mlir/include/mlir/IR/Visitors.h

public:
   bool wasSkipped() const { return result == Skip; }
 };
 
 /// Traversal order for region, block and operation walk utilities.
 enum class WalkOrder { PreOrder, PostOrder };
 
 /// This iterator enumerates the elements in "forward" order.
 struct ForwardIterator {
-  template <typename RangeT>
-  static constexpr RangeT &makeRange(RangeT &range) {
+  /// Make operations iterable: return the list of regions.
+  static MutableArrayRef<Region> makeIterable(Operation &range);
+
+  /// Regions and block are already iterable.
+  template <typename T>
+  static constexpr T &makeIterable(T &range) {
     return range;
   }
 };
 
 /// This iterator enumerates elements in "reverse" order. It is a wrapper around
 /// llvm::reverse.
 struct ReverseIterator {
   template <typename RangeT>
-  static constexpr auto makeRange(RangeT &&range) {
+  static constexpr auto makeIterable(RangeT &&range) {
     // llvm::reverse uses RangeT::rbegin and RangeT::rend.
-    return llvm::reverse(std::forward<RangeT>(range));
+    return llvm::reverse(
+        ForwardIterator::makeIterable(std::forward<RangeT>(range)));
   }
 };
 
 /// A utility class to encode the current walk stage for "generic" walkers.
 /// When walking an operation, we can either choose a Pre/Post order walker
 /// which invokes the callback on an operation before/after all its attached
 /// regions have been visited, or choose a "generic" walker where the callback
 /// is invoked on the operation N+1 times where N is the number of regions

 /// lexicographical order) is determined by 'Iterator'. The walk order for
 /// enclosing regions, blocks and operations with respect to their nested ones
 /// is specified by 'order'. These methods are invoked for void-returning
 /// callbacks. A callback on a block or operation is allowed to erase that block
 /// or operation only if the walk is in post-order. See non-void method for
 /// pre-order erasure.
 template <typename Iterator>
 void walk(Operation *op, function_ref<void(Region *)> callback,
-          WalkOrder order);
+          WalkOrder order) {
+  // We don't use early increment for regions because they can't be erased from
+  // a callback.
+  for (auto &region : Iterator::makeIterable(*op)) {
+    if (order == WalkOrder::PreOrder)
+      callback(&region);
+    for (auto &block : Iterator::makeIterable(region)) {
+      for (auto &nestedOp : Iterator::makeIterable(block))
+        walk<Iterator>(&nestedOp, callback, order);
+    }
+    if (order == WalkOrder::PostOrder)
+      callback(&region);
+  }
+}
+
 template <typename Iterator>
-void walk(Operation *op, function_ref<void(Block *)> callback, WalkOrder order);
+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))) {
+      if (order == WalkOrder::PreOrder)
+        callback(&block);
+      for (auto &nestedOp : Iterator::makeIterable(block))
+        walk<Iterator>(&nestedOp, callback, order);
+      if (order == WalkOrder::PostOrder)
+        callback(&block);
+    }
+  }
+}
+
 template <typename Iterator>
 void walk(Operation *op, function_ref<void(Operation *)> callback,
-          WalkOrder order);
+          WalkOrder order) {
+  if (order == WalkOrder::PreOrder)
+    callback(op);
+
+  // TODO: This walk should be iterative over the operations.
+  for (auto &region : Iterator::makeIterable(*op)) {
+    for (auto &block : Iterator::makeIterable(region)) {
+      // Early increment here in the case where the operation is erased.
+      for (auto &nestedOp :
+           llvm::make_early_inc_range(Iterator::makeIterable(block)))
+        walk<Iterator>(&nestedOp, callback, order);
+    }
+  }
+
+  if (order == WalkOrder::PostOrder)
+    callback(op);
+}
+
 /// Walk all of the regions, blocks, or operations nested under (and including)
 /// the given operation. The order in which regions, blocks and operations at
 /// the same nesting level are visited (e.g., lexicographical or reverse
 /// lexicographical order) is determined by 'Iterator'. The walk order for
 /// enclosing regions, blocks and operations with respect to their nested ones
 /// is specified by 'order'. This method is invoked for skippable or
 /// interruptible callbacks. A callback on a block or operation is allowed to
 /// erase that block or operation if either:
 ///   * the walk is in post-order, or
 ///   * the walk is in pre-order and the walk is skipped after the erasure.
 template <typename Iterator>
 WalkResult walk(Operation *op, function_ref<WalkResult(Region *)> callback,
-                WalkOrder order);
+                WalkOrder order) {
+  // We don't use early increment for regions because they can't be erased from
+  // a callback.
+  for (auto &region : Iterator::makeIterable(*op)) {
+    if (order == WalkOrder::PreOrder) {
+      WalkResult result = callback(&region);
+      if (result.wasSkipped())
+        continue;
+      if (result.wasInterrupted())
+        return WalkResult::interrupt();
+    }
+    for (auto &block : Iterator::makeIterable(region)) {
+      for (auto &nestedOp : Iterator::makeIterable(block))
+        if (walk<Iterator>(&nestedOp, callback, order).wasInterrupted())
+          return WalkResult::interrupt();
+    }
+    if (order == WalkOrder::PostOrder) {
+      if (callback(&region).wasInterrupted())
+        return WalkResult::interrupt();
+      // We don't check if this region was skipped because its walk already
+      // finished and the walk will continue with the next region.
+    }
+  }
+  return WalkResult::advance();
+}
+
 template <typename Iterator>
 WalkResult walk(Operation *op, function_ref<WalkResult(Block *)> callback,
-                WalkOrder order);
+                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))) {
+      if (order == WalkOrder::PreOrder) {
+        WalkResult result = callback(&block);
+        if (result.wasSkipped())
+          continue;
+        if (result.wasInterrupted())
+          return WalkResult::interrupt();
+      }
+      for (auto &nestedOp : Iterator::makeIterable(block))
+        if (walk<Iterator>(&nestedOp, callback, order).wasInterrupted())
+          return WalkResult::interrupt();
+      if (order == WalkOrder::PostOrder) {
+        if (callback(&block).wasInterrupted())
+          return WalkResult::interrupt();
+        // We don't check if this block was skipped because its walk already
+        // finished and the walk will continue with the next block.
+      }
+    }
+  }
+  return WalkResult::advance();
+}
+
 template <typename Iterator>
 WalkResult walk(Operation *op, function_ref<WalkResult(Operation *)> callback,
-                WalkOrder order);
+                WalkOrder order) {
+  if (order == WalkOrder::PreOrder) {
+    WalkResult result = callback(op);
+    // If skipped, caller will continue the walk on the next operation.
+    if (result.wasSkipped())
+      return WalkResult::advance();
+    if (result.wasInterrupted())
+      return WalkResult::interrupt();
+  }
+
+  // TODO: This walk should be iterative over the operations.
+  for (auto &region : Iterator::makeIterable(*op)) {
+    for (auto &block : Iterator::makeIterable(region)) {
+      // Early increment here in the case where the operation is erased.
+      for (auto &nestedOp :
+           llvm::make_early_inc_range(Iterator::makeIterable(block))) {
+        if (walk<Iterator>(&nestedOp, callback, order).wasInterrupted())
+          return WalkResult::interrupt();
+      }
+    }
+  }
+
+  if (order == WalkOrder::PostOrder)
+    return callback(op);
+  return WalkResult::advance();
+}
 
 // Below are a set of functions to walk nested operations. Users should favor
 // the direct `walk` methods on the IR classes(Operation/Block/etc) over these
 // methods. They are also templated to allow for statically dispatching based
 // upon the type of the callback function.
 
 /// Walk all of the regions, blocks, or operations nested under (and including)
 /// the given operation. The order in which regions, blocks and operations at

mlir/lib/IR/Visitors.cpp

 //===- Visitors.cpp - MLIR Visitor Utilities ------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "mlir/IR/Visitors.h"
 #include "mlir/IR/Operation.h"
 
 using namespace mlir;
 
 WalkStage::WalkStage(Operation *op)
     : numRegions(op->getNumRegions()), nextRegion(0) {}
 
-/// Walk all of the regions/blocks/operations nested under and including the
-/// given operation. The order in which regions, blocks and operations at the
-/// same nesting level are visited (e.g., lexicographical or reverse
-/// lexicographical order) is determined by 'Iterator'. The walk order for
-/// enclosing regions,  blocks and operations with respect to their nested ones
-/// is specified by 'order'. These methods are invoked for void-returning
-/// callbacks. A callback on a block or operation is allowed to erase that block
-/// or operation only if the walk is in post-order. See non-void method for
-/// pre-order erasure.
-template <typename Iterator>
-void detail::walk(Operation *op, function_ref<void(Region *)> callback,
-                  WalkOrder order) {
-  // We don't use early increment for regions because they can't be erased from
-  // a callback.
-  MutableArrayRef<Region> regions = op->getRegions();
-  for (auto &region : Iterator::makeRange(regions)) {
-    if (order == WalkOrder::PreOrder)
-      callback(&region);
-    for (auto &block : Iterator::makeRange(region)) {
-      for (auto &nestedOp : Iterator::makeRange(block))
-        walk<Iterator>(&nestedOp, callback, order);
-    }
-    if (order == WalkOrder::PostOrder)
-      callback(&region);
-  }
-}
-// Explicit template instantiations for all supported iterators.
-template void detail::walk<ForwardIterator>(Operation *,
-                                            function_ref<void(Region *)>,
-                                            WalkOrder);
-template void detail::walk<ReverseIterator>(Operation *,
-                                            function_ref<void(Region *)>,
-                                            WalkOrder);
-
-template <typename Iterator>
-void detail::walk(Operation *op, function_ref<void(Block *)> callback,
-                  WalkOrder order) {
-  MutableArrayRef<Region> regions = op->getRegions();
-  for (auto &region : Iterator::makeRange(regions)) {
-    // Early increment here in the case where the block is erased.
-    for (auto &block :
-         llvm::make_early_inc_range(Iterator::makeRange(region))) {
-      if (order == WalkOrder::PreOrder)
-        callback(&block);
-      for (auto &nestedOp : Iterator::makeRange(block))
-        walk<Iterator>(&nestedOp, callback, order);
-      if (order == WalkOrder::PostOrder)
-        callback(&block);
-    }
-  }
-}
-// Explicit template instantiations for all supported iterators.
-template void detail::walk<ForwardIterator>(Operation *,
-                                            function_ref<void(Block *)>,
-                                            WalkOrder);
-template void detail::walk<ReverseIterator>(Operation *,
-                                            function_ref<void(Block *)>,
-                                            WalkOrder);
-
-template <typename Iterator>
-void detail::walk(Operation *op, function_ref<void(Operation *)> callback,
-                  WalkOrder order) {
-  if (order == WalkOrder::PreOrder)
-    callback(op);
-
-  // TODO: This walk should be iterative over the operations.
-  MutableArrayRef<Region> regions = op->getRegions();
-  for (auto &region : Iterator::makeRange(regions)) {
-    for (auto &block : Iterator::makeRange(region)) {
-      // Early increment here in the case where the operation is erased.
-      for (auto &nestedOp :
-           llvm::make_early_inc_range(Iterator::makeRange(block)))
-        walk<Iterator>(&nestedOp, callback, order);
-    }
-  }
-
-  if (order == WalkOrder::PostOrder)
-    callback(op);
-}
-// Explicit template instantiations for all supported iterators.
-template void detail::walk<ForwardIterator>(Operation *,
-                                            function_ref<void(Operation *)>,
-                                            WalkOrder);
-template void detail::walk<ReverseIterator>(Operation *,
-                                            function_ref<void(Operation *)>,
-                                            WalkOrder);
+MutableArrayRef<Region> ForwardIterator::makeIterable(Operation &range) {
+  return range.getRegions();
 
 void detail::walk(Operation *op,
                   function_ref<void(Operation *, const WalkStage &)> callback) {
   WalkStage stage(op);
 
   for (Region &region : op->getRegions()) {
     // Invoke callback on the parent op before visiting each child region.
     callback(op, stage);
     stage.advance();
 
     for (Block &block : region) {
       for (Operation &nestedOp : block)
         walk(&nestedOp, callback);
     }
   }
 
   // Invoke callback after all regions have been visited.
   callback(op, stage);
 }
 
-/// Walk all of the regions/blocks/operations nested under and including the
-/// given operation. These functions walk operations until an interrupt result
-/// is returned by the callback. Walks on regions, blocks and operations may
-/// also be skipped if the callback returns a skip result. Regions, blocks and
-/// operations at the same nesting level are visited in lexicographical order.
-/// The walk order for enclosing regions, blocks and operations with respect to
-/// their nested ones is specified by 'order'. A callback on a block or
-/// operation is allowed to erase that block or operation if either:
-///   * the walk is in post-order, or
-///   * the walk is in pre-order and the walk is skipped after the erasure.
-template <typename Iterator>
-WalkResult detail::walk(Operation *op,
-                        function_ref<WalkResult(Region *)> callback,
-                        WalkOrder order) {
-  // We don't use early increment for regions because they can't be erased from
-  // a callback.
-  MutableArrayRef<Region> regions = op->getRegions();
-  for (auto &region : Iterator::makeRange(regions)) {
-    if (order == WalkOrder::PreOrder) {
-      WalkResult result = callback(&region);
-      if (result.wasSkipped())
-        continue;
-      if (result.wasInterrupted())
-        return WalkResult::interrupt();
-    }
-    for (auto &block : Iterator::makeRange(region)) {
-      for (auto &nestedOp : Iterator::makeRange(block))
-        if (walk<Iterator>(&nestedOp, callback, order).wasInterrupted())
-          return WalkResult::interrupt();
-    }
-    if (order == WalkOrder::PostOrder) {
-      if (callback(&region).wasInterrupted())
-        return WalkResult::interrupt();
-      // We don't check if this region was skipped because its walk already
-      // finished and the walk will continue with the next region.
-    }
-  }
-  return WalkResult::advance();
-}
-// Explicit template instantiations for all supported iterators.
-template WalkResult
-detail::walk<ForwardIterator>(Operation *, function_ref<WalkResult(Region *)>,
-                              WalkOrder);
-template WalkResult
-detail::walk<ReverseIterator>(Operation *, function_ref<WalkResult(Region *)>,
-                              WalkOrder);
-
-template <typename Iterator>
-WalkResult detail::walk(Operation *op,
-                        function_ref<WalkResult(Block *)> callback,
-                        WalkOrder order) {
-  MutableArrayRef<Region> regions = op->getRegions();
-  for (auto &region : Iterator::makeRange(regions)) {
-    // Early increment here in the case where the block is erased.
-    for (auto &block :
-         llvm::make_early_inc_range(Iterator::makeRange(region))) {
-      if (order == WalkOrder::PreOrder) {
-        WalkResult result = callback(&block);
-        if (result.wasSkipped())
-          continue;
-        if (result.wasInterrupted())
-          return WalkResult::interrupt();
-      }
-      for (auto &nestedOp : Iterator::makeRange(block))
-        if (walk<Iterator>(&nestedOp, callback, order).wasInterrupted())
-          return WalkResult::interrupt();
-      if (order == WalkOrder::PostOrder) {
-        if (callback(&block).wasInterrupted())
-          return WalkResult::interrupt();
-        // We don't check if this block was skipped because its walk already
-        // finished and the walk will continue with the next block.
-      }
-    }
-  }
-  return WalkResult::advance();
-}
-// Explicit template instantiations for all supported iterators.
-template WalkResult
-detail::walk<ForwardIterator>(Operation *, function_ref<WalkResult(Block *)>,
-                              WalkOrder);
-template WalkResult
-detail::walk<ReverseIterator>(Operation *, function_ref<WalkResult(Block *)>,
-                              WalkOrder);
-
-template <typename Iterator>
-WalkResult detail::walk(Operation *op,
-                        function_ref<WalkResult(Operation *)> callback,
-                        WalkOrder order) {
-  if (order == WalkOrder::PreOrder) {
-    WalkResult result = callback(op);
-    // If skipped, caller will continue the walk on the next operation.
-    if (result.wasSkipped())
-      return WalkResult::advance();
-    if (result.wasInterrupted())
-      return WalkResult::interrupt();
-  }
-
-  // TODO: This walk should be iterative over the operations.
-  MutableArrayRef<Region> regions = op->getRegions();
-  for (auto &region : Iterator::makeRange(regions)) {
-    for (auto &block : Iterator::makeRange(region)) {
-      // Early increment here in the case where the operation is erased.
-      for (auto &nestedOp :
-           llvm::make_early_inc_range(Iterator::makeRange(block))) {
-        if (walk<Iterator>(&nestedOp, callback, order).wasInterrupted())
-          return WalkResult::interrupt();
-      }
-    }
-  }
-
-  if (order == WalkOrder::PostOrder)
-    return callback(op);
-  return WalkResult::advance();
-}
-// Explicit template instantiations for all supported iterators.
-template WalkResult
-detail::walk<ForwardIterator>(Operation *,
-                              function_ref<WalkResult(Operation *)>, WalkOrder);
-template WalkResult
-detail::walk<ReverseIterator>(Operation *,
-                              function_ref<WalkResult(Operation *)>, WalkOrder);
-
 WalkResult detail::walk(
     Operation *op,
     function_ref<WalkResult(Operation *, const WalkStage &)> callback) {
   WalkStage stage(op);
 
   for (Region &region : op->getRegions()) {
     // Invoke callback on the parent op before visiting each child region.
     WalkResult result = callback(op, stage);

mlir/test/IR/visitors.mlir

 
 // CHECK-LABEL: Region post-order visits
 // CHECK:       Visiting region 0 from operation 'scf.if'
 // CHECK:       Visiting region 1 from operation 'scf.if'
 // CHECK:       Visiting region 0 from operation 'scf.for'
 // CHECK:       Visiting region 0 from operation 'func.func'
 // CHECK:       Visiting region 0 from operation 'builtin.module'
 
+// CHECK-LABEL: Op reverse post-order visits
+// CHECK:       Visiting op 'func.return'
+// CHECK:       Visiting op 'scf.yield'
+// CHECK:       Visiting op 'use3'
+// CHECK:       Visiting op 'scf.yield'
+// CHECK:       Visiting op 'use2'
+// CHECK:       Visiting op 'scf.yield'
+// CHECK:       Visiting op 'use1'
+// CHECK:       Visiting op 'scf.if'
+// CHECK:       Visiting op 'use0'
+// CHECK:       Visiting op 'scf.for'
+// CHECK:       Visiting op 'arith.constant'
+// CHECK:       Visiting op 'arith.constant'
+// CHECK:       Visiting op 'arith.constant'
+// CHECK:       Visiting op 'func.func'
+// CHECK:       Visiting op 'builtin.module'
+
 // CHECK-LABEL: Op pre-order erasures
 // CHECK:       Erasing op 'scf.for'
 // CHECK:       Erasing op 'func.return'
 
 // CHECK-LABEL: Block pre-order erasures
 // CHECK:       Erasing block ^bb0 from region 0 from operation 'scf.for'
 
 // CHECK-LABEL: Op post-order erasures (skip)

 // CHECK:       Visiting block ^bb0 from region 0 from operation 'func.func'
 // CHECK:       Visiting block ^bb0 from region 0 from operation 'builtin.module'
 
 // CHECK-LABEL: Region post-order visits
 // CHECK:       Visiting region 0 from operation 'regionOp0'
 // CHECK:       Visiting region 0 from operation 'func.func'
 // CHECK:       Visiting region 0 from operation 'builtin.module'
 
+// CHECK-LABEL: Op reverse post-order visits
+// CHECK:       Visiting op 'func.return'
+// CHECK:       Visiting op 'op2'
+// CHECK:       Visiting op 'cf.br'
+// CHECK:       Visiting op 'op1'
+// CHECK:       Visiting op 'cf.br'
+// CHECK:       Visiting op 'op0'
+// CHECK:       Visiting op 'regionOp0'
+// CHECK:       Visiting op 'func.func'
+// CHECK:       Visiting op 'builtin.module'
+
+// CHECK-LABEL: Block reverse post-order visits
+// CHECK:       Visiting block ^bb2 from region 0 from operation 'regionOp0'
+// CHECK:       Visiting block ^bb1 from region 0 from operation 'regionOp0'
+// CHECK:       Visiting block ^bb0 from region 0 from operation 'regionOp0'
+// CHECK:       Visiting block ^bb0 from region 0 from operation 'func.func'
+// CHECK:       Visiting block ^bb0 from region 0 from operation 'builtin.module'
+
+// CHECK-LABEL: Region reverse post-order visits
+// CHECK:       Visiting region 0 from operation 'regionOp0'
+// CHECK:       Visiting region 0 from operation 'func.func'
+// CHECK:       Visiting region 0 from operation 'builtin.module'
+
 // CHECK-LABEL: Op pre-order erasures (skip)
 // CHECK:       Erasing op 'regionOp0'
 // CHECK:       Erasing op 'func.return'
 
 // CHECK-LABEL: Block pre-order erasures (skip)
 // CHECK:       Erasing block ^bb0 from region 0 from operation 'regionOp0'
 // CHECK:       Erasing block ^bb0 from region 0 from operation 'regionOp0'
 // CHECK:       Erasing block ^bb0 from region 0 from operation 'regionOp0'

mlir/test/lib/IR/TestVisitors.cpp

llvm::outs() << "Op post-order visits"
                << "\n";
   op->walk<WalkOrder::PostOrder>(opPure);
   llvm::outs() << "Block post-order visits"
                << "\n";
   op->walk<WalkOrder::PostOrder>(blockPure);
   llvm::outs() << "Region post-order visits"
                << "\n";
   op->walk<WalkOrder::PostOrder>(regionPure);
+
+  llvm::outs() << "Op reverse post-order visits"
+               << "\n";
+  op->walk<WalkOrder::PostOrder, ReverseIterator>(opPure);
+  llvm::outs() << "Block reverse post-order visits"
+               << "\n";
+  op->walk<WalkOrder::PostOrder, ReverseIterator>(blockPure);
+  llvm::outs() << "Region reverse post-order visits"
+               << "\n";
+  op->walk<WalkOrder::PostOrder, ReverseIterator>(regionPure);
 }
 
 /// Tests erasure callbacks that skip the walk.
 static void testSkipErasureCallbacks(Operation *op) {
   auto skipOpErasure = [](Operation *op) {
     // Do not erase module and module children operations. Otherwise, there
     // wouldn't be too much to test in pre-order.
     if (isa<ModuleOp>(op) || isa<ModuleOp>(op->getParentOp()))