Skip to content

Add canonicalization for aten.add.tensor op #935

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 1 commit into from
Jun 23, 2022
Merged

Add canonicalization for aten.add.tensor op #935

merged 1 commit into from
Jun 23, 2022

Conversation

erman-gurses
Copy link
Collaborator

@erman-gurses erman-gurses commented Jun 13, 2022 •
edited
Loading

This PR is related to issue #911

@erman-gurses erman-gurses changed the title Add canonicalization for aten.add.tensor op [Draft] Add canonicalization for aten.add.tensor op Jun 13, 2022
@erman-gurses
Copy link
Collaborator Author

Added canonicalization based on the examples below:

Example1:

%0 = torch.prim.NumToTensor.Scalar %int0 : !torch.int -> !torch.vtensor<[],si64>
%1 = torch.prim.NumToTensor.Scalar %int2 : !torch.int -> !torch.vtensor<[],si64>
%2 = torch.aten.add.Tensor %0, %1, %int3 : !torch.vtensor<[],si64>, !torch.vtensor<[],si64>, !torch.int -> !torch.vtensor<[],si64>

Example2:

%0 = torch.vtensor.literal(dense<0> : tensor<si64>) : !torch.vtensor<[],si64>
%1 = torch.prim.NumToTensor.Scalar %int2 : !torch.int -> !torch.vtensor<[],si64>
%2 = torch.aten.add.Tensor %0, %1, %int3 : !torch.vtensor<[],si64>, !torch.vtensor<[],si64>, !torch.int -> !torch.vtensor<[],si64>

After canonicalization, they should look like this:

%0 =  aten.mul.int(%int2, %int3) : !torch.int, !torch.int -> !torch.int
%1 =  aten.add.int(%0, %int0) : !torch.int, !torch.int -> !torch.int
%2 = torch.prime.NumToTensor %1 :  !torch.int -> !torch.vtensor<[],si64>

@erman-gurses erman-gurses requested a review from pashu123 June 13, 2022 15:25
@nirvedhmeshram
Copy link
Collaborator

I think it would be helpful to add test in IR here
https://github.com/llvm/torch-mlir/blob/main/test/Dialect/Torch/canonicalize.mlir
to see how the pattern is working

@erman-gurses
Copy link
Collaborator Author

erman-gurses commented Jun 13, 2022 •
edited
Loading

@nirvedhmeshram Yes, I will do that, I am still missing to handle this part for now below. Working on it.
%0 = torch.vtensor.literal(dense<0> : tensor) : !torch.vtensor<[],si64>

silvasean
silvasean suggested changes Jun 13, 2022
View reviewed changes
@vivekkhandelwal1
Copy link
Collaborator

The issue that is being addressed with this PR: #911.

pashu123
pashu123 reviewed Jun 15, 2022
View reviewed changes
pashu123
pashu123 reviewed Jun 15, 2022
View reviewed changes
pashu123
pashu123 reviewed Jun 15, 2022
View reviewed changes
silvasean
silvasean reviewed Jun 15, 2022
View reviewed changes
pashu123
pashu123 reviewed Jun 17, 2022
View reviewed changes
pashu123
pashu123 reviewed Jun 17, 2022
View reviewed changes
@erman-gurses erman-gurses self-assigned this Jun 17, 2022
@erman-gurses erman-gurses marked this pull request as ready for review June 17, 2022 16:12
@erman-gurses erman-gurses requested a review from silvasean June 17, 2022 16:17
@pashu123 pashu123 changed the title [Draft] Add canonicalization for aten.add.tensor op Add canonicalization for aten.add.tensor op Jun 17, 2022
@erman-gurses erman-gurses mentioned this pull request Jun 17, 2022
Closed
@erman-gurses erman-gurses force-pushed the canonicalization-for-aten-add-tensor-op branch from 8bab003 to 837e563 Compare June 17, 2022 18:51
ramiro050
ramiro050 requested changes Jun 17, 2022
View reviewed changes
Copy link
Collaborator

@ramiro050 ramiro050 left a comment

Choose a reason for hiding this comment

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

Thanks for this! I have a few comments below

@erman-gurses erman-gurses force-pushed the canonicalization-for-aten-add-tensor-op branch from 837e563 to c6c40c5 Compare June 22, 2022 11:50
@Shukla-Gaurav Shukla-Gaurav mentioned this pull request Jun 22, 2022
Closed
@erman-gurses erman-gurses force-pushed the canonicalization-for-aten-add-tensor-op branch from c6c40c5 to 45523d6 Compare June 22, 2022 20:04
@erman-gurses erman-gurses requested a review from ramiro050 June 22, 2022 21:43
ramiro050
ramiro050 approved these changes Jun 22, 2022
View reviewed changes
Copy link
Collaborator

@ramiro050 ramiro050 left a comment

Choose a reason for hiding this comment

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

I just have a couple of small comments, but other than that it LGTM!

silvasean
silvasean reviewed Jun 22, 2022
View reviewed changes
@erman-gurses erman-gurses force-pushed the canonicalization-for-aten-add-tensor-op branch from 45523d6 to 80128df Compare June 23, 2022 13:52
@erman-gurses erman-gurses requested a review from silvasean June 23, 2022 14:00
@erman-gurses erman-gurses force-pushed the canonicalization-for-aten-add-tensor-op branch from 80128df to 0a198ca Compare June 23, 2022 16:14
@erman-gurses erman-gurses merged commit 5cff40c into llvm:main Jun 23, 2022
This was referenced Aug 2, 2022
Closed
Closed
This was referenced Aug 9, 2022
Closed
Merged
qedawkins pushed a commit to nod-ai/torch-mlir that referenced this pull request Oct 3, 2022
Change -Wno-everything to -w to account for old GNU toolchains. (llvm…
a913edb 
…#935)

Signed-off-by: Ettore Tiotto <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pashu123 pashu123 pashu123 left review comments

@nirvedhmeshram nirvedhmeshram nirvedhmeshram left review comments

@ramiro050 ramiro050 ramiro050 approved these changes

@Shukla-Gaurav Shukla-Gaurav Awaiting requested review from Shukla-Gaurav

@silvasean silvasean Awaiting requested review from silvasean

Assignees

@erman-gurses erman-gurses

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@erman-gurses @nirvedhmeshram @vivekkhandelwal1 @pashu123 @ramiro050 @silvasean