[flang][openacc] Add parsing support for dim in gang clause

[clementval]

Summary

Add parsing supprot for dim in gang clause

Depends on D151971

[Anastasia]

flang/test/Parser/acc-unparse.f90

57

Btw dim and num should not appear together, s2.9.2 paragraph 1 says:

The dim argument must be a constant positive integer with value 1, 2, or 3. <...> The number of gangs in dimension d is controlled by the parallel construct; the num argument is not allowed.

There are a number of other restrictions, for example the value of dim can only be an integer literal in the range 1-3 and it has to appear only once. But maybe you plan to add the diagnostics later?

[clementval]

flang/test/Parser/acc-unparse.f90

57

Thanks for catching this! I overlooked these restrictions when I checked the spec.
I’ll at least update this patch with the num argument is not allowed here. I might do another patch for the remaining restrictions.

[clementval]

Add semantic check

[jeanPerier]

Looks great, please double check/rebase, but I think the buildbot failures are unrelated to your patch.

[razvanlupusoru]

Looks great!

[clementval]

Rebase

[clementval]

clang-format :)

flang/examples/FeatureList/FeatureList.cpp

public:
   READ_FEATURE(AccClauseList)
   READ_FEATURE(AccCombinedDirective)
   READ_FEATURE(AccDataModifier)
   READ_FEATURE(AccDataModifier::Modifier)
   READ_FEATURE(AccDeclarativeDirective)
   READ_FEATURE(AccEndAtomic)
   READ_FEATURE(AccEndBlockDirective)
   READ_FEATURE(AccEndCombinedDirective)
-  READ_FEATURE(AccGangArgument)
+  READ_FEATURE(AccGangArg)
   READ_FEATURE(AccObject)
   READ_FEATURE(AccObjectList)
   READ_FEATURE(AccObjectListWithModifier)
   READ_FEATURE(AccObjectListWithReduction)
   READ_FEATURE(AccReductionOperator)
   READ_FEATURE(AccReductionOperator::Operator)
   READ_FEATURE(AccSizeExpr)
   READ_FEATURE(AccSizeExprList)

flang/include/flang/Parser/dump-parse-tree.h

#include "llvm/Frontend/OpenACC/ACC.inc"
   NODE(parser, AccClauseList)
   NODE(parser, AccCombinedDirective)
   NODE(parser, AccDataModifier)
   NODE_ENUM(parser::AccDataModifier, Modifier)
   NODE(parser, AccDeclarativeDirective)
   NODE(parser, AccEndAtomic)
   NODE(parser, AccEndBlockDirective)
   NODE(parser, AccEndCombinedDirective)
-  NODE(parser, AccGangArgument)
   NODE(parser, AccCollapseArg)
+  NODE(parser, AccGangArg)
+  NODE(AccGangArg, Num)
+  NODE(AccGangArg, Dim)
+  NODE(AccGangArg, Static)
+  NODE(parser, AccGangArgList)
   NODE(parser, AccObject)
   NODE(parser, AccObjectList)
   NODE(parser, AccObjectListWithModifier)
   NODE(parser, AccObjectListWithReduction)
   NODE(parser, AccReductionOperator)
   NODE_ENUM(parser::AccReductionOperator, Operator)
   NODE(parser, AccSizeExpr)
   NODE(parser, AccSizeExprList)

flang/include/flang/Parser/parse-tree.h

struct AccTileExpr {
   std::tuple<std::optional<ScalarIntConstantExpr>> t; // if null then *
 };
 
 struct AccTileExprList {
   WRAPPER_CLASS_BOILERPLATE(AccTileExprList, std::list<AccTileExpr>);
 };
 
 struct AccSizeExpr {
-  TUPLE_CLASS_BOILERPLATE(AccSizeExpr);
-  CharBlock source;
-  std::tuple<std::optional<ScalarIntExpr>> t; // if null then *
+  WRAPPER_CLASS_BOILERPLATE(AccSizeExpr, std::optional<ScalarIntExpr>);
 };
 
 struct AccSizeExprList {
   WRAPPER_CLASS_BOILERPLATE(AccSizeExprList, std::list<AccSizeExpr>);
 };
 
 struct AccSelfClause {
   UNION_CLASS_BOILERPLATE(AccSelfClause);
   std::variant<std::optional<ScalarLogicalExpr>, AccObjectList> u;
   CharBlock source;
 };
 
