[Mlir-commits] [mlir] [mlir][tosa] Add error if and level checks for COND_IF & WHILE_LOOP (PR #136194)

[Luke Hutton]

================
@@ -449,6 +449,42 @@ struct TosaValidation : public tosa::impl::TosaValidationBase<TosaValidation> {
     return true;
   }
 
+  // Preform depth-first search in Tosa IR structure to find the maximum nesting
+  // depth. Tosa nesting_depth starts at 0 and increase by one each time a new
+  // nested `region` is encountered.
+
+  static int32_t getMaxNestedDepth(Operation *op) {
+    int32_t depth = 0;
+    for (Region &region : op->getRegions())
+      depth = std::max(depth, getMaxNestedDepth(region));
+    return depth;
+  }
+
+  static int32_t getMaxNestedDepth(Block &block) {
+    int32_t depth = 0;
+    for (Operation &op : block.getOperations())
+      depth = std::max(depth, getMaxNestedDepth(&op));
+    return depth;
+  }
+
+  static int32_t getMaxNestedDepth(Region &region) {
+    int32_t depth = 0;
+    for (Block &block : region.getBlocks())
+      depth = std::max(depth, getMaxNestedDepth(block));
+    // Increase the nested depth.
+    return depth + 1;
+  }
+
+  bool levelCheckMaxNesting(Operation *op) {
----------------
lhutton1 wrote:

Thanks for the explanation! 

> I'm not sure how to do bottom-up traversal in an efficient way. Do you have one off the top of your head?

I was think of visiting the parent operation iteratively and increasing the nesting depth on each iteration, though perhaps there's a complexity here that I didn't consider?

> I would read tosa_nesting_depth as generally increasing its value when entering a new nested TOSA graph, rather than in response to specific operations.

My thinking was that the `tosa_nesting_depth++/--;` lines in the spec are part of the operator pesudo code, meaning they will only be executed only when encountering operations that have these lines (cond_if/while_loop). Your interpretation though does also cover the same cases as well as other operations added in the future that rely on entering a new tosa graph. So happy to stick with that interpretation :)

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