- Incorporates a reworked version of D106419 (which I have closed but has comments on it).
- Extends the standalone example to include a minimal CAPI (for registering its dialect) and a test which, from out of tree, creates an aggregate dylib and links a little sample program against it. This will likely only work today in *static* MLIR builds (until the TypeID fiasco is finally put to bed). It should work on all platforms, though (including Windows - albeit I haven't tried this exact incarnation there).
- This is the biggest pre-requisite to being able to build out of tree MLIR Python-based projects from an installed MLIR/LLVM.
- I am rather nauseated by the CMake shenanigans I had to endure to get this working. The primary complexity, above and beyond the previous patch is because (with no reason given), it is impossible to export target properties that contain generator expressions... because, of course it isn't. In this case, the primary reason we use generator expressions on the individual embedded libraries is to support arbitrary ordering. Since that need doesn't apply to out of tree (which import everything via FindPackage at the outset), we fall back to a more imperative way of doing the same thing if we detect that the target was imported. Gross, but I don't expect it to need a lot of maintenance.
- There should be a relatively straight-forward path from here to rebase libMLIR.so on top of this facility and also make it include the CAPI.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
mlir/cmake/modules/AddMLIR.cmake | ||
---|---|---|
63 | Can you document what ENABLE_AGGREGATION does here? is there a reason why we shouldn't always do this (other than that it requires an OBJLIB?) |
mlir/cmake/modules/AddMLIR.cmake | ||
---|---|---|
282–283 | Need some checks here in case the library was not exported with the right information. |
mlir/cmake/modules/AddMLIR.cmake | ||
---|---|---|
63 | Done and added a TODO to just make it a default. I had originally tried to do that, but there is cruft in a number of the in-tree libraries that keep them from being built with an object library. I'd like to do a second pass and clean all of those up and then switch this to a DISABLE flag (i.e. may want to disable when just building a single-use runtime library, for example -- need to futz with that a bit). |
This seems to be causing some build failures: https://github.com/google/mlir-hs/runs/3894495306?check_suite_focus=true
That is weird. It appears to be something odd in the debug logging the new
rule does (which I left enabled for everyone). I think we can just comment
that out with a note that it can be enabled if needed.
I don't know much about this project. If you have it buildable, would you
mind testing my theory. Comment out the two commands here:
https://github.com/llvm/llvm-project/blob/b6c218d4fdb74c0ee467e078721438c3396dc599/mlir/cmake/modules/AddMLIR.cmake#L342
mlir/cmake/modules/AddMLIR.cmake | ||
---|---|---|
371 | @stellaraccident is this really needed for anything? I mean which part is supposed to consume these *.o files? |
mlir/cmake/modules/AddMLIR.cmake | ||
---|---|---|
371 | The add_mlir_aggregate uses them when constructing a shared library for a slice of the dependencies. Installing them supports out of tree projects (building from an installed LLVM) and allows them to build such things in the same way that we do in-tree. This is used today for downstream Python and Haskell bindings, both of which build a subset of the MLIR dependencies to bundle for their native package manager. In the future, it could allow out of tree projects to roll their own libMLIR.so equivalent, which is a feature that keeps getting asked for (i.e. as far as I understand it, a lot of folks have workflows where one team/CI produces the LLVM installs for all of their architectures and then multiple "downstream" teams build MLIR based things on top of them). I assume you are asking because this universally adds bulk to the installs. I had considered gating this with a CMake option (i.e. MLIR_INSTALL_AGGREGATE_OBJECTS), which would let us scope the cost to just the folks who have this use case. |
mlir/cmake/modules/AddMLIR.cmake | ||
---|---|---|
371 | Thank you for the explanation.
That is correct. All of a sudden /usr/lib64/objects-RelWithDebInfo/obj.MLIRCAPI* files appeared that were installed but not packaged. I've asked my question because I'd like to understand where to put the files.
Sure, then packagers can gradually opt in to this kind of extra files. I'd suggest to make the default true though and add a very good explanation what it does. Then we have one build failing and you're forced to make a decision as a packager. I'd rather have this than going silent without ever knowing about these extra set of files. Make sense? |
mlir/cmake/modules/AddMLIR.cmake | ||
---|---|---|
371 | Makes sense (and also thank you for work/attention to packaging). Just to restate before rolling a patch:
|
Can you document what ENABLE_AGGREGATION does here? is there a reason why we shouldn't always do this (other than that it requires an OBJLIB?)