[GitHub] [incubator-tvm] LiangHao151941 commented on a change in pull request #4828: [QNN][TFLite] TFLite rounding mode support
LiangHao151941 commented on a change in pull request #4828: [QNN][TFLite]
TFLite rounding mode support
URL: https://github.com/apache/incubator-tvm/pull/4828#discussion_r388234666
##
File path: include/tvm/relay/qnn/attrs.h
##
@@ -44,14 +44,15 @@ struct RequantizeAttrs : public
tvm::AttrsNode {
.set_default(-1);
TVM_ATTR_FIELD(rounding).set_default("UPWARD")
.describe("Defines the rounding direction when the value is midway
between"
- "two representable values. There are two supported modes -
UPWARD"
- "or TONEAREST. Both modes behave exactly same except at the"
+ "two representable values. There are two 3 modes - UPWARD,
TONEAREST"
+ "or TFLITE. UP/TONEAREST modes behave exactly same except at
the"
"midpoints between the two representable values. At the
midpoint,"
"UPWARD rounds towards positive infinity (for example -1.5
will be"
"rounded to -1). TONEAREST is the standard rounding where
the"
"value is rounded away from zero at midpoints (for example,
-1.5"
"rounds to -2). More context can be found at following gblic
manual"
Review comment:
updated accordingly
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] LiangHao151941 commented on a change in pull request #4828: [QNN][TFLite] TFLite rounding mode support
LiangHao151941 commented on a change in pull request #4828: [QNN][TFLite]
TFLite rounding mode support
URL: https://github.com/apache/incubator-tvm/pull/4828#discussion_r376854118
##
File path: src/relay/qnn/util.cc
##
@@ -100,35 +134,54 @@ Expr FixedPointMultiply(Expr tensor, double multiplier,
const Array&
// (from the right, rightmost bit is bit 0). The computation is performed in
// higher precision to avoid overflow in multiplying two int32 values.
Expr scalar = MakeConstantScalar(hp_dtype, fixed_point_multiplier);
- tensor = Multiply(tensor, scalar);
+ Expr scaled_tensor = Multiply(tensor, scalar);
// 4) Find the rounding scalar. This depends on where the final decimal
// point sits. As we will be right shifting the multiplied_t, we need to
// first calculate the total_right_shift.
int total_right_shift = right_shift + 31;
int64_t pos_rounding_value = (1ll << (total_right_shift - 1));
+ auto nearest_rounding_scalar =
+[&](const Expr& input_tensor, int right_shift) -> Expr {
+ int64_t pos_rounding_value = (1ll << (right_shift - 1));
+ auto pos_rounder = MakeConstantScalar(hp_dtype, pos_rounding_value);
+ auto neg_rounder = MakeConstantScalar(hp_dtype, pos_rounding_value - 1);
+ auto pos_rounder_t = Full(pos_rounder, input_shape, hp_dtype);
+ auto neg_rounder_t = Full(neg_rounder, input_shape, hp_dtype);
+
+ auto zero_t = Zeros(input_shape, hp_dtype);
+ return Where(
+GreaterEqual(input_tensor, zero_t), pos_rounder_t, neg_rounder_t);
Review comment:
I'll add some comments for this lambda function, if it is the suggestion
here that I understand.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] LiangHao151941 commented on a change in pull request #4828: [QNN][TFLite] TFLite rounding mode support
LiangHao151941 commented on a change in pull request #4828: [QNN][TFLite]
TFLite rounding mode support
URL: https://github.com/apache/incubator-tvm/pull/4828#discussion_r376853818
##
File path: src/relay/qnn/util.cc
##
@@ -22,13 +22,47 @@
* \brief Utility functions for QNN.
*/
+#include
#include "util.h"
#include "../pass/pattern_util.h"
namespace tvm {
namespace relay {
namespace qnn {
+/* \brief This function implements the rounding part of ARMv7 NEON VQRDMULH
+ * instruction. For code reuse, the multiplied tensor is directly passed in
+ * as parameter.
+ */
+Expr SaturatingRoundingDoublingHigh32(const Expr& input_tensor,
+ const Expr& multiplier_expr,
+ const Expr& scaled_tensor,
+ const Array& input_shape) {
+DataType hp_dtype = DataType::Int(64);
+int32_t pos_nudge_value = (1ll << 30);
+int32_t neg_nudge_value = 1 - (1ll << 30);
+auto pos_nudge = MakeConstantScalar(hp_dtype, pos_nudge_value);
+auto neg_nudge = MakeConstantScalar(hp_dtype, neg_nudge_value);
+auto pos_nudge_t = Full(pos_nudge, input_shape, hp_dtype);
+auto neg_nudge_t = Full(neg_nudge, input_shape, hp_dtype);
+
+auto int32_min = MakeConstantScalar(
+hp_dtype, std::numeric_limits::min());
+auto int32_max = MakeConstantScalar(
+hp_dtype, std::numeric_limits::max());
+auto int32_min_t = Full(int32_min, input_shape, hp_dtype);
+auto int32_max_t = Full(int32_max, input_shape, hp_dtype);
Review comment:
This patch is for a unique request that needs bit-exact execution compared
to tflite implementation(not only on arm targets). I believe moving this
generic implementation into relay help us to gain more control over compiler
behavior and reduce gaps between different targets and implementations.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] LiangHao151941 commented on a change in pull request #4828: [QNN][TFLite] TFLite rounding mode support
LiangHao151941 commented on a change in pull request #4828: [QNN][TFLite]
TFLite rounding mode support
URL: https://github.com/apache/incubator-tvm/pull/4828#discussion_r376852996
##
File path: include/tvm/relay/qnn/attrs.h
##
@@ -44,14 +44,15 @@ struct RequantizeAttrs : public
tvm::AttrsNode {
.set_default(-1);
TVM_ATTR_FIELD(rounding).set_default("UPWARD")
.describe("Defines the rounding direction when the value is midway
between"
- "two representable values. There are two supported modes -
UPWARD"
- "or TONEAREST. Both modes behave exactly same except at the"
+ "two representable values. There are two 3 modes - UPWARD,
TONEAREST"
+ "or TFLITE. UP/TONEAREST modes behave exactly same except at
the"
"midpoints between the two representable values. At the
midpoint,"
"UPWARD rounds towards positive infinity (for example -1.5
will be"
"rounded to -1). TONEAREST is the standard rounding where
the"
"value is rounded away from zero at midpoints (for example,
-1.5"
"rounds to -2). More context can be found at following gblic
manual"
-
"https://www.gnu.org/software/libc/manual/html_node/Rounding.html.;);
+
"https://www.gnu.org/software/libc/manual/html_node/Rounding.html.;
+ "TFLITE mode is more complicated, referring to tflite
implementation.");
Review comment:
TFLite version is 1.13, conforming with relay tflite frontend. The rounding
implementation is here
https://github.com/tensorflow/tensorflow/blob/r1.13/tensorflow/lite/kernels/internal/common.h#L105,
but according to community norm, external links should be better removed to
avoid license issues? @FrozenGene
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] LiangHao151941 commented on a change in pull request #4828: [QNN][TFLite] TFLite rounding mode support
LiangHao151941 commented on a change in pull request #4828: [QNN][TFLite]
TFLite rounding mode support
URL: https://github.com/apache/incubator-tvm/pull/4828#discussion_r376313583
##
File path: include/tvm/relay/qnn/attrs.h
##
@@ -44,14 +44,16 @@ struct RequantizeAttrs : public
tvm::AttrsNode {
.set_default(-1);
TVM_ATTR_FIELD(rounding).set_default("UPWARD")
.describe("Defines the rounding direction when the value is midway
between"
- "two representable values. There are two supported modes -
UPWARD"
- "or TONEAREST. Both modes behave exactly same except at the"
+ "two representable values. There are two 3 modes - UPWARD,
TONEAREST"
+ "or TFLITE. Both modes behave exactly same except at the"
Review comment:
updated
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] LiangHao151941 commented on a change in pull request #4828: [QNN][TFLite] TFLite rounding mode support
LiangHao151941 commented on a change in pull request #4828: [QNN][TFLite]
TFLite rounding mode support
URL: https://github.com/apache/incubator-tvm/pull/4828#discussion_r376311994
##
File path: src/relay/qnn/util.cc
##
@@ -22,13 +22,49 @@
* \brief Utility functions for QNN.
*/
+#include
+
#include "util.h"
#include "../pass/pattern_util.h"
namespace tvm {
namespace relay {
namespace qnn {
+/* \brief This function implements the rounding part of ARMv7 NEON VQRDMULH
+ * instruction. For code reuse, the multiplied tensor is directly passed in
+ * as parameter. Reference implementation:
+ *
https://github.com/google/gemmlowp/blob/2483d846ad865dd4190fe4a1a1ba2d9cfcea78e1/fixedpoint/fixedpoint.h#L337
Review comment:
Sure, I'll respect that
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] LiangHao151941 commented on a change in pull request #4828: [QNN][TFLite] TFLite rounding mode support
LiangHao151941 commented on a change in pull request #4828: [QNN][TFLite]
TFLite rounding mode support
URL: https://github.com/apache/incubator-tvm/pull/4828#discussion_r376252855
##
File path: src/relay/qnn/util.cc
##
@@ -22,13 +22,49 @@
* \brief Utility functions for QNN.
*/
+#include
+
#include "util.h"
#include "../pass/pattern_util.h"
namespace tvm {
namespace relay {
namespace qnn {
+/* \brief This function implements the rounding part of ARMv7 NEON VQRDMULH
+ * instruction. For code reuse, the multiplied tensor is directly passed in
+ * as parameter. Reference implementation:
+ *
https://github.com/google/gemmlowp/blob/2483d846ad865dd4190fe4a1a1ba2d9cfcea78e1/fixedpoint/fixedpoint.h#L337
Review comment:
I assume a reference link will not trigger license problem? The code is
reimplemented in relay, my intension to add this reference link is to mark the
origin of this implementation so that other developers would get better idea
what we are doing here.
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:
us...@infra.apache.org
With regards,
Apache Git Services
[GitHub] [incubator-tvm] LiangHao151941 commented on a change in pull request #4828: [QNN][TFLite] TFLite rounding mode support
LiangHao151941 commented on a change in pull request #4828: [QNN][TFLite] TFLite rounding mode support URL: https://github.com/apache/incubator-tvm/pull/4828#discussion_r376249483 ## File path: python/tvm/relay/frontend/tflite.py ## @@ -1212,6 +1214,7 @@ def convert_conv(self, op, conv_type): input_zero_point=new_input_zero_point, output_scale=output_tensor.qnn_params['scale'], output_zero_point=output_tensor.qnn_params['zero_point'], + rounding="TFLITE", Review comment: thx, resolved 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: us...@infra.apache.org With regards, Apache Git Services
