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

[core IR] Add a core decomposition for floor_divide #110046

Closed
wants to merge 3 commits into from

Conversation

SS-JIA
Copy link
Contributor

@SS-JIA SS-JIA commented Sep 25, 2023

Stack from ghstack (oldest at bottom):

Context

Introduce a core decomposition for aten.floor_divide into other aten ops, and add it to the core ATen decomposition table.

This replaces the decomposition of floor_divide that was used by Inductor. I noticed there was a note on that decomposition

# TorchInductor-only decomposition. It should not be taken to core.
# See https://github.com/pytorch/torchdynamo/pull/1120

but couldn't discern the reason why this is the case. cc: @lezcano

cc @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @Xia-Weiwen @wenzhe-nrv @jiayisunx @peterbell10 @ipiszy @yf225 @chenyang78 @kadeng @muchulee8 @aakhundov

@pytorch-bot
Copy link

pytorch-bot bot commented Sep 25, 2023

🔗 Helpful Links

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

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

⏳ No Failures, 1 Pending

As of commit 75d4547 with merge base 52e1478 (image):
💚 Looks good so far! There are no failures yet. 💚

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

SS-JIA added a commit that referenced this pull request Sep 25, 2023
ghstack-source-id: 24e3f183889d3a2d988bd05cff681d45d191485e
Pull Request resolved: #110046
## Context

Introduce a core decomposition for `aten.floor_divide` into other `aten` ops, and add it to the core ATen decomposition table.


[ghstack-poisoned]
SS-JIA added a commit that referenced this pull request Sep 25, 2023
ghstack-source-id: 6accf4059304c69be09523b8ff0ee801da48475c
Pull Request resolved: #110046
@SS-JIA SS-JIA requested a review from lezcano September 25, 2023 23:51
@lezcano
Copy link
Collaborator

lezcano commented Sep 26, 2023

I don't quite remember what was the issue here really. I think that back then, our lowering for floordiv in triton was "faster but broken" and people were wary of decomposing floor_div as done in core. I think that @peterbell10 worked on fixing the lowering, and now, funnily enough, the decomposition in core is tighter than the lowering in triton (it does not use 2 wheres).

If this was indeed the reason (let's see if Peter remembers) then I guess we can move it to core, yep.

## Context

Introduce a core decomposition for `aten.floor_divide` into other `aten` ops, and add it to the core ATen decomposition table.

This replaces the decomposition of `floor_divide` that was used by Inductor. I noticed there was a note on that decomposition

```
# TorchInductor-only decomposition. It should not be taken to core.
# See pytorch/torchdynamo#1120
```

but couldn't discern the reason why this is the case. cc: lezcano 


cc voznesenskym penguinwu EikanWang jgong5 Guobing-Chen XiaobingSuper zhuhaozhe blzheng Xia-Weiwen wenzhe-nrv jiayisunx peterbell10 ipiszy yf225 chenyang78 kadeng muchulee8 aakhundov

[ghstack-poisoned]
SS-JIA added a commit that referenced this pull request Sep 26, 2023
ghstack-source-id: 204fa68c34379511fedb36e74d24cbcff3fcca77
Pull Request resolved: #110046
@SS-JIA
Copy link
Contributor Author

SS-JIA commented Sep 26, 2023

@pytorchbot merge

@pytorch-bot pytorch-bot bot added the ciflow/trunk Trigger trunk jobs on your pull request label Sep 26, 2023
@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

pytorchmergebot pushed a commit that referenced this pull request Sep 27, 2023
## Context

Add the decomposition for `aten.glu` as a decomposition in the core ATen decomposition table. Don't use it in the Inductor decomposition table since Inductor has a lowering for it.

Pull Request resolved: #110043
Approved by: https://github.com/peterbell10, https://github.com/lezcano
ghstack dependencies: #110046
ringohoffman pushed a commit to ringohoffman/pytorch that referenced this pull request Sep 27, 2023
## Context

Introduce a core decomposition for `aten.floor_divide` into other `aten` ops, and add it to the core ATen decomposition table.

This replaces the decomposition of `floor_divide` that was used by Inductor. I noticed there was a note on that decomposition

```
# TorchInductor-only decomposition. It should not be taken to core.
# See pytorch/torchdynamo#1120
```

but couldn't discern the reason why this is the case. cc: @lezcano

Pull Request resolved: pytorch#110046
Approved by: https://github.com/peterbell10
ringohoffman pushed a commit to ringohoffman/pytorch that referenced this pull request Sep 27, 2023
## Context

Add the decomposition for `aten.glu` as a decomposition in the core ATen decomposition table. Don't use it in the Inductor decomposition table since Inductor has a lowering for it.

Pull Request resolved: pytorch#110043
Approved by: https://github.com/peterbell10, https://github.com/lezcano
ghstack dependencies: pytorch#110046
Git-tjb pushed a commit to Git-tjb/MyPytorch that referenced this pull request Oct 6, 2023
ghstack-source-id: 37b9e14da1f577f3589e530d687af135c9dc9444
Pull Request resolved: pytorch/pytorch#110046
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants