- Remove the redundant collapse clause in MLIR OpenMP worksharing-loop operation.
- Fix several typos.
- Refactor the chunk size type conversion since CreateSExtOrTrunc has both type check and type conversion.

# Details

# Diff Detail

- Repository
- rG LLVM Github Monorepo

### Event Timeline

Have we handled the ordered clause with `n>0`? If the representation for that is similar to collapse then we might need some way to distinguish collapse and ordered loops.

Not yet. I had one try (https://reviews.llvm.org/D116292#change-bQfuPOutx59a), but there seems to be some problem with overflow. @Meinersbur is working on that. But I assume the implementation will be different with collapse clause.

Maybe I should have mentioned this in D116292, the standard specifies that for ordered(n), there are n loops that are associated with the ordered clause. One way to represent this in the IR is like the collapsed loop representation. If we don't bunch up the loops into a single operation then there is a possibility that FIR or other transformations can modify one of these loops which should not be permitted. So we might end up with a similar representation for collapsed and ordered loops, then we need to distinguish which one is collapse and which one is ordered. This can be by attributes, enums, or even additional information in the representation of the ordered clause (whose absence makes it a collapsed loop). Alternatively, we can leave this for now until we decide on the ordered loop representation.

One way to represent this in the IR is like the collapsed loop representation. If we don't bunch up the loops into a single operation then there is a possibility that FIR or other transformations can modify one of these loops which should not be permitted. So we might end up with a similar representation for collapsed and ordered loops, then we need to distinguish which one is collapse and which one is ordered. This can be by attributes, enums, or even additional information in the representation of the ordered clause (whose absence makes it a collapsed loop).

Agree. Michael mentioned there should be some transformation in canonical loop for the doacross loop nest. I am not sure if he plans to do it before MLIR or in OMPIRBuilder, so I abandon the whole patch including the doacross loops info in WsLoop operation. In addition, the ordered depend clause should be adjusted if necessary when supporting doacross loop nest for ordered clause (with n). This is why I also abandon the lowering of ordered depend directive in `fir-dev`. I have mentioned this to Michael and he took up both.

Alternatively, we can leave this for now until we decide on the ordered loop representation.

According to the above, I think we should leave this for now since we cannot have a final decision for the ordered loop representation if not supporting the complete doacross nest (including ordered clause and ordered depend) for now.

LGTM. One Nit comment.

mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td | ||
---|---|---|

328 | Nit: Can you instead add the following? Collapsed loops are represented by the worksharing loop having a list of indices, bounds and steps where the size of the list is equal to the collapse value. |