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

[TIR][Fix] Buffer slicing using index dtype as extent #13788

Merged

Conversation

MasterJH5574
Copy link
Contributor

This PR fixes a bug of TIR Buffer __getitem__ on Python side.

Whenever the __getitem__ parameter is a multi-dim slice, on non-slice dimension a Range will be created. Prior to this PR, that range will have int32 dtype in regardless of the start, while the proper behavior is to generate the extent according to the dtype of start when start is a PrimExpr.

This PR fixes the issue and provides a regression test.

@tvm-bot
Copy link
Collaborator

tvm-bot commented Jan 15, 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.

Generated by tvm-bot

@MasterJH5574
Copy link
Contributor Author

cc @cyx-6 @junrushao @tqchen

@MasterJH5574 MasterJH5574 changed the title [Fix] Buffer slicing using idnex dtype as extent [Fix] Buffer slicing using index dtype as extent Jan 15, 2023
@MasterJH5574 MasterJH5574 force-pushed the tvm-dev/2023-01-15-tir-parser-extent branch from f7e7528 to 84a9f8c Compare January 15, 2023 20:26
Copy link
Contributor

@cyx-6 cyx-6 left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for reporting and fixing this bug.

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, and would you like to revert the change to 3rdparty/cutlass? Seems like irrelevant

@MasterJH5574
Copy link
Contributor Author

LGTM, and would you like to revert the change to 3rdparty/cutlass? Seems like irrelevant

@junrushao Sorry my bad. I didn’t notice that... Will do later.

@MasterJH5574 MasterJH5574 force-pushed the tvm-dev/2023-01-15-tir-parser-extent branch from 84a9f8c to f951dcc Compare January 16, 2023 01:14
@junrushao junrushao changed the title [Fix] Buffer slicing using index dtype as extent [TIR][Fix] Buffer slicing using index dtype as extent Jan 16, 2023
test_different_dtype_assignment_to_var()
b = 1
test_var_capturing_order()
tvm.testing.main()
Copy link
Member

Choose a reason for hiding this comment

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

a = numpy.zeros((10, 10), dtype="int8") here is to test var capturing in #13640. I appreciate it if you could revert it and add a comment here.

@tqchen tqchen merged commit 49c92d9 into apache:main Jan 16, 2023
fzi-peccia pushed a commit to fzi-peccia/tvm that referenced this pull request Mar 27, 2023
[Fix] Buffer slicing using index dtype as extent
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.

6 participants