Skip to content

Support decomposition of torch.broadcast_tensors #4253

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 17 commits into from
Aug 28, 2025
Merged

Support decomposition of torch.broadcast_tensors #4253

merged 17 commits into from
Aug 28, 2025

Conversation

vinitdeodhar
Copy link
Contributor

@vinitdeodhar vinitdeodhar commented Jul 2, 2025 •
edited
Loading

Added support for torch.broadcast op and decomposition of broadcast_tensor to a sequence of aten.broadcast_to

closes #4240

@sahas3 sahas3 requested review from vivekkhandelwal1 and sahas3 July 3, 2025 14:26
@vinitdeodhar
Copy link
Contributor Author

@sahas3 , @vivekkhandelwal1 can you please review ? The failing checks are unrelated to the change and similar failures are seen for other recent PRs

sahas3
sahas3 requested changes Jul 10, 2025
View reviewed changes
@vinitdeodhar
Copy link
Contributor Author

Hi @vivekkhandelwal1, can you please provide your feedback ?

@vinitdeodhar vinitdeodhar requested a review from sahas3 July 18, 2025 18:00
@vinitdeodhar
Copy link
Contributor Author

@zjgarvey I am not sure who is the code owner who could review this change. Can you please provide feedback on this if possible ?

@zjgarvey zjgarvey self-requested a review August 7, 2025 17:30
zjgarvey
zjgarvey requested changes Aug 11, 2025
View reviewed changes
Copy link
Collaborator

@zjgarvey zjgarvey left a comment

Choose a reason for hiding this comment

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

Thanks! A few things to address before approval (most importantly the potentially wrong check on shape index).

@zjgarvey
Copy link
Collaborator

I'll re-review tomorrow morning, thanks for the update.

zjgarvey
zjgarvey reviewed Aug 21, 2025
View reviewed changes
Copy link
Collaborator

@zjgarvey zjgarvey left a comment •
edited
Loading

Choose a reason for hiding this comment

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

This looks good, but I have a few small suggestions after looking through again. Let me know what you think and I can re-review/approve/merge.

Mostly I think improving the folder would be nice, and it would be good to give the "reverse dim" indices names in the broadcast util function (+ some other nit suggestions there).

sahas3
sahas3 approved these changes Aug 21, 2025
View reviewed changes
Copy link
Member

@sahas3 sahas3 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 addressing the feedbacks/clarifications.

@vinitdeodhar
Copy link
Contributor Author

@sahas3 @zjgarvey thanks for your feedback. Can you help merge this change ? The failing check is an unrelated issue

@sahas3
Copy link
Member

sahas3 commented Aug 22, 2025 •
edited
Loading

@sahas3 @zjgarvey thanks for your feedback. Can you help merge this change ? The failing check is an unrelated issue

Looks like the version torch==2.8.0.dev20250622 is not available anymore. We need to upgrade to a more recent torch nightly version, I see two PRs: #4298 and #4270 for updating torch version. Hopefully one of them will be merged soon to re-enable the CIs.

I'll wait for @zjgarvey's final approval before merging. Thanks!

zjgarvey
zjgarvey approved these changes Aug 22, 2025
View reviewed changes
Copy link
Collaborator

@zjgarvey zjgarvey left a comment

Choose a reason for hiding this comment

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

We need to wait for #4298 to land and sync with main to unblock CI.

Thanks for the changes.

@zjgarvey
Copy link
Collaborator

Looks good, let me know if you want me to merge!

@vinitdeodhar
Copy link
Contributor Author

Looks good, let me know if you want me to merge!

Yes it would be great if you can help merge !

@rafaelubalmw rafaelubalmw merged commit 4b6bf0d into llvm:main Aug 28, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sahas3 sahas3 sahas3 approved these changes

@zjgarvey zjgarvey zjgarvey approved these changes

@vivekkhandelwal1 vivekkhandelwal1 Awaiting requested review from vivekkhandelwal1

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support for torch.broadcast_tensors in linalg backend
4 participants
@vinitdeodhar @zjgarvey @sahas3 @rafaelubalmw