This repository was archived by the owner on Aug 1, 2025. It is now read-only.
[inductor] Revise #956 to avoid a perf regression #981
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
desertfire
changed the title
ngimel
reviewed
Aug 24, 2022
torchinductor/codegen/triton.py
Outdated
| index, mask = self.indexing(index) | ||
| if is_index_0 and "tl.zeros" not in index: | ||
| # Need dense_indexing when index == 0 | ||
| index = f"{index} + tl.zeros({self.dense_size_str()}, tl.int32)" |
ngimel
reviewed
Aug 24, 2022
Comment on lines
+655
to
657
| if need_dense and not have_dense: | ||
| mask = dense_mask | ||
| index_str = f"{index_str} + tl.zeros({self.dense_size_str()}, tl.int32)" |
There was a problem hiding this comment.
Suggested change
| if need_dense and not have_dense: | |
| mask = dense_mask | |
| index_str = f"{index_str} + tl.zeros({self.dense_size_str()}, tl.int32)" | |
| if need_dense and not have_dense or index == 0: | |
| index_str = f"{index_str} + tl.zeros({self.dense_size_str()}, tl.int32)" | |
| if index == 0: | |
| mask = ["None"] | |
| else: | |
| mask = dense_mask |
and remove changes to load. If index is 0, you don't need mask at all, a single element load is always valid.
desertfire
force-pushed
the
binbao/torchinductor_3
branch
from
396d8a1 to
4689b59
Compare
The perf regression is because my previous change will cause some torchdynamo/torchinductor/codegen/triton.py Line 721 in fb2fb47 I am actually surprised to see how perf measurement on AWS is stable enough to capture a 3% regression correctly. I will look into how stable the numbers are on the CI and consider add perf checking for CI tasks. |
ngimel
approved these changes
Aug 24, 2022
desertfire
added a commit
that referenced
this pull request
Aug 25, 2022
Summary: #981 introduced a misaligned address bug which relates how tl.load from index 0 should be written in triton.
desertfire
added a commit
that referenced
this pull request
Aug 25, 2022
Summary: #981 introduced a misaligned address bug which relates how tl.load from index 0 should be written in triton.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.