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

Adding facebook dlrm in SHARK/tank #185

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Conversation

vidsinghal
Copy link

WIP: dlrm lowering through shark.
Failing to lower to Torch right now.
Exception:
Lowering TorchScript IR -> Torch Backend IR failed with the following diagnostics:
error: found a non-value tensor type, this is likely due to a missing case in the MaximizeValueSemantics pass
note: see current operation: %25 = "torch.copy.to_tensor"(%24) : (!torch.vtensor) -> !torch.tensor

@pashu123
Copy link
Collaborator

Hi @vid-999 I saw in the torch IR that there are some lowering of the OPs missing like %32:4 = torch.operator "aten.embedding_bag.padding_idx"(%31, %30, %29, %false, %int0, %true, %none, %false, %none) : (!torch.tensor<[4,2],f32>, !torch.tensor<[3],si64>, !torch.tensor<[1],si64>, !torch.bool, !torch.int, !torch.bool, !torch.none, !torch.bool, !torch.none) -> (!torch.tensor<[1,2],f32>, !torch.tensor<[0],si64>, !torch.tensor<[1],si64>, !torch.tensor<[1],si64>) and hence the above issue is common. Could you add all the missing lowerings and then try? I see there are two torch.operator s.

@vidsinghal
Copy link
Author

vidsinghal commented Jul 21, 2022

Hi @pashu123 thanks for pointing that out. I am actually in the process of writing the linalg lowering for the aten.embedding_bag.padding_idx op.
The only other torch.operator that I see is aten.to.device which I think I should be able to remove from the model code.

Also, maybe I misunderstood but when you say "try" you mean try to lower from TORCH to LINALG ?

@pashu123
Copy link
Collaborator

Hi @pashu123 thanks for pointing that out. I am actually in the process of writing the linalg lowering for the aten.embedding_bag.padding_idx op. The only other torch.operator that I see is aten.to.device which I think I should be able to remove from the model code.

Also, maybe I misunderstood but when you say "try" you mean try to lower from TORCH to LINALG ?

Yes, I mean torch to linalg. Also, since torch.operator is present, that op is not even registered in torch-mlir so the lowering means first adding it to torch-mlir and then lowering to linalg.

@vidsinghal
Copy link
Author

Yup. I have already registered it. I'm working on a branch.
Here is a draft PR. Haven't pushed the latest changes here though. llvm/torch-mlir#1066

@pashu123
Copy link
Collaborator

Yup. I have already registered it. I'm working on a branch.
Here is a draft PR. Haven't pushed the latest changes here though. llvm/torch-mlir#1066

Cool!

Copy link
Contributor

@powderluv powderluv left a comment

Choose a reason for hiding this comment

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

I would avoid checking in the .linalg and .torch . Maybe add a comment on how to generate it

@pashu123
Copy link
Collaborator

We can save the generated linalg, input, and golden output in the shark tank and then enable the tests.

@powderluv
Copy link
Contributor

We can save the generated linalg, input, and golden output in the shark tank and then enable the tests.

should just add to the gen_shark_tank.py script right ? the upload should happen via @dan-garvey nightly build change.

@vidsinghal
Copy link
Author

vidsinghal commented Jul 28, 2022

I added 2 manual tests for QrEmbedding and PrEmbedding modes.
It was not straightforward to make artificial inputs for dlrm that would run those modes.
But the code is representative of the code in the actual model (from the function create_emb)

For the QrEmbedding mode, one more op needs to be implemented:
torch.operator "aten.remainder.Scalar"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants