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

[Shraiysh via Phabricator via llvm-commits]

shraiysh added a comment.

Thanks for the comments @kiranchandramohan. I have addressed them.



================
Comment at: mlir/test/Dialect/OpenMP/invalid.mlir:150
+  // expected-error @below {{inclusive is not a valid clause}}
+  omp.wsloop (%iv) : index = (%lb) to (%ub) step (%step) nowait inclusive {
+    omp.yield
----------------
kiranchandramohan wrote:
> Is this an error because inclusive is not placed after ub?
Yes, as you suggested, `inclusive` is not a clause and so, it is expected outside the clause-list. I placed it right after the upper bound because it only affects the upper bound.


================
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) {
----------------
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.


================
Comment at: mlir/test/Dialect/OpenMP/ops.mlir:146
+
+  return
 }
----------------
kiranchandramohan wrote:
> Nit: Is this a formatting character?
I don't think it is a formatting character, I have tried to remove it and add spaces and upload the diff, but it is staying. I believe this is just a new-line diff.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112198