Page MenuHomePhabricator

Re-engineer MLIR python build support.
ClosedPublic

Authored by stellaraccident on Wed, Jul 21, 9:43 PM.

Details

Summary
  • Implements all of the discussed features:
    • Links against common CAPI libraries that are self contained.
    • Stops using the 'python/' directory at the root for everything, opening the namespace up for multiple projects to embed the MLIR python API.
    • Separates declaration of sources (py and C++) needed to build the extension from building, allowing external projects to build custom assemblies from core parts of the API.
    • Makes the core python API relocatable (i.e. it could be embedded as something like 'npcomp.ir', 'npcomp.dialects', etc). Still a bit more to do to make it truly isolated but the main structural reset is done.
    • When building statically, installed python packages are completely self contained, suitable for direct setup and upload to PyPi, et al.
    • Lets external projects assemble their own CAPI common runtime library that all extensions use. No more possibilities for TypeID issues.
    • Begins modularizing the API so that external projects that just include a piece pay only for what they use.
  • I also rolled in a re-organization of the native libraries that matches how I was packaging these out of tree and is a better layering (i.e. all libraries go into a nested _mlir_libs package). There is some further cleanup that I resisted since it would have required source changes that I'd rather do in a followup once everything stabilizes.
  • Note that I made a somewhat odd choice in choosing to recompile all extensions for each project they are included into (as opposed to compiling once and just linking). While not leveraged yet, this will let us set definitions controlling the namespacing of the extensions so that they can be made to not conflict across projects (with preprocessor definitions).
  • This will be a relatively substantial breaking change for downstreams. I will handle the npcomp migration and will coordinate with the circt folks before landing. We should stage this and make sure it isn't causing problems before landing.
  • Fixed a couple of absolute imports that were causing issues.

Diff Detail

Event Timeline

stellaraccident requested review of this revision.Wed, Jul 21, 9:43 PM

if not installing into python/ anymore, can you document where things will get installed? I didn't quite parse it out of the code.

if not installing into python/ anymore, can you document where things will get installed? I didn't quite parse it out of the code.

Updated the Python.md documentation:

For interactive use, it is sufficient to add the
tools/mlir/python_packages/mlir_core/ directory in your build/ directory to
the PYTHONPATH. Typically:

shell
export PYTHONPATH=$(cd build && pwd)/tools/mlir/python_packages/mlir_core

Note that if you have installed (i.e. via ninja install, et al), then
python packages for all enabled projects will be in your install tree under
python_packages/ (i.e. python_packages/mlir_core). Official distributions
are built with a more specialized setup.

Side note: I really wish that MLIR's binary directory was not tools/mlir (and I wish it wasn't gated on the LLVM_BUILD_TOOLS option.

I haven't reviewed yet, but it's great to see this. I'll dig in today, and I'll help integrate the changes into CIRCT. The CI builds of CIRCT use the github.com/circt/llvm repo, and we can stage this there for testing as needed.

I've reviewed, and I think I'm understanding what's going on. I don't think I have enough CMake chops to provide serious suggestions/alternatives on the approach, so I'm just going to try it out with CIRCT and see how it goes. Assuming it works and we don't have to work around TypeID issues and whatnot, I'm satisfied!

I've reviewed, and I think I'm understanding what's going on. I don't think I have enough CMake chops to provide serious suggestions/alternatives on the approach, so I'm just going to try it out with CIRCT and see how it goes. Assuming it works and we don't have to work around TypeID issues and whatnot, I'm satisfied!

Thanks for the honesty. It is a wise path to not have such CMake chops (otherwise, you end up "me") :)

I have this working with one of our internal projects, modulo a couple of minor tweaks I'll push momentarily. If you'd like to wait until tomorrow, I'll have an npcomp branch staged with changes and you should be able to draft onto those. Probably easier than being the first to apply in a new project.

A couple of minor fixes found while integrating a third party project.

If you'd like to wait until tomorrow, I'll have an npcomp branch staged with changes and you should be able to draft onto those. Probably easier than being the first to apply in a new project.

Sounds good, I will apply the latest patch to my local MLIR but I won't dig into the circt changes just yet.

Add sources custom target to ALL.

We have pre-patched npcomp (https://github.com/llvm/mlir-npcomp/pull/251) and the iree-llvm-sandbox (https://github.com/google/iree-llvm-sandbox) successfully based on this patch. Mike says that he has a pending patch for CIRCT. So I think this is ready to land. I'll plan to do so tomorrow.

mikeurbach accepted this revision.Tue, Jul 27, 6:54 AM

This has been working out of the box for the CIRCT project. No more TypeID suprises, etc. Thanks Stella, this is super nice!

This revision is now accepted and ready to land.Tue, Jul 27, 6:54 AM
This revision was landed with ongoing or failed builds.Tue, Jul 27, 9:03 AM
This revision was automatically updated to reflect the committed changes.