[RFC] DSL-style / dialect-specific builders

This is essentially just pushing the cost of fragmentation downstream. This makes sense to me if there are distinct non-overlapping sets of maintainers, which is arguably not the case today, at least in MLIR. And the cost for the same set of folks to maintain multiple projects with dependency chains sounds higher. To be blunt and selfish, if I had to start a separate glue project around MLIR, what would be the incentive for me to contribute back anything? I would be also concerned if we refused the in-depth technical discussion because of high-level principles, otherwise we would be encouraging people to form camps (that we don’t necessarily know of and can’t influence).

It feels like the builders discussion is challenging because it falls into the grey zone between style or paradigm changes and functionality changes. Adding a new backend or a new dialect wouldn’t be much of a problem, even if it uses additional abstractions inside. Making them available to others starts to get interesting.

Let me put this back into context. Disregarding all the other arguments, is LLVM API using builders a sufficient reason for MLIR not to explore anything else?

Such “expansion” transformations are much more common in MLIR due to its multi-level and progressive nature than they are in LLVM proper where most of the expansion actually happens in frontends, each of which can adopt its preferred way.

There are certainly costs in having a larger API surface, namely maintenance and entry barrier, but I am not convinced why we should accept this costs for the APIs that manage transformations (direct IR manipulation vs. DRR/simple rewrites vs. dialect conversion) and not for APIs that implement transformations (builders).

This is essentially just pushing the cost of fragmentation downstream. This makes sense to me if there are distinct non-overlapping sets of maintainers, which is arguably not the case today, at least in MLIR. And the cost for the same set of folks to maintain multiple projects with dependency chains sounds higher. To be blunt and selfish, if I had to start a separate glue project around MLIR, what would be the incentive for me to contribute back anything? I would be also concerned if we refused the in-depth technical discussion because of high-level principles, otherwise we would be encouraging people to form camps (that we don’t necessarily know of and can’t influence).

This claim, in general, seems to me to have some affinities with my old comment at Numpy/scipy op set - #9 by bhack

Fwiw, with llvm-aligned frontends like npcomp, we are running straight into this for the simple pragmatic reason that I have little desire to do it again in the hard/brittle way that tf2xla was done (same problem: reducing an open set of thousands of “subroutine” style ops to dozens of primitives). My sense is that some kind of macro expander and associated DSL needs to exist, and that is at a higher level than any of the builder discussions (and not in conflict with them). Being a “frontend” top level project, I think it is proper for it to be trying to build tooling that increases it’s efficacy, and I suspect that some of that will eventually be general enough to promote out in some form if desired.

I really appreciate all of the work to increase the usability of the builder APIs and I think what I could benefit from the most is a layer above this for solving a different level of complexity. I’d like to not see 15 divergent attempts at such a thing in sub-projects, but I also am not sure we know what we need enough yet to be trying to layer new abstractions into the core APIs. You have more visibility into the other projects that exist and maybe this is clearer for you?

1 Like

With API instability and the difficulty of keeping up with upstream, I don’t really see such approach having chances of success or lead to a widespread adoption. It risks instead having every client forking for their own convenience, improving locally, and not contributing back.
But more importantly, if this is about downstream clients that are shared by the upstream developers, it seems to me that we would make our own lives harder for a reason that I haven’t totally grasped yet.
(Also in this case there is the technical aspect that we’d need to make ODS and the generator externally extensible in the first place to achieve this out-of-tree).

My take is that if there are utilities or tools that are “general enough” to be shared by multiple/most
downstream project: having them in-tree is what ensures the best maintenance path as well as the most chance of having something well integrated and consistent across the project.

The EDSC suffered from being “that thing on the side” previously, I the current proposal as a good direction to unify back things behind a single API that integrates well with ODS. I don’t see us removing EDSC without a good alternative right now, and just for this I’m very supportive of Alex’s work here.


We may have a fundamentally different viewpoint on this. I see fragmentation as inevitable.

LLVM needs to have a single consistent coding style, this is really important for project management, allows us to scale the codebase better, and makes it easier to explain LLVM to new people. I’ve fought this problem (only semi-successfully!) for years when thing like LLDB got merged in etc - LLDB having a “house style” that was significantly different than LLVM’s was a major issue that reduced cross pollination. This argues that we should make sure the LLVM standards are high quality, consistently apply them, and improve them when needed.

That said, I see zero chance that the world will adopt LLVM standards uniformly, no matter how great they are. This means that there will always be downstream fragmentation. The argument you’re making is that we should privilege one or two of those fragments and merge those into LLVM - that doesn’t work, because now you have internal fragmentation and can’t manage the LLVM project effectively. It also means that other fragments will want first class support.

