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

[TOPI] Fix tuple unpack in conv2d NCHWc int8 #13761

Merged
merged 8 commits into from
Jan 17, 2023
Merged

Conversation

michalpiszczek
Copy link
Contributor

@michalpiszczek michalpiszczek commented Jan 11, 2023

Fixes a tuple being incorrectly unpacked (too many elements expected) in a Conv2D TOPI int8 op, similarly to #13566 which addressed the issue specifically in the ARM implementation (which calls the function modified in this PR).

This explicit unpacking helps improve flexibility with other incoming layouts.

@tvm-bot
Copy link
Collaborator

tvm-bot commented Jan 11, 2023

Thanks for contributing to TVM! Please refer to the contributing guidelines https://tvm.apache.org/docs/contribute/ for useful information and tips. Please request code reviews from Reviewers by @-ing them in a comment.

  • No users to tag found in teams: topi See #10317 for details

Generated by tvm-bot

@michalpiszczek michalpiszczek changed the title Fix tuple unpack in conv2d nchw int8 Fix tuple unpack in conv2d NCHWc int8 Jan 11, 2023
@michalpiszczek
Copy link
Contributor Author

cc @jwfromm

@michalpiszczek
Copy link
Contributor Author

@tvm-bot rerun

@ekalda
Copy link
Contributor

ekalda commented Jan 11, 2023

I know it's a miniature PR, but https://tvm.apache.org/docs/contribute/pull_request.html#commit-message-guideline

@michalpiszczek michalpiszczek changed the title Fix tuple unpack in conv2d NCHWc int8 [TOPI] Fix tuple unpack in conv2d NCHWc int8 Jan 11, 2023
@michalpiszczek
Copy link
Contributor Author

@tvm-bot rerun

@jwfromm
Copy link
Contributor

jwfromm commented Jan 11, 2023

Looks like this needs to be linted. I think its also worth throwing in a comment explaining this explicit unpacking helps improve flexibility with other incoming layouts.

@michalpiszczek
Copy link
Contributor Author

@tvm-bot rerun

1 similar comment
@michalpiszczek
Copy link
Contributor Author

@tvm-bot rerun

Copy link
Member

@junrushao junrushao left a comment

Choose a reason for hiding this comment

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

LGTM!

@junrushao junrushao merged commit 1258863 into apache:main Jan 17, 2023
fzi-peccia pushed a commit to fzi-peccia/tvm that referenced this pull request Mar 27, 2023
Fixes a tuple being incorrectly unpacked (too many elements expected) in a Conv2D TOPI int8 op, similarly to apache#13566 which addressed the issue specifically in the ARM implementation (which calls the function modified in this PR). 

This explicit unpacking helps improve flexibility with other incoming layouts.
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.

5 participants