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 2020/04/17 08:44:48 UTC
[GitHub] [incubator-tvm] Menooker opened a new pull request #5357: enable
blocking format in x86 conv2d and fold scale axis
Menooker opened a new pull request #5357: enable blocking format in x86 conv2d and fold scale axis
URL: https://github.com/apache/incubator-tvm/pull/5357
Thanks for contributing to TVM! Please refer to guideline https://tvm.apache.org/docs/contribute/ for useful information and tips. After the pull request is submitted, please request code reviews from [Reviewers](https://github.com/apache/incubator-tvm/blob/master/CONTRIBUTORS.md#reviewers) by @ them in the pull request thread.
* Enable conv2d in blocking format for x86
* Enable fold scale axis optimization for blocking format conv2d
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-tvm] Menooker commented on a change in pull request #5357: [Relay] enable blocking format in x86 conv2d and fold scale axis
Posted by GitBox <gi...@apache.org>.
Menooker commented on a change in pull request #5357:
URL: https://github.com/apache/incubator-tvm/pull/5357#discussion_r420067574
##########
File path: python/tvm/relay/op/strategy/x86.py
##########
@@ -128,6 +135,15 @@ def conv2d_strategy_cpu(attrs, inputs, out_type, target):
wrap_compute_conv2d(topi.nn.depthwise_conv2d_nchw),
wrap_topi_schedule(topi.generic.schedule_depthwise_conv2d_nchw),
name="depthwise_conv2d_nchw.generic")
+ if layout == "NCHW":
+ assert kernel_layout == "OIHW"
+ channel_multiplier = get_const_tuple(inputs[1].shape)[1]
+ add_implementation_depthwise_nchw(channel_multiplier)
+ elif _NCHWc_matcher.match(layout): # check if layout is NCHWxc
+ assert _OIHWio_matcher.match(kernel_layout) # check if kernel is OIHWio
+ kernel_shape = get_const_tuple(inputs[1].shape)
+ channel_multiplier = kernel_shape[1] * kernel_shape[4]
+ add_implementation_depthwise_nchw(channel_multiplier)
Review comment:
ok, now redirected to `depthwise_conv2d_NCHWc_strategy_cpu`
----------------------------------------------------------------
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
[GitHub] [incubator-tvm] tqchen commented on pull request #5357: [Relay] enable blocking format in x86 conv2d and fold scale axis
Posted by GitBox <gi...@apache.org>.
tqchen commented on pull request #5357:
URL: https://github.com/apache/incubator-tvm/pull/5357#issuecomment-626411470
ping @yzhliu
----------------------------------------------------------------
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
[GitHub] [incubator-tvm] Menooker commented on pull request #5357: [Relay] enable blocking format in x86 conv2d and fold scale axis
Posted by GitBox <gi...@apache.org>.
Menooker commented on pull request #5357:
URL: https://github.com/apache/incubator-tvm/pull/5357#issuecomment-621165175
@icemelon9
If you unchenge these, you will get
```
File "/home/menooker/work/mobilenet/tvm/python/tvm/relay/op/nn/_nn.py", line 97, in alter_op_layout_conv2d
return topi.nn.conv2d_alter_layout(attrs, inputs, tinfos, out_type)
File "</home/menooker/anaconda3/envs/mobilenet/lib/python3.6/site-packages/decorator.py:decorator-gen-35>", line 2, in conv2d_alter_layout
File "/home/menooker/work/mobilenet/tvm/python/tvm/target/generic_func.py", line 267, in dispatch_func
return dispatch_dict[k](*args, **kwargs)
File "/home/menooker/work/mobilenet/tvm/topi/python/topi/x86/conv2d_alter_op.py", line 43, in _alter_conv2d_layout
relay.op.get("nn.conv2d"), attrs, tinfos, out_type, target)
File "/home/menooker/work/mobilenet/tvm/python/tvm/relay/backend/compile_engine.py", line 183, in select_implementation
all_impls = get_valid_implementations(op, attrs, inputs, out_type, target)
File "/home/menooker/work/mobilenet/tvm/python/tvm/relay/backend/compile_engine.py", line 124, in get_valid_implementations
strategy = fstrategy(attrs, inputs, out_type, target)
File "/home/menooker/work/mobilenet/tvm/python/tvm/target/generic_func.py", line 45, in __call__
return _ffi_api.GenericFuncCallFunc(self, *args)
File "/home/menooker/work/mobilenet/tvm/python/tvm/_ffi/_ctypes/packed_func.py", line 213, in __call__
raise get_last_ffi_error()
[bt] (3) /home/menooker/work/mobilenet/tvm/build/libtvm.so(TVMFuncCall+0x52) [0x7f4a967f5272]
[bt] (2) /home/menooker/work/mobilenet/tvm/build/libtvm.so(+0x7b19c8) [0x7f4a963779c8]
[bt] (1) /home/menooker/work/mobilenet/tvm/build/libtvm.so(tvm::GenericFunc::CallPacked(tvm::runtime::TVMArgs, tvm::runtime::TVMRetValue*) const+0x1b6) [0x7f4a96377746]
[bt] (0) /home/menooker/work/mobilenet/tvm/build/libtvm.so(+0xc2aa43) [0x7f4a967f0a43]
File "/home/menooker/work/mobilenet/tvm/python/tvm/_ffi/_ctypes/packed_func.py", line 78, in cfun
rv = local_pyfunc(*pyargs)
File "/home/menooker/work/mobilenet/tvm/python/tvm/relay/op/strategy/x86.py", line 108, in conv2d_strategy_cpu
raise RuntimeError("Unsupported conv2d layout {} for x86".format(layout))
RuntimeError: Unsupported conv2d layout NCHW1c for x86
```
----------------------------------------------------------------
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
[GitHub] [incubator-tvm] Menooker commented on a change in pull request #5357: [Relay] enable blocking format in x86 conv2d and fold scale axis
Posted by GitBox <gi...@apache.org>.
Menooker commented on a change in pull request #5357:
URL: https://github.com/apache/incubator-tvm/pull/5357#discussion_r415204685
##########
File path: src/relay/transforms/fold_scale_axis.cc
##########
@@ -518,13 +564,30 @@ Expr Conv2DForwardRewrite(const Call& ref_call,
// match the ic_axis
if (is_depthwise_conv2d) {
- Expr scale = ExpandBiasToMatchAxis(
- sdata->scale, kernel_layout.ndim(), {big_oc_axis});
- weight = Multiply(weight, scale);
+ if (is_simple) {
+ Expr scale = ExpandBiasToMatchAxis(
+ sdata->scale, kernel_layout.ndim(), {big_ko_axis});
+ weight = Multiply(weight, scale);
+ } else {
+ weight = Multiply(weight, ReshapeToMatchAxis(sdata->scale,
+ weight->type_as<TensorTypeNode>()->shape,
+ {big_ko_axis, small_ko_axis}));
+ if (!weight.defined())
+ return Expr();
Review comment:
I can see in line 542 and 543 in the same function:
```
if (sdata == nullptr) return Expr();
if (sweight != nullptr) return Expr();
```
Looks like returning `Expr()` just mean that the optimization fails?
----------------------------------------------------------------
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
[GitHub] [incubator-tvm] Menooker commented on a change in pull request #5357: [Relay] enable blocking format in x86 conv2d and fold scale axis
Posted by GitBox <gi...@apache.org>.
Menooker commented on a change in pull request #5357:
URL: https://github.com/apache/incubator-tvm/pull/5357#discussion_r418919219
##########
File path: src/relay/transforms/fold_scale_axis.cc
##########
@@ -390,8 +430,10 @@ Expr AddSubForwardRewrite(const Call& ref_call,
} else {
CHECK(srhs != nullptr);
CHECK(MatchBroadcastToLeftAxes(trhs, tlhs, srhs->axes));
- Expr scale = ExpandBiasToMatchAxis(
- srhs->scale, trhs->shape.size(), srhs->axes);
+ Expr scale = ReshapeOrExpandToMatchAxis(
+ srhs->scale, trhs->shape, srhs->axes);
+ if (!scale.defined())
+ return Expr();
Review comment:
changed as required
##########
File path: src/relay/transforms/fold_scale_axis.cc
##########
@@ -314,6 +316,42 @@ class ForwardPrep : private ExprVisitor {
}
};
+static bool IsIntInArray(const Array<Integer>& axis, int v) {
+ for (size_t i = 0; i < axis.size(); i++) {
+ if (axis[i] == v)
+ return true;
+ }
+ return false;
+}
+
+static Expr ReshapeToMatchAxis(Expr scale, const Array<PrimExpr>& shape,
+ const Array<Integer>& axis) {
+ Array<Integer> arr;
+ for (size_t i = 0; i < shape.size(); i++) {
+ if (IsIntInArray(axis, i)) {
+ auto node = shape[i].as<IntImmNode>();
+ if (!node) {
+ // if the shape is not a constant, use normal transform
+ return Expr();
+ }
+ arr.push_back(node->value);
+ } else {
+ arr.push_back(1);
+ }
+ }
+ return MakeReshape(scale, std::move(arr));
+}
+
+// if only one axis, use expand dim. Else, use reshape
+static Expr ReshapeOrExpandToMatchAxis(Expr scale, const Array<PrimExpr>& shape,
+ const Array<Integer>& axis) {
Review comment:
changed as required
----------------------------------------------------------------
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
[GitHub] [incubator-tvm] Menooker commented on a change in pull request #5357: [Relay] enable blocking format in x86 conv2d and fold scale axis
Posted by GitBox <gi...@apache.org>.
Menooker commented on a change in pull request #5357:
URL: https://github.com/apache/incubator-tvm/pull/5357#discussion_r415207561
##########
File path: src/relay/transforms/fold_scale_axis.cc
##########
@@ -39,6 +39,10 @@ namespace relay {
*
* Use namespace to reduce potential naming conflict.
*/
+
+extern Expr MakeReshape(Expr data,
+ Array<Integer> newshape);
Review comment:
Would you plz suggest a `.h` where we can move this declaration to? The function is defined in `relay/transform.cc`. But it is not declared anywhere in a header file.
----------------------------------------------------------------
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
[GitHub] [incubator-tvm] icemelon9 commented on a change in pull request #5357: [Relay] enable blocking format in x86 conv2d and fold scale axis
Posted by GitBox <gi...@apache.org>.
icemelon9 commented on a change in pull request #5357:
URL: https://github.com/apache/incubator-tvm/pull/5357#discussion_r419567958
##########
File path: python/tvm/relay/op/strategy/x86.py
##########
@@ -18,13 +18,17 @@
# pylint: disable=invalid-name,unused-argument,wildcard-import,unused-wildcard-import
import logging
+import re
import topi
from tvm.te import SpecializedCondition
from .generic import *
from .. import op as _op
logger = logging.getLogger('strategy')
+_NCHWc_matcher = re.compile("^NCHW[0-9]+c$")
+_OIHWio_matcher = re.compile("^OIHW[0-9]+i[-+]?[0-9]+o$")
Review comment:
```suggestion
_OIHWio_matcher = re.compile("^OIHW[0-9]+i[0-9]+o$")
```
##########
File path: python/tvm/relay/op/strategy/x86.py
##########
@@ -96,6 +99,12 @@ def conv2d_strategy_cpu(attrs, inputs, out_type, target):
wrap_compute_conv2d(topi.x86.conv2d_nchw),
wrap_topi_schedule(topi.x86.schedule_conv2d_nchw),
name="conv2d_nchw.x86")
+ if layout == "NCHW":
+ assert kernel_layout == "OIHW"
+ add_implementation_nchw()
+ elif _NCHWc_matcher.match(layout): # check if layout is NCHWxc
+ assert _OIHWio_matcher.match(kernel_layout) # check if kernel is OIHWio
+ add_implementation_nchw()
Review comment:
Here you should directly add `topi.x86.conv2d_NCHWc` since the op already has NCHWc layout.
##########
File path: python/tvm/relay/op/strategy/x86.py
##########
@@ -128,6 +135,15 @@ def conv2d_strategy_cpu(attrs, inputs, out_type, target):
wrap_compute_conv2d(topi.nn.depthwise_conv2d_nchw),
wrap_topi_schedule(topi.generic.schedule_depthwise_conv2d_nchw),
name="depthwise_conv2d_nchw.generic")
+ if layout == "NCHW":
+ assert kernel_layout == "OIHW"
+ channel_multiplier = get_const_tuple(inputs[1].shape)[1]
+ add_implementation_depthwise_nchw(channel_multiplier)
+ elif _NCHWc_matcher.match(layout): # check if layout is NCHWxc
+ assert _OIHWio_matcher.match(kernel_layout) # check if kernel is OIHWio
+ kernel_shape = get_const_tuple(inputs[1].shape)
+ channel_multiplier = kernel_shape[1] * kernel_shape[4]
+ add_implementation_depthwise_nchw(channel_multiplier)
Review comment:
same here
##########
File path: topi/python/topi/x86/conv2d_alter_op.py
##########
@@ -31,6 +32,9 @@
logger = logging.getLogger('topi')
+_NCHWc_matcher = re.compile("^NCHW[0-9]+c$")
+_OIHWio_matcher = re.compile("^OIHW[0-9]+i[-+]?[0-9]+o$")
Review comment:
```suggestion
_OIHWio_matcher = re.compile("^OIHW[0-9]+i[0-9]+o$")
```
----------------------------------------------------------------
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
[GitHub] [incubator-tvm] Menooker commented on a change in pull request #5357: [Relay] enable blocking format in x86 conv2d and fold scale axis
Posted by GitBox <gi...@apache.org>.
Menooker commented on a change in pull request #5357:
URL: https://github.com/apache/incubator-tvm/pull/5357#discussion_r418919351
##########
File path: python/tvm/relay/op/strategy/x86.py
##########
@@ -84,8 +88,13 @@ def conv2d_strategy_cpu(attrs, inputs, out_type, target):
raise ValueError("dilation should be positive value")
if groups == 1:
- if layout == "NCHW":
- assert kernel_layout == "OIHW"
+ if layout.startswith("NCHW"):
Review comment:
Ok, changed. Extract the same part of code handling HCWH and NCHWc to an nested function.
##########
File path: python/tvm/relay/op/strategy/x86.py
##########
@@ -113,8 +122,13 @@ def conv2d_strategy_cpu(attrs, inputs, out_type, target):
else:
raise RuntimeError("Unsupported conv2d layout {} for x86".format(layout))
elif is_depthwise_conv2d(data.shape, layout, kernel.shape, kernel_layout, groups):
- if layout == "NCHW":
- assert kernel_layout == "OIHW"
+ if layout.startswith("NCHW"):
Review comment:
changed as required
----------------------------------------------------------------
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
[GitHub] [incubator-tvm] yzhliu commented on a change in pull request #5357: [Relay] enable blocking format in x86 conv2d and fold scale axis
Posted by GitBox <gi...@apache.org>.
yzhliu commented on a change in pull request #5357:
URL: https://github.com/apache/incubator-tvm/pull/5357#discussion_r416315915
##########
File path: src/relay/transforms/fold_scale_axis.cc
##########
@@ -39,6 +39,10 @@ namespace relay {
*
* Use namespace to reduce potential naming conflict.
*/
+
+extern Expr MakeReshape(Expr data,
+ Array<Integer> newshape);
Review comment:
how about src/relay/op/tensor/transform.h ?
----------------------------------------------------------------
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
[GitHub] [incubator-tvm] Menooker commented on a change in pull request #5357: [Relay] enable blocking format in x86 conv2d and fold scale axis
Posted by GitBox <gi...@apache.org>.
Menooker commented on a change in pull request #5357:
URL: https://github.com/apache/incubator-tvm/pull/5357#discussion_r420068669
##########
File path: topi/python/topi/x86/conv2d_alter_op.py
##########
@@ -31,6 +32,9 @@
logger = logging.getLogger('topi')
+_NCHWc_matcher = re.compile("^NCHW[0-9]+c$")
+_OIHWio_matcher = re.compile("^OIHW[0-9]+i[-+]?[0-9]+o$")
Review comment:
Sorry I missed that. Changed now
----------------------------------------------------------------
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
[GitHub] [incubator-tvm] yzhliu commented on a change in pull request #5357: [Relay] enable blocking format in x86 conv2d and fold scale axis
Posted by GitBox <gi...@apache.org>.
yzhliu commented on a change in pull request #5357:
URL: https://github.com/apache/incubator-tvm/pull/5357#discussion_r415164446
##########
File path: src/relay/transforms/fold_scale_axis.cc
##########
@@ -314,6 +318,42 @@ class ForwardPrep : private ExprVisitor {
}
};
+static bool IsIntInArray(const Array<Integer>& axis, int v) {
+ for (size_t i = 0; i < axis.size(); i++) {
+ if (axis[i] == v)
+ return true;
+ }
+ return false;
+}
+
+static Expr ReshapeToMatchAxis(Expr scale, const Array<PrimExpr>& shape,
+ const Array<Integer>& axis) {
+ Array<Integer> arr;
+ for (size_t i = 0; i < shape.size(); i++) {
+ if (IsIntInArray(axis, i)) {
+ auto node = shape[i].as<IntImmNode>();
+ if (!node) {
+ // if the shape is not a constant, use normal transform
+ return Expr();
Review comment:
this will cause failure as you later do `CHECK(scale.defined());` ?
##########
File path: src/relay/transforms/fold_scale_axis.cc
##########
@@ -39,6 +39,10 @@ namespace relay {
*
* Use namespace to reduce potential naming conflict.
*/
+
+extern Expr MakeReshape(Expr data,
+ Array<Integer> newshape);
Review comment:
can we move it to .h?
##########
File path: src/relay/transforms/fold_scale_axis.cc
##########
@@ -518,13 +564,30 @@ Expr Conv2DForwardRewrite(const Call& ref_call,
// match the ic_axis
if (is_depthwise_conv2d) {
- Expr scale = ExpandBiasToMatchAxis(
- sdata->scale, kernel_layout.ndim(), {big_oc_axis});
- weight = Multiply(weight, scale);
+ if (is_simple) {
+ Expr scale = ExpandBiasToMatchAxis(
+ sdata->scale, kernel_layout.ndim(), {big_ko_axis});
+ weight = Multiply(weight, scale);
+ } else {
+ weight = Multiply(weight, ReshapeToMatchAxis(sdata->scale,
+ weight->type_as<TensorTypeNode>()->shape,
+ {big_ko_axis, small_ko_axis}));
+ if (!weight.defined())
+ return Expr();
Review comment:
would this cause problem?
----------------------------------------------------------------
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
[GitHub] [incubator-tvm] icemelon9 commented on pull request #5357: [Relay] enable blocking format in x86 conv2d and fold scale axis
Posted by GitBox <gi...@apache.org>.
icemelon9 commented on pull request #5357:
URL: https://github.com/apache/incubator-tvm/pull/5357#issuecomment-621364453
@Menooker Could you share the script that can reproduce the error? It's a bit weird that `_alter_conv2d_layout` receive a conv2d op with `NCHWc` layout.
----------------------------------------------------------------
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
[GitHub] [incubator-tvm] Menooker commented on a change in pull request #5357: [Relay] enable blocking format in x86 conv2d and fold scale axis
Posted by GitBox <gi...@apache.org>.
Menooker commented on a change in pull request #5357:
URL: https://github.com/apache/incubator-tvm/pull/5357#discussion_r415206359
##########
File path: src/relay/transforms/fold_scale_axis.cc
##########
@@ -518,13 +564,30 @@ Expr Conv2DForwardRewrite(const Call& ref_call,
// match the ic_axis
if (is_depthwise_conv2d) {
- Expr scale = ExpandBiasToMatchAxis(
- sdata->scale, kernel_layout.ndim(), {big_oc_axis});
- weight = Multiply(weight, scale);
+ if (is_simple) {
+ Expr scale = ExpandBiasToMatchAxis(
+ sdata->scale, kernel_layout.ndim(), {big_ko_axis});
+ weight = Multiply(weight, scale);
+ } else {
+ weight = Multiply(weight, ReshapeToMatchAxis(sdata->scale,
+ weight->type_as<TensorTypeNode>()->shape,
+ {big_ko_axis, small_ko_axis}));
+ if (!weight.defined())
+ return Expr();
Review comment:
If I am understanding correctly, line 166~175 in `src/relay/transforms/forward_rewrite.cc` will take care of `Expr()`
----------------------------------------------------------------
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
[GitHub] [incubator-tvm] icemelon9 commented on a change in pull request #5357: [Relay] enable blocking format in x86 conv2d and fold scale axis
Posted by GitBox <gi...@apache.org>.
icemelon9 commented on a change in pull request #5357:
URL: https://github.com/apache/incubator-tvm/pull/5357#discussion_r417472667
##########
File path: python/tvm/relay/op/strategy/x86.py
##########
@@ -84,8 +88,13 @@ def conv2d_strategy_cpu(attrs, inputs, out_type, target):
raise ValueError("dilation should be positive value")
if groups == 1:
- if layout == "NCHW":
- assert kernel_layout == "OIHW"
+ if layout.startswith("NCHW"):
Review comment:
No, inside `_alter_conv2d_layout`, the `conv2d` layout should still be NCHW, and this function will then convert layout into NCHWc
----------------------------------------------------------------
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
[GitHub] [incubator-tvm] Menooker commented on a change in pull request #5357: [Relay] enable blocking format in x86 conv2d and fold scale axis
Posted by GitBox <gi...@apache.org>.
Menooker commented on a change in pull request #5357:
URL: https://github.com/apache/incubator-tvm/pull/5357#discussion_r420067349
##########
File path: python/tvm/relay/op/strategy/x86.py
##########
@@ -96,6 +99,12 @@ def conv2d_strategy_cpu(attrs, inputs, out_type, target):
wrap_compute_conv2d(topi.x86.conv2d_nchw),
wrap_topi_schedule(topi.x86.schedule_conv2d_nchw),
name="conv2d_nchw.x86")
+ if layout == "NCHW":
+ assert kernel_layout == "OIHW"
+ add_implementation_nchw()
+ elif _NCHWc_matcher.match(layout): # check if layout is NCHWxc
+ assert _OIHWio_matcher.match(kernel_layout) # check if kernel is OIHWio
+ add_implementation_nchw()
Review comment:
Ok. now redirected to `conv2d_NCHWc_strategy_cpu`
----------------------------------------------------------------
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
[GitHub] [incubator-tvm] tqchen commented on issue #5357: [Relay] enable blocking format in x86 conv2d and fold scale axis
Posted by GitBox <gi...@apache.org>.
tqchen commented on issue #5357:
URL: https://github.com/apache/incubator-tvm/pull/5357#issuecomment-617507224
cc @icemelon9 @yzhliu @anijain2305
----------------------------------------------------------------
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
[GitHub] [incubator-tvm] Menooker commented on pull request #5357: [Relay] enable blocking format in x86 conv2d and fold scale axis
Posted by GitBox <gi...@apache.org>.
Menooker commented on pull request #5357:
URL: https://github.com/apache/incubator-tvm/pull/5357#issuecomment-621567041
> @Menooker Could you share the script that can reproduce the error? It's a bit weird that `_alter_conv2d_layout` receive a conv2d op with `NCHWc` layout.
My usecase is that, I want to use `nn.conv2d` with "NCHWc' layout in the frontend. I noticed that there are `relay.nn.contrib_conv2d_nchwc` and `contrib_depthwise_conv2d_nchwc`. I think users do not need to choose from `conv2d_nchwc` and `depthwise_conv2d_nchwc` manually. And I want TVM to decide which to use automatically - just like what TVM is now doing on `conv2d` with "NCHW'. So I use "NCHW16c" in `nn.conv2d`:
```python
from tvm import relay
import tvm
img = relay.var("img", shape=(128, 3, 224, 224, 1), dtype='float')
weight = relay.var("weight", shape=(1, 3, 3, 3, 1, 16 ), dtype='float')
conv = relay.nn.conv2d(img, weight, strides=(2, 2), padding=(1, 1),
dilation=(1, 1), groups=1, channels=16, kernel_size=[3, 3],
data_layout='NCHW1c', kernel_layout='OIHW1i16o', out_layout='NCHW16c')
func = relay.Function([img, weight], conv)
target = 'llvm -mcpu=skylake-avx512'
ctx = tvm.context(target, 0)
print(func)
with relay.build_config(opt_level=3):
graph, lib, params_fwd = relay.build(func, target)
```
Supporting "NCHWc" in `nn.conv2d` will make the frontend cleaner: users can use `nn.conv2d` for depthwise or not, and blocking format or not. TVM will choose the implementation (like `depthwise_conv2d_nchwc`) under the hood.
----------------------------------------------------------------
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
[GitHub] [incubator-tvm] Menooker commented on a change in pull request #5357: [Relay] enable blocking format in x86 conv2d and fold scale axis
Posted by GitBox <gi...@apache.org>.
Menooker commented on a change in pull request #5357:
URL: https://github.com/apache/incubator-tvm/pull/5357#discussion_r418919208
##########
File path: src/relay/transforms/fold_scale_axis.cc
##########
@@ -380,8 +418,10 @@ Expr AddSubForwardRewrite(const Call& ref_call,
if (slhs != nullptr) {
CHECK(srhs == nullptr);
CHECK(MatchBroadcastToLeftAxes(tlhs, trhs, slhs->axes));
- Expr scale = ExpandBiasToMatchAxis(
- slhs->scale, tlhs->shape.size(), slhs->axes);
+ Expr scale = ReshapeOrExpandToMatchAxis(
+ slhs->scale, tlhs->shape, slhs->axes);
+ if (!scale.defined())
+ return Expr();
Review comment:
changed as required
----------------------------------------------------------------
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
[GitHub] [incubator-tvm] Menooker commented on a change in pull request #5357: [Relay] enable blocking format in x86 conv2d and fold scale axis
Posted by GitBox <gi...@apache.org>.
Menooker commented on a change in pull request #5357:
URL: https://github.com/apache/incubator-tvm/pull/5357#discussion_r417267279
##########
File path: python/tvm/relay/op/strategy/x86.py
##########
@@ -84,8 +88,13 @@ def conv2d_strategy_cpu(attrs, inputs, out_type, target):
raise ValueError("dilation should be positive value")
if groups == 1:
- if layout == "NCHW":
- assert kernel_layout == "OIHW"
+ if layout.startswith("NCHW"):
Review comment:
Actually the changes here are necessary. `_alter_conv2d_layout` which is used in `AlterOpLayout` pass will call `select_implementation(relay.op.get('nn.conv2d'))` which will finally go to this function `conv2d_strategy_cpu`. And the layout is passed as blocking layout
----------------------------------------------------------------
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
[GitHub] [incubator-tvm] icemelon9 commented on a change in pull request #5357: [Relay] enable blocking format in x86 conv2d and fold scale axis
Posted by GitBox <gi...@apache.org>.
icemelon9 commented on a change in pull request #5357:
URL: https://github.com/apache/incubator-tvm/pull/5357#discussion_r418430603
##########
File path: python/tvm/relay/op/strategy/x86.py
##########
@@ -18,13 +18,17 @@
# pylint: disable=invalid-name,unused-argument,wildcard-import,unused-wildcard-import
import logging
+import re
import topi
from tvm.te import SpecializedCondition
from .generic import *
from .. import op as _op
logger = logging.getLogger('strategy')
+_NCHWc_matcher = re.compile("^NCHW[-+]?[0-9]+c$")
Review comment:
Why do we need `[-+]?` here?
##########
File path: python/tvm/relay/op/strategy/x86.py
##########
@@ -84,8 +88,13 @@ def conv2d_strategy_cpu(attrs, inputs, out_type, target):
raise ValueError("dilation should be positive value")
if groups == 1:
- if layout == "NCHW":
- assert kernel_layout == "OIHW"
+ if layout.startswith("NCHW"):
Review comment:
Could you separate "NCHW" and "NCHWc" into two different if branches?
##########
File path: src/relay/op/tensor/transform.h
##########
@@ -36,6 +36,9 @@
namespace tvm {
namespace relay {
+extern Expr MakeReshape(Expr data,
+ Array<Integer> newshape);
Review comment:
fix indent
##########
File path: src/relay/transforms/fold_scale_axis.cc
##########
@@ -390,8 +430,10 @@ Expr AddSubForwardRewrite(const Call& ref_call,
} else {
CHECK(srhs != nullptr);
CHECK(MatchBroadcastToLeftAxes(trhs, tlhs, srhs->axes));
- Expr scale = ExpandBiasToMatchAxis(
- srhs->scale, trhs->shape.size(), srhs->axes);
+ Expr scale = ReshapeOrExpandToMatchAxis(
+ srhs->scale, trhs->shape, srhs->axes);
+ if (!scale.defined())
+ return Expr();
Review comment:
same here
##########
File path: src/relay/transforms/fold_scale_axis.cc
##########
@@ -380,8 +418,10 @@ Expr AddSubForwardRewrite(const Call& ref_call,
if (slhs != nullptr) {
CHECK(srhs == nullptr);
CHECK(MatchBroadcastToLeftAxes(tlhs, trhs, slhs->axes));
- Expr scale = ExpandBiasToMatchAxis(
- slhs->scale, tlhs->shape.size(), slhs->axes);
+ Expr scale = ReshapeOrExpandToMatchAxis(
+ slhs->scale, tlhs->shape, slhs->axes);
+ if (!scale.defined())
+ return Expr();
Review comment:
still use { } to scope the if condition
##########
File path: src/relay/transforms/fold_scale_axis.cc
##########
@@ -314,6 +316,42 @@ class ForwardPrep : private ExprVisitor {
}
};
+static bool IsIntInArray(const Array<Integer>& axis, int v) {
+ for (size_t i = 0; i < axis.size(); i++) {
+ if (axis[i] == v)
+ return true;
+ }
+ return false;
+}
+
+static Expr ReshapeToMatchAxis(Expr scale, const Array<PrimExpr>& shape,
+ const Array<Integer>& axis) {
+ Array<Integer> arr;
+ for (size_t i = 0; i < shape.size(); i++) {
+ if (IsIntInArray(axis, i)) {
+ auto node = shape[i].as<IntImmNode>();
+ if (!node) {
+ // if the shape is not a constant, use normal transform
+ return Expr();
+ }
+ arr.push_back(node->value);
+ } else {
+ arr.push_back(1);
+ }
+ }
+ return MakeReshape(scale, std::move(arr));
+}
+
+// if only one axis, use expand dim. Else, use reshape
+static Expr ReshapeOrExpandToMatchAxis(Expr scale, const Array<PrimExpr>& shape,
+ const Array<Integer>& axis) {
Review comment:
fix indent
----------------------------------------------------------------
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
[GitHub] [incubator-tvm] Menooker commented on a change in pull request #5357: [Relay] enable blocking format in x86 conv2d and fold scale axis
Posted by GitBox <gi...@apache.org>.
Menooker commented on a change in pull request #5357:
URL: https://github.com/apache/incubator-tvm/pull/5357#discussion_r418919431
##########
File path: python/tvm/relay/op/strategy/x86.py
##########
@@ -18,13 +18,17 @@
# pylint: disable=invalid-name,unused-argument,wildcard-import,unused-wildcard-import
import logging
+import re
import topi
from tvm.te import SpecializedCondition
from .generic import *
from .. import op as _op
logger = logging.getLogger('strategy')
+_NCHWc_matcher = re.compile("^NCHW[-+]?[0-9]+c$")
Review comment:
Now changed to `NCHW?[0-9]+c`
----------------------------------------------------------------
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
[GitHub] [incubator-tvm] Menooker commented on a change in pull request #5357: [Relay] enable blocking format in x86 conv2d and fold scale axis
Posted by GitBox <gi...@apache.org>.
Menooker commented on a change in pull request #5357:
URL: https://github.com/apache/incubator-tvm/pull/5357#discussion_r417047631
##########
File path: src/relay/transforms/fold_scale_axis.cc
##########
@@ -39,6 +39,10 @@ namespace relay {
*
* Use namespace to reduce potential naming conflict.
*/
+
+extern Expr MakeReshape(Expr data,
+ Array<Integer> newshape);
Review comment:
OK, changed as required
----------------------------------------------------------------
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
[GitHub] [incubator-tvm] Menooker commented on a change in pull request #5357: [Relay] enable blocking format in x86 conv2d and fold scale axis
Posted by GitBox <gi...@apache.org>.
Menooker commented on a change in pull request #5357:
URL: https://github.com/apache/incubator-tvm/pull/5357#discussion_r415208733
##########
File path: src/relay/transforms/fold_scale_axis.cc
##########
@@ -314,6 +318,42 @@ class ForwardPrep : private ExprVisitor {
}
};
+static bool IsIntInArray(const Array<Integer>& axis, int v) {
+ for (size_t i = 0; i < axis.size(); i++) {
+ if (axis[i] == v)
+ return true;
+ }
+ return false;
+}
+
+static Expr ReshapeToMatchAxis(Expr scale, const Array<PrimExpr>& shape,
+ const Array<Integer>& axis) {
+ Array<Integer> arr;
+ for (size_t i = 0; i < shape.size(); i++) {
+ if (IsIntInArray(axis, i)) {
+ auto node = shape[i].as<IntImmNode>();
+ if (!node) {
+ // if the shape is not a constant, use normal transform
+ return Expr();
Review comment:
I have changed `CHECK(scale.defined())` now. It now fallbacks to a "optimization failure" rather than a compilation error.
For ForwardRewrite, if the shape is not constant, the rewriter will return `Expr()`.
For BackwardRewrite, if the shape is not constant, the rewriter will return `transformer->NormalCallTransform(call.operator->())`.
----------------------------------------------------------------
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