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

[Frank Schlimbach]

fschlimb wrote:

> Very nice work and sorry for the late review.
> 
> Unfortunately, there are some issues with the dependencies on MPI headers and packages. I highly doubt that this is an acceptable thing to depend on in the build system. Note that test might break when someone has another header around that depends different values for the used constants.
> 
> I suggest to restructure the pass to take all the necessary values as configuration parameters. In the worst case, this could even be a json file, if the list of options is growing. I'm not really sure who could consult on this.

Yes, there is no straight-forward solution for this right now. We could depend on yet another (external) library which unifies the interfaces. To me that's not really tempting for getting a few defines.

Why would configuration parameters change anything about the issue you are mentioning? Whatever was configured can or cannot conflict later, no matter what. Notice that later is not when the compiler is built, but when the compiler is executed. If you are concerned about other passes/dialects using a different MPI in the same build, we could move finding the header-file to an upper level cmake file. Frankly, I don't see this as a problem.

As mentioned above, as soon as the new MPI standard defines a stable ABI this problem is gone anyway. I suggest keeping this like this for the time being.


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