[MLIR] Remove redundant scf.if builder

[frgossen]

Summary

The builder with withElseRegion implicitly builds a terminator depending on the op types, which is unnecessarily complex.
The same can be achieved with scf::buildTerminatedBody which makes the intention clear.

[frgossen]

Rebase

[frgossen]

Fix flang

[frgossen]

Fix formatting

[frgossen]

Rebase

flang/lib/Lower/OpenMP.cpp

if (const auto &privateClause =
           // lastprivate FIR can reside. Later canonicalizations
           // will optimize away this operation.
 
           omp::SectionOp *sectionOp = dyn_cast<omp::SectionOp>(&op);
           mlir::scf::IfOp ifOp = firOpBuilder.create<mlir::scf::IfOp>(
               sectionOp->getLoc(),
               firOpBuilder.createIntegerConstant(
                   sectionOp->getLoc(), firOpBuilder.getIntegerType(1), 0x1),
-              /*else*/ false);
+              mlir::scf::buildTerminatedBody);
           firOpBuilder.setInsertionPointToStart(&ifOp.getThenRegion().front());
 
           const Fortran::parser::OpenMPConstruct *parentOmpConstruct =
               eval.parentConstruct->getIf<Fortran::parser::OpenMPConstruct>();
           assert(parentOmpConstruct &&
                  "Expected a valid enclosing OpenMP construct");
           const Fortran::parser::OpenMPSectionsConstruct *sectionsConstruct =
               std::get_if<Fortran::parser::OpenMPSectionsConstruct>(
if (const auto &privateClause =
         // clauses.
         if (!hasLastPrivateOp) {
           cmpOp = firOpBuilder.create<mlir::arith::CmpIOp>(
               wsLoopOp->getLoc(), mlir::arith::CmpIPredicate::eq,
               wsLoopOp->getRegion().front().getArguments()[0],
               wsLoopOp->getUpperBound()[0]);
         }
         mlir::scf::IfOp ifOp = firOpBuilder.create<mlir::scf::IfOp>(
-            wsLoopOp->getLoc(), cmpOp, /*else*/ false);
+            wsLoopOp->getLoc(), cmpOp, mlir::scf::buildTerminatedBody);
         firOpBuilder.setInsertionPointToStart(&ifOp.getThenRegion().front());
         lastPrivIP = firOpBuilder.saveInsertionPoint();
       } else {
         TODO(converter.getCurrentLocation(),
              "lastprivate clause in constructs other than worksharing-loop");
       }
       collectOmpObjectListSymbol(lastPrivateClause->v, privatizedSymbols);
       hasLastPrivateOp = true;

mlir/include/mlir/Dialect/SCF/IR/SCFOps.td

