[Mlir-commits] [mlir] [mlir][tosa] Fix validation check on controlflow operators (PR #159754)

[Luke Hutton]

https://github.com/lhutton1 updated https://github.com/llvm/llvm-project/pull/159754

>From 1ae1cf8b89ecbca1ad240b0325dc3e633ebb8c1d Mon Sep 17 00:00:00 2001
From: Luke Hutton <luke.hutton at arm.com>
Date: Fri, 19 Sep 2025 10:43:41 +0000
Subject: [PATCH] [mlir][tosa] Fix validation check on controlflow operators

Previoulsy the error_if check for controlflow operators would
silently fail on valid controflow operators. This was due to
incorrect return logic in the validation function. This commit
fixes that logic.

Change-Id: I3c69c726028ff387202eef1d1f56b3c40ce80157
---
 .../Tosa/Transforms/TosaValidation.cpp        |  8 ++---
 mlir/test/Dialect/Tosa/error_if_check.mlir    | 33 ------------------
 .../Tosa/tosa-validation-valid-strict.mlir    | 34 +++++++++++++++++++
 3 files changed, 38 insertions(+), 37 deletions(-)
 create mode 100644 mlir/test/Dialect/Tosa/tosa-validation-valid-strict.mlir

diff --git a/mlir/lib/Dialect/Tosa/Transforms/TosaValidation.cpp b/mlir/lib/Dialect/Tosa/Transforms/TosaValidation.cpp
index 790bbf77877bc..e6091df367754 100644
--- a/mlir/lib/Dialect/Tosa/Transforms/TosaValidation.cpp
+++ b/mlir/lib/Dialect/Tosa/Transforms/TosaValidation.cpp
@@ -1257,8 +1257,8 @@ bool checkErrorIfCondIf(Operation *op) {
   //   tosa.yield %arg4
   // }
 
-  return failed(checkIsolatedRegion(op, ifOp.getThenGraph(), "then")) ||
-         failed(checkIsolatedRegion(op, ifOp.getElseGraph(), "else"));
+  return succeeded(checkIsolatedRegion(op, ifOp.getThenGraph(), "then")) &&
+         succeeded(checkIsolatedRegion(op, ifOp.getElseGraph(), "else"));
 }
 
 bool checkErrorIfWhileLoop(Operation *op) {
@@ -1266,8 +1266,8 @@ bool checkErrorIfWhileLoop(Operation *op) {
   if (!whileOp)
     return true;
 
-  return failed(checkIsolatedRegion(op, whileOp.getCondGraph(), "cond")) ||
-         failed(checkIsolatedRegion(op, whileOp.getBodyGraph(), "body"));
+  return succeeded(checkIsolatedRegion(op, whileOp.getCondGraph(), "cond")) &&
+         succeeded(checkIsolatedRegion(op, whileOp.getBodyGraph(), "body"));
 }
 
 bool checkErrorIfScatter(Operation *op) {
diff --git a/mlir/test/Dialect/Tosa/error_if_check.mlir b/mlir/test/Dialect/Tosa/error_if_check.mlir
index 290773b23193f..2f9421c43d2fb 100644
--- a/mlir/test/Dialect/Tosa/error_if_check.mlir
+++ b/mlir/test/Dialect/Tosa/error_if_check.mlir
@@ -269,20 +269,6 @@ func.func @test_cond_if_simplified_form_not_isolated_from_above(%arg0: tensor<f3
 
 // -----
 
-// Check isolated cond_if's are valid
-func.func @test_cond_if_isolated_from_above(%arg0: tensor<f32>, %arg1: tensor<f32>, %arg2: tensor<i1>) -> tensor<f32> {
-  %0 = "tosa.cond_if"(%arg2, %arg0, %arg1) ({
-    ^bb0(%arg3: tensor<f32>, %arg4: tensor<f32>):
-      tosa.yield %arg3 : tensor<f32>
-    },  {
-    ^bb0(%arg3: tensor<f32>, %arg4: tensor<f32>):
-      tosa.yield %arg4 : tensor<f32>
-    }) : (tensor<i1>, tensor<f32>, tensor<f32>) -> tensor<f32>
-  return %0 : tensor<f32>
-}
-
-// -----
-
 func.func @test_while_loop_cond_not_isolated_from_above(%arg0: tensor<i32>, %arg1: tensor<i32>, %arg2: tensor<f32>) {
   %0 = "tosa.const"() {values = dense<0> : tensor<i32>} : () -> tensor<i32>
   // expected-error at +1 {{'tosa.while_loop' op is not conformant to the TOSA specification. It requires the 'cond' region is isolated from above.}}
@@ -318,22 +304,3 @@ func.func @test_while_loop_body_not_isolated_from_above(%arg0: tensor<i32>, %arg
   }) : (tensor<i32>) -> (tensor<i32>)
   return
 }
-
-// -----
-
-// Check isolated while_loops are valid
-func.func @test_while_loop_isolated_from_above(%arg0: tensor<f32>, %arg1: tensor<i32>) {
-  %0 = "tosa.const"() {values = dense<0> : tensor<i32>} : () -> tensor<i32>
-  %1:3 = "tosa.while_loop"(%0, %arg0, %arg1) ({
-  ^bb0(%arg3: tensor<i32>, %arg4: tensor<f32>, %arg5: tensor<i32>):
-    %2 = "tosa.greater_equal"(%arg3, %arg5) : (tensor<i32>, tensor<i32>) -> tensor<i1>
-    %3 = "tosa.logical_not"(%2) : (tensor<i1>) -> tensor<i1>
-    "tosa.yield"(%3) : (tensor<i1>) -> ()
-  },  {
-  ^bb0(%arg3: tensor<i32>, %arg4: tensor<f32>, %arg5: tensor<i32>):
-    %2 = "tosa.const"() {values = dense<1> : tensor<i32>} : () -> tensor<i32>
-    %3 = "tosa.add"(%arg3, %2) : (tensor<i32>, tensor<i32>) -> tensor<i32>
-    "tosa.yield"(%3, %arg4, %arg5) : (tensor<i32>, tensor<f32>, tensor<i32>) -> ()
-  }) : (tensor<i32>, tensor<f32>, tensor<i32>) -> (tensor<i32>, tensor<f32>, tensor<i32>)
-  return
-}
diff --git a/mlir/test/Dialect/Tosa/tosa-validation-valid-strict.mlir b/mlir/test/Dialect/Tosa/tosa-validation-valid-strict.mlir
new file mode 100644
index 0000000000000..f05ae7f58261d
--- /dev/null
+++ b/mlir/test/Dialect/Tosa/tosa-validation-valid-strict.mlir
@@ -0,0 +1,34 @@
+// RUN: mlir-opt %s -split-input-file -verify-diagnostics --tosa-validate="profile=pro_int,pro_fp extension=int16,int4,bf16,fp8e4m3,fp8e5m2,fft,variable,controlflow,doubleround,inexactround strict-op-spec-alignment" | FileCheck %s
+
+// -----
+
+// CHECK-LABEL: test_cond_if_isolated_from_above
+func.func @test_cond_if_isolated_from_above(%arg0: tensor<f32>, %arg1: tensor<f32>, %arg2: tensor<i1>) -> tensor<f32> {
+  %0 = "tosa.cond_if"(%arg2, %arg0, %arg1) ({
+    ^bb0(%arg3: tensor<f32>, %arg4: tensor<f32>):
+      tosa.yield %arg3 : tensor<f32>
+    },  {
+    ^bb0(%arg3: tensor<f32>, %arg4: tensor<f32>):
+      tosa.yield %arg4 : tensor<f32>
+    }) : (tensor<i1>, tensor<f32>, tensor<f32>) -> tensor<f32>
+  return %0 : tensor<f32>
+}
+
+// -----
+
+// CHECK-LABEL: test_while_loop_isolated_from_above
+func.func @test_while_loop_isolated_from_above(%arg0: tensor<f32>, %arg1: tensor<i32>) {
+  %0 = "tosa.const"() {values = dense<0> : tensor<i32>} : () -> tensor<i32>
+  %1:3 = "tosa.while_loop"(%0, %arg0, %arg1) ({
+  ^bb0(%arg3: tensor<i32>, %arg4: tensor<f32>, %arg5: tensor<i32>):
+    %2 = "tosa.greater_equal"(%arg3, %arg5) : (tensor<i32>, tensor<i32>) -> tensor<i1>
+    %3 = "tosa.logical_not"(%2) : (tensor<i1>) -> tensor<i1>
+    "tosa.yield"(%3) : (tensor<i1>) -> ()
+  },  {
+  ^bb0(%arg3: tensor<i32>, %arg4: tensor<f32>, %arg5: tensor<i32>):
+    %2 = "tosa.const"() {values = dense<1> : tensor<i32>} : () -> tensor<i32>
+    %3 = "tosa.add"(%arg3, %2) : (tensor<i32>, tensor<i32>) -> tensor<i32>
+    "tosa.yield"(%3, %arg4, %arg5) : (tensor<i32>, tensor<f32>, tensor<i32>) -> ()
+  }) : (tensor<i32>, tensor<f32>, tensor<i32>) -> (tensor<i32>, tensor<f32>, tensor<i32>)
+  return
+}