[PATCH] D78397: [mlir][Transforms] Add pass to perform sparse conditional constant propagation

Sun Apr 19 13:53:28 PDT 2020

fhahn added a comment.

In D78397#1990777 <https://reviews.llvm.org/D78397#1990777>, @rriddle wrote:

> In D78397#1990753 <https://reviews.llvm.org/D78397#1990753>, @fhahn wrote:
>
> > Conceptually this looks very similar to LLVM's SCCP. Do you anticipate using any MLIR specific properties to cover more cases than the LLVM version? I am not really up-to-speed with the latest on MLIR dialects, but to me it seems like there is a set of transformations where there is currently no additional information to use from MLIR dialects compared to LLVM IR, like SCCP or GVN.
> >
> > Viewed very simplistically, they only require the following from an IR:  SSA, a way to traverse a function along CFG edges, a way to simplify instructions and a way to replace values. Do you think it would be feasible to provide some kind of interface to abstract those and then work towards sharing the implementations of the underlying algorithms between LLVM & MLIR? Or is the plan to duplicate various passes from LLVM also in MLIR?
>
>
> Thanks for the comment Florian! I would be very +1 on sharing common implementations of the more generalized algorithms if we can. From a technical perspective, the major problems I've encountered with bridging the gap are a few impedance mismatches between LLVM and MLIR that arise from different implementation decisions:
>
> - The multi-level aspect leads to special handling for various constructs in MLIR(e.g. regions/structured control flow/etc.) that aren't present in LLVM.
> - MLIR globals(like functions) are not SSA values, and require specific handling because they do not use or interact with the traditional SSA use list.
> - Block arguments vs PHIs

Thanks for sharing the list. It seems like handling nested regions/structured control flow would be the most tricky to abstract. I am not really up-to-date on what is possible with nested regions, but would the interactions allowed between different regions be similar across all types of regions or could it be dependent on the region type? I.e. is it safe to propagate constants to all regions or is it unsafe to propagate constants between certain types of regions?

I think the other 2 issues should be fairly straight-forward to abstract.

> When adding things in MLIR that already exist in LLVM, at least for me, it is a cost-benefit computation between the amount of work it would take to refactor the thing in LLVM to be usable by MLIR. By amount of work I mean not only the technical aspects, but also the effort to convince the community that it is the right thing to do. I'd love to share as much as possible(as we do for things like ADT/Support/etc) and I'm happy to collaborate in that direction, but if I'm driving it alone the cost has often out-weighed the benefits.

I think that's a reasonable trade-off. I mostly wanted to get a discussions started on that topic, which would definitely not be a short-term kind of project. I'm not too familiar with the MLIR side of things, but I might have a bit of time to collaborate on the LLVM side of such a project. It might be good to sync up/discuss ideas somewhere else than the current review ;)

In D78397#1990865 <https://reviews.llvm.org/D78397#1990865>, @mehdi_amini wrote:

> In D78397#1990753 <https://reviews.llvm.org/D78397#1990753>, @fhahn wrote:
>
> > Viewed very simplistically, they only require the following from an IR:  SSA, a way to traverse a function along CFG edges, a way to simplify instructions and a way to replace values. Do you think it would be feasible to provide some kind of interface to abstract those and then work towards sharing the implementations of the underlying algorithms between LLVM & MLIR? Or is the plan to duplicate various passes from LLVM also in MLIR?
>
>
> That'll be an interesting and likely recurring question moving forward. I wonder however if when applicable LLVM would be OK to take a slow-down from generalizing passes to MLIR and going through interfaces for the sake of sharing the implementation?

I think the answer here depends a lot on how this would look like in practice. Are you referring to slow-down in terms of runtime or development?

In terms of run-time overhead, the abstraction would have to be very cheap I think, but that should be feasible as long as it is limited to a few key aspects (like traversing functions/blocks/regions and an interface to simplify instructions given a set of input values (which LLVM has)). In terms of slowing down development, I am personally not too concerned. The passes that seem likely candidates (SCCP, GVN) don't seem to see a huge amount of ongoing development.

I think sharing implementations could be beneficial for both LLVM and MLIR, as it would hopefully mean more users = more testing = more people willing to work on fixes/improvements.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78397/new/

https://reviews.llvm.org/D78397