[Mlir-commits] [mlir] [OpenMP Dialect] Add omp.canonical_loop operation. (PR #65380)

[llvmlistbot at llvm.org]

kiranchandramohan wrote:

> I agree that `omp.unroll` should result in an `omp.canonical_loop` object. My concern was how does this canonical loop object fit with the existing loop objects. I will try to rephrase and elaborate my point.
> 
> Here is one possible translation of `#pragma omp for`
> 
> ```
> %cli = omp.canonical_loop [0, %tc) {...}
> %tiled = omp.tiled(%cli) { tile_size = 4 } -> omp.canonical_loop, omp.canonical_loop
> omp.unroll(%tiled#1)
> omp.wsloop(%tiled#0)
> ```
> 
> In this case, during compilation, after parsing the second line, `%tiled#1` is a child loop of `%tiled#0`. After the third statement `omp.unroll(%tiled#1)`, the unrolled loop replaces `%tiled#1` in-place as a child of `%tiled#0`. So, when `omp.wsloop(%tiled#0)` is called, worksharing is applied to the outer tiled loop (`%tiled#0`) and the code is generated for outer loop with the unrolled loop in the body. In this case, we do not need the `omp.unrol` operation to return a result, because it is modifying the canonical loop info in-place.
> 
> A second possible translation would be `#pragma omp for`
> 
> ```
> %cli = omp.canonical_loop [0, %tc) {...}
> %tiled = omp.tiled(%cli) { tile_size = 4 } -> omp.canonical_loop, omp.canonical_loop
> %unrolled = omp.unroll(%tiled#0) -> omp.canonical_loop
> %merged_loop = omp.replace(parent=%tiled#0, old_value=%tiled#1, new_value=%unrolled)
> omp.wsloop(%merged_loop)
> ```
> 
> In this case, after the second line, we have two tiled loops. `%tiled#0` is parent of `%tiled#1`. After the third line, we have an independent loop `%unrolled`. The next line then creates a nested loop, where it replaces `%tiled#1` under `%tiled#0` with `%unrolled`. Now, this `%merged_loop` is the one that worksharing is applied on. In this case we need a new operation to somehow compose the `CanonicalLoopInfo` objects to create new objects.
> 
> I personally like the second way of doing things, because it keeps IR clean and easy to understand, but wanted to know if there were other opinions about this or if I had misunderstood something.

I believe the former is the original proposal from @Meinersbur. But Michael can confirm it.


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