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

Migration from MLIR-HLO to StableHLO #1835

Closed
ashay opened this issue Jan 30, 2023 · 6 comments
Closed

Migration from MLIR-HLO to StableHLO #1835

ashay opened this issue Jan 30, 2023 · 6 comments

Comments

@ashay
Copy link
Collaborator

ashay commented Jan 30, 2023

Currently, torch-mlir contains a lowering from the torch dialect to the mhlo dialect. However, lowering to the StableHLO dialect seems beneficial for several reasons:

  1. StableHLO supports versioning (thus forward and backward compatibility within major versions) and serialization.

  2. StableHLO has a smaller codebase than MLIR-HLO, thus causing less churn in downstream projects like torch-mlir due to updates (e.g. reorganized directories, renamed targets).

  3. Unlike the CI for the StableHLO repository, the CI for the MLIR-HLO repository does not use CMake (it uses Bazel), so the CMake build for MLIR-HLO breaks often. I fix these breakages in the MHLO green commits, but it's recurring work to fix them and submit patches upstream:

So my questions for the TorchToMhlo maintainers are: (a) would there be any loss in functionality if we switched to using StableHLO and (b) would you (or someone familiar with that code) be willing to update the code to switch to StableHLO please? Thanks!

@ashay
Copy link
Collaborator Author

ashay commented Jan 30, 2023

cc: @tanyokwok, @ZihengJiang, @Vremold

@burmako
Copy link

burmako commented Jan 31, 2023

Hi everyone! Smaller footprint is one of the reasons why we started StableHLO in a separate repository, and it's great that this could potentially come in handy.

We have recently migrated JAX from producing MHLO to producing StableHLO (jax-ml/jax@a1480c4), and that was fairly straightforward thanks to how close StableHLO is to MHLO. Based on this experience, I think that StableHLO would be a good fit for Torch-MLIR as well.

We'll be happy to support the migration effort - please let us know if there's anything we can do to help.

@joker-eph
Copy link
Contributor

Unlike the CI for the StableHLO repository, the CI for the MLIR-HLO repository does not use CMake

Actually I'm pretty it does, before Bazel files were even added to this repo. I filed tensorflow/mlir-hlo#58 to make it publicly visible.

so the CMake build for MLIR-HLO breaks often.

This may be a coverage problem with the cmake configuration used by the CI, so in the same ticket as above I mentioned that open-sourcing the script used by the Kokoro CI (even if the runs aren't visible) may already help figuring out why you found issues that weren't uncovered by the CI?

@ashay
Copy link
Collaborator Author

ashay commented Feb 1, 2023

My apologies, I wasn't aware of the CMake build for MHLO. Thanks for creating the issue! That should help me understand why my builds fail (or what else needs to be added to the existing build).

@ashay
Copy link
Collaborator Author

ashay commented Feb 1, 2023

Here (#1840) is an initial attempt at the migration.

@ashay
Copy link
Collaborator Author

ashay commented Feb 6, 2023

Closing, since #1840 resolves this issue.

@ashay ashay closed this as completed Feb 6, 2023
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

No branches or pull requests

3 participants