[Mlir-commits] [mlir] [mlir][tosa] Robustify Tosa_IfOp against null dereference and wrong assertion (PR #159756)

[Luke Hutton]

================
@@ -438,6 +438,31 @@ func.func @test_pad_invalid_padding_value(%arg0: tensor<10xi8>, %arg1: tensor<1x
   return %1 : tensor<10xi8>
 }
 
+// -----
+func.func @test_cond_if_wrong_terminator_op(%arg0: tensor<i1>) -> tensor<i32> {
+  %0 = "tosa.cond_if"(%arg0) ({
+    %1 = "tosa.const"() <{values = dense<1> : tensor<i32>}> : () -> tensor<i32>
+    "tosa.yield"(%1) : (tensor<i32>) -> ()
+  }, {
+    // expected-error at +2 {{'func.return' op expects parent op 'func.func'}}
+    %2 = "tosa.const"() <{values = dense<2> : tensor<i32>}> : () -> tensor<i32>
+    "func.return"(%2) : (tensor<i32>) -> ()
+  }) : (tensor<i1>) -> tensor<i32>
+  return %0 : tensor<i32>
+}
+
+// -----
+func.func @test_cond_if_missing_terminator(%arg0: tensor<i1>) -> tensor<i32> {
+  %0 = "tosa.cond_if"(%arg0) ({
+    %1 = "tosa.const"() <{values = dense<1> : tensor<i32>}> : () -> tensor<i32>
+    "tosa.yield"(%1) : (tensor<i32>) -> ()
+  }, {
+    // expected-error at +1 {{block with no terminator}}
+    %2 = "tosa.const"() <{values = dense<2> : tensor<i32>}> : () -> tensor<i32>
+  }) : (tensor<i1>) -> tensor<i32>
+  return %0 : tensor<i32>
+}
+
----------------
lhutton1 wrote:

I think empty body is already covered by `SizedRegion<1>` on the operators definition. e.g.
```
func.func @test_cond_if(%arg0: tensor<i1>) -> tensor<i32> {
  %0 = "tosa.cond_if"(%arg0) ({

  }, {

  }) : (tensor<i1>) -> tensor<i32>
  return %0 : tensor<i32>
}

$ mlir-opt test.mlir 
test.mlir:2:8: error: 'tosa.cond_if' op region #0 ('then_graph') failed to verify constraint: region with 1 blocks
```
 It would be good to add a test case to check the `then` case though

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