Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix x86 depthwise conv2d alter_op_layout #3264

Merged
merged 8 commits into from Jun 6, 2019

Conversation

@kevinthesun
Copy link
Contributor

commented May 31, 2019

Fix x86 depthwise conv2d alter_op_layout. Related discussion topic: https://discuss.tvm.ai/t/x86-relay-auto-tune-mobilefacenet-error-using-relay/2508/22

kevinthesun added some commits May 30, 2019

kevinthesun added some commits May 31, 2019

@@ -415,7 +415,8 @@ def _alter_conv2d_layout(attrs, inputs, tinfo, F):

dtype = data.dtype
out_dtype = dtype if out_dtype in ("same", "") else out_dtype
is_depthwise = groups == in_channel and groups == out_channel
kshape = get_const_tuple(kernel.shape)
is_depthwise = groups == kshape[0] and kshape[1] == 1

This comment has been minimized.

Copy link
@yzhliu

yzhliu Jun 1, 2019

Member

do we need to check kernel layout?

kevinthesun added some commits Jun 3, 2019

@kevinthesun kevinthesun force-pushed the kevinthesun:MoveX86DepthwiseConv branch from 0a6a274 to 39c622e Jun 3, 2019

@@ -415,11 +415,15 @@ def _alter_conv2d_layout(attrs, inputs, tinfo, F):

dtype = data.dtype
out_dtype = dtype if out_dtype in ("same", "") else out_dtype
is_depthwise = groups == in_channel and groups == out_channel

# only optimize for NCHW
if layout != 'NCHW':

This comment has been minimized.

Copy link
@yzhliu

yzhliu Jun 5, 2019

Member
Suggested change
if layout != 'NCHW':
if layout != 'NCHW' or attrs["kernel_layout"] != "OIHW":

and remove the assert below.

@kevinthesun kevinthesun requested review from Huyuwei and Laurawly as code owners Jun 5, 2019

@yzhliu

yzhliu approved these changes Jun 6, 2019

@yzhliu yzhliu merged commit d7bc4fd into dmlc:master Jun 6, 2019

1 check passed

continuous-integration/jenkins/pr-merge This commit looks good
Details
@yzhliu

This comment has been minimized.

Copy link
Member

commented Jun 6, 2019

Thanks @kevinthesun !

@kevinthesun kevinthesun deleted the kevinthesun:MoveX86DepthwiseConv branch Jun 13, 2019

wweic pushed a commit to wweic/tvm that referenced this pull request Jun 26, 2019

Fix x86 depthwise conv2d alter_op_layout (dmlc#3264)
* Fix x86 depthwise conv2d alter_op_layout

* Small fix

* Add test case

* Fix test

* Assert kernel layout

* Minor fix

* Add get_shape function

* Minor change

wweic added a commit to neo-ai/tvm that referenced this pull request Jun 27, 2019

Fix x86 depthwise conv2d alter_op_layout (dmlc#3264)
* Fix x86 depthwise conv2d alter_op_layout

* Small fix

* Add test case

* Fix test

* Assert kernel layout

* Minor fix

* Add get_shape function

* Minor change
@apivovarov

This comment has been minimized.

Copy link
Contributor

commented Jul 17, 2019

@kevinthesun Looks like this PR makes autotvm to "skips depthwise_conv2d_nchw workloads for llvm x86_64"
More details: https://discuss.tvm.ai/t/autotvm-skips-depthwise-conv2d-nchw-workloads-for-llvm-x86-64/3353/1

@apivovarov

This comment has been minimized.

Copy link
Contributor

commented Jul 17, 2019

Issue: #3557

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.