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

[llvmlistbot at llvm.org]

Author: Luke Hutton
Date: 2025-09-24T09:06:35+01:00
New Revision: 14f55046bc7429ca5d49affa9f0b082651b9e10c

URL: https://github.com/llvm/llvm-project/commit/14f55046bc7429ca5d49affa9f0b082651b9e10c
DIFF: https://github.com/llvm/llvm-project/commit/14f55046bc7429ca5d49affa9f0b082651b9e10c.diff

LOG: [mlir][tosa] Fix validation check on controlflow operators (#159754)

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.

Added: 
    mlir/test/Dialect/Tosa/tosa-validation-valid-strict.mlir

Modified: 
    mlir/lib/Dialect/Tosa/Transforms/TosaValidation.cpp
    mlir/test/Dialect/Tosa/error_if_check.mlir

Removed: 
    


################################################################################
diff  --git a/mlir/lib/Dialect/Tosa/Transforms/TosaValidation.cpp b/mlir/lib/Dialect/Tosa/Transforms/TosaValidation.cpp
index 91fea676ac44a..e9fdcbdc15837 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
+}