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

[downstream project integration] Enable use of LLVM_EXTERNAL_PROJECTS #1795

Open
christopherbate opened this issue Oct 18, 2022 · 4 comments

Comments

@christopherbate
Copy link
Collaborator

christopherbate commented Oct 18, 2022

Other MLIR repositories such as mlir-hlo support using the LLVM_EXTERNAL_PROJECTS (https://github.com/tensorflow/mlir-hlo/blob/master/CMakeLists.txt#L93) as a build mechanism for plugging in the provided dialects into a downstream project.

IREE also uses this mechanism for various dialects (https://github.com/iree-org/iree/tree/main/llvm-external-projects).

It would be great if the build system could be updated to support this mechanism for at least the ONNX dialect, which allows for easier integration of onnx-mlir into downstream projects.

Currently it seems like the project has a very sensitive build system, and downstream users who may only need a small part of the project don't want to build all the dialects or the runtime if they only want to use onnx-mlir as a frontend layer that imports an ONNX model to the ONNX IR level. Such a user might end up rolling their own CMake integration instead of using the current build system.

Related: #1597

Edit:

Also related, other orgs seem to be rolling their own forks: https://github.com/nod-ai/onnx-mlir-exporter

@AlexandreEichenberger
Copy link
Collaborator

Opened an issue in the onnx-mlir-exporter to see if they can provide their expertise to merge what they need back in onnx-mlir
nod-ai/onnx-mlir-exporter#1

@christopherbate We have asked the MHLO and Pytorch-MLIR to have a switch so that folks don't necessarily have to build their stuff, though for our CIs we want to test everything. Would you be able to contribute and/or collaborate with the folks from onnx-mlir-exporter to properly switch off what you don't need?

Historically, the only way to use this repo was to lower to binary... but now that there are new bridges from onnx-mlir to mlir dialects being built, through TOSA/MHLO/Torch-MLIR, obviously what used to be necessary is not so anymore.

We have a dependence on ONNX as I believe we use some of the protobuf tools that ONNX provide to parse the input files. Presumably any project using onnx-mlir GitHub repo would need that component. But beyond that, I can see that the lowering from ONNX to Krnl/Zhigh/Zlow and everthing after that could be switched off under the right cmake variables.

Further opinions @gongsu832 @caoimhinuibrian ?

@powderluv
Copy link

Just following up on this thread from nod-ai/onnx-mlir-exporter#1

We created that fork mostly to refactor / untangle the different layering of the monolithic onnx-mlir build system so we could "exit" with the onnx-mlir dialect. We did that to start prototyping where to do the torch-mlir backend contract integration which eventually lead to #1639 and the corresponding first PR #1731 . Unfortunately we are still navigating the CI to make sure all the tests pass. Having the ability to run the CI locally would help too (we have something like https://github.com/llvm/torch-mlir/blob/main/docs/development.md#mimicing-cirelease-builds in torch-mlir). Once that is done we have a set of follow-on patches for further lowering.

On the build infra side we are happy to help contribute. We setup a similar setup with torch-mlir CI with in-tree LLVM and out-of-tree LLVM builds. @ashay has helped a lot and could possibly have some guidance here too.

In general having a core set that is buildable / testable and others that can be enabled / disabled (like we have MHLO and LTC in torch-mlir) would be super useful for developer velocity.

Overall we (nod.ai team) have multiple customers that are interested in the the onnx-mlir path (ONNX->onnx-mlir->torch_backend-->mhlo/tosa/tcp/linalg/custom) and are willing to help with this effort (once we get our onnx-mlir->torch-mlir backend contract PRs in).

@christopherbate
Copy link
Collaborator Author

christopherbate commented Oct 19, 2022

Thanks for your response.

Would you be able to contribute and/or collaborate with the folks from onnx-mlir-exporter to properly switch off what you don't need?

I see this less as a problem with predicating project features in the build and more of an organizational problem. Specifically, in this issue I'm recommending to use the LLVM "External Projects" feature to isolate your core dialect (ONNX) that is probably the most useful to downstream projects from the rest of your project. You would essentially be treating it as a dependency in a manner that prevents developers from breaking the "use of ONNXDialect out of onnx-mlir tree" feature. I've submitted a simple rearrangement of your build that shows how this works:

#1796

I do not know how much time I would be able to put in to drive this to completion. Ideally you guys could help with the issues I describe in the PR (if you agree with this approach). If those small issues are solved, landing the feature should be easy. I believe that having a repo maintainer take over the PR from that point would be best.

Edit: typos/clarity

@ashay
Copy link
Contributor

ashay commented Oct 20, 2022

This would be useful for our downstream project too. Currently, we include all of onnx-mlir in our project (by way of add_subdirectory(onnx-mlir)) when we really only need a couple of dialects. Our ad-hoc workaround to avoid long-ish build times is to recommend that folks build specific targets only. Being able to just build a few dialects would probably help organize our project better as well.

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

4 participants