[flang][openacc][NFC] Make self clause value optional in ACC.td and extract the parser

[clementval]

Summary

Set the isOptional flag for the self clause. Move the optional and parenthesis part of the parser. Update the rest of the code to deal with the optional value.

Preparatory work for D106968.

[clementval]

Rebase

[kiranchandramohan]

flang/lib/Lower/OpenACC.cpp

439

I guess you mean if it is not a logical expression or an object list it is an empty self clause and should be represented by the selfAttr. If so, I think this is not nested correctly.

[clementval]

flang/lib/Lower/OpenACC.cpp

439

The parser changed a bit so this is normally correct. If the clause is present without value then it's the simple self. There are couple of test for this in acc-parallel.f90 and acc-parallel-loop.f90.

[clementval]

flang/lib/Lower/OpenACC.cpp

439

The change is mainly due because of the isOptional added in ACC.td

[kiranchandramohan]

LGTM.

flang/lib/Lower/OpenACC.cpp

439

Ahh OK. I missed the AccClause::Self that is outside and represents the presence of the self clause.

flang/lib/Lower/OpenACC.cpp

if (const auto *asyncClause =
                        &clause.u)) {
       vectorLength = fir::getBase(converter.genExprValue(
           *Fortran::semantics::GetExpr(vectorLengthClause->v), stmtCtx));
     } else if (const auto *ifClause =
                    std::get_if<Fortran::parser::AccClause::If>(&clause.u)) {
       genIfClause(converter, ifClause, ifCond, stmtCtx);
     } else if (const auto *selfClause =
                    std::get_if<Fortran::parser::AccClause::Self>(&clause.u)) {
-      const Fortran::parser::AccSelfClause &accSelfClause = selfClause->v;
+      const std::optional<Fortran::parser::AccSelfClause> &accSelfClause =
+          selfClause->v;
+      if (accSelfClause) {
         if (const auto *optCondition =
                 std::get_if<std::optional<Fortran::parser::ScalarLogicalExpr>>(
-                  &accSelfClause.u)) {
+                    &(*accSelfClause).u)) {
           if (*optCondition) {
             mlir::Value cond = fir::getBase(converter.genExprValue(
                 *Fortran::semantics::GetExpr(*optCondition), stmtCtx));
-          selfCond = firOpBuilder.createConvert(currentLocation,
-                                                firOpBuilder.getI1Type(), cond);
-        } else {
-          addSelfAttr = true;
+            selfCond = firOpBuilder.createConvert(
+                currentLocation, firOpBuilder.getI1Type(), cond);
           }
         } else if (const auto *accClauseList =
                        std::get_if<Fortran::parser::AccObjectList>(
-                         &accSelfClause.u)) {
+                           &(*accSelfClause).u)) {
           // TODO This would be nicer to be done in canonicalization step.
           if (accClauseList->v.size() == 1) {
             const auto &accObject = accClauseList->v.front();
             if (const auto *designator =
                     std::get_if<Fortran::parser::Designator>(&accObject.u)) {
               if (const auto *name = getDesignatorNameIfDataRef(*designator)) {
                 auto cond = converter.getSymbolAddress(*name->symbol);
                 selfCond = firOpBuilder.createConvert(
                     currentLocation, firOpBuilder.getI1Type(), cond);
               }
             }
           }
         }
