[Mlir-commits] [mlir] [mlir][mpi] Lowering Mpi To LLVM (PR #127053)

[Frank Schlimbach]

fschlimb wrote:

> Very nice, thanks for changing this to use the DLTI. I mainly added nit comments, related to things from the style guide: https://mlir.llvm.org/getting_started/DeveloperGuide/
> 
> Considering the new structure, it might be more elegant to rely on virtual dispatch to implement the different lowering strategies. This use case very nicely fits the strategy pattern, I believe. This would ensure that you only need to check the DLTI once and can thereafter rely on the virtual dispatch to delegate to the proper implementation.

Yes, I have been thinking about this as well. Virtual dispatch requires an object. The easy lifetime of that object is each `matchAndRewrite`. What we ideally want is one object per module. Without a proper pass, I don't know how to do this.

Yes, the current solution is not perfect. We can go on like this forever, or we declare this as good enough as a first step and together work on continuous improvements in future PRs. I vote for the latter.

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