[Mlir-commits] [mlir] [mlir][tosa] Migrate tosa.mul shift check from verifier to tosa-validate (PR #234092)

[llvmlistbot at llvm.org]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir-tosa

Author: Wijkqwe

<details>
<summary>Changes</summary>

Part of #<!-- -->233457.

Remove the constant-value check on `tosa.mul`'s `shift` operand from the
verifier. Verifiers should only check information local to an operation;
reading a `tosa.const`'s contents reaches through to another op, so this
belongs in `tosa-validate`. `checkErrorIfMul` already implements the rule and
is strictly more complete (it also enforces `0 <= shift <= 63` for `int32_t`),
so no coverage is lost.

Moves the `shift`-value test to `error_if_check.mlir` alongside the other
`tosa.mul` shift cases.

Assisted-by: AI.

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


3 Files Affected:

- (modified) mlir/lib/Dialect/Tosa/IR/TosaOps.cpp (-9) 
- (modified) mlir/test/Dialect/Tosa/error_if_check.mlir (+10) 
- (modified) mlir/test/Dialect/Tosa/invalid.mlir (-10) 


``````````diff
diff --git a/mlir/lib/Dialect/Tosa/IR/TosaOps.cpp b/mlir/lib/Dialect/Tosa/IR/TosaOps.cpp
index 99c352ab8ed3c3..6b6f295c625f5b 100644
--- a/mlir/lib/Dialect/Tosa/IR/TosaOps.cpp
+++ b/mlir/lib/Dialect/Tosa/IR/TosaOps.cpp
@@ -2561,15 +2561,6 @@ LogicalResult tosa::MulOp::verify() {
         return emitOpError(
             "requires the same element type for all operands and results");
     }
-
-    // verify shift has value 0 for non-integer types
-    ElementsAttr shiftElem;
-    if (matchPattern(getShift(), m_Constant(&shiftElem))) {
-      int32_t shift = shiftElem.getValues<IntegerAttr>()[0].getInt();
-      if (shift != 0) {
-        return emitOpError() << "require shift to be 0 for float type";
-      }
-    }
   }
 
   // Verify the op has same ranks for all main operands (excludes extra operands
diff --git a/mlir/test/Dialect/Tosa/error_if_check.mlir b/mlir/test/Dialect/Tosa/error_if_check.mlir
index b87f124f64ac27..0301711cd67898 100644
--- a/mlir/test/Dialect/Tosa/error_if_check.mlir
+++ b/mlir/test/Dialect/Tosa/error_if_check.mlir
@@ -146,6 +146,16 @@ func.func @test_mul_non_zero_shift(%arg0: tensor<1x8x8x8xi16>, %arg1: tensor<1x8
   return %mul : tensor<1x8x8x8xi32>
 }
 
+// -----
+
+// CHECK-LABEL: test_mul_float_non_zero_shift
+func.func @test_mul_float_non_zero_shift(%arg0: tensor<13x21x3xf32>, %arg1: tensor<13x1x3xf32>) -> tensor<13x21x3xf32> {
+  %shift = "tosa.const" () <{ values = dense<1> : tensor<1xi8> }> : () -> tensor<1xi8>
+  // expected-error at +1 {{'tosa.mul' op requires shift = 0 for all input data types that are not int32_t, but got: 1}}
+  %mul = tosa.mul %arg0, %arg1, %shift : (tensor<13x21x3xf32>, tensor<13x1x3xf32>, tensor<1xi8>) -> tensor<13x21x3xf32>
+  return %mul : tensor<13x21x3xf32>
+}
+
 // -----
 // CHECK-LABEL: test_i16_table_size
 func.func @test_i16_table_size(%arg0: tensor<2x64xi16>, %arg1: tensor<256xi16>) -> tensor<2x64xi32> {
diff --git a/mlir/test/Dialect/Tosa/invalid.mlir b/mlir/test/Dialect/Tosa/invalid.mlir
index bf05491164f404..1973736b28b222 100644
--- a/mlir/test/Dialect/Tosa/invalid.mlir
+++ b/mlir/test/Dialect/Tosa/invalid.mlir
@@ -637,16 +637,6 @@ func.func @test_mul_int_type_mismatch(%arg0: tensor<1xf32>, %arg1: tensor<1xf32>
 
 // -----
 
-// CHECK-LABEL: test_mul_invalid_shift
-func.func @test_mul_invalid_shift(%arg0: tensor<13x21x3xf32>, %arg1: tensor<13x1x3xf32>) -> tensor<13x21x3xf32> {
-  %shift = "tosa.const"() <{values = dense<1> : tensor<1xi8>}> : () -> tensor<1xi8>
-  // expected-error at +1 {{'tosa.mul' op require shift to be 0 for float type}}
-  %0 = tosa.mul %arg0, %arg1, %shift : (tensor<13x21x3xf32>, tensor<13x1x3xf32>, tensor<1xi8>) -> tensor<13x21x3xf32>
-  return %0 : tensor<13x21x3xf32>
-}
-
-// -----
-
 // CHECK-LABEL: test_mul_missing_shift
 func.func @test_mul_missing_shift(%arg0: tensor<13x21x3xi32>, %arg1: tensor<13x1x3xi32>) -> tensor<13x21x3xi32> {
   // expected-error at +1 {{'tosa.mul' op expected 3 operands, but found 2}}

``````````

</details>


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