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

Roll Torch-MLIR_Architecture.png into main diagram #1265

Merged
merged 1 commit into from
Sep 15, 2022
Merged

Conversation

powderluv
Copy link
Collaborator

Update all references to torch dialect
Roll the Torch-MLIR_Architecture.png contents into main diagram
Remove torch_dispatch() which was originally FX lowering

Copy link
Contributor

@silvasean silvasean left a comment

Choose a reason for hiding this comment

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

The "backend contract" box is super important for the architecture.md use of Torch-MLIR_Architecture.png, but probably not for the README. I think these two diagrams (README and architecture.md) have two slightly different target audiences, so let's keep them as separate files for now.

Also, the Torch dialect is actually used throughout the "purple" boxes in the Torch-MLIR "frontend". It is really deceptive to have it only in the backend.

Overall I think the original diagram is really good, and we just need to remove the torch_dispatch and replace Torch-MLIR Dialct with "Torch Dialect".

@sjain-stanford
Copy link
Member

Gentle check-in on this :)

I believe this is good to go with these two boxes removed?

image

Add MHLO and Custom dialects
@powderluv
Copy link
Collaborator Author

Overall I think the original diagram is really good, and we just need to remove the torch_dispatch and replace Torch-MLIR Dialct with "Torch Dialect".

Updated per @silvasean comment. PTAL

@powderluv powderluv merged commit 9111b9a into main Sep 15, 2022
@sjain-stanford
Copy link
Member

sjain-stanford commented Sep 15, 2022

Remove torch_dispatch() which was originally FX lowering

I don't think this was removed in the commit you just force pushed @powderluv . I believe we do want to remove the torch_dispatch path:

we just need to remove the torch_dispatch

@powderluv
Copy link
Collaborator Author

Ah ok. I missed the first part. Thanks for fixing in the follow on

sjain-stanford added a commit that referenced this pull request Sep 15, 2022
…1372)

Addresses leftover comment from earlier PRs: #1254 , #1265 to remove `torch_dispatch` frontend. In addition, moves the main arch diagram into `docs/` directory for consistency.
@powderluv powderluv deleted the update-diagram branch September 15, 2022 22:10
qedawkins pushed a commit to nod-ai/torch-mlir that referenced this pull request Oct 3, 2022
* add Krnl Ops

Signed-off-by: chentong319 <[email protected]>

* lowering

Signed-off-by: chentong319 <[email protected]>

* test

Signed-off-by: chentong319 <[email protected]>

* fix makefile

Signed-off-by: chentong319 <[email protected]>

* format

Signed-off-by: chentong319 <[email protected]>

* format

Signed-off-by: chentong319 <[email protected]>

Co-authored-by: Tung D. Le <[email protected]>
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