-struct AccGangArgument {
-  TUPLE_CLASS_BOILERPLATE(AccGangArgument);
-  std::tuple<std::optional<ScalarIntExpr>, std::optional<AccSizeExpr>> t;
+// num, dim, static
+struct AccGangArg {
+  UNION_CLASS_BOILERPLATE(AccGangArg);
+  WRAPPER_CLASS(Num, ScalarIntExpr);
+  WRAPPER_CLASS(Dim, ScalarIntExpr);
+  WRAPPER_CLASS(Static, AccSizeExpr);
+  std::variant<Num, Dim, Static> u;
+  CharBlock source;
+};
+
+struct AccGangArgList {
+  WRAPPER_CLASS_BOILERPLATE(AccGangArgList, std::list<AccGangArg>);
 };
 
 struct AccCollapseArg {
   TUPLE_CLASS_BOILERPLATE(AccCollapseArg);
   std::tuple<bool, ScalarIntConstantExpr> t;
 };
 
 struct AccClause {

flang/lib/Lower/OpenACC.cpp

createLoopOp(Fortran::lower::AbstractConverter &converter,
   llvm::SmallVector<mlir::Attribute> privatizations, reductionRecipes;
   bool hasGang = false, hasVector = false, hasWorker = false;
 
   for (const Fortran::parser::AccClause &clause : accClauseList.v) {
     mlir::Location clauseLocation = converter.genLocation(clause.source);
     if (const auto *gangClause =
             std::get_if<Fortran::parser::AccClause::Gang>(&clause.u)) {
       if (gangClause->v) {
-        const Fortran::parser::AccGangArgument &x = *gangClause->v;
-        if (const auto &gangNumValue =
-                std::get<std::optional<Fortran::parser::ScalarIntExpr>>(x.t)) {
+        const Fortran::parser::AccGangArgList &x = *gangClause->v;
+        for (const Fortran::parser::AccGangArg &gangArg : x.v) {
+          if (const auto *num =
+                  std::get_if<Fortran::parser::AccGangArg::Num>(&gangArg.u)) {
             gangNum = fir::getBase(converter.genExprValue(
-              *Fortran::semantics::GetExpr(gangNumValue.value()), stmtCtx));
-        }
-        if (const auto &gangStaticValue =
-                std::get<std::optional<Fortran::parser::AccSizeExpr>>(x.t)) {
-          const auto &expr =
-              std::get<std::optional<Fortran::parser::ScalarIntExpr>>(
-                  gangStaticValue.value().t);
-          if (expr) {
+                *Fortran::semantics::GetExpr(num->v), stmtCtx));
+          } else if (const auto *staticArg =
+                         std::get_if<Fortran::parser::AccGangArg::Static>(
+                             &gangArg.u)) {
+
+            const Fortran::parser::AccSizeExpr &sizeExpr = staticArg->v;
+            if (sizeExpr.v) {
               gangStatic = fir::getBase(converter.genExprValue(
-                *Fortran::semantics::GetExpr(*expr), stmtCtx));
+                  *Fortran::semantics::GetExpr(*sizeExpr.v), stmtCtx));
             } else {
               // * was passed as value and will be represented as a special
               // constant.
               gangStatic = builder.createIntegerConstant(
                   clauseLocation, builder.getIndexType(), starCst);
             }
           }
         }
+      }
       hasGang = true;
     } else if (const auto *workerClause =
                    std::get_if<Fortran::parser::AccClause::Worker>(&clause.u)) {
       if (workerClause->v) {
         workerNum = fir::getBase(converter.genExprValue(
             *Fortran::semantics::GetExpr(*workerClause->v), stmtCtx));
       }
       hasWorker = true;

flang/lib/Parser/openacc-parsers.cpp

 // tile size is one of:
 //   * (represented as an empty std::optional<ScalarIntExpr>)
 //   constant-int-expr
 TYPE_PARSER(construct<AccTileExpr>(scalarIntConstantExpr) ||
     construct<AccTileExpr>(
         "*" >> construct<std::optional<ScalarIntConstantExpr>>()))
 TYPE_PARSER(construct<AccTileExprList>(nonemptyList(Parser<AccTileExpr>{})))
 
-// 2.9 (1607) gang-arg is:
-//   [[num:]int-expr][[,]static:size-expr]
-TYPE_PARSER(construct<AccGangArgument>(
-    maybe(("NUM:"_tok >> scalarIntExpr || scalarIntExpr)),
-    maybe(", STATIC:" >> Parser<AccSizeExpr>{})))
+// 2.9 gang-arg is one of :
+//   [num:]int-expr
+//   dim:int-expr
+//   static:size-expr
+TYPE_PARSER(construct<AccGangArg>(construct<AccGangArg::Static>(
+                "STATIC: " >> Parser<AccSizeExpr>{})) ||
+    construct<AccGangArg>(
+        construct<AccGangArg::Dim>("DIM: " >> scalarIntExpr)) ||
+    construct<AccGangArg>(
+        construct<AccGangArg::Num>(maybe("NUM: "_tok) >> scalarIntExpr)))
+
+// 2.9 gang-arg-list
+TYPE_PARSER(
+    construct<AccGangArgList>(many(maybe(","_tok) >> Parser<AccGangArg>{})))
 
 // 2.9.1 collapse
 TYPE_PARSER(construct<AccCollapseArg>(
     "FORCE:"_tok >> pure(true) || pure(false), scalarIntConstantExpr))
 
 // 2.5.13 Reduction
 // Operator for reduction
 TYPE_PARSER(sourced(construct<AccReductionOperator>(

flang/lib/Parser/unparse.cpp

case llvm::acc::DefaultValue::ACC_Default_none:
       Put("NONE");
       break;
     case llvm::acc::DefaultValue::ACC_Default_present:
       Put("PRESENT");
       break;
     }
   }
   void Unparse(const AccClauseList &x) { Walk(" ", x.v, " "); }
-  void Unparse(const AccGangArgument &x) {
-    Walk("NUM:", std::get<std::optional<ScalarIntExpr>>(x.t));
-    Walk(", STATIC:", std::get<std::optional<AccSizeExpr>>(x.t));
+  void Unparse(const AccGangArgList &x) { Walk(x.v, ","); }
+  void Before(const AccSizeExpr &x) {
+    if (!x.v)
+      Put("*");
+  }
+  void Before(const AccGangArg &x) {
+    common::visit(common::visitors{
+                      [&](const AccGangArg::Num &) { Word("NUM:"); },
+                      [&](const AccGangArg::Dim &) { Word("DIM:"); },
+                      [&](const AccGangArg::Static &) { Word("STATIC:"); },
+                      [](const StatOrErrmsg &) {},
+                  },
+        x.u);
   }
   void Unparse(const AccCollapseArg &x) {
     const auto &force{std::get<bool>(x.t)};
     const auto &collapseValue{std::get<parser::ScalarIntConstantExpr>(x.t)};
     if (force) {
       Put("FORCE:");
     }
     Walk(collapseValue);

flang/lib/Semantics/check-acc-structure.cpp

 CHECK_SIMPLE_CLAUSE(Detach, ACCC_detach)
 CHECK_SIMPLE_CLAUSE(Device, ACCC_device)
 CHECK_SIMPLE_CLAUSE(DeviceNum, ACCC_device_num)
 CHECK_SIMPLE_CLAUSE(Deviceptr, ACCC_deviceptr)
 CHECK_SIMPLE_CLAUSE(DeviceResident, ACCC_device_resident)
 CHECK_SIMPLE_CLAUSE(DeviceType, ACCC_device_type)
 CHECK_SIMPLE_CLAUSE(Finalize, ACCC_finalize)
 CHECK_SIMPLE_CLAUSE(Firstprivate, ACCC_firstprivate)
-CHECK_SIMPLE_CLAUSE(Gang, ACCC_gang)
 CHECK_SIMPLE_CLAUSE(Host, ACCC_host)
 CHECK_SIMPLE_CLAUSE(If, ACCC_if)
 CHECK_SIMPLE_CLAUSE(IfPresent, ACCC_if_present)
 CHECK_SIMPLE_CLAUSE(Independent, ACCC_independent)
 CHECK_SIMPLE_CLAUSE(Link, ACCC_link)
 CHECK_SIMPLE_CLAUSE(NoCreate, ACCC_no_create)
 CHECK_SIMPLE_CLAUSE(Nohost, ACCC_nohost)
 CHECK_SIMPLE_CLAUSE(NumGangs, ACCC_num_gangs)
if (modifier->v != parser::AccDataModifier::Modifier::Zero) {
           parser::ToUpperCaseLetters(
               llvm::acc::getOpenACCClauseName(llvm::acc::Clause::ACCC_copyout)
                   .str()),
           ContextDirectiveAsFortran());
     }
   }
 }
 
+void AccStructureChecker::Enter(const parser::AccClause::Gang &g) {
+  CheckAllowed(llvm::acc::Clause::ACCC_gang);
+
+  if (g.v) {
+    bool hasNum = false;
+    bool hasDim = false;
+    const Fortran::parser::AccGangArgList &x = *g.v;
+    for (const Fortran::parser::AccGangArg &gangArg : x.v) {
+      if (std::get_if<Fortran::parser::AccGangArg::Num>(&gangArg.u))
+        hasNum = true;
+      else if (std::get_if<Fortran::parser::AccGangArg::Dim>(&gangArg.u))
+        hasDim = true;
+    }
+
+    if (hasDim && hasNum)
+      context_.Say(GetContext().clauseSource,
+          "The num argument is not allowed when dim is specified"_err_en_US);
+  }
+}
+
 void AccStructureChecker::Enter(const parser::AccClause::Self &x) {
   CheckAllowed(llvm::acc::Clause::ACCC_self);
   const std::optional<parser::AccSelfClause> &accSelfClause = x.v;
   if (GetContext().directive == llvm::acc::Directive::ACCD_update &&
       ((accSelfClause &&
            std::holds_alternative<std::optional<parser::ScalarLogicalExpr>>(
                (*accSelfClause).u)) ||
           !accSelfClause)) {

flang/test/Parser/acc-unparse.f90

 end program
 
 !CHECK-LABEL: PROGRAM bug47659
 !CHECK: !$ACC PARALLEL LOOP
 
 
 subroutine acc_loop()
   integer :: i, j
+  real :: a(10)
+  integer :: gangNum, gangDim, gangStatic
+
+!CHECK-LABEL: SUBROUTINE acc_loop
 
   !$acc loop collapse(force: 2)
   do i = 1, 10
     do j = 1, 10
     end do
   end do
-end subroutine
-
-!CHECK-LABEL: SUBROUTINE acc_loop
 !CHECK: !$ACC LOOP COLLAPSE(FORCE:2_4)
+
+  !$acc loop gang
+  do i = 1, 10
+    a(i) = i
+  end do
+! CHECK: !$ACC LOOP GANG
+
+  !$acc loop gang(gangNum)
+  do i = 1, 10
+    a(i) = i
+  end do
+! CHECK: !$ACC LOOP GANG(NUM:gangnum)
+
+  !$acc loop gang(num: gangNum)
+  do i = 1, 10
+    a(i) = i
+  end do
+! CHECK: !$ACC LOOP GANG(NUM:gangnum)
+
+  !$acc loop gang(dim: gangDim)
+  do i = 1, 10
+    a(i) = i
+  end do
+! CHECK: !$ACC LOOP GANG(DIM:gangdim)
+
+  !$acc loop gang(static:gangStatic)
+  do i = 1, 10
+    a(i) = i
+  end do
+! CHECK: !$ACC LOOP GANG(STATIC:gangstatic)
+
+  !$acc loop gang(static:*)
+  do i = 1, 10
+    a(i) = i
+  end do
+! CHECK: !$ACC LOOP GANG(STATIC:*)
+
+  !$acc loop gang(static:gangStatic, dim: gangDim)
+  do i = 1, 10
+    a(i) = i
+  end do
+! CHECK: !$ACC LOOP GANG(STATIC:gangstatic,DIM:gangdim)
+
+end subroutine

flang/test/Semantics/OpenACC/acc-loop.f90

program openacc_loop_validity
 
   !$acc loop collapse(2)
   do i = 1, N
     !ERROR: Loop control is not present in the DO LOOP
     do
       a(i) = 3.14
     end do
   end do
+
+  !ERROR: The num argument is not allowed when dim is specified
+  !$acc loop gang(1, dim: 2)
+  do i = 1, N
+  end do
+
 end program openacc_loop_validity

llvm/include/llvm/Frontend/OpenACC/ACC.td

 
 // 2.5.13
 def ACCC_FirstPrivate : Clause<"firstprivate"> {
   let flangClass = "AccObjectList";
 }
 
 // 2.9.2
 def ACCC_Gang : Clause<"gang"> {
-  let flangClass = "AccGangArgument";
+  let flangClass = "AccGangArgList";
   let isValueOptional = true;
 }
 
 // 2.14.4
 def ACCC_Host : Clause<"host"> {
   let flangClass = "AccObjectList";
 }