[Mlir-commits] [mlir] [mlir][tosa] Fix TosaNarrowTypes conversion order to prevent rollback crash (PR #207372)

[llvmlistbot at llvm.org]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-mlir-tosa

@llvm/pr-subscribers-mlir

Author: Hocky Yudhiono (hockyy)

<details>
<summary>Changes</summary>

Fix #<!-- -->206952 by doing `convertRegionTypes` before moving and unlinking the blocks when handling `tosa.cond_if` ops. This will cause a crash when doing rollback. 

---
Full diff: https://github.com/llvm/llvm-project/pull/207372.diff


2 Files Affected:

- (modified) mlir/lib/Dialect/Tosa/Transforms/TosaNarrowTypes.cpp (+2-2) 
- (modified) mlir/test/Dialect/Tosa/tosa-narrow-i64-to-i32.mlir (+21) 


``````````diff
diff --git a/mlir/lib/Dialect/Tosa/Transforms/TosaNarrowTypes.cpp b/mlir/lib/Dialect/Tosa/Transforms/TosaNarrowTypes.cpp
index 983472762e791..d441f90c2a074 100644
--- a/mlir/lib/Dialect/Tosa/Transforms/TosaNarrowTypes.cpp
+++ b/mlir/lib/Dialect/Tosa/Transforms/TosaNarrowTypes.cpp
@@ -426,10 +426,10 @@ LogicalResult convertGenericOp(Operation *op, ValueRange operands,
   }
 
   for (Region &region : op->getRegions()) {
+    if (failed(rewriter.convertRegionTypes(&region, *typeConverter)))
+      return failure();
     Region *newRegion = state.addRegion();
     rewriter.inlineRegionBefore(region, *newRegion, newRegion->begin());
-    if (failed(rewriter.convertRegionTypes(newRegion, *typeConverter)))
-      return failure();
   }
 
   Operation *newOp = rewriter.create(state);
diff --git a/mlir/test/Dialect/Tosa/tosa-narrow-i64-to-i32.mlir b/mlir/test/Dialect/Tosa/tosa-narrow-i64-to-i32.mlir
index 9349bd1c1cf7b..f51c5b714378e 100644
--- a/mlir/test/Dialect/Tosa/tosa-narrow-i64-to-i32.mlir
+++ b/mlir/test/Dialect/Tosa/tosa-narrow-i64-to-i32.mlir
@@ -75,6 +75,27 @@ func.func @test_regions(%arg0: tensor<1x2xi32>, %arg1: tensor<1xi32>, %arg2: ten
 
 // -----
 
+// CHECK-LABEL: test_cond_if_i64_yield_with_call
+module {
+  func.func @m0() -> () {
+    return
+  }
+  func.func @test_cond_if_i64_yield_with_call(%arg0: tensor<1xi1>, %arg1: tensor<4xi64>, %arg2: tensor<4xi64>) -> () {
+    %0 = tosa.cond_if %arg0 (%arg3 = %arg1) : tensor<1xi1> (tensor<4xi64>) -> tensor<4xi64> {
+    ^bb0(%arg3: tensor<4xi64>):
+      tosa.yield %arg3 : tensor<4xi64>
+    } else {
+    ^bb0(%arg3: tensor<4xi64>):
+      tosa.yield %arg3 : tensor<4xi64>
+    }
+    // expected-error @+1 {{failed to legalize operation 'func.call'}}
+    call @m0() : () -> ()
+    return
+  }
+}
+
+// -----
+
 // CHECK-LABEL: test_concat
 func.func @test_concat(%arg0: tensor<13x21x3xi64>, %arg1: tensor<13x21x3xi64>) -> tensor<26x21x3xi64> {
   // COMMON: tosa.concat %{{.*}}, %{{.*}} {axis = 0 : i32} : (tensor<13x21x3xi32>, tensor<13x21x3xi32>) -> tensor<26x21x3xi32>

``````````

</details>


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