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/09/23 14:28:44 UTC
[GitHub] [tvm] lhutton1 opened a new pull request, #12887: [ETHOSN] Support conversion of add/mul to requantize where possible
lhutton1 opened a new pull request, #12887:
URL: https://github.com/apache/tvm/pull/12887
Add/mul operations that correspond to identity operations can be converted to a simple reinterpret quantize operation. This conversion takes place in the convert equivalents pass similar to the depthwise counter-part.
In addtion, an issue was noticed that would cause unsupported operations to raise an error rather than not being offloaded. This has been fixed by allowing the conversion to return Null when the conversion is not supported.
cc @ashutosh-arm @leandron
--
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
[GitHub] [tvm] lhutton1 commented on pull request #12887: [ETHOSN] Support conversion of add/mul to requantize where possible
Posted by GitBox <gi...@apache.org>.
lhutton1 commented on PR #12887:
URL: https://github.com/apache/tvm/pull/12887#issuecomment-1260906802
Will wait on #12770 before pushing an update to pass CI
--
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
[GitHub] [tvm] lhutton1 commented on a diff in pull request #12887: [ETHOSN] Support conversion of add/mul to requantize where possible
Posted by GitBox <gi...@apache.org>.
lhutton1 commented on code in PR #12887:
URL: https://github.com/apache/tvm/pull/12887#discussion_r981075665
##########
tests/python/contrib/test_ethosn/test_convert_equivalents.py:
##########
@@ -74,7 +74,7 @@ def before():
relay.const(output_sc, "float32"),
relay.const(output_zp, "int32"),
)
- composite = tei.make_ethosn_composite(expr, "ethos-n.qnn_mul")
Review Comment:
This was just because the composite name was changed to show more intent, TVM mul -> NPU mul was never supported
--
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
[GitHub] [tvm] lhutton1 commented on a diff in pull request #12887: [ETHOSN] Support conversion of add/mul to requantize where possible
Posted by GitBox <gi...@apache.org>.
lhutton1 commented on code in PR #12887:
URL: https://github.com/apache/tvm/pull/12887#discussion_r981318065
##########
tests/python/contrib/test_ethosn/test_addition.py:
##########
@@ -117,13 +119,15 @@ def test_addition(dtype, shape):
@requires_ethosn
@pytest.mark.parametrize("dtype", ["uint8", "int8"])
@pytest.mark.parametrize(
- "lhs_shape,rhs_shape",
+ "lhs_shape,lhs_is_constant,rhs_shape,rhs_is_constant",
[
- ((1, 4, 4, 8), (1, 1, 1, 8)),
- ((1, 16, 12, 4), (4,)),
+ ((1, 4, 4, 8), False, (1, 1, 1, 8), True),
+ ((4,), True, (1, 16, 12, 4), False),
+ ((1, 1, 1, 8), True, (1, 4, 4, 8), False),
+ ((1, 16, 12, 4), False, (4,), True),
Review Comment:
see above - I'm of the opinion we should block this PR though as it doesn't add the const+const functionality. A test for this case should be added in a separate PR though, WDYT?
--
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
[GitHub] [tvm] leandron merged pull request #12887: [ETHOSN] Support conversion of add/mul to requantize where possible
Posted by GitBox <gi...@apache.org>.
leandron merged PR #12887:
URL: https://github.com/apache/tvm/pull/12887
--
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
[GitHub] [tvm] lhutton1 commented on a diff in pull request #12887: [ETHOSN] Support conversion of add/mul to requantize where possible
Posted by GitBox <gi...@apache.org>.
lhutton1 commented on code in PR #12887:
URL: https://github.com/apache/tvm/pull/12887#discussion_r981078225
##########
python/tvm/relay/op/contrib/ethosn.py:
##########
@@ -227,11 +227,13 @@ def qnn_add_pattern():
is_constant(),
is_constant(),
)
- two_inputs = gen_add_inputs(wildcard(), wildcard())
- input_is_left = gen_add_inputs(wildcard(), is_constant())
- input_is_right = gen_add_inputs(is_constant(), wildcard())
- return input_is_left | input_is_right | two_inputs
+ if has_constant_input:
+ input_is_left = gen_add_inputs(wildcard(), is_constant())
+ input_is_right = gen_add_inputs(is_constant(), wildcard())
+ return input_is_left | input_is_right
+ else:
+ return gen_add_inputs(wildcard(), wildcard())
Review Comment:
I believe this is a quirk of the pattern matching in that constant values are only propagated to the operator when `is_constant()` is used, otherwise they remain as inputs to the composite function ~- I will double check this though~
--
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
[GitHub] [tvm] ashutosh-arm commented on a diff in pull request #12887: [ETHOSN] Support conversion of add/mul to requantize where possible
Posted by GitBox <gi...@apache.org>.
ashutosh-arm commented on code in PR #12887:
URL: https://github.com/apache/tvm/pull/12887#discussion_r980029874
##########
python/tvm/relay/op/contrib/ethosn.py:
##########
@@ -227,11 +227,13 @@ def qnn_add_pattern():
is_constant(),
is_constant(),
)
- two_inputs = gen_add_inputs(wildcard(), wildcard())
- input_is_left = gen_add_inputs(wildcard(), is_constant())
- input_is_right = gen_add_inputs(is_constant(), wildcard())
- return input_is_left | input_is_right | two_inputs
+ if has_constant_input:
+ input_is_left = gen_add_inputs(wildcard(), is_constant())
+ input_is_right = gen_add_inputs(is_constant(), wildcard())
+ return input_is_left | input_is_right
+ else:
+ return gen_add_inputs(wildcard(), wildcard())
Review Comment:
Since all types of inputs are supported, can the function simply return with an op check and ignore the input types?
##########
tests/python/contrib/test_ethosn/test_convert_equivalents.py:
##########
@@ -74,7 +74,7 @@ def before():
relay.const(output_sc, "float32"),
relay.const(output_zp, "int32"),
)
- composite = tei.make_ethosn_composite(expr, "ethos-n.qnn_mul")
Review Comment:
Same as one of the previously asked question: are the direct TVM mul -> NPU MUL test cases not needed any more?
##########
python/tvm/relay/op/contrib/ethosn.py:
##########
@@ -299,16 +301,24 @@ def check_leaky_relu(extract):
return _ethosn.leaky_relu(extract)
- def check_mul(extract):
- """Check if Mul is supported."""
+ def check_mul_to_reinterpret_quantize(extract):
+ """Check if Mul is supported by converting to reinterpret quantize"""
if not ethosn_available():
return False
- # Do not support scalar constants for now
- check_scalar = lambda i: isinstance(i, tvm.relay.Constant) and len(i.data.shape) == 0
- if check_scalar(extract.args[0]) or check_scalar(extract.args[1]):
Review Comment:
What is the behavior of TVM when both the inputs are constants. Does it not reach here? If it does, then does it need more simplifications to avoid getting offloaded to support library?
##########
src/relay/backend/contrib/ethosn/convert_equivalent.cc:
##########
@@ -39,100 +39,117 @@ namespace relay {
namespace contrib {
namespace ethosn {
+/*!
+ * \brief Helper class to extract inputs and quantization information from binary
+ * elementwise operations ready to convert.
+ */
+class BinaryElementwiseParams {
+ public:
+ static BinaryElementwiseParams ExtractBinaryElementwiseParams(const Call& call) {
+ auto params = BinaryElementwiseParams();
+ params.input1 = call->args[0];
+ params.input2 = call->args[1];
+ params.input1_scale = call->args[2];
+ params.input1_zero_point = call->args[3];
+ params.input2_scale = call->args[4];
+ params.input2_zero_point = call->args[5];
+ // Reverse the inputs if the constant is first input
+ if (call->args[0]->IsInstance<ConstantNode>()) {
+ params.input1 = call->args[1];
+ params.input2 = call->args[0];
+ params.input1_scale = call->args[4];
+ params.input1_zero_point = call->args[5];
+ params.input2_scale = call->args[2];
+ params.input2_zero_point = call->args[3];
+ }
+ params.output_scale = call->args[6];
+ params.output_zero_point = call->args[7];
+ return params;
+ }
+
+ Expr input1;
+ Expr input2;
+ Expr input1_scale;
+ Expr input1_zero_point;
+ Expr input2_scale;
+ Expr input2_zero_point;
+ Expr output_scale;
+ Expr output_zero_point;
Review Comment:
Would it be better to have the members as constants to avoid duplication of casts where this struct is consumed? I can imagine its not possible to do the same for inputs.
##########
tests/python/contrib/test_ethosn/test_addition.py:
##########
@@ -117,13 +119,15 @@ def test_addition(dtype, shape):
@requires_ethosn
@pytest.mark.parametrize("dtype", ["uint8", "int8"])
@pytest.mark.parametrize(
- "lhs_shape,rhs_shape",
+ "lhs_shape,lhs_is_constant,rhs_shape,rhs_is_constant",
[
- ((1, 4, 4, 8), (1, 1, 1, 8)),
- ((1, 16, 12, 4), (4,)),
+ ((1, 4, 4, 8), False, (1, 1, 1, 8), True),
+ ((4,), True, (1, 16, 12, 4), False),
+ ((1, 1, 1, 8), True, (1, 4, 4, 8), False),
+ ((1, 16, 12, 4), False, (4,), True),
Review Comment:
do we need to test for true, true case as well?
##########
python/tvm/relay/op/contrib/ethosn.py:
##########
@@ -328,19 +338,40 @@ def check_add(extract):
"""Check if an addition is supported by Ethos-N."""
if not ethosn_available():
return False
- # Do not support scalar constants for now
- check_scalar = lambda i: isinstance(i, tvm.relay.Constant) and len(i.data.shape) == 0
- if check_scalar(extract.args[0]) or check_scalar(extract.args[1]):
- return False
- inputs = extract.args[0:2]
- if any([isinstance(i, tvm.relay.Constant) for i in inputs]):
- extract = _ethosn.ConvertQnnAdd(extract)
- return _ethosn.conv2d(extract)
return _ethosn.addition(extract)
+ def check_add_to_reinterpret_quantize(extract):
+ """Check if addition can be converted to a reinterpret quantize operation."""
+ if not ethosn_available():
+ return False
+ converted_extract = _ethosn.ConvertQnnAddToReinterpretQuantize(extract)
+ if converted_extract:
+ return _ethosn.reinterpret_quantize(converted_extract)
+ return False
+
+ def check_add_to_depthwise(extract):
+ """Check if addition can be converted to a depthwise operation."""
+ if not ethosn_available():
+ return False
+ converted_extract = _ethosn.ConvertQnnAddToDepthwise(extract)
+ if converted_extract:
+ return _ethosn.conv2d(converted_extract)
+ return False
+
return [
- ("ethos-n.qnn_mul", qnn_mul_pattern(), check_mul),
Review Comment:
Do the following mul-transforms cover all cases? If not, should the standalone pattern still be listed in addition to the mul2req / mul2depthwise?
##########
python/tvm/relay/op/contrib/ethosn.py:
##########
@@ -328,19 +338,40 @@ def check_add(extract):
"""Check if an addition is supported by Ethos-N."""
if not ethosn_available():
return False
- # Do not support scalar constants for now
- check_scalar = lambda i: isinstance(i, tvm.relay.Constant) and len(i.data.shape) == 0
- if check_scalar(extract.args[0]) or check_scalar(extract.args[1]):
- return False
- inputs = extract.args[0:2]
- if any([isinstance(i, tvm.relay.Constant) for i in inputs]):
- extract = _ethosn.ConvertQnnAdd(extract)
- return _ethosn.conv2d(extract)
return _ethosn.addition(extract)
+ def check_add_to_reinterpret_quantize(extract):
+ """Check if addition can be converted to a reinterpret quantize operation."""
+ if not ethosn_available():
+ return False
+ converted_extract = _ethosn.ConvertQnnAddToReinterpretQuantize(extract)
+ if converted_extract:
+ return _ethosn.reinterpret_quantize(converted_extract)
+ return False
+
+ def check_add_to_depthwise(extract):
+ """Check if addition can be converted to a depthwise operation."""
+ if not ethosn_available():
+ return False
+ converted_extract = _ethosn.ConvertQnnAddToDepthwise(extract)
Review Comment:
I think I have heard of some explanation before, but could you please share why just knowing that the conversion is possible not enough in this function? Why can't the next step be ignored?
--
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
[GitHub] [tvm] lhutton1 commented on a diff in pull request #12887: [ETHOSN] Support conversion of add/mul to requantize where possible
Posted by GitBox <gi...@apache.org>.
lhutton1 commented on code in PR #12887:
URL: https://github.com/apache/tvm/pull/12887#discussion_r981078225
##########
python/tvm/relay/op/contrib/ethosn.py:
##########
@@ -227,11 +227,13 @@ def qnn_add_pattern():
is_constant(),
is_constant(),
)
- two_inputs = gen_add_inputs(wildcard(), wildcard())
- input_is_left = gen_add_inputs(wildcard(), is_constant())
- input_is_right = gen_add_inputs(is_constant(), wildcard())
- return input_is_left | input_is_right | two_inputs
+ if has_constant_input:
+ input_is_left = gen_add_inputs(wildcard(), is_constant())
+ input_is_right = gen_add_inputs(is_constant(), wildcard())
+ return input_is_left | input_is_right
+ else:
+ return gen_add_inputs(wildcard(), wildcard())
Review Comment:
I believe this is a quirk of the pattern matching in that constant values are only propagated to the operator when `is_constant()` is used, otherwise they remain as inputs to the composite function - I will double check this though
--
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
[GitHub] [tvm] lhutton1 commented on a diff in pull request #12887: [ETHOSN] Support conversion of add/mul to requantize where possible
Posted by GitBox <gi...@apache.org>.
lhutton1 commented on code in PR #12887:
URL: https://github.com/apache/tvm/pull/12887#discussion_r981316150
##########
python/tvm/relay/op/contrib/ethosn.py:
##########
@@ -299,16 +301,24 @@ def check_leaky_relu(extract):
return _ethosn.leaky_relu(extract)
- def check_mul(extract):
- """Check if Mul is supported."""
+ def check_mul_to_reinterpret_quantize(extract):
+ """Check if Mul is supported by converting to reinterpret quantize"""
if not ethosn_available():
return False
- # Do not support scalar constants for now
- check_scalar = lambda i: isinstance(i, tvm.relay.Constant) and len(i.data.shape) == 0
- if check_scalar(extract.args[0]) or check_scalar(extract.args[1]):
Review Comment:
It's a good point - I hadn't considered this case before now. Currently (and prior to this PR) a const+const will result in a segfault during codegen. Ideally a const+const op (or similar) should have been folded, although we currently don't require that to be run before the NPU codegen - so we should probably look at adding some constant folding remove these kind of operations from the graph
--
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
[GitHub] [tvm] lhutton1 commented on a diff in pull request #12887: [ETHOSN] Support conversion of add/mul to requantize where possible
Posted by GitBox <gi...@apache.org>.
lhutton1 commented on code in PR #12887:
URL: https://github.com/apache/tvm/pull/12887#discussion_r981318065
##########
tests/python/contrib/test_ethosn/test_addition.py:
##########
@@ -117,13 +119,15 @@ def test_addition(dtype, shape):
@requires_ethosn
@pytest.mark.parametrize("dtype", ["uint8", "int8"])
@pytest.mark.parametrize(
- "lhs_shape,rhs_shape",
+ "lhs_shape,lhs_is_constant,rhs_shape,rhs_is_constant",
[
- ((1, 4, 4, 8), (1, 1, 1, 8)),
- ((1, 16, 12, 4), (4,)),
+ ((1, 4, 4, 8), False, (1, 1, 1, 8), True),
+ ((4,), True, (1, 16, 12, 4), False),
+ ((1, 1, 1, 8), True, (1, 4, 4, 8), False),
+ ((1, 16, 12, 4), False, (4,), True),
Review Comment:
see above - I'm of the opinion we should not block this PR though as it doesn't add the const+const functionality. A test for this case should be added in a separate PR when this issue is fixed, WDYT?
--
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
[GitHub] [tvm] ashutosh-arm commented on a diff in pull request #12887: [ETHOSN] Support conversion of add/mul to requantize where possible
Posted by GitBox <gi...@apache.org>.
ashutosh-arm commented on code in PR #12887:
URL: https://github.com/apache/tvm/pull/12887#discussion_r981454820
##########
tests/python/contrib/test_ethosn/test_addition.py:
##########
@@ -117,13 +119,15 @@ def test_addition(dtype, shape):
@requires_ethosn
@pytest.mark.parametrize("dtype", ["uint8", "int8"])
@pytest.mark.parametrize(
- "lhs_shape,rhs_shape",
+ "lhs_shape,lhs_is_constant,rhs_shape,rhs_is_constant",
[
- ((1, 4, 4, 8), (1, 1, 1, 8)),
- ((1, 16, 12, 4), (4,)),
+ ((1, 4, 4, 8), False, (1, 1, 1, 8), True),
+ ((4,), True, (1, 16, 12, 4), False),
+ ((1, 1, 1, 8), True, (1, 4, 4, 8), False),
+ ((1, 16, 12, 4), False, (4,), True),
Review Comment:
A follow up later sounds good.
--
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
[GitHub] [tvm] lhutton1 commented on a diff in pull request #12887: [ETHOSN] Support conversion of add/mul to requantize where possible
Posted by GitBox <gi...@apache.org>.
lhutton1 commented on code in PR #12887:
URL: https://github.com/apache/tvm/pull/12887#discussion_r980229549
##########
src/relay/backend/contrib/ethosn/convert_equivalent.cc:
##########
@@ -39,100 +39,117 @@ namespace relay {
namespace contrib {
namespace ethosn {
+/*!
+ * \brief Helper class to extract inputs and quantization information from binary
+ * elementwise operations ready to convert.
+ */
+class BinaryElementwiseParams {
+ public:
+ static BinaryElementwiseParams ExtractBinaryElementwiseParams(const Call& call) {
+ auto params = BinaryElementwiseParams();
+ params.input1 = call->args[0];
+ params.input2 = call->args[1];
+ params.input1_scale = call->args[2];
+ params.input1_zero_point = call->args[3];
+ params.input2_scale = call->args[4];
+ params.input2_zero_point = call->args[5];
+ // Reverse the inputs if the constant is first input
+ if (call->args[0]->IsInstance<ConstantNode>()) {
+ params.input1 = call->args[1];
+ params.input2 = call->args[0];
+ params.input1_scale = call->args[4];
+ params.input1_zero_point = call->args[5];
+ params.input2_scale = call->args[2];
+ params.input2_zero_point = call->args[3];
+ }
+ params.output_scale = call->args[6];
+ params.output_zero_point = call->args[7];
+ return params;
+ }
+
+ Expr input1;
+ Expr input2;
+ Expr input1_scale;
+ Expr input1_zero_point;
+ Expr input2_scale;
+ Expr input2_zero_point;
+ Expr output_scale;
+ Expr output_zero_point;
Review Comment:
Good point, yep I think that makes more sense, thanks!
##########
python/tvm/relay/op/contrib/ethosn.py:
##########
@@ -328,19 +338,40 @@ def check_add(extract):
"""Check if an addition is supported by Ethos-N."""
if not ethosn_available():
return False
- # Do not support scalar constants for now
- check_scalar = lambda i: isinstance(i, tvm.relay.Constant) and len(i.data.shape) == 0
- if check_scalar(extract.args[0]) or check_scalar(extract.args[1]):
- return False
- inputs = extract.args[0:2]
- if any([isinstance(i, tvm.relay.Constant) for i in inputs]):
- extract = _ethosn.ConvertQnnAdd(extract)
- return _ethosn.conv2d(extract)
return _ethosn.addition(extract)
+ def check_add_to_reinterpret_quantize(extract):
+ """Check if addition can be converted to a reinterpret quantize operation."""
+ if not ethosn_available():
+ return False
+ converted_extract = _ethosn.ConvertQnnAddToReinterpretQuantize(extract)
+ if converted_extract:
+ return _ethosn.reinterpret_quantize(converted_extract)
+ return False
+
+ def check_add_to_depthwise(extract):
+ """Check if addition can be converted to a depthwise operation."""
+ if not ethosn_available():
+ return False
+ converted_extract = _ethosn.ConvertQnnAddToDepthwise(extract)
Review Comment:
This will just do the conversion to a depth-wise operation without any NPU specific constraints. The second part will check whether the generated depth-wise operation can actually be supported by the NPU
##########
python/tvm/relay/op/contrib/ethosn.py:
##########
@@ -328,19 +338,40 @@ def check_add(extract):
"""Check if an addition is supported by Ethos-N."""
if not ethosn_available():
return False
- # Do not support scalar constants for now
- check_scalar = lambda i: isinstance(i, tvm.relay.Constant) and len(i.data.shape) == 0
- if check_scalar(extract.args[0]) or check_scalar(extract.args[1]):
- return False
- inputs = extract.args[0:2]
- if any([isinstance(i, tvm.relay.Constant) for i in inputs]):
- extract = _ethosn.ConvertQnnAdd(extract)
- return _ethosn.conv2d(extract)
return _ethosn.addition(extract)
+ def check_add_to_reinterpret_quantize(extract):
+ """Check if addition can be converted to a reinterpret quantize operation."""
+ if not ethosn_available():
+ return False
+ converted_extract = _ethosn.ConvertQnnAddToReinterpretQuantize(extract)
+ if converted_extract:
+ return _ethosn.reinterpret_quantize(converted_extract)
+ return False
+
+ def check_add_to_depthwise(extract):
+ """Check if addition can be converted to a depthwise operation."""
+ if not ethosn_available():
+ return False
+ converted_extract = _ethosn.ConvertQnnAddToDepthwise(extract)
+ if converted_extract:
+ return _ethosn.conv2d(converted_extract)
+ return False
+
return [
- ("ethos-n.qnn_mul", qnn_mul_pattern(), check_mul),
Review Comment:
Mul is a bit different to addition in that the driver stack doesn't support it as an op directly
--
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
[GitHub] [tvm] lhutton1 commented on a diff in pull request #12887: [ETHOSN] Support conversion of add/mul to requantize where possible
Posted by GitBox <gi...@apache.org>.
lhutton1 commented on code in PR #12887:
URL: https://github.com/apache/tvm/pull/12887#discussion_r981318065
##########
tests/python/contrib/test_ethosn/test_addition.py:
##########
@@ -117,13 +119,15 @@ def test_addition(dtype, shape):
@requires_ethosn
@pytest.mark.parametrize("dtype", ["uint8", "int8"])
@pytest.mark.parametrize(
- "lhs_shape,rhs_shape",
+ "lhs_shape,lhs_is_constant,rhs_shape,rhs_is_constant",
[
- ((1, 4, 4, 8), (1, 1, 1, 8)),
- ((1, 16, 12, 4), (4,)),
+ ((1, 4, 4, 8), False, (1, 1, 1, 8), True),
+ ((4,), True, (1, 16, 12, 4), False),
+ ((1, 1, 1, 8), True, (1, 4, 4, 8), False),
+ ((1, 16, 12, 4), False, (4,), True),
Review Comment:
see above - I'm of the opinion we should not block this PR though as it doesn't add the const+const functionality. A test for this case should be added in a separate PR though, WDYT?
--
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
[GitHub] [tvm] lhutton1 commented on a diff in pull request #12887: [ETHOSN] Support conversion of add/mul to requantize where possible
Posted by GitBox <gi...@apache.org>.
lhutton1 commented on code in PR #12887:
URL: https://github.com/apache/tvm/pull/12887#discussion_r981032909
##########
src/relay/backend/contrib/ethosn/convert_equivalent.cc:
##########
@@ -39,100 +39,117 @@ namespace relay {
namespace contrib {
namespace ethosn {
+/*!
+ * \brief Helper class to extract inputs and quantization information from binary
+ * elementwise operations ready to convert.
+ */
+class BinaryElementwiseParams {
+ public:
+ static BinaryElementwiseParams ExtractBinaryElementwiseParams(const Call& call) {
+ auto params = BinaryElementwiseParams();
+ params.input1 = call->args[0];
+ params.input2 = call->args[1];
+ params.input1_scale = call->args[2];
+ params.input1_zero_point = call->args[3];
+ params.input2_scale = call->args[4];
+ params.input2_zero_point = call->args[5];
+ // Reverse the inputs if the constant is first input
+ if (call->args[0]->IsInstance<ConstantNode>()) {
+ params.input1 = call->args[1];
+ params.input2 = call->args[0];
+ params.input1_scale = call->args[4];
+ params.input1_zero_point = call->args[5];
+ params.input2_scale = call->args[2];
+ params.input2_zero_point = call->args[3];
+ }
+ params.output_scale = call->args[6];
+ params.output_zero_point = call->args[7];
+ return params;
+ }
+
+ Expr input1;
+ Expr input2;
+ Expr input1_scale;
+ Expr input1_zero_point;
+ Expr input2_scale;
+ Expr input2_zero_point;
+ Expr output_scale;
+ Expr output_zero_point;
Review Comment:
Ah, this isn't actually necessary, the quantization params are not cast to Constants at any point - either the value they hold is retrieved from them, or they are passed when creating a new operator which expects Expr anyway.
--
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