[Mlir-commits] [mlir] [MLIR][OpenMP] Add omp.simd operation (PR #79843)

[Jan Leyonberg]

jsjodin wrote:


> Thanks for the clarification. However, I still can't quite understand how using composite operations to represent distribute-parallel-for in that example vs wrapper ops is any different with regards to this loop transformation problem. If we're able to represent the loop transformations in MLIR, even if we only actually produce these loops in the OMPIRBuilder due to it being the place where loop transformations are eventually applied, we should be able to then apply the corresponding parallelization/worksharing constructs to the resulting top-level loop.

Yes, I think I misunderstood a bit how to interpret the composite ops. If they simply are collapsed versions of the nested ones both are equivalent. It would also be possible to keep the wrapper ops and have lowering functions that would would identify and lower multiple ops in one step.

> It seems to me that `convertOmpDistParWsLoop` could be split up into calls to the equivalent of `convertOmpDistribute` + `convertOmpParallel` + `convertOmpWsLoop` if that's the best way to deal with that composite construct. If anything, it looks like it would give us some flexibility as to how to share information (for e.g. reductions) across these partial translations of the composite construct rather than having to add more state to the OMPIRBuilder.

If we can keep the information sharing within a single lowering function would be nice. Adding more state to the OMPIRBuilder is a concern, because it is fairly easy to create an inconsistent state if data is not added and cleared in the right places during lowering. If we decide to use wrapper ops, we could still have lowering functions that handle composite constructs to avoid adding state to the OMPIRBuilder. That way the representation is more flexible with no risk of a combinatorial explosion, but the implementation of the lowering would have this problem to some extent, although it can be minimized by having functions to share the implementation as you mentioned.

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