+      } else {
+        addSelfAttr = true;
+      }
     } else if (const auto *copyClause =
                    std::get_if<Fortran::parser::AccClause::Copy>(&clause.u)) {
       genObjectList(copyClause->v, converter, copyOperands);
     } else if (const auto *copyinClause =
                    std::get_if<Fortran::parser::AccClause::Copyin>(&clause.u)) {
       genObjectListWithModifier<Fortran::parser::AccClause::Copyin>(
           copyinClause, converter,
           Fortran::parser::AccDataModifier::Modifier::ReadOnly,

flang/lib/Parser/openacc-parsers.cpp

"PRESENT" >> construct<AccClause>(construct<AccClause::Present>(
                      parenthesized(Parser<AccObjectList>{}))) ||
     "PRIVATE" >> construct<AccClause>(construct<AccClause::Private>(
                      parenthesized(Parser<AccObjectList>{}))) ||
     "READ" >> construct<AccClause>(construct<AccClause::Read>()) ||
     "REDUCTION" >> construct<AccClause>(construct<AccClause::Reduction>(
                        parenthesized(construct<AccObjectListWithReduction>(
                            Parser<AccReductionOperator>{} / ":",
                            Parser<AccObjectList>{})))) ||
-    "SELF" >> construct<AccClause>(
-                  construct<AccClause::Self>(Parser<AccSelfClause>{})) ||
+    "SELF" >> construct<AccClause>(construct<AccClause::Self>(
+                  maybe(parenthesized(Parser<AccSelfClause>{})))) ||
     "SEQ" >> construct<AccClause>(construct<AccClause::Seq>()) ||
     "TILE" >> construct<AccClause>(construct<AccClause::Tile>(
                   parenthesized(Parser<AccTileExprList>{}))) ||
     "USE_DEVICE" >> construct<AccClause>(construct<AccClause::UseDevice>(
                         parenthesized(Parser<AccObjectList>{}))) ||
     "VECTOR_LENGTH" >> construct<AccClause>(construct<AccClause::VectorLength>(
                            parenthesized(scalarIntExpr))) ||
     "VECTOR" >>

 // 2.5.14 Default clause
 TYPE_PARSER(construct<AccDefaultClause>(parenthesized(
     first("NONE" >> pure(llvm::acc::DefaultValue::ACC_Default_none),
         "PRESENT" >> pure(llvm::acc::DefaultValue::ACC_Default_present)))))
 
 // SELF clause is either a simple optional condition for compute construct
 // or a synonym of the HOST clause for the update directive 2.14.4 holding
 // an object list.
-TYPE_PARSER(construct<AccSelfClause>(parenthesized(Parser<AccObjectList>{})) ||
-    construct<AccSelfClause>(maybe(parenthesized(scalarLogicalExpr))))
+TYPE_PARSER(construct<AccSelfClause>(Parser<AccObjectList>{}) ||
+    construct<AccSelfClause>(scalarLogicalExpr))
 
 // Modifier for copyin, copyout, cache and create
 TYPE_PARSER(construct<AccDataModifier>(
     first("ZERO:" >> pure(AccDataModifier::Modifier::Zero),
         "READONLY:" >> pure(AccDataModifier::Modifier::ReadOnly))))
 
 // Combined directives
 TYPE_PARSER(sourced(construct<AccCombinedDirective>(

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

if (modifier->v != parser::AccDataModifier::Modifier::Zero) {
                   .str()),
           ContextDirectiveAsFortran());
     }
   }
 }
 
 void AccStructureChecker::Enter(const parser::AccClause::Self &x) {
   CheckAllowed(llvm::acc::Clause::ACCC_self);
-  const parser::AccSelfClause &accSelfClause = x.v;
+  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).u)) ||
+          !accSelfClause)) {
     context_.Say(GetContext().clauseSource,
         "SELF clause on the %s directive must have a var-list"_err_en_US,
         ContextDirectiveAsFortran());
   } else if (GetContext().directive != llvm::acc::Directive::ACCD_update &&
-      std::holds_alternative<parser::AccObjectList>(accSelfClause.u)) {
+      accSelfClause &&
+      std::holds_alternative<parser::AccObjectList>((*accSelfClause).u)) {
     const auto &accObjectList =
-        std::get<parser::AccObjectList>(accSelfClause.u);
+        std::get<parser::AccObjectList>((*accSelfClause).u);
     if (accObjectList.v.size() != 1) {
       context_.Say(GetContext().clauseSource,
           "SELF clause on the %s directive only accepts optional scalar logical"
           " expression"_err_en_US,
           ContextDirectiveAsFortran());
     }
   }
 }

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

 // 2.5.14
 def ACCC_Reduction : Clause<"reduction"> {
   let flangClass = "AccObjectListWithReduction";
 }
 
 // 2.5.6
 def ACCC_Self : Clause<"self"> {
   let flangClass = "AccSelfClause";
+  let isValueOptional = true;
 }
 
 // 2.9.5
 def ACCC_Seq : Clause<"seq"> {}
 
 // 2.9.4
 def ACCC_Vector : Clause<"vector"> {
   let flangClass = "ScalarIntExpr";