You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2022/11/29 18:31:32 UTC

[GitHub] [tvm] AndrewZhaoLuo commented on a diff in pull request #13469: [QNN] support zero points as variable scalar for QnnBatchMatMul op

AndrewZhaoLuo commented on code in PR #13469:
URL: https://github.com/apache/tvm/pull/13469#discussion_r1035126503


##########
src/relay/qnn/op/batch_matmul.cc:
##########
@@ -175,27 +200,32 @@ Expr QnnBatchMatmulCanonicalize(const Attrs& attrs, const Array<Expr>& new_args,
 
   const auto* qnn_batch_matmul_attrs = attrs.as<BatchMatmulAttrs>();
 
-  // Extract the integer zero points.
-  auto y_zero_point_int = GetScalarFromConstant<int>(y_zero_point);
-  auto x_zero_point_int = GetScalarFromConstant<int>(x_zero_point);
-
   // Get all the terms as described in the comments.
   auto term1 = BatchMatmulFirstTerm(quantized_x, quantized_y, qnn_batch_matmul_attrs);
   auto term2 = BatchMatmulSecondTerm(quantized_x, y_zero_point);
   auto term3 = BatchMatmulThirdTerm(quantized_y, x_zero_point, broadcast_dim_size);
-  auto term4 = BatchMatmulFourthTerm(x_zero_point_int, y_zero_point_int, reduction_dim_size);
-
-  // Combine those 4 terms depending on the zero points to get the best lowering.
-  if (x_zero_point_int == 0 && y_zero_point_int == 0) {
-    // term 2, 3 and 4 become zero.
-    return term1;
-  } else if (x_zero_point_int == 0 && y_zero_point_int != 0) {
-    // term 3 and term 4 become zero.
-    return Subtract(term1, term2);
-  } else if (x_zero_point_int != 0 && y_zero_point_int == 0) {
-    // term 2 and term 4 become zero.
-    return Subtract(term1, term3);
+
+  // TODO(vvchernov): how do additional checs (equal) influence on performance?
+  if (IsConstScalar(x_zero_point) && IsConstScalar(y_zero_point)) {
+    // Extract the integer zero points.
+    auto y_zero_point_int = GetScalarFromConstant<int>(y_zero_point);
+    auto x_zero_point_int = GetScalarFromConstant<int>(x_zero_point);
+    auto term4 = BatchMatmulFourthTerm(x_zero_point_int, y_zero_point_int, reduction_dim_size);
+    // Combine those 4 terms depending on the zero points to get the best lowering.
+    if (x_zero_point_int == 0 && y_zero_point_int != 0) {

Review Comment:
   I believe some copy pasting was wrong here.
   
   We want
   `if (x_zero_point_int == 0 && y_zero_point_int == 0) {`



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org