I’ve been trying to drive the incubators in this direction, where a repo (with its dialects, tools, and extensions) becomes a packaging unit, with CMake exports as the (only?) information transfer between them. It is still clumsier than I would like: The export mechanism itself is non-trivial to set up. Using git submodules to integrate things together is imperfect. Versioning overall is a bit of a nightmare, etc. A proper packaging infrastructure would really help here and I think there’s alot of newish technology out there that might help. I’d be really hesitant to just go build something.
Every time I’ve done this thought experiment, I’ve found the limits to be somewhat technical but largely development process related: doing this right requires committing to levels of stability and boundaries that we have so far been unwilling to commit to. Unless if something changes there, I don’t think there are any technical solutions that make the whole story substantially better (feel free to accept this as a challenge 
).
2 Likes
I think we should differentiate the C API from other languages: it is the implementation of the low level bindings of the core API that everyone else consumes (potentially including some form of high level C API should there ever be demand for such a thing). Colocating the dialect specific parts of the CAPI with the C++ internal implementations makes sense to me. I wouldn’t necessarily extend that to other languages (many of which have their own idea of directory structures, etc which have to be adhered to).
Might be worth breaking this out as a top level thing to discuss/explore. A lot of people I trust have been sliding vcpkg to me as a possible solution to our woes (which, has a really unfortunate first two letter association that has nothing to do with reality).
For sure, and I just had a very interesting discussion with @River707 on that topic as well recently. But my comment was only related to the directory reorganization. Currently we have one include directory, with underneath a structure that mimics the structure under lib (viz. top/include/X/Y with top/lib/X/Y). I like the proposed (top/X/Y with top/X/Y/include).
I don’t think C is a special case, just because other bindings often use it. E.g. Swift can progressively talk directly to C++ these days, so it has a shrinking need for the C bindings.
The only scalable approach here is to treat the bindings just like any other library in the ecosystem. What I would recommend is a system like:
llvm/lib/Bindings/ // Generic LLVM support for bindings in general
C/*
Swift/*
Python/*
mlir/lib/IR/Bindings/ // Bindings for MLIR's IR construction
C/*
Swift/*
Python/*
mlir/lib/Dialect/Affine/Bindings/ // Bindings for Affine Dialect
C/*
Swift/*
Python/*
The observation is that the LLVM family of tech is much larger than MLIR, so bindings aren’t an MLIR thing. Similarly, the IR construction stuff is dialect independent and is itself hard. Finally, each specific dialect will want to customize things in different ways.
The other point here is that the bindings for each part of this can depend on whatever they want to: if the python binding for affine wants to depend on the C binding for Affine that is cool. If the Swift binding for Affine does not, that is also totally fine.
I think this would provide a much more modular and scalable approach than what we have now for bindings.
-Chris
One major problem with this is that it causes a proliferation of -I flags, which makes the build logic more complicated. I personally think the current approach of a single include directory works fine. Is there a specific problem you’re looking to solve?
-Chris
I don’t have a strong opinion, but I will note that CMake does solve this transparently for the build tree but LLVM isn’t currently using it right. It’s on my list to fix (and in fact we fix it in downstreams by annotating the LLVM targets properly).
However, at install time, there is a strong case for having one include tree. Mirroring that without translation in the build tree is valuable, imo.
2 Likes
Oh I agree This is in my “favorite” “cascading SLA” area. Depending on anything that has less of an SLA guarantee that you are providing means you are signing up to do work for them (in this case, if your tool depends on dialect(s) that update less frequently than you, then you are probably going to need to update that/those dialect(s)). Which effectively means things in core can’t depend on things not in core, you have strata of stability/support that should be in monotonic order and so even in core (where everything should at least build always at same revision) you can’t have more supported parts depending on more experimental parts (nothing surprising there). But yes given no stable APIs, dialects, data structures, serialization formats etc the best today is a “usable at hash” level [if going across repos], and there can be, if folks aren’t actually actively developing/updating/supporting a point where one is stuck and can’t update - but that’s true due to relying on unsupported components and indeed no technical solution for that
1 Like
We had a related discussion this week in the CIRCT ODM. One of the nice things about the CIRCT repo is that it has maintained somewhat stricter layering and dialect separation than MLIR upstream. This has made it easier to take on known-researchy stuff, because it doesn’t impact other things.
I think a few policies could go a long way here to capture the intent (e.g. Canonicalize shouldn’t depend on affine or any other dialects), clarify the goal of the end game, etc. If we nail that down, we can then work to push the tree towards it.
-Chris
The comments next to the directory names in the original post are roughly listing out these rules but not completely. For MLIRTransforms (transformations/ in the proposed tree), initially, all the transforms being developed were dumped there and many were subsequently moved to MLIR<dialect>Transforms libraries – however, several utilities that were shared by two dialects’ transforms or passes that touched multiple dialects were left behind in MLIRTransforms and MLIRTransformsUtils. If these are to move out, it appears that the proposed tree is missing a place for them. If MLIRTransforms in the future would depend only on core (which would only include the builtin and pdl dialects), there has to be another place in dialects/ or elsewhere to host the rest. Likewise for analysis/.
1 Like
There should be a layering relationship between these dialects, so something that spans two of them can go in the more-derived dialect’s transforms directory.
Alternatively, if two dialects are often used together but are not layered, there can be dedicated dual-dialect libraries (common for conversions) or an interface can make the transform dialect independent.
1 Like
Should the two dialects supporting loops (affine/scf?) and ‘async’ continue to be in dialects? They are rather low level from what high level languages offer and look to fit in the IR category… that may encourage wider scale adoption.
I am new, so sorry in advance in case this has already been clarified.
Yes, they absolutely should. Any execution-related entity in MLIR is an operation, and all operations must belong to some dialect. Modules are operations. Functions are operations. There is no reason why loops wouldn’t be operations.
It doesn’t matter if these execution-related entities are high-level or low-level from any point of view. We can have dialects that match hardware ISAs or even hardware components and dialects with operations that correspond to entire binaries.
IR is for the most fundamental concepts: attributes, operations, types, values.
If there is something specific that you think prevents adoption of these dialects, please start a new topic to discuss that so we can see what can be done about it.
I would highly recommend watching our CGO 2021 talk that explains the overall design briefly.
I will watch the video, for sure. The concern is that lowering steps that are often cleanly described with loops is based on these dialects or off-tree code. If things are based off on core, maybe this step need not be based on an isolated group’s work.
The restructuring won’t change anything about the availability of these dialects and the infrastructure we’ll build around them in-tree: scf will still be an important part of MLIR for the foreseeable future.
I wouldn’t expect it to matter from an adoption point of view that these are located in IR or dialects: there are maintained components.
Is there a perception problem you’re reporting here?
1 Like
Moving things in a manner you suggested looks appropriate to me. Other suggestions I will follow up elsewhere, thanks.
I took a fresh pass through the proposal and made some concrete notes/questions:
Core notes
Generally the layout of the proposed mlir/core lgtm. A few comments.
CAPI
Headers going into a unfied mlir-c directory makes sense to me. I wonder if this is not a chance to break the library part, though: I think we should co-locate the actual C-API libraries with the thing they are exposing (we can start with them in the monolithic CAPI directory to make transition easier, I guess). I think this would make it structurally easier to tablegen more of the C implementation (i.e. when generating code for your dialect, also generate a FooDialectCAPI.cpp, as an example). Concretely, I propose that CAPI definitions are either defined in a file like mlir/core/Interfaces/InterfacesCAPI.cpp (what is currently in mlir/core/CAPI/Interfaces/Interfaces.cpp), or simply include them in the same module defining the C++ API (for simple things that do not benefit from separation: e.g. this may make sense for Passes.cpp or similarly auto-generated adjacent code).
A step further still would be to unify the headers. I think this would break a lot of precedent and make the layering more opaque, and I don’t think we should do that (i.e. we should keep the mlir-c header directory).
Unifying the libraries would also let us easily do the build work to extend the current aggregate library work so that it can produce either C++ dynamic libraries (pending bug fixes) and/or C dynamic libraries.
Python
I’m going back and forth on the Python bindings organization. These need to coexist in a unified deployment directory structure (for everything that shares the mlir namespace), and from that perspective, I might want a top-level Bindings/Python (peer to core). We put everything there (current lib/Bindings/Python, python). This has the downside of making the python bindings monolithic from a build perspective, but I would argue that is not unusual. Sub-projects can still have private Bindings/Python, but they would need to have their own namespace in the tree (i.e. mlir.compilers.tosa or another top-level entirely, which might get built as its own wheel, etc) and not just an overlay into the core parts of the tree. This is something like how we have it for sub-projects, and there is tension no matter what. Having all of the python bits under one Bindings/Python directory also gives us a place to put things like project descriptors, setup.py, etc, and it will be more like a real python project.
Tests
Are we sticking with a monolithic test/ tree?
Include trees
I’m concerned by some of the include trees and not sure if it is an artifact of a POC diff. Example: mlir/core/include has mlir and mlir-c subdirectories, and the tree matches include paths. dialects/Arithmetic/include directly contains only the “local” parts of the path. I assume that you would still want to include these as #include "mlir/Dialect/Arithmetic/IR/Arithmetic.h" (both internally to the codebase and externally/intstalled)? It gets pretty dicey to have the path in the source tree differ from how it is included.
I do like that each component has its include/ directories local to it vs split at the top level. I believe the main negative to this is in-tree build complexity and longer compiler command lines. On the build complexity side, CMake should handle this for us (we just need to tell it to), and I think the reason LLVM has eschewed using that are historic. Another benefit to splitting like this is that it would naturally enforce library layering better than is done today: if you fail to depend on a library, you won’t have the corresponding include directory in your compiler invocation, and it will be a hard failure. Not perfect, but it should be a significant improvement in terms of build robustness.
An easy fix to the issue is to just include the full path components in the local-include directories. E.g. dialects/Arithmetic/include/mlir/Dialect/Arithmetic/IR/Arithmetic.h. Quite a few more directory nodes involved, but modern IDEs will typically collapse empty directories.
Another benefit of the component-local-include directories is that we have a parent path to put CMake code generation rules that can emit both implementation and include files. Should open the possibility of reducing build boilerplate while better controlling the public header surface area.
- Nit: We are somehwat inconsistent in our treatment of singular/plural form of “dialect” vs “dialects”. The top-level directory is “dialects” but the include path segment is “Dialect”. In addition, the Python package is
mlir.dialects. Possibly update the include path segment to be “Dialects” while doing this?
“compilers/”
I’m still a bit sketched out by the compilers/ dir (I think more on the name), but I do agree something here is needed and it is more than what traditionally fits into tools/..., in that these can be entire sub-projects with inbound dependencies, etc. Maybe just split the difference and call this projects/ and make it for things that don’t rise to the level of top-level, aren’t worth having a dedicated incubator repo for, and that we want to be able to control direction of dependencies on (and have optionality). I’ve also heard extras/ for this. Just spit-balling other names: components/, sub-projects/, integrations/, … that’s all I’ve got.
In short, it is a medium-high bar area for starting things (same high bar on actual code quality, etc as usual) that depend on the MLIR infra, are not part of the core/builtin-dialects/runtime/targets/tools, should exist in relative isolation (possibly forever, possibly until growing into a real thing), and that we want build system optionality for.
Organization of build tree
Currently all of mlir lives under tools/mlir in the build tree. Given that we are adding some indirection with this proposal, and some of us live on the command-line a lot and this will cost keystrokes, can we agree that as part of this change to move mlir to the top-level of the build tree (not under tools/).
Layering with respect to the rest of LLVM
We should probably prepare for a world where other core project in LLVM are depending on some subset of MLIR infra. The thing I don’t like about this directory layering is that there is not a parent-dir cut-point that neatly encapsulates what this boundary might be. core is close. Depending on how we resolve the test/ and unittest/ directories question, it might even be sufficient for such uses.
I’m really just looking for an answer to: “when MLIR core is unconditionally built as part of the overall LLVM build, how do we enable optionality on everything else in this directory?”
CMake shenanigans may be the answer, in that there is always a way, but I think we will ultimately want to eject core/ into its own peer directory of the rest of this entire mlir/ tree. As long as we don’t cross wires so we can’t do that, I guess it should be ok.
1 Like
The below makes me think of dialects being akin to projects dir. Each seeming to be its own self-contained thing (own include, own lib), which makes me think test should be there too and split (makes the separation between code and test smaller too).
That sounds quite nice. integrations matches a lot of these and feels most accurate. But not as intuitive. extras probably follows convention LLVM currently has, but I like projects more. So my vote would be split between accuracy and intuition
2 Likes
Thanks Stella!
“Integrations” is likely the most descriptive to what I had in mind, but I almost like “projects@ better actually!
“Extras” has a precedent with “clang-tools-extras” but note that “extras” is a qualifier for “tools” I believe, otherwise it can be a bag of anything potentially.
As of the tests, collocating them with the libraries looks fine to me, but we’ll still likely want a unique mlir-opt tool driving it all.