[Mlir-commits] [mlir] [OpenMP][MLIR] Add omp.distribute op to the OMP dialect (PR #67720)

[Razvan Lupusoru]

razvanlupusoru wrote:

Nice! The change seems OK to me.

However, it re-kindles that there is a need for an MLIR form that doesn't break this operation's expected properties. And more specifically, since by construction, distribute needs to be associated with loops, it needs some way to maintain that constraint. Being separate, it cannot guarantee this property across transformations. It also cannot guarantee the data-sharing property of the loop's IV must be private - since an inner loop may be "canonicalized" and it wouldn't maintain the user's IV. It also makes it unclear who executes instructions between distribute and the actual loop - likely each team - but that code is not distributed across iterations even though it currently would appear inside the omp.distribute construct. Thus, it would be ideal if these properties are captured at construction time and cannot be broken by later passes.

I've given some thought and I imagine that doing some of the checking in verifier is hard when omp dialect is used with dialects that don't have a native loop syntax - like LLVM dialect. So I am OK with your PR as-is - but I would be happier to see this operation evolve in a way that the constraints are maintained by construction instead of just hoping it doesn't go through passes that invalidate the properties.

By the way, I haven't evaluated the merits of the omp.canonical_loop proposal/implementation (and I noticed you commented which makes me think you are aware). But, from a high level, I think that is likely the right direction for omp.distribute as well.

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