[flang] Adopt NoRegionArguments (WhereOp) and ParentOneOf (ResultOp) traits

[jurahul]

Ping. Not sure if flang is accepting such changes, if not please let me know and I will abandon this.

[mehdi_amini]

Nice cleanup! Thanks.

[schweitz]

In D83522#2152131, @jurahul wrote:

Ping. Not sure if flang is accepting such changes, if not please let me know and I will abandon this.

Cleanup is welcome. Thanks for doing this.

We would like llvm-project not to diverge from the working development branch. Can you submit these changes here as well? https://github.com/flang-compiler/f18-llvm-project/tree/fir-dev It'll help make sure the branches stay in sync and changes don't get lost.

Thanks in advance.

[mehdi_amini]

In D83522#2153157, @schweitz wrote:

In D83522#2152131, @jurahul wrote:

Ping. Not sure if flang is accepting such changes, if not please let me know and I will abandon this.

Cleanup is welcome. Thanks for doing this.

We would like llvm-project not to diverge from the working development branch. Can you submit these changes here as well? https://github.com/flang-compiler/f18-llvm-project/tree/fir-dev It'll help make sure the branches stay in sync and changes don't get lost.

This quite an unusual request /setup. I am not sure I understand this “working development branch”, but in general I’d expect any downstream fork of LLVM to be maintained by the fork owner.

[jurahul]

Holding off on pushing till the dev branch issue is clarified. Please let me know if Mehdi'd understanding is correct. How do you plan to manage API breaking changes in MLIR that may not be in this dev branch?

[sscalpone]

Hi @jurahul, there's certainly no objection for you to commit this change in llvm-project, especially if the change is required to keep llvm building properly.

The story behind Eric's request is that the majority of the development in flang/.../Optimizer is happening in the fir-dev branch. That branch is being upstreamed to llvm-project as it is reviewed and approved. The request to commit the change to fir-dev is simply to make the rebase easier. It's solely up to you.

[jurahul]

Thanks. I have created a pull request as well here: https://github.com/flang-compiler/f18-llvm-project/pull/275.

[jurahul]

Fix build failure

[schweitz]

In D83522#2155799, @jurahul wrote:

Fix build failure

Thanks for your fix and getting this upstreamed for test in fir-dev. It is greatly appreciated.

flang/include/flang/Optimizer/Dialect/FIROps.td

let extraClassDeclaration = [{
     }
   }];
 }
 
 //===----------------------------------------------------------------------===//
 // Fortran loops
 //===----------------------------------------------------------------------===//
 
-def fir_ResultOp : fir_Op<"result", [NoSideEffect, ReturnLike, Terminator]> {
+def fir_ResultOp : fir_Op<"result",
+    [NoSideEffect, ReturnLike, Terminator,
+     ParentOneOf<["WhereOp", "LoopOp", "IterWhileOp"]>]> {
   let summary = "special terminator for use in fir region operations";
 
   let description = [{
     Result takes a list of ssa-values produced in the block and forwards them
     as a result to the operation that owns the region of the block. The
     operation can retain the values or return them to its parent block
     depending upon its semantics.
   }];
let extraClassDeclaration = [{
 
     void setUnordered() {
       getOperation()->setAttr(unorderedAttrName(),
                               mlir::UnitAttr::get(getContext()));
     }
   }];
 }
 
-def fir_WhereOp : region_Op<"if"> {
+def fir_WhereOp : region_Op<"if", [NoRegionArguments]> {
   let summary = "if-then-else conditional operation";
   let description = [{
     Used to conditionally execute operations. This operation is the FIR
     dialect's version of `loop.if`.
 
     ```mlir
       %56 = ... : i1
       %78 = ... : !fir.ref<!T>

flang/lib/Optimizer/Dialect/FIROps.cpp

 // ResultOp
 //===----------------------------------------------------------------------===//
 
 static mlir::LogicalResult verify(fir::ResultOp op) {
   auto parentOp = op.getParentOp();
   auto results = parentOp->getResults();
   auto operands = op.getOperands();
 
-  if (isa<fir::WhereOp>(parentOp) || isa<fir::LoopOp>(parentOp) ||
-      isa<fir::IterWhileOp>(parentOp)) {
-    if (parentOp->getNumResults() != op.getNumOperands())
-      return op.emitOpError() << "parent of result must have same arity";
-    for (auto e : llvm::zip(results, operands)) {
-      if (std::get<0>(e).getType() != std::get<1>(e).getType())
-        return op.emitOpError()
-               << "types mismatch between result op and its parent";
-    }
-  } else {
-    return op.emitOpError()
-           << "result only terminates if, do_loop, or iterate_while regions";
-  }
+  for (auto e : llvm::zip(results, operands))
   return success();
 }
 
 //===----------------------------------------------------------------------===//
 // SelectOp
 //===----------------------------------------------------------------------===//
 
 static constexpr llvm::StringRef getCompareOffsetAttr() {
static mlir::ParseResult parseWhereOp(OpAsmParser &parser,
   // Parse the optional attribute list.
   if (parser.parseOptionalAttrDict(result.attributes))
     return mlir::failure();
 
   return mlir::success();
 }
 
 static LogicalResult verify(fir::WhereOp 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)
-      if (b.getNumArguments() != 0)
-        return op.emitOpError(
-            "requires that child entry blocks have no arguments");
-  }
   if (op.getNumResults() != 0 && op.otherRegion().empty())
     return op.emitOpError("must have an else block if defining values");
 
   return mlir::success();
 }
 
 static void print(mlir::OpAsmPrinter &p, fir::WhereOp op) {
   bool printBlockTerminators = false;