There are analogies to this in LLVM subprojects. Early in the project, we struggled with “why not merge my research project into LLVM”? There were many examples of this including VMKit, “HLVM”, TVKit, and others. We eventually decided that it was more important for the code base to be high quality and actively maintained than for it to include “all the exciting things that might want to work out”. As you probably know, we’ve recently added an LLVM Incubator process as a sort of relief valve to enable a new state.

I agree with you. What I’m arguing is that we should take the style issues first, resolve those w.r.t. the existing builders, and see what is left to discuss. This is why I say things like “there needs to be a right answer: are locations always specified, or not?”. Whatever the answer is should be consistently applied to both. Once we decide, we roll it out and have reduced one deviation, and then can discuss the next topics with less ambiguity, etc.

I’m not sure what you mean here. Are you saying “we have OpBuilder, does that prevent having a higher level abstraction in tree?”. If so, I would say “no, it doesn’t prevent it categorically, but we would look like it as any other infra extension to llvm”. The problem with EDSC’s (as we discussed) is that they were landed early in the cycle of MLIR where a lot of research’y stuff was underway. If we didn’t have them and someone proposed their inclusion today, I would be strongly opposed to them for all the reasons I’m explaining above.

I agree that the risk of “not contributing back” is real, but I don’t see how this discussion relates to it. I don’t think that EDSCs are a significant hinge that will affect this one way or the other.


@ftynse, with regard to the things mentioned under “Interaction with EDSC”, I wanted to check what happens to code like the one appended below from Linalg that’s using not just different builders but a lot of abstractions and new patterns around it. Could anyone even guess from the code below that it’s actually creating load and store ops?! Would such things go away with this proposal? (progressively or at once?) I feel parallel patterns like these are a big hindrance for readability - not just for new developers but primarily even for other MLIR developers! Even the most active MLIR developers would have to dig deeper to figure out it’s creating two load ops and one store op.

IndexedValueType output(op.output());
IndexedValueType input(op.input());
Value lhs = output(indices.outputs);
Value rhs = input(indices.inputs);
output(indices.outputs) = maxValue;

I strongly feel patterns like these shouldn’t have been allowed in the first place upstream.

I’m curious to know what these are - if you could just summarily list them. I’ve only heard about the EDSC used mainly by Linalg and Vector, and I’m a -1 on these abstractions for all the reasons mentioned upthread. Replacing these completely with what’s proposed here or some form of it would still be a net positive move IMO. But it’s still not fully clear to what extent that would happen - the statements at the end of “Interactions with EDSC” are a bit concerning.

I feel the proposal could benefit from an allude to how the higher order builders proposed could be taken even further. The benefits of IDE/clangd autocompletion, prompts, inline errors are really great to have for productivity, but those could be achieved with what @jurahul suggests above or a lower order form of this proposal, in which case they could just replace standard builders. This is all assuming clangd’s code completion can’t be made to work with such function templates. If it can, the standard builders could just be retained. Separately from this, I think we should aspire for something even “higher order” if we want to add a new builder abstraction upstream — because if the code completion part is tackled by changing the standard builders slightly, I feel the proposal is adding a thin layer or not spelling everything out.

As a general observation, MLIR positions itself as a partly productivity-oriented compiler framework. If one of the original goals was not having to write n-th CSE implementation for each compiler (see early presentations from Chris), I don’t see why we at the same time encourage users to write n-th IR “expander”.

I don’t know what folks need in general, my proposition is abstracting from what we have in-tree and several TF-related projects. By putting this up for discussion, I hoped to receive some feedback on what else is necessary or simply useful.

As a matter of fact, I agree that fragmentation is inevitable. Therefore, I am trying to find ways to minimize its cost.

Maybe we can actually reduce this discussion to what is considered to be LLVM standards and which parts of the proposal contradict those standards, so we can discuss those parts separately.

I don’t fundamentally disagree with this. But there is cost to everything, roll-out duration and api churn will affect most users here.

LLVM and derived projects (including translation in MLIR) uses llvm::IRBuilder API to construct (most of) the LLVM IR. Should MLIR adopt the same API pattern, i.e. Builder, only for the sake of having the same API patterns as LLVM (see “standards” above).

They were also landed because they were, and still are, used. That’s why I am trying to converge EDSC and Builders and reach a state where we have one thing that is good enough for everyone. The alternative at this point is to keep the status quo.

We’ve gotten so meta in this discussion that I would like to see what are the ways of mitigating the risk, regardless of EDSC or this proposal :slight_smile:

As you probably know, EDSC is a combination of many different things. I am trying to progressively integrate them with the rest of the infrastructure as explained in Evolving Builder APIs based on lessons learned from EDSC. In his posts above, Chris argues that even the current proposal is too broad. Considering such helper structures is definitely out of scope here. For practical purposes, such helper structures will likely remain unchanged in the first iteration and have their constructors updated to take a builder if we decide to remove edsc::ScopedContext. Let’s not derail this discussion even further though.

To be honest, I don’t understand how affine structures are modeled in MLIR without digging deeper, and I would guess neither does the majority of the contributors. This does not preclude them from being in tree.

I listed some examples in my previous answer, each word is a different link:

I can’t even imagine the amount of pushback I’d would have received had I proposed to replace builders and EDSC by a completely new API; nor the amount of work to do. If we want progressive change that does not introduce more divergence, components need to interact with each other.

It seems like both you and @stellaraccident have some ideas about higher-order builders. I am interested to hear them and potentially incorporate them in the proposal.

The signature of the function is template <typename OpTy, typename... Args> OpBuilder::create(Location, Args &&... args), which means it accepts just about any argument list. While I think it’s technically feasible, although not trivial, to discover that the argument pack is forwarded through another function template with variadic template parameters to a method on a dependent type, and use the list of possible overloads of that method as helper prompt, I would consider this to be too niche to be included in clangd. Even if it is included, it only gives you the documentation, implicit conversions are not triggered (they are in @jurahul’s proposal and in mine).

I’m not referring to internals or anything specific to the dialect, but plainly to IR building - something that is common ground for all MLIR developers that they shouldn’t have to dig deeper to understand. The code snippet I showed from Linalg Transforms (again reproduced below) is creating load/store ops as you know — this isn’t anything dialect specific, and yet it’s unreadable to me. (The IR creation code isn’t clearly spelt out due to the metaprogramming and lost/mixed among regular code.) IR constructing methods/code in other dialects upstream aren’t like this.

IndexedValueType output(op.output());
IndexedValueType input(op.input());
Value lhs = output(indices.outputs);
Value rhs = input(indices.inputs);
output(indices.outputs) = maxValue;

To provide some more concrete examples, here how the first pattern Sean’s patch above would look like with different APIs:

1 Like

Thanks Alex for taking the time to put these examples together!

Speaking personally, the “current proposal w/o create” reads best to me. I like that it is succinct and reads very similarly to the corresponding textual .mlir (there’s something about the . vs edsc’s _ after the dialect name which makes it much easier to read). The create is just line noise once we get to this level of fluency.

For the “even more DSL” version, it is too difficult to see what concrete ops are being created.

I would really like to have a “current proposal w/o create”-like API available for these sorts of “expansion” patterns.

1 Like

These are quite useful. Did you paste the wrong link for the first one? There is no create there - the builder is just named create. I assume you meant to have it use createCmpI etc.

If these are the current proposals, I’m not sure why the proposal says standard builders are going to stay. With either of the first two options listed by @ftynse, one can just replace the standard builders. I assume passing an explicit location will also be supported if needed. I felt that was the confusing part of the proposal - the first post neither says standard builders would be replaced nor deprecated and replaced over time.

I really don’t understand what you mean here. MLIR already has OpBuilder - we are not talking about removing the presence of builder APIs! I think you’re arguing that the existing builders are not good enough - I’m arguing that we should either improve them or delete them. There should be one builder abstraction that is “good”.

FWIW, I don’t agree that the create-less APIs are “good”. Beyond the problems with naming conventions, I am very concerned about their reliance on tblgen code generation is also suboptimal – because it means that anything built on top of the builders need to be similarly code generated (they can’t just use C++ templates). This is one of the reasons that I can’t see this ever replacing the existing builders. Given that, this direction cannot lead to unification behind a single “good” API.

Let me give you one example of what I mean. With the existing builder APIs, a downstream project could say “hey, I want a builder that does extra stuff when I build operations” (e.g. maybe make the location implicit). RIght now, you can do this easily:

struct ImplicitLocOpBuilder {
  Location loc;
  OpBuilder underlying;

  template<typename T>
  T create(....) { return underlying.create<T>(loc, ...));

You can’t do this sort of thing when you rely on tblgen code generation for the builder APIs - you’d have to tblgen codegen ImplicitLocOpBuilder, which would require adoption by every dialect that wants to participate.

This breaks scalability of the ecosystem, which is a critical issue to me. I can see how this approach can be used for a couple of dialects, but unless it can be “the” thing, I have a hard time getting excited about forking and fragmenting the ecosystem.

Furthermore, EDSC’s have not been widely adopted outside of the code that ntv’s code, so I don’t see strong empirical evidence that there is a “pull” for this outside a couple of specific (and very amazing) engineers that prefer it.


Brainstorming here: What if builder.setInsertionPoint*() took a code point and a Loc. Then each create<> call wouldn’t have to specify the loc explicitly? It might be more awkward for pieces of code that are operating on large numbers of OPs, but we could also keep the existing create<> calls that take a loc for those cases?

No, I did exactly what I intended. I do not propose the createCmpI version, Chris does and then explains why that version isn’t a significant improvement over the current state.

Because the current proposal would be using builders internally. I have not considered fully replacing them, yet, as the proposal is large and contentious enough already. For example, it is not fully clear to me what will happen with various type- and attribute-constructing functions currently in Builder (although this creates a disparity in the API between “privileged” types that have Builder API and dialect-specific types that do not), with PatternRewriter and related functionality, and with the actual ::build functions that currently take an OpBuilder.

That being said, if not removing builders is the main blocker for going through with this proposal, let’s discuss that as well.

MLIR also has the possibility of populating an OperationState and calling Operation::create to construct operations. It was not removed despite having builders. I don’t see why the same should not be allowed for one more API abstraction level.

I agree this is a concern. However, we already generate a lot of APIs: interfaces, passes, accessors to Op operands/attributes. Realistically, how many of such builder extensions can one come up with? Caching locations is one good example, and also one of the motivations for EDSC :slight_smile: If there another where one wants to modify the op creation behavior? If one only needs to be notified that the op is about to be create or has been created, this is already possible through the listener, no need to wrap the builder in such cases.

I’ve seen folks attempting to use it in HLO and then getting asked to remove it during code review. So the visible lack of adoption is, at least partly, due to the pushback against adoption. Note that I don’t propose that everyone just adopts EDSC, and that I listed several examples above where similar API patterns were added locally.

Assuming we don’t get hit by overload resolution+templates, this would still be subject to the lack of autocomplete/documentation and lack of implicit conversions for a small decrease in verbosity.

Let me flip this. What would be the alternative that meets the following requirements:

  • minimal redundancy in subsequent calls (e.g., no repeated location, ideally no repeated meaningless prefix);
  • supports autocomplete/documentation;
  • fully replaces or extends builders?

I see what you’re trying to show, but these serve two different purposes. The OperationState is the underlying implementation details that power the infrastructure in a “type erased” way. The OpBuilder::create<> template is the typed layer that is intended to be used by users of the MLIR APIs.

The EDSL stuff is another “typed layer that is intended to be used by users of the MLIR APIs”. That is why it is redundant, and only one should stand. If the ultimate approach is to use the EDSL inspired API, then it should be built on top of the OperationState API directly, there isn’t a clear reason it should go through another layer of indirection.

I don’t understand what you’re saying here. Surely you agree that there is a difference in significance between ops and interfaces/passes/accessors. Ops are the primal thing in MLIR and exposed by everything, building generic code over families of them is extremely common. We should be doing more here to make this easier, not less.

I think you’re arguing here for a monolithic builder design that ties all concerns into one API. I don’t think this is a good or scalable approach - we should go for simple things that can be layered/composed on top of each other.

Fair point, but I think we should resolve that by deciding whether it is the right thing to do and apply it uniformly. The signal you’re seeing above is just an indicator that people are not happy with EDSCs. If that is the case, then the right answer is to just replace all uses of them with OpBuilder in tree and remove EDSCs (or sink them into other MLIR derived projects that want them). I would also be very +1 on this, it combines very nicely with the orthogonal goal of “improving OpBuilder”.

I don’t understand this “flip” at all. You’re getting pushback on this thread about various design points you’re proposing to achieve those three goals, so it isn’t clear that we “have to” solve those problems.

I love that you’re pushing the envelope here to try to resolve the fissure between OpBuilder and EDSCs, but I am fairly serious that we should consider the alternate path: just remove EDSCs (replacing with direct use of OpBuilder), and then continue a discussion of improving OpBuilder. The fork in the universe is really a separable problem from the “how do we get the best builder API possible”.

I can’t see any strong argument that we should remove OpBuilder and keep EDSCs given the compile time and many other pushbacks that people have raised.


Here’s another idea on how to make incremental progress here:

It has been observed that some use-cases would benefit from a builder that makes the location implicit at the create site: the location is a mutable part of the state in the builder itself. This is controversial because some of us want to make sure that location processing is thought through and would like to put some burden on compiler engineers to do the right thing with locations. OTOH, it is clearly useful for specific design patterns, including entire passes that work in a structured way for which this makes sense, as well as the primary EDSC use case of “splat out a bunch of IR” which presumably comes from one location.

I think we can have the best of both worlds here with the existing opbuilder stuff. I think we keep OpBuilder the way it currently is - it supports creation of arbitrary IR nodes, requires the location to be explicit in the create call, and is the thing that is generally passed around between the core parts of the compiler.

To solve for the use-case above, we introduce the new ImplicitLocOpBuilder described above. This allows clients with the structured creation logic to opt-in to implicit location handling.

I think this covers both bases well. WDYT?


Hi all,

I implemented the ImplicitLocOpBuilder concept in CIRCT and we’ve been using it for some time now. It works well, so I think it would be useful to upstream it to MLIR. Here is a patch that does so, I’d appreciate feedback and review, thanks!


1 Like