[flang][openacc] Enforce no modifier on enter data and exit data clauses

[clementval]

Summary

Enter data can have the copyin clause and exit data can have the copyout clause.
Both clauses support modifier with other directive but for these two directives no modifier
are supported. This semantic check enforce this rule.

[kiranktp]

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

128

Possible to rephrase this?
"Modifier is not allowed for the %s clause on the %s directive"

[clementval]

Address review comment

[clementval]

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

128

Thanks for catching this. Patch has been updated.

[kiranktp]

flang/test/Semantics/acc-clause-validity.f90

48–53

I think you forgot to modify the error message here.

[clementval]

Fix error message

[clementval]

flang/test/Semantics/acc-clause-validity.f90

48–53

My bad! Tired to go to fast and of course I missed it :-)

[kiranktp]

LGTM

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

#include "llvm/Frontend/OpenACC/ACC.cpp.inc"
   void Enter(const parser::AccClause::Write &);
 
 private:
 
   void CheckNoBranching(const parser::Block &block,
       const llvm::acc::Directive directive,
       const parser::CharBlock &directiveSource) const;
 
+  bool CheckAllowedModifier(llvm::acc::Clause clause);
+
   llvm::StringRef getClauseName(llvm::acc::Clause clause) override;
   llvm::StringRef getDirectiveName(llvm::acc::Directive directive) override;
 };
 
 } // namespace Fortran::semantics
 
 #endif // FORTRAN_SEMANTICS_CHECK_ACC_STRUCTURE_H_

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

private:
   }
 
   SemanticsContext &context_;
   parser::CharBlock currentStatementSourcePosition_;
   parser::CharBlock sourcePosition_;
   llvm::acc::Directive currentDirective_;
 };
 
+bool AccStructureChecker::CheckAllowedModifier(llvm::acc::Clause clause) {
+  if (GetContext().directive == llvm::acc::ACCD_enter_data ||
+      GetContext().directive == llvm::acc::ACCD_exit_data) {
+    context_.Say(GetContext().clauseSource,
+        "Modifier is not allowed on the %s clause "
+        "on the %s directive"_err_en_US,
+        parser::ToUpperCaseLetters(getClauseName(clause).str()),
+        ContextDirectiveAsFortran());
+    return true;
+  }
+  return false;
+}
+
 void AccStructureChecker::Enter(const parser::AccClause &x) {
   SetContextClause(x);
 }
 
 void AccStructureChecker::Leave(const parser::AccClauseList &) {}
 
 void AccStructureChecker::Enter(const parser::OpenACCBlockConstruct &x) {
   const auto &beginBlockDir{std::get<parser::AccBeginBlockDirective>(x.t)};
void AccStructureChecker::Enter(const parser::AccClause::Create &c) {
   }
 }
 
 void AccStructureChecker::Enter(const parser::AccClause::Copyin &c) {
   CheckAllowed(llvm::acc::Clause::ACCC_copyin);
   const auto &modifierClause{c.v};
   if (const auto &modifier{
           std::get<std::optional<parser::AccDataModifier>>(modifierClause.t)}) {
+    if (CheckAllowedModifier(llvm::acc::Clause::ACCC_copyin))
+      return;
     if (modifier->v != parser::AccDataModifier::Modifier::ReadOnly) {
       context_.Say(GetContext().clauseSource,
           "Only the READONLY modifier is allowed for the %s clause "
           "on the %s directive"_err_en_US,
           parser::ToUpperCaseLetters(
               llvm::acc::getOpenACCClauseName(llvm::acc::Clause::ACCC_copyin)
                   .str()),
           ContextDirectiveAsFortran());
     }
   }
 }
 
 void AccStructureChecker::Enter(const parser::AccClause::Copyout &c) {
   CheckAllowed(llvm::acc::Clause::ACCC_copyout);
   const auto &modifierClause{c.v};
   if (const auto &modifier{
           std::get<std::optional<parser::AccDataModifier>>(modifierClause.t)}) {
+    if (CheckAllowedModifier(llvm::acc::Clause::ACCC_copyout))
+      return;
     if (modifier->v != parser::AccDataModifier::Modifier::Zero) {
       context_.Say(GetContext().clauseSource,
           "Only the ZERO modifier is allowed for the %s clause "
           "on the %s directive"_err_en_US,
           parser::ToUpperCaseLetters(
               llvm::acc::getOpenACCClauseName(llvm::acc::Clause::ACCC_copyout)
                   .str()),
           ContextDirectiveAsFortran());

flang/test/Semantics/acc-clause-validity.f90

program openacc_clause_validity
   !$acc init device_num(i)
   !$acc init device_type(i)
   !$acc init device_type(2, i, j)
   !$acc init device_num(i) device_type(i, j) if(ifCondition)
 
   !ERROR: At least one of ATTACH, COPYIN, CREATE clause must appear on the ENTER DATA directive
   !$acc enter data
 
-  !ERROR: Only the READONLY modifier is allowed for the COPYIN clause on the ENTER DATA directive
+  !ERROR: Modifier is not allowed on for the COPYIN clause on the ENTER DATA directive
   !$acc enter data copyin(zero: i)
 
+  !ERROR: Modifier is not allowed on for the COPYOUT clause on the EXIT DATA directive
+  !$acc exit data copyout(zero: i)
+
   !ERROR: Only the ZERO modifier is allowed for the CREATE clause on the ENTER DATA directive
   !$acc enter data create(readonly: i)
 
   !ERROR: Only the ZERO modifier is allowed for the COPYOUT clause on the DATA directive
   !$acc data copyout(readonly: i)
   !$acc end data
 
   !ERROR: COPYOUT clause is not allowed on the ENTER DATA directive