[MLIR] Insert loop.yield to IfOp regions only if it's void.

[pifon2a]

Resolve merge conflicts.

[nmostafa]

mlir/include/mlir/Dialect/LoopOps/LoopOps.td

227

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.

235

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.

[pifon2a]

Addressed the comments.

[pifon2a]

Remove whitespace.

[pifon2a]

@nmostafa thank you!

mlir/include/mlir/Dialect/LoopOps/LoopOps.td

227

It became a refactoring after I rebased. I had a similar patch last week to add a builder for IfOp with results, but I did not send it out on Friday. Today I sent it out and then after rebasing I noticed your patch. I do not really care about order of resultTypes args. I care only about loop.yield not being inserted for non-void ifops.

235

thank you for noticing!

[nmostafa]

LGTM. Thanks for fixing this !

mlir/include/mlir/Dialect/LoopOps/LoopOps.td

234

nit: extra space after "?" @ line 240 as well.

[pifon2a]

Fixing stuff.

[pifon2a]

remove more whitespaces

mlir/include/mlir/Dialect/LoopOps/LoopOps.td

def IfOp : Loop_Op<"if",
   let results = (outs Variadic<AnyType>:$results);
   let regions = (region SizedRegion<1>:$thenRegion, AnyRegion:$elseRegion);
 
   let skipDefaultBuilders = 1;
   let builders = [
     OpBuilder<"Builder *builder, OperationState &result, "
               "Value cond, bool withElseRegion">,
     OpBuilder<"Builder *builder, OperationState &result, "
-              "TypeRange resultTypes, Value cond, "
-              "bool withElseRegion">
+              "TypeRange resultTypes, Value cond, bool withElseRegion">
   ];
 
   let extraClassDeclaration = [{
     OpBuilder getThenBodyBuilder() {
       assert(!thenRegion().empty() && "Unexpected empty 'then' region.");
       Block &body = thenRegion().front();
-      return OpBuilder(&body, std::prev(body.end()));
+      return OpBuilder(&body,
+                       results().empty() ?  std::prev(body.end()) : body.end());
     }
     OpBuilder getElseBodyBuilder() {
       assert(!elseRegion().empty() && "Unexpected empty 'else' region.");
       Block &body = elseRegion().front();
-      return OpBuilder(&body, std::prev(body.end()));
+      return OpBuilder(&body,
+                       results().empty() ?  std::prev(body.end()) : body.end());
     }
   }];
 }
 
 def ParallelOp : Loop_Op<"parallel",
     [AttrSizedOperandSegments, SingleBlockImplicitTerminator<"YieldOp">]> {
   let summary = "parallel for operation";
   let description = [{

mlir/lib/Dialect/LoopOps/LoopOps.cpp

 }
 
 //===----------------------------------------------------------------------===//
 // IfOp
 //===----------------------------------------------------------------------===//
 
 void IfOp::build(Builder *builder, OperationState &result, Value cond,
                  bool withElseRegion) {
     build(builder, result, /*resultTypes=*/llvm::None, cond, withElseRegion);
 }
 
 void IfOp::build(Builder *builder, OperationState &result,
                  TypeRange resultTypes, Value cond, bool withElseRegion) {
   result.addOperands(cond);
   result.addTypes(resultTypes);
+
   Region *thenRegion = result.addRegion();
-  Region *elseRegion = result.addRegion();
+  thenRegion->push_back(new Block());
+  if (resultTypes.empty())
     IfOp::ensureTerminator(*thenRegion, *builder, result.location);
-  if (withElseRegion)
+
+  Region *elseRegion = result.addRegion();
+  if (withElseRegion) {
+    elseRegion->push_back(new Block());
+    if (resultTypes.empty())
       IfOp::ensureTerminator(*elseRegion, *builder, result.location);
   }
+}
 
 static LogicalResult verify(IfOp op) {
   // Verify that the entry of each child region does not have arguments.
   for (auto &region : op.getOperation()->getRegions()) {
     if (region.empty())
       continue;
 
     for (auto &b : region)