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

Int8 NHWC pack tensorize failure #3598

Closed
anijain2305 opened this issue Jul 23, 2019 · 9 comments
Closed

Int8 NHWC pack tensorize failure #3598

anijain2305 opened this issue Jul 23, 2019 · 9 comments

Comments

@anijain2305
Copy link
Contributor

If I replace llvm with llvm -mcpu=skylake-avx512 at

https://github.com/dmlc/tvm/blob/master/topi/tests/python/test_topi_conv2d_nhwc_pack_int8.py#L67

, the test fails with the following error

  File "/home/ubuntu/workplace/t1/tvm/src/op/tensorize.cc", line 226
TVMError: Check failed: is_one(e.region[i]->extent): Tensorize tensor_intrin: Input dimension mismatch with tensor intrin  expected shape=[16, 4], given region=[range(min=0, ext=1), range(min=0, ext=1), range(min=((ff.outer*16)/16), ext=(((((ff.outer*16) + 15)/16) + 1) - ff.outer)), range(min=(((((rc.outer.outer*16) + rc.outer.inner)*4)/4)*16), ext=((((((((rc.outer.outer*64) + (rc.outer.inner*4)) + 3)/4)*16) + 16) - (rc.outer.inner*16)) - (rc.outer.outer*256))), range(min=0, ext=4)]

@llyfacebook Can you please take a look?

@anijain2305 anijain2305 changed the title Int8 NHWC pack Int8 NHWC pack tensorize failure Jul 23, 2019
@tqchen
Copy link
Member

tqchen commented Jul 23, 2019

It seems was due to the recent revamp of integer simplification infra. One solution could be to introduce the bound information of the indices, (to hint ff.outer is non-negative) of Analyzer infra to tensorize.cc so the division simplification can be carried effectively.

Another way would be to carry through the floordiv/mod change(I could do that but need to wait a few weeks due to recent time constrains).

Would be helpful if anyone could try the first approach

@lly-zero-one
Copy link
Contributor

It seems to be broken some time ago (Even before #3526). Fortunately, I still have a working copy locally and try to dig into it this week.

@tqchen
Copy link
Member

tqchen commented Jul 23, 2019

@llyfacebook @anijain2305 it is likely due what i mentioned in the last post(not being able to simplify certain expressions), we could try the proposed solutions

@anijain2305
Copy link
Contributor Author

@tqchen Makes sense. As of now, I am busy with other stuff. Let's keep status help wanted for now. Maybe I can take a look later when I get some free cycles.

@anijain2305
Copy link
Contributor Author

@tqchen @sgrechanik-h I am seeing this failure for my own tensorize implementations, prompting me to make this a priority to fix.

However, I have never touched this piece of code. Can you please do more little more hand-holding to help me solve the problem?

@jwfromm
Copy link
Contributor

jwfromm commented Sep 1, 2019

Just wanted to add that I'm also encountering this bug when using tensorize for arm_cpu in some places (and also dont quite understand tianqis recommended fix).

@tqchen
Copy link
Member

tqchen commented Sep 2, 2019

To elaborate a bit, the simplification would need hints about bounds of the iteration variables(that they are not negative). This information can be put into the simplifier of tensorizer so that the expression can be simplified

@anijain2305
Copy link
Contributor Author

#3981

@kimishpatel and @tchen are working on this

@tqchen
Copy link
Member

tqchen commented Dec 19, 2019

closing for now due to inactive status, the new simplifier might resolve the problem, feel free to reopen

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants