-
Notifications
You must be signed in to change notification settings - Fork 49
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
Add a CPU lowering pass #26
Add a CPU lowering pass #26
Conversation
Thank you so much @shintaro-iwasaki! I'll start taking a look 😄 |
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.
@shintaro-iwasaki I left a few nit comments, other than that, everything looks good! Thanks again for contributing this. @manbearian mentioned that he couldn't get tensor store to work, so I'll be investigating that separately once we merge this.
@kshama-msft Would you mind helping Shintaro with the CLA question? Thank you! |
@shintaro-iwasaki if the CLA has been generated by the github bot then feel free to sign it. You should review with your internal open-source contribution legal teams to confirm that it is okay for you to sign. |
Thanks a lot for your review, @nhat-nguyen! |
@microsoft-github-policy-service agree [company="Meta Platforms, Inc."] |
@microsoft-github-policy-service agree company="Meta Platforms, Inc." |
The legal thing has been cleared on my side. Let me fix the issues pointed out and update the PR by the end of today. |
321ff98
to
c5b7d8b
Compare
Thanks! I updated the commit as well as rebased onto the latest (Do you want the PR author (me) to resolve the comments? Or should it be done by reviewers?) |
Thank you @shintaro-iwasaki, everything looks good! We usually let the PR authors resolve the comments but let me know if you'd prefer otherwise. Once the pipeline runs finish, I can merge in the changes. 😄 |
Merged! Thank @shintaro-iwasaki again! |
Per discussion in #24, this PR creates a reference-CPU backend for
triton-shared
using the standard MLIR->LLVM lowering passes.triton-shared
on CPU from Python-Triton code.reduce.py
) runs and succeeds.memref::tensorStore
->LLVM seems not implemented, so many kernels are not supported at this point.Help from the community and triton-shared core developers is needed to improve and maintain it. I'd appreciate your feedback and suggestions.
Note: compared with the PR I used in the discussion, this PR version can run a compute kernel (
reduce.py
, as an example) by using MLIR'sCRunnerUtils.h
.(EDIT: let me understand if I can sign this CLA)