[GitHub] [incubator-tvm] giuseros commented on a change in pull request #6675: [QNN] Optimize requantize for power of 2 and fix dequantize for per-channel quantized input

[GitBox <gi...@apache.org>]

giuseros commented on a change in pull request #6675:
URL: https://github.com/apache/incubator-tvm/pull/6675#discussion_r512038744



##########
File path: src/relay/qnn/op/requantize.cc
##########
@@ -155,17 +155,22 @@ Expr RequantizeLower(const Expr& input_tensor, const Expr& input_scale,
     if (!IsEqualScalar(input_scale, output_scale)) {
       int32_t fixed_point_multiplier, shift;
       std::tie(fixed_point_multiplier, shift) = GetFixedPointMultiplierShift(double_multiplier);
-
       const bool is_upward_rounding = (param->rounding == "UPWARD");
 
-      // When using upward rounding (i.e., x.5 rounded to x+1), leverage
-      // the FixedPointMultiply operator
-      scaled_int32_t =
-          (is_upward_rounding
-               ? FixedPointMultiply(scaled_int32_t, fixed_point_multiplier, shift)
-               : FixedPointMultiplyToNearest(scaled_int32_t, double_multiplier, input_shape));
+      if (is_upward_rounding && fixed_point_multiplier == (1 << 30)) {
+        // Power of 2 is determined by the fixed_point_multiplier == 1 << 30. In case of power of 2,
+        // fixed point multiplier will represent a float value of 0.5. In fixed point, this is
+        // represented by 1 << 30.
+        scaled_int32_t = PowerOfTwoMultiply(scaled_int32_t, shift - 1);

Review comment:
       Does it make sense for this to go in `FixedPointMultiply`? This would give the possibility to everybody using FixedPointMultiply to exploit this fix. 

##########
File path: src/relay/qnn/utils.cc
##########
@@ -56,6 +56,21 @@ std::pair<int32_t, int32_t> GetFixedPointMultiplierShift(double double_multiplie
   return std::make_pair(significand, exponent);
 }
 
+Expr PowerOfTwoMultiply(Expr tensor, int32_t exp) {
+  Expr out;
+  if (exp > 0) {
+    // power of 2 is greater than 0, apply left shift.
+    out = LeftShift(tensor, MakeConstantScalar(DataType::Int(32), exp));
+  } else {
+    // power of 2 is less than 0, round and then apply right shift.
+    exp = -exp;
+    auto rounding_factor = 1 << (exp - 1);
+    auto rounded_t = Add(tensor, MakeConstantScalar(DataType::Int(32), rounding_factor));
+    out = RightShift(rounded_t, MakeConstantScalar(DataType::Int(32), exp));

Review comment:
       Are you sure you don't need to convert to `int64` upfront and then cast back to int32?




----------------------------------------------------------------
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.

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