[PATCH] D72394: [mlir] Add skeleton implementation of loop.parallel, loop.yield and loop.yield.return operations.

[Nicolas Vasilache via Phabricator via llvm-commits]

nicolasvasilache added inline comments.


================
Comment at: mlir/include/mlir/Dialect/LoopOps/LoopOps.td:152
+    The parallel loop operation supports reduction of values produced by
+    indiviual iterations into a single result. This is modelled using the
+    loop.yield operation (see loop.yield for details). Each result of a
----------------
typo: individual
typo2: modeled


================
Comment at: mlir/include/mlir/Dialect/LoopOps/LoopOps.td:163
+
+    ```mlir
+       loop.parallel (%iv) = (%lb) to (%ub) step (%step) {
----------------
Please add a few more examples that also illustrate reduction and the yield ops.


================
Comment at: mlir/include/mlir/Dialect/LoopOps/LoopOps.td:198
 
+def YieldOp : Loop_Op<"yield", [HasParent<"ParallelOp">]> {
+  let summary = "yield operation for parallel for";
----------------
Do we foresee another use case for this than reduction?
If not, does it make sense to call this a ReductionOp or a ReduceOp?


================
Comment at: mlir/include/mlir/Dialect/LoopOps/LoopOps.td:227
+        ^bb0(%lhs : f32, %rhs: f32):
+	  loop.yield.return %rhs: f32
+      } : f32
----------------
rriddle wrote:
> This looks like it needs to be formatted.
can we make this example a bit more realistic?
Unless I am misunderstanding, there should be a reduction operation in the region?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72394