Skip to content

[inductor] return constant shape for ops that do not return #163023

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
Closed
isuruf wants to merge 2 commits into from
Closed

[inductor] return constant shape for ops that do not return #163023

isuruf wants to merge 2 commits into from

Conversation

@isuruf
Copy link
Collaborator

@isuruf isuruf commented Sep 15, 2025 •
edited by pytorch-bot bot
Loading

Stack from ghstack (oldest at bottom):

Returning None is bad because None means that we don't know the shape.
Returning () is a good choice as that shape broadcasts with any other
shape.

cc @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @wenzhe-nrv @jiayisunx @ipiszy @chenyang78 @kadeng @muchulee8 @amjames @chauhang @aakhundov @coconutruben

@isuruf
Update
35880f8 
[ghstack-poisoned]
@pytorch-bot
Copy link

pytorch-bot bot commented Sep 15, 2025 •
edited
Loading

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/163023

Note: Links to docs will display an error until the docs builds have been completed.

✅ No Failures

As of commit bb028ba with merge base cfc539f (image):
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Sep 15, 2025 •
edited
Loading

CLA Signed

The committers listed above are authorized under a signed CLA.

@isuruf isuruf mentioned this pull request Sep 15, 2025
Closed
@isuruf
Copy link
Collaborator Author

isuruf commented Sep 15, 2025

/easycla

@isuruf isuruf added the topic: not user facing topic category label Sep 15, 2025
@isuruf
Copy link
Collaborator Author

isuruf commented Sep 16, 2025

/easycla

@isuruf
Update
bb028ba 
[ghstack-poisoned]
@isuruf isuruf requested review from eellison and mlazos September 16, 2025 14:29
eellison
eellison reviewed Sep 16, 2025
View reviewed changes
Copy link
Contributor

@eellison eellison left a comment

Choose a reason for hiding this comment

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

Returning None is more consistent with other handlers which return None from these ops, like

pytorch/torch/_inductor/dtype_propagation.py

Lines 235 to 237 in d4554bc

@staticmethod
def store_reduction(name: str, index, value: DTypeArg) -> None:
return None
.

() could also be conflated with a single-element tensor. Is there a reason we need to do this ?

@isuruf
Copy link
Collaborator Author

isuruf commented Sep 16, 2025

There's code like CSEProxy where the return value of all the ops are wrapped in a TritonCSEVariable at

pytorch/torch/_inductor/codegen/triton.py

Lines 3656 to 3661 in d4554bc

return cse.generate(
helper,
getattr(overrides, name)(*args, **kwargs),
dtype=output_dtype,
shape=output_shape,
)

Since there's no assignment for ops.device_assert_async, the created TritonCSEVariable is discarded, but not before it raises an error that the shape is None.

Test failure:
https://hud.pytorch.org/pytorch/pytorch/pull/162275?sha=3da8dc6a9c149a653eff2b877ddc2ef5a79cf21b

@eellison
Copy link
Contributor

eellison commented Sep 16, 2025

Can we update device_assert_async so it behaves more like store ? cc @mlazos @karthickai

@karthickai
Copy link
Collaborator

karthickai commented Sep 16, 2025

@eellison, sure, I'll update the device_assert_async

@karthickai karthickai mentioned this pull request Sep 23, 2025
Closed
karthickai added a commit that referenced this pull request Sep 24, 2025
@karthickai
Update on "[Inductor] Update DeviceAssert op to behave like store"
21310b2 
Updated the DeviceAssert operation to match the behavior of Store, it will fixes the issue mentioned in [this PR](#163023) and updated testcases as Elias [suggested](#160677 (comment)).



cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng wenzhe-nrv jiayisunx ipiszy chenyang78 kadeng muchulee8 amjames chauhang aakhundov coconutruben mlazos choijon5 

[ghstack-poisoned]
pytorchmergebot pushed a commit that referenced this pull request Sep 24, 2025
@karthickai @pytorchmergebot
[Inductor] Update DeviceAssert op to behave like store (#163696)
8c98aee 
Updated the DeviceAssert operation to match the behavior of Store, it will fixes the issue mentioned in [this PR](#163023) and updated testcases as Elias [suggested](#160677 (comment)).

Pull Request resolved: #163696
Approved by: https://github.com/mlazos
@isuruf
Copy link
Collaborator Author

isuruf commented Sep 26, 2025

Thanks @karthickai for your PR

@isuruf isuruf closed this Sep 26, 2025
jainapurva pushed a commit that referenced this pull request Sep 29, 2025
@karthickai @jainapurva
[Inductor] Update DeviceAssert op to behave like store (#163696)
9afcf80 
Updated the DeviceAssert operation to match the behavior of Store, it will fixes the issue mentioned in [this PR](#163023) and updated testcases as Elias [suggested](#160677 (comment)).

Pull Request resolved: #163696
Approved by: https://github.com/mlazos
@github-actions github-actions bot deleted the gh/isuruf/153/head branch October 27, 2025 02:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@eellison eellison eellison left review comments

@mlazos mlazos Awaiting requested review from mlazos

Assignees

No one assigned

Labels

ciflow/inductor module: inductor open source topic: not user facing topic category

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@isuruf @eellison @karthickai @pytorchbot