Skip to content

Conversation

@lhutton1
Copy link
Contributor

The TOSA specification allows the zero point of conv ops to be variable when the dynamic extension is being used, but information about which extensions are in use is only known when the validation pass is run. A variable zero point should be allowed in the conv ops verifiers.

In terms of testing, there didn't seem to be an existing set of tests for the verifiers to add this check to, so the opportunity has been taken to run the verifiers on the tests in ops.mlir. Since the conv2d test there had variable zero points, this change in functionality is being tested.

The TOSA specification allows the zero point of conv ops to be
variable when the dynamic extension is being used, but information
about which extensions are in use is only known when the validation
pass is run. A variable zero point should be allowed in the conv ops
verifiers.

In terms of testing, there didn't seem to be an existing set of tests
for the verififers to add this check to, so the opportunity has been
taken to run the verifiers on the tests in `ops.mlir`. Since the conv2d
test there had variable zero points, this change in functionality is
being tested.

Change-Id: I2cb5533a6a059290641f2e145f28c712eada0dd7
Signed-off-by: Luke Hutton <[email protected]>
@llvmbot
Copy link
Member

llvmbot commented Feb 24, 2025

@llvm/pr-subscribers-mlir-tosa

@llvm/pr-subscribers-mlir

Author: Luke Hutton (lhutton1)

Changes

The TOSA specification allows the zero point of conv ops to be variable when the dynamic extension is being used, but information about which extensions are in use is only known when the validation pass is run. A variable zero point should be allowed in the conv ops verifiers.

In terms of testing, there didn't seem to be an existing set of tests for the verifiers to add this check to, so the opportunity has been taken to run the verifiers on the tests in ops.mlir. Since the conv2d test there had variable zero points, this change in functionality is being tested.


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

2 Files Affected:

  • (modified) mlir/lib/Dialect/Tosa/IR/TosaOps.cpp (+2-4)
  • (modified) mlir/test/Dialect/Tosa/ops.mlir (+1-1)
diff --git a/mlir/lib/Dialect/Tosa/IR/TosaOps.cpp b/mlir/lib/Dialect/Tosa/IR/TosaOps.cpp
index e9c33e1b1bf10..295b010da0ee0 100644
--- a/mlir/lib/Dialect/Tosa/IR/TosaOps.cpp
+++ b/mlir/lib/Dialect/Tosa/IR/TosaOps.cpp
@@ -291,12 +291,10 @@ static LogicalResult verifyConvOp(T op) {
   ElementsAttr weightZpAttr;
   if (!matchPattern(op.getInputZp(), m_Constant(&inputZpAttr)) ||
       !matchPattern(op.getWeightZp(), m_Constant(&weightZpAttr))) {
-    op.emitOpError(
-        "bail out if the actual value of zero points cannot be determined");
-    return failure();
+    return success();
   }
 
-  // Get and verify explicit zero points.
+  // Get and verify explicit constant zero points.
   int64_t inputZpVal;
   int64_t weightZpVal;
 
diff --git a/mlir/test/Dialect/Tosa/ops.mlir b/mlir/test/Dialect/Tosa/ops.mlir
index cd73377c7f587..27132fa71fda4 100644
--- a/mlir/test/Dialect/Tosa/ops.mlir
+++ b/mlir/test/Dialect/Tosa/ops.mlir
@@ -1,4 +1,4 @@
-// RUN: mlir-opt %s | mlir-opt | FileCheck %s
+// RUN: mlir-opt %s --verify-each | mlir-opt | FileCheck %s
 // RUN: mlir-opt %s --mlir-print-op-generic | mlir-opt | FileCheck %s
 
 

@lhutton1 lhutton1 merged commit a4656bb into llvm:main Feb 25, 2025
11 checks passed
@lhutton1 lhutton1 deleted the conv2d-variable-zp branch February 25, 2025 09:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants