[Mlir-commits] [mlir] [mlir][IR] Make verifyDominanceOfContainedRegions iterative (PR #74428)

[llvmlistbot at llvm.org]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-mlir

@llvm/pr-subscribers-mlir-core

Author: Hideto Ueno (uenoku)

<details>
<summary>Changes</summary>

This commit refactors `verifyDominanceOfContainedRegions` to iterative algorithms similar to https://reviews.llvm.org/D154925 to fix stack overflow for deeply nested regions (https://github.com/llvm/circt/issues/5316). There should be no functional change except that this could result in slightly different order of verification. The original order could be used with a few tweaks (using reverse iterator and split the inner most loop into two loops) but I just kept the implementation simple so LMK if it's important to preserve the exact verification order. 



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


1 Files Affected:

- (modified) mlir/lib/IR/Verifier.cpp (+28-28) 


``````````diff
diff --git a/mlir/lib/IR/Verifier.cpp b/mlir/lib/IR/Verifier.cpp
index 0d2fa6486e219..a09b47ee981c9 100644
--- a/mlir/lib/IR/Verifier.cpp
+++ b/mlir/lib/IR/Verifier.cpp
@@ -378,39 +378,39 @@ static void diagnoseInvalidOperandDominance(Operation &op, unsigned operandNo) {
 LogicalResult
 OperationVerifier::verifyDominanceOfContainedRegions(Operation &op,
                                                      DominanceInfo &domInfo) {
-  for (Region &region : op.getRegions()) {
-    // Verify the dominance of each of the held operations.
-    for (Block &block : region) {
-      // Dominance is only meaningful inside reachable blocks.
-      bool isReachable = domInfo.isReachableFromEntry(&block);
-
-      for (Operation &op : block) {
-        if (isReachable) {
-          // Check that operands properly dominate this use.
-          for (const auto &operand : llvm::enumerate(op.getOperands())) {
-            if (domInfo.properlyDominates(operand.value(), &op))
-              continue;
-
-            diagnoseInvalidOperandDominance(op, operand.index());
-            return failure();
+  llvm::SmallVector<Operation *, 8> worklist{&op};
+  while (!worklist.empty()) {
+    auto *op = worklist.pop_back_val();
+    for (auto &region : op->getRegions())
+      for (auto &block : region.getBlocks()) {
+        // Dominance is only meaningful inside reachable blocks.
+        bool isReachable = domInfo.isReachableFromEntry(&block);
+        for (auto &op : block) {
+          if (isReachable) {
+            // Check that operands properly dominate this use.
+            for (const auto &operand : llvm::enumerate(op.getOperands())) {
+              if (domInfo.properlyDominates(operand.value(), &op))
+                continue;
+
+              diagnoseInvalidOperandDominance(op, operand.index());
+              return failure();
+            }
           }
-        }
 
-        // Recursively verify dominance within each operation in the block, even
-        // if the block itself is not reachable, or we are in a region which
-        // doesn't respect dominance.
-        if (verifyRecursively && op.getNumRegions() != 0) {
-          // If this operation is IsolatedFromAbove, then we'll handle it in the
-          // outer verification loop.
-          if (op.hasTrait<OpTrait::IsIsolatedFromAbove>())
-            continue;
-
-          if (failed(verifyDominanceOfContainedRegions(op, domInfo)))
-            return failure();
+          // Recursively verify dominance within each operation in the block,
+          // even if the block itself is not reachable, or we are in a region
+          // which doesn't respect dominance.
+          if (verifyRecursively && op.getNumRegions() != 0) {
+            // If this operation is IsolatedFromAbove, then we'll handle it in
+            // the outer verification loop.
+            if (op.hasTrait<OpTrait::IsIsolatedFromAbove>())
+              continue;
+            worklist.push_back(&op);
+          }
         }
       }
-    }
   }
+
   return success();
 }
 

``````````

</details>


https://github.com/llvm/llvm-project/pull/74428