[PATCH] D76603: [MLIR] Refactor builder of `loop.if` op with support of non-void ops.

[Nagy Mostafa via Phabricator via llvm-commits]

nmostafa added inline comments.


================
Comment at: mlir/include/mlir/Dialect/LoopOps/LoopOps.td:224
+    OpBuilder<"Builder *builder, OperationState &result, Value cond, "
+              "bool withElseRegion, ArrayRef<Type> resultTypes = {}">
   ];
----------------
Any reason for this refactoring ? I prefer to have results before uses to be consistent with tbl-gen'ed builders. Also, use TypeRange instead of ArrayRef. 


================
Comment at: mlir/include/mlir/Dialect/LoopOps/LoopOps.td:233
+                       results().empty() ? body.end() : std::prev(body.end()));
     }
     OpBuilder getElseBodyBuilder() {
----------------
Is the condition here reversed ? If we built a void IfOp, the YieldOp is auto-inserted, else it is missing, and we insert at end of block. 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76603