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

[Hideto Ueno]

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

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. 



>From b5212f07c7b1d2bd07dae372a74328192dee17ee Mon Sep 17 00:00:00 2001
From: Hideto Ueno <uenoku.tokotoko at gmail.com>
Date: Mon, 4 Dec 2023 22:59:21 -0800
Subject: [PATCH] [mlir][IR] Make verifyDominanceOfContainedRegions iterative

This commit refactors `verifyDominanceOfContainedRegions` to iterative
algorithms. This change coiuld result in slightly different order of
verification.
---
 mlir/lib/IR/Verifier.cpp | 56 ++++++++++++++++++++--------------------
 1 file changed, 28 insertions(+), 28 deletions(-)

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();
 }