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

[River Riddle via Phabricator via llvm-commits]

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


================
Comment at: mlir/lib/Dialect/LoopOps/LoopOps.cpp:123
+    auto operands = op.getIterOperands();
+    for (auto e : llvm::zip(regionArgs, operands)) {
+      p << std::get<0>(e) << " = " << std::get<1>(e);
----------------
nit:
```
llvm::interleaveComma(llvm::zip(...), p, [&](auto it) {
  p << ...
}); 
p << ")";
```


================
Comment at: mlir/lib/Dialect/LoopOps/LoopOps.cpp:127
+    }
+    assert(!op.results().empty());
+    p << " -> (" << op.getResultTypes() << ")";
----------------
Why assert here? This seems like something that would already be verified.


================
Comment at: mlir/lib/Dialect/LoopOps/LoopOps.cpp:521
+             << "yield inside loop.parallel is not allowed to have operands";
+  } else
+    return op.emitOpError()
----------------
nit: Add {} for each if/else clause if any of them have one.


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

https://reviews.llvm.org/D74174