[PATCH] D86071: [MLIR][OpenMP] Add omp.wsloop operation

[Kiran Chandramohan via Phabricator via llvm-commits]

kiranchandramohan marked 6 inline comments as done.
kiranchandramohan added inline comments.


================
Comment at: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td:103
+
+def WsLoopOp : OpenMP_Op<"wsloop", [AttrSizedOperandSegments]> {
+  let summary = "workshare loop construct";
----------------
ftynse wrote:
> clementval wrote:
> > Would it make sense to add the `DeclareOpInterfaceMethods<LoopLikeOpInterface>` trait since you added `lowerBound`, `upperBound` and `step`? 
> `LoopLikeOpInterface` is a bit of a misnomer. It has nothing to do with bounds, but instead registers the op to be processed by LICM. It will likely break OpenMP loops.
Skipping since they are not applicable to OpenMP loops.


================
Comment at: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td:106
+  let description = [{
+    The workshare loop construct specifies that the iterations of the associated
+    loops will be executed in parallel by threads in the current context. These
----------------
clementval wrote:
> Since it is now a loop should the `associated loops` be rephrased? 
Did a minor rephrasing.


================
Comment at: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td:152
+
+  let regions = (region AnyRegion:$region);
+}
----------------
ftynse wrote:
> How does one terminate such loops?
Added a yield terminator. Current usage will be an empty yield.


================
Comment at: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td:134
+    associate loops are executed in. Currently the only option for this
+    attribute is "concurrent".
+  }];
----------------
bondhugula wrote:
> This description is missing a customary example in the end. (Please use triple backticks to include an example.)
Added an example. Note that the pretty printer and parser are not part of this patch but still i am using the pretty syntax.


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

https://reviews.llvm.org/D86071