Skip to content

Fix a bug related to expanded broadcast IterDomains.#1489

Merged
wujingyue merged 2 commits intomainfrom
wjy/broadcast
Dec 11, 2023
Merged

Fix a bug related to expanded broadcast IterDomains.#1489
wujingyue merged 2 commits intomainfrom
wjy/broadcast

Conversation

@wujingyue
Copy link
Collaborator

My #1295 changed some tests in the wrong way so this bug has been hidden since then.

@wujingyue
Copy link
Collaborator Author

!build

@wujingyue wujingyue requested a review from jacobhinkle December 8, 2023 23:59
Copy link
Collaborator

@jacobhinkle jacobhinkle left a comment

Choose a reason for hiding this comment

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

A bit more explanation would be much appreciated. As I understand it, this applies when the input of a ViewOp has an allocation domain that is a permutation of its rfactor. The input's allocation domain is used to determine the contiguity of the output at its root domain (as opposed to .contiguity() which refers to out's allocation domain). That is then propagated from root to rfactor to check if we require a copy. The change seems reasonable to me but it's not clear to me yet how this fails currently.

@wujingyue
Copy link
Collaborator Author

A bit more explanation would be much appreciated. As I understand it, this applies when the input of a ViewOp has an allocation domain that is a permutation of its rfactor. The input's allocation domain is used to determine the contiguity of the output at its root domain (as opposed to .contiguity() which refers to out's allocation domain). That is then propagated from root to rfactor to check if we require a copy. The change seems reasonable to me but it's not clear to me yet how this fails currently.

Your understanding described above is all correct. The code currently fails because due to #1126 (or a similar issue that your #1174 is trying to fix) nvFuser doesn't propagate the has-expanded-extent flag from in_rfactor to out_root. As a result, the expressions between out_root and out_rfactor see out_root's IterDomains as a t or f instead of an e:

if (!outer_contiguity.has_value() && !outer_id->hasExpandedExtent()) {
.

@wujingyue
Copy link
Collaborator Author

!build

Copy link
Collaborator

@jjsjann123 jjsjann123 left a comment

Choose a reason for hiding this comment

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

LGTM

@wujingyue wujingyue merged commit 001f3f0 into main Dec 11, 2023
@wujingyue wujingyue deleted the wjy/broadcast branch December 11, 2023 19:25
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