def IfOp : SCF_Op<"if", [DeclareOpInterfaceMethods<RegionBranchOpInterface, [
   let regions = (region SizedRegion<1>:$thenRegion,
                         MaxSizedRegion<1>:$elseRegion);
 
   let skipDefaultBuilders = 1;
   let builders = [
     OpBuilder<(ins "TypeRange":$resultTypes, "Value":$cond)>,
     OpBuilder<(ins "TypeRange":$resultTypes, "Value":$cond,
       "bool":$addThenBlock, "bool":$addElseBlock)>,
-    OpBuilder<(ins "Value":$cond, "bool":$withElseRegion)>,
     OpBuilder<(ins "TypeRange":$resultTypes, "Value":$cond,
       "bool":$withElseRegion)>,
     OpBuilder<(ins "Value":$cond,
       CArg<"function_ref<void(OpBuilder &, Location)>",
            "buildTerminatedBody">:$thenBuilder,
       CArg<"function_ref<void(OpBuilder &, Location)>",
            "nullptr">:$elseBuilder)>,
   ];

mlir/lib/Conversion/SCFToGPU/SCFToGPU.cpp

if (isMappedToProcessor(processor)) {
           bounds[processor] = launchBound;
         }
         if (!boundIsPrecise) {
           // We are using an approximation, create a surrounding conditional.
           Value originalBound = std::get<3>(config);
           arith::CmpIOp pred = rewriter.create<arith::CmpIOp>(
               loc, arith::CmpIPredicate::slt, newIndex,
               cloningMap.lookupOrDefault(originalBound));
-          scf::IfOp ifOp = rewriter.create<scf::IfOp>(loc, pred, false);
+          scf::IfOp ifOp = rewriter.create<scf::IfOp>(loc, pred);
           rewriter.setInsertionPointToStart(&ifOp.getThenRegion().front());
           // Put a sentinel into the worklist so we know when to pop out of the
           // if body again. We use the launchOp here, as that cannot be part of
           // the bodies instruction.
           worklist.push_back(launchOp.getOperation());
         }
       }
     } else {

mlir/lib/Dialect/GPU/TransformOps/GPUTransformOps.cpp

static DiagnosedSilenceableFailure rewriteOneForallToGpuThreads(
 
   // Step 5. Move the body of forallOp.
   // Erase the terminator first, it will not be used.
   rewriter.eraseOp(forallOp.getTerminator());
   Block *targetBlock;
   Block::iterator insertionPoint;
   if (predicate) {
     // Step 5.a. If predicated, move at the beginning.
-    auto ifOp =
-        rewriter.create<scf::IfOp>(loc, predicate, /*withElseRegion=*/false);
+    auto ifOp = rewriter.create<scf::IfOp>(loc, predicate);
     targetBlock = ifOp.thenBlock();
     insertionPoint = ifOp.thenBlock()->begin();
   } else {
     // Step 5.b. Otherwise, move inline just before forallOp.
     targetBlock = forallOp->getBlock();
     insertionPoint = Block::iterator(forallOp);
   }
   Block &sourceBlock = forallOp.getRegion().front();

mlir/lib/Dialect/SCF/IR/SCF.cpp

void IfOp::build(OpBuilder &builder, OperationState &result,
   Region *thenRegion = result.addRegion();
   if (addThenBlock)
     builder.createBlock(thenRegion);
   Region *elseRegion = result.addRegion();
   if (addElseBlock)
     builder.createBlock(elseRegion);
 }
 
-void IfOp::build(OpBuilder &builder, OperationState &result, Value cond,
-                 bool withElseRegion) {
-  build(builder, result, TypeRange{}, cond, withElseRegion);
-}
-
 void IfOp::build(OpBuilder &builder, OperationState &result,
                  TypeRange resultTypes, Value cond, bool withElseRegion) {
   result.addTypes(resultTypes);
   result.addOperands(cond);
 
   // Build then region.
   OpBuilder::InsertionGuard guard(builder);
   Region *thenRegion = result.addRegion();

mlir/lib/Dialect/SCF/Transforms/LoopSpecialization.cpp

static void specializeParallelLoopForUnrolling(ParallelOp op) {
   for (auto bound : llvm::zip(op.getUpperBound(), constantIndices)) {
     Value constant =
         b.create<arith::ConstantIndexOp>(op.getLoc(), std::get<1>(bound));
     Value cmp = b.create<arith::CmpIOp>(op.getLoc(), arith::CmpIPredicate::eq,
                                         std::get<0>(bound), constant);
     cond = cond ? b.create<arith::AndIOp>(op.getLoc(), cond, cmp) : cmp;
     map.map(std::get<0>(bound), constant);
   }
-  auto ifOp = b.create<scf::IfOp>(op.getLoc(), cond, /*withElseRegion=*/true);
+  auto ifOp = b.create<scf::IfOp>(op.getLoc(), cond, scf::buildTerminatedBody,
+                                  scf::buildTerminatedBody);
   ifOp.getThenBodyBuilder().clone(*op.getOperation(), map);
   ifOp.getElseBodyBuilder().clone(*op.getOperation());
   op.erase();
 }
 
 /// Rewrite a for loop with bounds defined by an affine.min with a constant into
 /// 2 loops after checking if the bounds are equal to that constant. This is
 /// beneficial if the loop will almost always have the constant bound and that
if (minConstant == std::numeric_limits<int64_t>::max())
     return;
 
   OpBuilder b(op);
   IRMapping map;
   Value constant = b.create<arith::ConstantIndexOp>(op.getLoc(), minConstant);
   Value cond = b.create<arith::CmpIOp>(op.getLoc(), arith::CmpIPredicate::eq,
                                        bound, constant);
   map.map(bound, constant);
-  auto ifOp = b.create<scf::IfOp>(op.getLoc(), cond, /*withElseRegion=*/true);
+  auto ifOp = b.create<scf::IfOp>(op.getLoc(), cond, scf::buildTerminatedBody,
+                                  scf::buildTerminatedBody);
   ifOp.getThenBodyBuilder().clone(*op.getOperation(), map);
   ifOp.getElseBodyBuilder().clone(*op.getOperation());
   op.erase();
 }
 
 /// Rewrite a for loop with bounds/step that potentially do not divide evenly
 /// into a for loop where the step divides the iteration space evenly, followed
 /// by an scf.if for the last (partial) iteration (if any).

mlir/lib/Dialect/SparseTensor/Transforms/CodegenUtils.cpp

void mlir::sparse_tensor::deallocDenseTensor(OpBuilder &builder, Location loc,
                                              Value buffer) {
   builder.create<memref::DeallocOp>(loc, buffer);
 }
 
 Value mlir::sparse_tensor::genValueForDense(OpBuilder &builder, Location loc,
                                             Value tensor, ValueRange ivs) {
   Value val = builder.create<tensor::ExtractOp>(loc, tensor, ivs);
   Value cond = genIsNonzero(builder, loc, val);
-  scf::IfOp ifOp = builder.create<scf::IfOp>(loc, cond, /*else*/ false);
+  scf::IfOp ifOp = builder.create<scf::IfOp>(loc, cond);
   builder.setInsertionPointToStart(&ifOp.getThenRegion().front());
   return val;
 }
 
 // FIXME:
 // 1. Dense tensors loop should be generated by loop emitter.
 // 2. Support reduction variables to propagate SSA chains properly.
 void mlir::sparse_tensor::genDenseTensorOrSparseConstantIterLoop(

mlir/lib/Dialect/SparseTensor/Transforms/SparseBufferRewriting.cpp

static void createShiftDownFunc(OpBuilder &builder, ModuleOp module,
   ValueRange args = entryBlock->getArguments().drop_back();
   Value first = args[loIdx];
   Value start = args[hiIdx];
 
   // If (n >= 2).
   Value c2 = constantIndex(builder, loc, 2);
   Value condN =
       builder.create<arith::CmpIOp>(loc, arith::CmpIPredicate::uge, n, c2);
-  scf::IfOp ifN = builder.create<scf::IfOp>(loc, condN, /*else=*/false);
+  scf::IfOp ifN =
+      builder.create<scf::IfOp>(loc, TypeRange{}, condN, /*else=*/false);
   builder.setInsertionPointToStart(&ifN.getThenRegion().front());
   Value child = builder.create<arith::SubIOp>(loc, start, first);
 
   // If ((n-2)/2 >= child).
   Value t = createSubTwoDividedByTwo(builder, loc, n);
   Value condNc =
       builder.create<arith::CmpIOp>(loc, arith::CmpIPredicate::uge, t, child);
-  scf::IfOp ifNc = builder.create<scf::IfOp>(loc, condNc, /*else=*/false);
+  scf::IfOp ifNc =
+      builder.create<scf::IfOp>(loc, TypeRange{}, condNc, /*else=*/false);
 
   builder.setInsertionPointToStart(&ifNc.getThenRegion().front());
   Value c1 = constantIndex(builder, loc, 1);
   SmallVector<Value> compareOperands{start, start};
   uint64_t numXBuffers = isCoo ? 1 : nx;
   compareOperands.append(args.begin() + xStartIdx,
                          args.begin() + xStartIdx + numXBuffers);
   Type i1Type = IntegerType::get(module.getContext(), 1, IntegerType::Signless);

 
 static void createQuickSort(OpBuilder &builder, ModuleOp module,
                             func::FuncOp func, ValueRange args, uint64_t nx,
                             uint64_t ny, bool isCoo, uint32_t nTrailingP) {
   MLIRContext *context = module.getContext();
   Location loc = func.getLoc();
   Value lo = args[loIdx];
   Value hi = args[hiIdx];
+  Value cond =
+      builder.create<arith::CmpIOp>(loc, arith::CmpIPredicate::ult, lo, hi);
+  scf::IfOp ifOp = builder.create<scf::IfOp>(loc, cond);
+
+  // The if-stmt true branch.
+  builder.setInsertionPointToStart(&ifOp.getThenRegion().front());
   FlatSymbolRefAttr partitionFunc = getMangledSortHelperFunc(
       builder, func, {IndexType::get(context)}, kPartitionFuncNamePrefix, nx,
       ny, isCoo, args.drop_back(nTrailingP), createPartitionFunc);
   auto p = builder.create<func::CallOp>(loc, partitionFunc,
                                         TypeRange{IndexType::get(context)},
                                         args.drop_back(nTrailingP));
 
   SmallVector<Value> lowOperands{lo, p.getResult(0)};
static void createQuickSortFunc(OpBuilder &builder, ModuleOp module,
   Location loc = func.getLoc();
   ValueRange args = entryBlock->getArguments();
   Value lo = args[loIdx];
   Value hi = args[hiIdx];
   Value loCmp =
       builder.create<arith::AddIOp>(loc, lo, constantIndex(builder, loc, 1));
   Value cond =
       builder.create<arith::CmpIOp>(loc, arith::CmpIPredicate::ult, loCmp, hi);
-  scf::IfOp ifOp = builder.create<scf::IfOp>(loc, cond, /*else=*/false);
+  scf::IfOp ifOp =
+      builder.create<scf::IfOp>(loc, TypeRange{}, cond, /*else=*/false);
 
   // The if-stmt true branch.
   builder.setInsertionPointToStart(&ifOp.getThenRegion().front());
   Value pDepthLimit;
   Value savedDepthLimit;
   scf::IfOp depthIf;
 
   if (isHybrid) {
     Value len = builder.create<arith::SubIOp>(loc, hi, lo);
     Value lenLimit = constantIndex(builder, loc, 30);
     Value lenCond = builder.create<arith::CmpIOp>(
         loc, arith::CmpIPredicate::ule, len, lenLimit);
-    scf::IfOp lenIf = builder.create<scf::IfOp>(loc, lenCond, /*else=*/true);
+    scf::IfOp lenIf =
+        builder.create<scf::IfOp>(loc, TypeRange{}, lenCond, /*else=*/true);
 
     // When len <= limit.
     builder.setInsertionPointToStart(&lenIf.getThenRegion().front());
     FlatSymbolRefAttr insertionSortFunc = getMangledSortHelperFunc(
         builder, func, TypeRange(), kSortStableFuncNamePrefix, nx, ny, isCoo,
         args.drop_back(nTrailingP), createSortStableFunc);
     builder.create<func::CallOp>(loc, insertionSortFunc, TypeRange(),
                                  ValueRange(args.drop_back(nTrailingP)));
 
     // When len > limit.
     builder.setInsertionPointToStart(&lenIf.getElseRegion().front());
     pDepthLimit = args.back();
     savedDepthLimit = builder.create<memref::LoadOp>(loc, pDepthLimit);
     Value depthLimit = builder.create<arith::SubIOp>(
         loc, savedDepthLimit, constantI64(builder, loc, 1));
     builder.create<memref::StoreOp>(loc, depthLimit, pDepthLimit);
     Value depthCond =
         builder.create<arith::CmpIOp>(loc, arith::CmpIPredicate::ule,
                                       depthLimit, constantI64(builder, loc, 0));
-    depthIf = builder.create<scf::IfOp>(loc, depthCond, /*else=*/true);
+    depthIf =
+        builder.create<scf::IfOp>(loc, TypeRange{}, depthCond, /*else=*/true);
 
     // When depth exceeds limit.
     builder.setInsertionPointToStart(&depthIf.getThenRegion().front());
     FlatSymbolRefAttr heapSortFunc = getMangledSortHelperFunc(
         builder, func, TypeRange(), kHeapSortFuncNamePrefix, nx, ny, isCoo,
         args.drop_back(nTrailingP), createHeapSortFunc);
     builder.create<func::CallOp>(loc, heapSortFunc, TypeRange(),
                                  ValueRange(args.drop_back(nTrailingP)));

mlir/lib/Dialect/SparseTensor/Transforms/SparseTensorConversion.cpp

static void genSparseCOOIterationLoop(
   Block *after = rewriter.createBlock(&whileOp.getAfter(), {}, noTypes);
   rewriter.setInsertionPointToStart(after);
 
   const bool hasDenseDim =
       llvm::any_of(stt.getEncoding().getDimLevelType(), isDenseDLT);
   if (hasDenseDim) {
     Value elemV = rewriter.create<memref::LoadOp>(loc, elemPtr);
     Value isZero = genIsNonzero(rewriter, loc, elemV);
-    scf::IfOp ifOp = rewriter.create<scf::IfOp>(loc, isZero, /*else*/ false);
+    scf::IfOp ifOp = rewriter.create<scf::IfOp>(loc, isZero);
     rewriter.setInsertionPointToStart(&ifOp.getThenRegion().front());
   }
   // Callback here to build loop body.
   bodyBuilder(rewriter, loc, srcIdx, elemPtr);
 
   // Exit the scope from the IfOp.
   if (hasDenseDim)
     rewriter.setInsertionPointToEnd(after);

mlir/lib/Dialect/Vector/Transforms/VectorDistribute.cpp

LogicalResult matchAndRewrite(WarpExecuteOnLane0Op warpOp,
     // Passed all checks. Start rewriting.
     OpBuilder::InsertionGuard g(rewriter);
     rewriter.setInsertionPoint(warpOp);
 
     // Step 1: Create scf.if op.
     Value c0 = rewriter.create<arith::ConstantIndexOp>(loc, 0);
     Value isLane0 = rewriter.create<arith::CmpIOp>(
         loc, arith::CmpIPredicate::eq, warpOp.getLaneid(), c0);
-    auto ifOp = rewriter.create<scf::IfOp>(loc, isLane0,
-                                           /*withElseRegion=*/false);
+    auto ifOp = rewriter.create<scf::IfOp>(loc, isLane0);
     rewriter.eraseOp(ifOp.thenBlock()->getTerminator());
 
     // Step 2: insert appropriate (alloc, write)-pairs before the scf.if and
     // reads within the scf.if to transit the values captured from above.
     SmallVector<Value> bbArgReplacements;
     for (const auto &it : llvm::enumerate(warpOp.getArgs())) {
       Value sequentialVal = warpOpBody->getArgument(it.index());
       Value distributedVal = it.value();