Update Torch-MLIR Architecture Diagram

[powderluv]

Add MHLO path
Add custom accelarator dialects
Rename Torch Dialect back to original Torch-MLIR Dialect
(Surrounding text still refers to Torch-MLIR dialect)
Check in source for Excalidraw(https://excalidraw.com/)
so anyone can use / update it using the open source version

[henrytwo]

Mostly LGTM, but shouldn't there be an arrow from the MLIR dialects to the backends?

Is the dialect itself actually going to be renamed to the Torch-MLIR dialect, or is this just a clarification for this diagram?

[henrytwo]

Another minor thing: Isn't it TorchScript instead of Torchscript? (https://pytorch.org/docs/stable/jit.html)

And nit: Maybe github.com -> github.com

[powderluv]

Mostly LGTM, but shouldn't there be an arrow from the MLIR dialects to the backends?

I had to show too many arrows for every combo so dropped it. Happy to add it back if we feel like we want it.

Is the dialect itself actually going to be renamed to the Torch-MLIR dialect, or is this just a clarification for this diagram?

I was just trying to make the diagram consistent with the text around it (previous images had it as torch-dialect https://github.com/llvm/torch-mlir/blob/e59a91620a1afd341b7eef9153cc9da35cb8e876/Torch-MLIR.png). I don't think we need to change the code.

Will update to TorchScript. and github.com as part of any further feedback.

[henrytwo]

Wait now it says gitHub instead of github 😆

[powderluv]

Will update again in the next revision :D

[sjain-stanford]

Thank you for doing this!
Should we drop the torch_dispatch frontend, and bring it in sync with the architecture diagram here.
A few easy-to-fix nits:

• perhaps rename "Torch-MLIR Dialect" with "Torch Dialect"
• s/Torchscript/TorchScript in the box that says "Torchscript/MLIR Converter"

[powderluv]

Should we drop the torch_dispatch frontend, and bring it in sync with the architecture diagram here.

I think we should reconcile both at some point. Maybe @silvasean has that as a more low level diagram ?

Should we drop the torch_dispatch frontend

We use the eager mode and FX tracing support with torch_dispatch. It used to be called TorchFX lowering but I think we still need it there.

perhaps rename "Torch-MLIR Dialect" with "Torch Dialect"

Personally I think Torch-MLIR Dialect is better than just Torch Dialect since we may have other entry points like ONNX (#1255) but open to whatever we want to name / call it.

s/Torchscript/TorchScript in the box that says "Torchscript/MLIR Converter"

Will do in next update along with any other feedback.

[sjain-stanford]

I think we should reconcile both at some point. Maybe @silvasean has that as a more low level diagram ?

Ah I should clarify - by "bring it in sync with the architecture diagram here" I meant syncing the # of frontends (three vs two) without including the low level details (e.g. the various simplification passes) which I agree are a better fit for the architecture doc. I didn't see the torch_dispatch input in @silvasean 's diagram so this difference is a bit confusing (at least to me). IIUC there are plans to add a 3rd edge (TorchDynamo) but we are not there yet.

[ashay]

LGTM!

[ZihengJiang]

This looks great!

[sstamenova]

Thanks for keeping this updated!

[silvasean]

My architecture diagram doesn't include the torch_dispatch one because it is not really being invested in and from an architectural perspective it is the same as the TorchScript path. It's not really a "PyTorch" thing it's something we rolled ourselves. I would drop torch_dispatch from the diagram personally -- it's not something users would actually be like "oh, that one".

I think the dialect needs to be called "torch" in the diagram. The dialect name is torch.

[powderluv]

I will document two Action Items / discussions here:

1. Do we drop torch_dispatch ?
2. Rename torch-mlir dialect in diagrams / documentation to match torch dialect in code.

It may be worth considering renaming code to torch-mlir dialect especially if we add support for things like ONNX import #1255 torch-mlir dialect seems more extensible - but I'm ok either way.

Happy to update both based on further discussions.

[silvasean]

@powderluv Can you please call the dialect "torch" dialect? That is the name of the dialect.

[silvasean]

As far as the dialect name, a more likely direction would be to call it torch_jit_ir since that is what it is.

[powderluv]

So should I rename the doc and image to replace what is now called "torch-mlir dialect" to "torch_jit_ir dialect" or just "torch dialect". Either is fine with me.

[silvasean]

The present state is that the dialect is called the torch dialect. All docs and diagrams should use that name.

[powderluv]

#1265 to address the two outstanding comments and rolls in @silvasean diagram too