[Mlir-commits] [mlir] [mlir] Add config for PDL (PR #69927)

[Mehdi Amini]

joker-eph wrote:

Need to iterate a bit more on framing the problem:

> MLIR is a general infrastructure and as far as possible no dialect is intended to be special or privileged

Yes, and you can use MLIR without PDL, it's not a mandatory component. I believe that the minimal examples `mlir-cat` and `mlir-minimal-opt` don't have PDL linked in (so you can already have your *-opt tool without it!

> Users should be able to use MLIR and the parts they need to customize for their solution (e.g., one doesn't have to include any dialect except the ones one uses). 

I don't see PDL as "just a dialect": I see it more as an infrastructure component. 
That is no one will have PDL used within their IR! 
It isn't comparable to any other dialect and completely unique from this point of view (do we have other dialects that aren't targeting user-written compilers? The transform dialect might be in-between)

> Many parts are elided when not referenced (LTO DCE'd etc), but this is not possible with PDL given how its integrated with the common rewrite drivers. This results in it always being included even when not used.

Now that seems more accurate to me: the dependency on PDL is only from the rewrite drivers, which is a "bring your own" thing by the way.
So the underlying question for motivating this here would be better framed IMO as "should we have an option to build the GreedyPatternRewriter without PDL?" 
I see it less as a strong need here (because again, bring your own driver if you don't like it).

Without a stronger case, this seems like something I would be supportive if we can make it minimally intrusive: that is localize the changes to a maximum and not spread through the codebase.
There are far too many #ifdef  to me right now, this may need some more refactoring first to avoid this and make PDL a more proper "separate component" inside libMLIRRewrite so that is can be enabled/disabled more naturally.

https://github.com/llvm/llvm-project/pull/69927