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

[llvmlistbot at llvm.org]

shraiysh 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.

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