[Mlir-commits] [mlir] [OpenMP] Enable simd in non-reduction composite constructs (PR #146097)

[Sergio Afonso]

skatrak wrote:

> Question for reviewers who have worked on this before - the current behaviour is fairly inconsistent, that is to say, using "simd reduction" on its own results in a crash because it's not implemented, whereas using "simd reduction" as part of a composite construct simply ignores the simd. Is it desirable to keep this behaviour? For instance, the following program will crash:

I was actually planning on proposing changes to this soon, so I can just explain from my side what the thinking behind some of these differences are. Currently, we entirely ignore simd within a composite construct, which means that we don't check whether there are unimplemented simd clauses being specified. In the case of standalone simd, though, there is some existing support, so there are checks for unimplemented features. The result of that is that `do simd reduction(+: x)` compiles and runs as if it was just `do reduction(+: x)`, whereas `simd reduction(+: x)` triggers a not-yet-implemented error for the `reduction` clause.

The reason for ignoring `simd` information on composite constructs was to prevent the introduction of `simd` from causing not-yet-implemented errors when it would be legal by the spec to always codegen for SIMD lanes of 1 element, especially when there are reductions involved, as @tblah described. So, composite constructs behave as if they always resulted in 1-element SIMD lanes. This is very arbitrary, so I like your proposal to handle simd in composite constructs. At the moment I'm most concerned about what to do about unsupported clauses.

How much of an issue would it be to always emit warnings instead of errors for all unsupported `simd` clauses? Does this become an issue only after we generate 2+ element SIMD lanes? I'm thinking that from the target side we don't currently want to be doing anything with `simd` constructs, so we might still want to have the option to ignore them.

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