[PATCH] D74174: [MLIR] Allow Loop dialect IfOp and ForOp to define values

Mon Feb 10 13:13:21 PST 2020

nmostafa marked an inline comment as done.
nmostafa added inline comments.

================
Comment at: mlir/include/mlir/Dialect/LoopOps/LoopOps.td:353

-def TerminatorOp : Loop_Op<"terminator", [Terminator]> {
-  let summary = "cf terminator operation";
+def YieldOp : Loop_Op<"yield", [Terminator]> {
+  let summary = "cf yield and termination operation";
----------------
herhut wrote:
> I wonder whether we should keep the TerminatorOp for the `loop.parallel`. It is always implicit there anyway and has different semantics (the return values of `loop.parallel` are defined by `loop.reduce` operation. Having the extra op is not costing much and avoid the extra checks in verifiers.
There are mixed opinions on this .. see https://llvm.discourse.group/t/rfc-adding-operands-and-results-to-loop-for/459/14
IMO, unifying under one terminator makes more sense, and yield semantics is really: Terminate and Yield values. 

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

https://reviews.llvm.org/D74174