[flang][openacc] Add clause validity tests for the update directive

Add couple of clause validity tests for the update directive and check for
the restriction where at least self, host or device clause must appear on the directive.

Reviewed By: sameeranjoshi

Differential Revision: https://reviews.llvm.org/D92447

diff --git a/flang/include/flang/Parser/dump-parse-tree.h b/flang/include/flang/Parser/dump-parse-tree.h
index 61c5bdd..c86c2ec 100644 (file)
--- a/flang/include/flang/Parser/dump-parse-tree.h
+++ b/flang/include/flang/Parser/dump-parse-tree.h
@@ -85,6 +85,7 @@ public:
   NODE_ENUM(parser::AccReductionOperator, Operator)
   NODE(parser, AccSizeExpr)
   NODE(parser, AccSizeExprList)
+  NODE(parser, AccSelfClause)
   NODE(parser, AccStandaloneDirective)
   NODE(parser, AccTileExpr)
   NODE(parser, AccTileExprList)

diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index 2dcb92b..c990ddd 100644 (file)
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -3873,6 +3873,12 @@ 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;

diff --git a/flang/lib/Lower/OpenACC.cpp b/flang/lib/Lower/OpenACC.cpp
index f9a60ca..6f45bb6 100644 (file)
--- a/flang/lib/Lower/OpenACC.cpp
+++ b/flang/lib/Lower/OpenACC.cpp
@@ -335,13 +335,18 @@ genACCParallelOp(Fortran::lower::AbstractConverter &converter,
                                           firOpBuilder.getI1Type(), cond);
     } else if (const auto *selfClause =
                    std::get_if<Fortran::parser::AccClause::Self>(&clause.u)) {
-      if (selfClause->v) {
-        Value cond = fir::getBase(converter.genExprValue(
-            *Fortran::semantics::GetExpr(*(selfClause->v))));
-        selfCond = firOpBuilder.createConvert(currentLocation,
-                                              firOpBuilder.getI1Type(), cond);
-      } else {
-        addSelfAttr = true;
+      const Fortran::parser::AccSelfClause &accSelfClause = selfClause->v;
+      if (const auto *optCondition =
+              std::get_if<std::optional<Fortran::parser::ScalarLogicalExpr>>(
+                  &accSelfClause.u)) {
+        if (*optCondition) {
+          Value cond = fir::getBase(converter.genExprValue(
+              *Fortran::semantics::GetExpr(*optCondition)));
+          selfCond = firOpBuilder.createConvert(currentLocation,
+                                                firOpBuilder.getI1Type(), cond);
+        } else {
+          addSelfAttr = true;
+        }
       }
     } else if (const auto *copyClause =
                    std::get_if<Fortran::parser::AccClause::Copy>(&clause.u)) {

diff --git a/flang/lib/Parser/openacc-parsers.cpp b/flang/lib/Parser/openacc-parsers.cpp
index 510b802..bba886f 100644 (file)
--- a/flang/lib/Parser/openacc-parsers.cpp
+++ b/flang/lib/Parser/openacc-parsers.cpp
@@ -98,13 +98,8 @@ TYPE_PARSER("AUTO" >> construct<AccClause>(construct<AccClause::Auto>()) ||
                        parenthesized(construct<AccObjectListWithReduction>(
                            Parser<AccReductionOperator>{} / ":",
                            Parser<AccObjectList>{})))) ||
-    // 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.
-    "SELF" >> construct<AccClause>(construct<AccClause::Self>(
-                  maybe(parenthesized(scalarLogicalExpr)))) ||
-    construct<AccClause>(
-        construct<AccClause::Host>(parenthesized(Parser<AccObjectList>{}))) ||
+    "SELF" >> construct<AccClause>(
+                  construct<AccClause::Self>(Parser<AccSelfClause>{})) ||
     "SEQ" >> construct<AccClause>(construct<AccClause::Seq>()) ||
     "TILE" >> construct<AccClause>(construct<AccClause::Tile>(
                   parenthesized(Parser<AccTileExprList>{}))) ||
@@ -176,6 +171,12 @@ TYPE_PARSER(construct<AccDefaultClause>(
     parenthesized(first("NONE" >> pure(AccDefaultClause::Arg::None),
         "PRESENT" >> pure(AccDefaultClause::Arg::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))))
+
 // Modifier for copyin, copyout, cache and create
 TYPE_PARSER(construct<AccDataModifier>(
     first("ZERO:" >> pure(AccDataModifier::Modifier::Zero),

diff --git a/flang/lib/Semantics/check-acc-structure.cpp b/flang/lib/Semantics/check-acc-structure.cpp
index edcb41e..d6be718 100644 (file)
--- a/flang/lib/Semantics/check-acc-structure.cpp
+++ b/flang/lib/Semantics/check-acc-structure.cpp
@@ -201,6 +201,8 @@ void AccStructureChecker::Leave(const parser::OpenACCStandaloneConstruct &x) {
     CheckRequireAtLeastOneOf();
     break;
   case llvm::acc::Directive::ACCD_update:
+    // Restriction - line 2636
+    CheckRequireAtLeastOneOf();
     // Restriction - 2301
     CheckOnlyAllowedAfter(llvm::acc::Clause::ACCC_device_type,
         updateOnlyAllowedAfterDeviceTypeClauses);
@@ -281,7 +283,6 @@ CHECK_SIMPLE_CLAUSE(Present, ACCC_present)
 CHECK_SIMPLE_CLAUSE(Private, ACCC_private)
 CHECK_SIMPLE_CLAUSE(Read, ACCC_read)
 CHECK_SIMPLE_CLAUSE(Reduction, ACCC_reduction)
-CHECK_SIMPLE_CLAUSE(Self, ACCC_self)
 CHECK_SIMPLE_CLAUSE(Seq, ACCC_seq)
 CHECK_SIMPLE_CLAUSE(Tile, ACCC_tile)
 CHECK_SIMPLE_CLAUSE(UseDevice, ACCC_use_device)
@@ -346,6 +347,28 @@ void AccStructureChecker::Enter(const parser::AccClause::Copyout &c) {
   }
 }
 
+void AccStructureChecker::Enter(const parser::AccClause::Self &x) {
+  CheckAllowed(llvm::acc::Clause::ACCC_self);
+  const parser::AccSelfClause &accSelfClause = x.v;
+  if (GetContext().directive == llvm::acc::Directive::ACCD_update &&
+      std::holds_alternative<std::optional<parser::ScalarLogicalExpr>>(
+          accSelfClause.u)) {
+    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)) {
+    const auto &accObjectList =
+        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::StringRef AccStructureChecker::getClauseName(llvm::acc::Clause clause) {
   return llvm::acc::getOpenACCClauseName(clause);
 }

diff --git a/flang/test/Semantics/acc-clause-validity.f90 b/flang/test/Semantics/acc-clause-validity.f90
index 717b5cd..22b3328 100644 (file)
--- a/flang/test/Semantics/acc-clause-validity.f90
+++ b/flang/test/Semantics/acc-clause-validity.f90
@@ -173,9 +173,40 @@ program openacc_clause_validity
   !ERROR: Unmatched PARALLEL directive
   !$acc end parallel
 
+  !ERROR: At least one of DEVICE, HOST, SELF clause must appear on the UPDATE directive
+  !$acc update
+
   !$acc update self(a, f) host(g) device(h)
 
-  !$acc update device(i) device_type(*) async
+  !$acc update host(aa) async(1)
+
+  !$acc update device(bb) async(async1)
+
+  !ERROR: At most one ASYNC clause can appear on the UPDATE directive
+  !$acc update host(aa, bb) async(1) async(2)
+
+  !$acc update self(bb, cc(:)) wait(1)
+
+  !ERROR: SELF clause on the UPDATE directive must have a var-list
+  !$acc update self
+
+  !$acc update device(aa, bb, cc) wait(wait1)
+
+  !$acc update host(aa) host(bb) device(cc) wait(1,2)
+
+  !$acc update device(aa, cc) wait(wait1, wait2)
+
+  !$acc update device(aa) device_type(*) async
+
+  !$acc update host(bb) device_type(*) wait
+
+  !$acc update self(cc) device_type(1,2) async device_type(3) wait
+
+  !ERROR: At most one IF clause can appear on the UPDATE directive
+  !$acc update device(aa) if(.true.) if(ifCondition)
+
+  !ERROR: At most one IF_PRESENT clause can appear on the UPDATE directive
+  !$acc update device(bb) if_present if_present
 
   !ERROR: Clause IF is not allowed after clause DEVICE_TYPE on the UPDATE directive
   !$acc update device(i) device_type(*) if(.TRUE.)
@@ -205,6 +236,12 @@ program openacc_clause_validity
     a(i) = 3.14
   end do
 
+  !ERROR: SELF clause on the PARALLEL LOOP directive only accepts optional scalar logical expression
+  !$acc parallel loop self(bb, cc(:))
+  do i = 1, N
+    a(i) = 3.14
+  end do
+
   !$acc parallel loop self(.true.)
   do i = 1, N
     a(i) = 3.14

diff --git a/llvm/include/llvm/Frontend/OpenACC/ACC.td b/llvm/include/llvm/Frontend/OpenACC/ACC.td
index 206f82a..4c1d632 100644 (file)
--- a/llvm/include/llvm/Frontend/OpenACC/ACC.td
+++ b/llvm/include/llvm/Frontend/OpenACC/ACC.td
@@ -210,8 +210,7 @@ def ACCC_Reduction : Clause<"reduction"> {
 
 // 2.5.5
 def ACCC_Self : Clause<"self"> {
-  let flangClassValue = "ScalarLogicalExpr";
-  let isValueOptional = true;
+  let flangClassValue = "AccSelfClause";
 }
 
 // 2.9.5