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

[Alex Zinenko via Phabricator via llvm-commits]

ftynse accepted this revision.
ftynse added inline comments.
This revision is now accepted and ready to land.


================
Comment at: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td:124-125
+
+    The body region can contain any number of blocks. The region is terminated
+    by "omp.yield" instruction without operands.
+
----------------
What is the semantics of multiple blocks terminated with `omp.yield` in the loop body?  (Regions are not necessarily single-exit).


================
Comment at: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td:137
+
+    The $private_vars, $firstprivate_vars, $lastprivate_vars and $linear_vars
+    parameters are a variadic list of variables that specify the data sharing
----------------
Nit, here and below: the `$` sign only appears in ODS input, not in the generated documentation or code.


================
Comment at: mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td:138
+    The $private_vars, $firstprivate_vars, $lastprivate_vars and $linear_vars
+    parameters are a variadic list of variables that specify the data sharing
+    attribute of those variables. The $linear_step_vars variable additionally
----------------
Nit: there are no "variables" in MLIR. In these case, you are likely referring to operand groups.


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

https://reviews.llvm.org/D86071