Skip to content

Add packing for int8 1x1 convolution and support the int8 group convolution on X86 - #2991

Merged
yzhliu merged 15 commits into
apache:masterfrom
lly-zero-one:cv
May 22, 2019
Merged

Add packing for int8 1x1 convolution and support the int8 group convolution on X86#2991
yzhliu merged 15 commits into
apache:masterfrom
lly-zero-one:cv

Conversation

@lly-zero-one

Copy link
Copy Markdown
Contributor

Thanks for contributing to TVM! Please refer to guideline https://docs.tvm.ai/contribute/ for useful information and tips. After the pull request is submitted, please request code reviews from Reviewers.

@lly-zero-one

lly-zero-one commented Apr 9, 2019

Copy link
Copy Markdown
Contributor Author

cc @yzhliu @yidawang @anijain2305 @ajtulloch
Basically, we want to upstream some changes, which are used in our optimization for quantized ResNext101.

@tqchen

tqchen commented Apr 18, 2019

Copy link
Copy Markdown
Member

@llyfacebook Please check the testcase, because of not all test machines support avx512, we need to optionally skip them.

@tqchen

tqchen commented Apr 18, 2019

Copy link
Copy Markdown
Member

@yzhliu @kevinthesun can you help follow up and review this PR?

@lly-zero-one

Copy link
Copy Markdown
Contributor Author

Thanks, I will skip the tests.

@lly-zero-one
lly-zero-one requested a review from phisiart as a code owner April 23, 2019 02:14
@lly-zero-one
lly-zero-one changed the base branch from master to v0.5 April 23, 2019 03:03
@lly-zero-one
lly-zero-one changed the base branch from v0.5 to master April 23, 2019 03:03
@yzhliu yzhliu self-assigned this Apr 23, 2019

@yzhliu yzhliu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@llyfacebook would you please help to elaborate the data/kernel layout for each scenario? It's pretty confusing to me so far.

Comment thread topi/python/topi/nn/conv2d.py Outdated
raise ValueError("not support this layout {} yet".format(data_layout))


if data_layout == 'NHWC':

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shall we use kernel_layout instead? as data_layout might not be necessarily binded to kernal_layout.
I'm actually a bit confused with the int8 conv layout, for NHWC data, what kernal layout is it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I was mainly following the data layout and kernel layout corresponding relationship here: https://github.com/dmlc/tvm/blob/147ea3b0ca147b527086228d524a2f68f872112d/topi/python/topi/nn/conv2d.py#L284

Comment thread topi/python/topi/x86/conv2d.py Outdated
oc, _, kh, kw = kshape
elif layout == 'NHWC':
n, h, w, ic = dshape
oc, _, kh, kw = kshape

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kh, kw, oc, _ ?

Comment thread topi/python/topi/x86/conv2d.py Outdated
strides = strides if isinstance(strides, (tuple, list)) else (strides, strides)
dilation = dilation if isinstance(dilation, (tuple, list)) else (dilation, dilation)

_, _, kh, kw = get_const_tuple(kernel.shape)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so, for data=NHWC & fp32, kernel=HWIO, while for data=NHWC & int8, kernel=OIHW?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I will spend some time this week unifying them.

Comment thread topi/python/topi/x86/conv2d.py Outdated
conv2d_avx_1x1._schedule_conv_nhwc_pack_int8(*args)
else:
raise ValueError("Only support 1x1 kernel with "
"schedule template.")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please make the fatal msg more detailed other than just "schedule template"

@tqchen tqchen added the status: need update need update based on feedbacks label May 8, 2019
@tqchen

tqchen commented May 8, 2019

Copy link
Copy Markdown
Member

@llyfacebook please update as per review comments

@tqchen

tqchen commented May 16, 2019

Copy link
Copy Markdown
Member

@yzhliu please followup on this PR

@yzhliu yzhliu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

otherwise looks good to me.

Comment thread topi/python/topi/x86/conv2d.py Outdated
_get_default_config(cfg, data, kernel, strides, padding, out_dtype)
return _declaration_conv_impl(cfg, data, kernel, strides,
padding, dilation, layout, out_dtype)
# KHOI kernel layout is for NHWC and HWCN

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# KHOI kernel layout is for NHWC and HWCN
# HWOI kernel layout is for NHWC and HWCN

@yzhliu
yzhliu merged commit f7d7fdc into apache:master May 22, 2019
@yzhliu yzhliu added status: accepted and removed status: need review status: need update need update based on feedbacks labels May 22, 2019
@yzhliu

yzhliu commented May 22, 2019

Copy link
Copy Markdown
Member

Thanks @llyfacebook This is merged

wweic pushed a commit to wweic/tvm that referenced this pull request Jun 26, 2019
…lution on X86 (apache#2991)

* Support the 1x1 int8 conv with NHWC layout and weight packing

fix linter

* fix the memoize issue

* fix the failed nhwc test

* add the schedule for pack to unbreak other tests

* skip avx512 compile

* Support the 1x1 int8 conv with NHWC layout and weight packing

fix linter

* fix the memoize issue

* fix the failed nhwc test

* add the schedule for pack to unbreak other tests

* skip avx512 compile

* Unify the data_layout and kernel_layout relation

* add asf header

* fix the comment

* retrigger the build/test
wweic pushed a commit to neo-ai/tvm that referenced this pull request Jun 27, 2019
…lution on X86 (apache#2991)

* Support the 1x1 int8 conv with NHWC layout and weight packing

fix linter

* fix the memoize issue

* fix the failed nhwc test

* add the schedule for pack to unbreak other tests

* skip avx512 compile

* Support the 1x1 int8 conv with NHWC layout and weight packing

fix linter

* fix the memoize issue

* fix the failed nhwc test

* add the schedule for pack to unbreak other tests

* skip avx512 compile

* Unify the data_layout and kernel_layout relation

* add asf header

* fix the comment

* retrigger the build/test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants