-
Notifications
You must be signed in to change notification settings - Fork 514
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
[uncategorized_lowerings] Add lowering for torch.aten.round.decimals #3811
base: main
Are you sure you want to change the base?
Conversation
c7a3666
to
286443f
Compare
Implement missing lowering for the op in a similar fashion as done by torch inductor. Also fix data movement and reduce op variants patterns to correctly handle explicitly declared legal ops. Signed-off-by: Prathamesh Tagore <[email protected]>
286443f
to
0f5a2dc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not able to understand why the lowering for the op torch.aten.round.decimals
is added as a custom op lowering, why don't do it the right way?
Please follow the steps specified here https://github.com/llvm/torch-mlir/blob/main/docs/Torch-ops-E2E-implementation.md to add the op lowering.
Hi @meshtag, is this PR still of interest to you? |
Hey @vivekkhandelwal1, looks like I missed this PR. Apologies for that.
AFAICT, we have two options here:
This PR tries to go via path 1. I am not sure if we should discard path 2 though, can you please share your thoughts on this.
Sure, I can do this (if we want to go via this path). Thanks for pointing it out. |
If you want to go thorough the path 1, then I don't think you have to do much. Just register this op in Torch-MLIR, and the same lowering which you have written can be used. Also, you would be able to test the correctness of your lowering e2e. |
Implement missing lowering for the op in a similar fashion as done by torch inductor. Also fix data movement and reduce op variants patterns to correctly handle explicitly declared legal ops.
Inductor decomposition ref: https://github.com/pytorch/pytorch/blob/main/torch/_inductor/decomposition.py#L223.