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

Remove reference tensor creation in producer tensor indexing path #1750

Merged
merged 31 commits into from
Jun 25, 2022

Conversation

shmsong
Copy link

@shmsong shmsong commented Jun 4, 2022

Fixes #ISSUE_NUMBER

Copy link
Owner

@csarofeen csarofeen left a comment

Choose a reason for hiding this comment

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

LGTM

torch/csrc/jit/codegen/cuda/index_compute.cpp Show resolved Hide resolved
@@ -151,6 +165,32 @@ IndexingParameters getGlobalIndexParameters(
loop_indexing.loopDomains(),
index_parameters.initial_concrete_id_index);

// Setup double buffer increment for producer case:
Copy link
Owner

Choose a reason for hiding this comment

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

I don't remember the double buffering part of indexing. @naoyam could you double check the double buffering portions of the changes in this PR stack.

Copy link
Author

@shmsong shmsong Jun 25, 2022

Choose a reason for hiding this comment

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

This is just incrementing producer index by one on the main stage of double buffer loop, if the consumer is double buffered, i.e. prefetching. We can now make the logic more explicit.

Will do that in a follow up as the merged PR is already getting huge.

Copy link
Collaborator

Choose a reason for hiding this comment

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

That's right. double_buffer_loop is a loop that's double buffered in the current loop nest. We can just skip the rest if it's nullptr.

Base automatically changed from nonglobal_consumer_index to alloc_idx_val June 25, 2022 04:00
@shmsong shmsong changed the title WIP: remove reference tensor creation in producer tensor indexing path Remove reference tensor creation in producer tensor indexing path Jun 25, 2022
@shmsong shmsong merged commit 0de6736 into alloc_idx_val Jun 25, 2022
@shmsong shmsong deleted the producer_indexing_refactor branch June 25, 2022 04:23
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.

3 participants