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

[Bug Fix] Fix bug in dense_nopack of x86 when using dynamic input shape #8166

Merged
merged 1 commit into from
Jun 5, 2021

Conversation

lygztq
Copy link
Contributor

@lygztq lygztq commented Jun 1, 2021

When K is dynamic, this line k = te.reduce_axis((0, K // vec), "k") cannot work.

@jroesch @kevinthesun @tkonolige @tqchen

@tqchen
Copy link
Member

tqchen commented Jun 4, 2021

Thanks @lygztq , can you please add a test case to cover this bug?

@tqchen tqchen added the status: need test case need test cases to cover the change label Jun 4, 2021
@tqchen tqchen assigned ke4qqq and kevinthesun and unassigned ke4qqq Jun 4, 2021
@lygztq
Copy link
Contributor Author

lygztq commented Jun 5, 2021

Thanks @lygztq , can you please add a test case to cover this bug?

I think there is already a test case for this in relay (see here), but it does not seem to be used in CI. This test will fail on my machine if we do not use cublas for cuda devices and may not invoke this x86 implementation on cpu. And I am not sure whether adding a dynamic shape tensor test here directly is a good choice.

@tqchen
Copy link
Member

tqchen commented Jun 5, 2021

I see, in that case let us merge it for now and add tests later if we have a better idea. Thanks @lygztq !

@tqchen tqchen merged commit 43387d0 into apache:main Jun 5, 2021
@tqchen tqchen removed the status: need test case need test cases to cover the change label Jun 5, 2021
trevor-m pushed a commit to trevor-m/tvm that referenced this pull request Jun 17, 2021
trevor-m pushed a commit to neo-ai/tvm that referenced this pull request Jun 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants