I'm curious about @jpienaar 's thoughts on the overall direction here

LGTM, but @rriddle should have another look

I rather not write code that is over-restrictive unnecessarily (if you think about it, the entirety of MLIR exists because we generalize early, otherwise we'd have rebuilt LLVM or XLA ;) )

Ping here @frgossen ?

In D144814#4187700, @mehdi_amini wrote:

In D144814#4163506, @Tyker wrote:

The data-flow analysis framework has a concept very similar to IRUnit its called ProgramPoint maybe this should be shared ?

I'm not sure how to unify it, it feels that somehow the ProgramPoint could inherit from IRUnit but it is declared as a more extensible framework: public PointerUnion<GenericProgramPoint *, Operation *, Value, Block *> so I'm not sure how to unify.

You should look into the title and description of the commit: https://cbea.ms/git-commit/

In D146036#4192786, @nicolasvasilache wrote:

if anything, I'd go for dest personally with a doc that the dest may or may not be read.

In D141098#4111809, @owenpan wrote:

Can we just fix the buildbot so that it runs git-clang-format with --binary? Using clang-format-15 to format a patch to clang-format-16 source still looks wrong to me.

I'll wait to hear at least from @mravishankar @nicolasvasilache here.

Please get reviews from @rriddle here.

In D136875#3918674, @dcaballe wrote:

we can reconsider a name that is more intuitive like outputs

This LGTM, but River may have thoughts on simplifying getChildOpIndex(), let's wait to hear back from him.

In D145991#4191352, @rriddle wrote:

In D145991#4191347, @mehdi_amini wrote:

This assumes less threads than shards overall right?

Yeah, which is effectively always a guarantee (even on my beefy desktop) given the number of parametric builtin attributes/types we have.

This assumes less threads than shards overall right?

Can you add tests please?

Address comments

One more comment addressed

Rebase & address comments

Minor fix to PAssExecutionAction

clang-format

minor cleanup

Address comment

Use ... instead of "/*skip region*/" comment

Update with one extra no-inline indirection

In D144816#4172601, @Mogball wrote:

LGTM but River mentioned there were some perf issues. Do have a quick capture of the before vs. after time to run a pattern-heavy workload?

address comments

address comments

In D144814#4163506, @Tyker wrote:

The data-flow analysis framework has a concept very similar to IRUnit its called ProgramPoint maybe this should be shared ?

Address comments

Address comments

This LGTM, but @rriddle and @Mogball had concerned earlier, it would be great if they could take another look here!

My feeling is that we explicitly do not want a pass to depend on a specific IRDL dialect. In fact, this is already something that we do not want to have for DynamicDialect.
Having a pass relying on a dynamic dialect would be too dangerous and error-prone, and if you are already paying the cost of recompiling to write your pass, you are willing to pay the cost of compiling your dialect to C++ (in my opinion).
Also, manipulating IRDL dialects in C++ is a bit annoying, since you need to use the generic way of accessing operands/results/attributes, meaning that you have almost no guarantee that those operands/results/attributes exist in the first place.

In D144693#4186322, @math-fehr wrote:

In D144693#4186106, @mehdi_amini wrote:

I feel like you're using "register" when it is actually more "loading" here. (I'm not sure IRDL has a concept equivalent to "registering" dialects?)

Coming back to this: how do we differentiate registering from loading in IRDL? Do you see some difference somehow?

In IRDL, there are not really any registration, since the dialects are loaded right away, without going through the `DialectRegistry.
While there may be a way to allow registering without loading, I'm not completely sure what would be the actual benefits, and I'm not sure how easy this would be.

In D145066#4182035, @rriddle wrote:

In D145066#4168413, @mehdi_amini wrote:

In D145066#4167514, @rriddle wrote:

I'd be fine landing this patch for now to fix the issue. The root of the inliner issues should be fixed when I rebase+land https://reviews.llvm.org/D134480

Will this be enough to solve the entire dump including the members of the InlinerPass class?

Yeah, it should. The callable stuff gets killed, and the list is already specified as an option.

I feel like you're using "register" when it is actually more "loading" here. (I'm not sure IRDL has a concept equivalent to "registering" dialects?)

Right now you have test failures. On my Mac locally I see:

Do you have commit access or do you need help to land this?

In D143647#4185686, @mehdi_amini wrote:

LGTM, thanks for being so patient through the reviews!

Let's wait for @rriddle to give a final approval.

LGTM, thanks for being so patient through the reviews!

In general I am quite concerned about messing with the optimizer this way, this should be extremely exceptional. This'll be ad-hoc, forces a performance tradeoff specific to a given use case, and couple the "heuristic" to the exact compiler you're using (what does it do on Windows? On Mac?).
Have you tried building your project with -Os? -Oz? PGO? FullLTO?

~3.4 MB reduction in binary in release mode.

In D144692#4184072, @rengolin wrote:

In D144692#4183069, @mehdi_amini wrote:

I don't believe there is any rule like that: I merge my patches as they get approved in general. We mostly need to agree about the overall design, then we can develop incrementally in tree. Stacked patches are mostly a convenient way of streamlining the development and review process.

Approving a patch-set in bulk isn't new or seldom used. We've been doing this in LLVM for at least a decade for all sorts of new features, including all new targets. What you describe is a completely different scenario.

In D144693#4183083, @math-fehr wrote:

Is there a way to add a check for this?

In D144693#4174249, @math-fehr wrote:

Is it possible to register new dialects during a pass?

In D144692#4183066, @rengolin wrote:

As with other patch series, we review all patches and only when all patches are approved you can merge them all at once. This is to make sure all points are covered, and to make it easier to pre-approve the good ones and focus discussion on the hard ones.

My personal motivation was to be able to ship MLIR without any static ctor.
Fixing all of LLVM seemed too much, but MLIR (when not needing the JIT) does only depends on libSupport, which was very much doable (in particular a key piece was that the command line parsing entry point is part of the lib, so we can lazy load libSupport options)

In D144812#4176575, @mehdi_amini wrote:

In D144812#4174341, @rriddle wrote:

Is there a commit in this stack that handles updating/adding documentation?

No not yet, I was waiting because I anticipated API changes during the review.

In D144812#4174341, @rriddle wrote:

Is there a commit in this stack that handles updating/adding documentation?

Address more comments