[PATCH] D112198: [MLIR][OpenMP] Fixed the missing inclusive clause in omp.wsloop and fix order clause

Tue Oct 26 18:25:52 PDT 2021

peixin added a comment.

LGTM.

================
Comment at: mlir/test/Dialect/OpenMP/invalid.mlir:158
+func @order_value(%lb : index, %ub : index, %step : index) {
+  // expected-error @below {{attribute 'order_val' failed to satisfy constraint: OrderKind Clause}}
+  omp.wsloop (%iv) : index = (%lb) to (%ub) step (%step) order(default) {
----------------
shraiysh wrote:
> peixin wrote:
> > shraiysh wrote:
> > > kiranchandramohan wrote:
> > > > Why is this an error?
> > > This is an error because `order(default)` is not allowed to be specified by the user according to the standard. Order can either be present with the `concurrent` value, or it should not be there.
> > It seems that order clause can only be present with the `concurrent` value. Where does the `OMP_ORDER_default` in OMP.td come from? I don't see this in OpenMP 5.0 Spec.
> Yes, that is correct. It is not in the OpenMP Spec, but as far as I understand, every Clause in OMP.td is supposed to have a default value (which signifies no value, I believe). I faced a similar issue when dealing with the `memory_order` clause. There was no `default` memory clause in the standard, and not adding a default value was leading to some errors in either one of llvm/clang/flang (I don't exactly remember which one). @clementval please correct me if I have missed something.
Thanks for the explanation. This is fine with me.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112198/new/

https://reviews.llvm.org/D112198