Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[flang][openacc] Apply mutually exclusive clauses restriction to routine #77802

Merged
merged 1 commit into from Jan 11, 2024

Conversation

clementval
Copy link
Contributor

this patch enforce or fix the enforcement of two restrictions from section 2.15.1:

Only the gang, worker, vector, seq and bind clauses may follow a device_type clause.

seq was not allowed after device_type with the current implementation.

Exactly one of the gang, worker, vector, or seq clauses must appear.

This was not properly checked. This patch check correctly for mutually exclusion as described in section 2.4. Mutually exclusive clauses may appear on the same directive if they apply for different device_type.

@llvmbot llvmbot added flang Flang issues not falling into any other category openacc flang:semantics labels Jan 11, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Jan 11, 2024

@llvm/pr-subscribers-openacc

Author: Valentin Clement (バレンタイン クレメン) (clementval)

Changes

this patch enforce or fix the enforcement of two restrictions from section 2.15.1:

> Only the gang, worker, vector, seq and bind clauses may follow a device_type clause.

seq was not allowed after device_type with the current implementation.

> Exactly one of the gang, worker, vector, or seq clauses must appear.

This was not properly checked. This patch check correctly for mutually exclusion as described in section 2.4. Mutually exclusive clauses may appear on the same directive if they apply for different device_type.


Full diff: https://github.com/llvm/llvm-project/pull/77802.diff

3 Files Affected:

  • (modified) flang/lib/Semantics/check-acc-structure.cpp (+48-14)
  • (modified) flang/lib/Semantics/check-directive-structure.h (+42)
  • (modified) flang/test/Semantics/OpenACC/acc-routine.f90 (+111)
diff --git a/flang/lib/Semantics/check-acc-structure.cpp b/flang/lib/Semantics/check-acc-structure.cpp
index 4a5798a8a531a4..d69e1c597e2fbf 100644
--- a/flang/lib/Semantics/check-acc-structure.cpp
+++ b/flang/lib/Semantics/check-acc-structure.cpp
@@ -69,7 +69,12 @@ static constexpr inline AccClauseSet updateOnlyAllowedAfterDeviceTypeClauses{
 
 static constexpr inline AccClauseSet routineOnlyAllowedAfterDeviceTypeClauses{
     llvm::acc::Clause::ACCC_bind, llvm::acc::Clause::ACCC_gang,
-    llvm::acc::Clause::ACCC_vector, llvm::acc::Clause::ACCC_worker};
+    llvm::acc::Clause::ACCC_vector, llvm::acc::Clause::ACCC_worker,
+    llvm::acc::Clause::ACCC_seq};
+
+static constexpr inline AccClauseSet routineMutuallyExclusiveClauses{
+    llvm::acc::Clause::ACCC_gang, llvm::acc::Clause::ACCC_worker,
+    llvm::acc::Clause::ACCC_vector, llvm::acc::Clause::ACCC_seq};
 
 bool AccStructureChecker::CheckAllowedModifier(llvm::acc::Clause clause) {
   if (GetContext().directive == llvm::acc::ACCD_enter_data ||
@@ -388,7 +393,6 @@ CHECK_SIMPLE_CLAUSE(NoCreate, ACCC_no_create)
 CHECK_SIMPLE_CLAUSE(Nohost, ACCC_nohost)
 CHECK_SIMPLE_CLAUSE(Private, ACCC_private)
 CHECK_SIMPLE_CLAUSE(Read, ACCC_read)
-CHECK_SIMPLE_CLAUSE(Seq, ACCC_seq)
 CHECK_SIMPLE_CLAUSE(UseDevice, ACCC_use_device)
 CHECK_SIMPLE_CLAUSE(Wait, ACCC_wait)
 CHECK_SIMPLE_CLAUSE(Write, ACCC_write)
@@ -532,18 +536,40 @@ void AccStructureChecker::Enter(const parser::AccClause::DeviceType &d) {
                 .str()),
         ContextDirectiveAsFortran());
   }
+  ResetCrtGroup();
+}
+
+void AccStructureChecker::Enter(const parser::AccClause::Seq &g) {
+  llvm::acc::Clause crtClause = llvm::acc::Clause::ACCC_seq;
+  if (GetContext().directive == llvm::acc::Directive::ACCD_routine) {
+    CheckMutuallyExclusivePerGroup(crtClause,
+        llvm::acc::Clause::ACCC_device_type, routineMutuallyExclusiveClauses);
+  }
+  CheckAllowed(crtClause);
 }
 
 void AccStructureChecker::Enter(const parser::AccClause::Vector &g) {
-  CheckAllowed(llvm::acc::Clause::ACCC_vector);
-  CheckAllowedOncePerGroup(
-      llvm::acc::Clause::ACCC_vector, llvm::acc::Clause::ACCC_device_type);
+  llvm::acc::Clause crtClause = llvm::acc::Clause::ACCC_vector;
+  if (GetContext().directive == llvm::acc::Directive::ACCD_routine) {
+    CheckMutuallyExclusivePerGroup(crtClause,
+        llvm::acc::Clause::ACCC_device_type, routineMutuallyExclusiveClauses);
+  }
+  CheckAllowed(crtClause);
+  if (GetContext().directive != llvm::acc::Directive::ACCD_routine) {
+    CheckAllowedOncePerGroup(crtClause, llvm::acc::Clause::ACCC_device_type);
+  }
 }
 
 void AccStructureChecker::Enter(const parser::AccClause::Worker &g) {
-  CheckAllowed(llvm::acc::Clause::ACCC_worker);
-  CheckAllowedOncePerGroup(
-      llvm::acc::Clause::ACCC_worker, llvm::acc::Clause::ACCC_device_type);
+  llvm::acc::Clause crtClause = llvm::acc::Clause::ACCC_worker;
+  if (GetContext().directive == llvm::acc::Directive::ACCD_routine) {
+    CheckMutuallyExclusivePerGroup(crtClause,
+        llvm::acc::Clause::ACCC_device_type, routineMutuallyExclusiveClauses);
+  }
+  CheckAllowed(crtClause);
+  if (GetContext().directive != llvm::acc::Directive::ACCD_routine) {
+    CheckAllowedOncePerGroup(crtClause, llvm::acc::Clause::ACCC_device_type);
+  }
 }
 
 void AccStructureChecker::Enter(const parser::AccClause::Tile &g) {
@@ -553,24 +579,32 @@ void AccStructureChecker::Enter(const parser::AccClause::Tile &g) {
 }
 
 void AccStructureChecker::Enter(const parser::AccClause::Gang &g) {
-  CheckAllowed(llvm::acc::Clause::ACCC_gang);
-  CheckAllowedOncePerGroup(
-      llvm::acc::Clause::ACCC_gang, llvm::acc::Clause::ACCC_device_type);
+  llvm::acc::Clause crtClause = llvm::acc::Clause::ACCC_gang;
+  if (GetContext().directive == llvm::acc::Directive::ACCD_routine) {
+    CheckMutuallyExclusivePerGroup(crtClause,
+        llvm::acc::Clause::ACCC_device_type, routineMutuallyExclusiveClauses);
+  }
+  CheckAllowed(crtClause);
+  if (GetContext().directive != llvm::acc::Directive::ACCD_routine) {
+    CheckAllowedOncePerGroup(crtClause, llvm::acc::Clause::ACCC_device_type);
+  }
 
   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))
+      if (std::get_if<Fortran::parser::AccGangArg::Num>(&gangArg.u)) {
         hasNum = true;
-      else if (std::get_if<Fortran::parser::AccGangArg::Dim>(&gangArg.u))
+      } else if (std::get_if<Fortran::parser::AccGangArg::Dim>(&gangArg.u)) {
         hasDim = true;
+      }
     }
 
-    if (hasDim && hasNum)
+    if (hasDim && hasNum) {
       context_.Say(GetContext().clauseSource,
           "The num argument is not allowed when dim is specified"_err_en_US);
+    }
   }
 }
 
diff --git a/flang/lib/Semantics/check-directive-structure.h b/flang/lib/Semantics/check-directive-structure.h
index f7a6233a32eddd..829405f99d64c0 100644
--- a/flang/lib/Semantics/check-directive-structure.h
+++ b/flang/lib/Semantics/check-directive-structure.h
@@ -197,6 +197,7 @@ class DirectiveStructureChecker : public virtual BaseChecker {
     const PC *clause{nullptr};
     ClauseMapTy clauseInfo;
     std::list<C> actualClauses;
+    std::list<C> crtGroup;
     Symbol *loopIV{nullptr};
   };
 
@@ -261,6 +262,12 @@ class DirectiveStructureChecker : public virtual BaseChecker {
     GetContext().actualClauses.push_back(type);
   }
 
+  void AddClauseToCrtGroupInContext(C type) {
+    GetContext().crtGroup.push_back(type);
+  }
+
+  void ResetCrtGroup() { GetContext().crtGroup.clear(); }
+
   // Check if the given clause is present in the current context
   const PC *FindClause(C type) { return FindClause(GetContext(), type); }
 
@@ -353,6 +360,9 @@ class DirectiveStructureChecker : public virtual BaseChecker {
   // separator clause appears.
   void CheckAllowedOncePerGroup(C clause, C separator);
 
+  void CheckMutuallyExclusivePerGroup(
+      C clause, C separator, common::EnumSet<C, ClauseEnumSize> set);
+
   void CheckAtLeastOneClause();
 
   void CheckNotAllowedIfClause(
@@ -526,6 +536,7 @@ void DirectiveStructureChecker<D, C, PC, ClauseEnumSize>::CheckAllowed(
   }
   SetContextClauseInfo(clause);
   AddClauseToCrtContext(clause);
+  AddClauseToCrtGroupInContext(clause);
 }
 
 // Enforce restriction where clauses in the given set are not allowed if the
@@ -570,6 +581,37 @@ void DirectiveStructureChecker<D, C, PC,
   }
 }
 
+template <typename D, typename C, typename PC, std::size_t ClauseEnumSize>
+void DirectiveStructureChecker<D, C, PC,
+    ClauseEnumSize>::CheckMutuallyExclusivePerGroup(C clause, C separator,
+    common::EnumSet<C, ClauseEnumSize> set) {
+
+  // Checking of there is any offending clauses before the first separator.
+  for (auto cl : GetContext().actualClauses) {
+    if (cl == separator) {
+      break;
+    }
+    if (set.test(cl)) {
+      context_.Say(GetContext().directiveSource,
+          "Clause %s is not allowed if clause %s appears on the %s directive"_err_en_US,
+          parser::ToUpperCaseLetters(getClauseName(clause).str()),
+          parser::ToUpperCaseLetters(getClauseName(cl).str()),
+          ContextDirectiveAsFortran());
+    }
+  }
+
+  // Checking for mutually exclusive clauses in the current group.
+  for (auto cl : GetContext().crtGroup) {
+    if (set.test(cl)) {
+      context_.Say(GetContext().directiveSource,
+          "Clause %s is not allowed if clause %s appears on the %s directive"_err_en_US,
+          parser::ToUpperCaseLetters(getClauseName(clause).str()),
+          parser::ToUpperCaseLetters(getClauseName(cl).str()),
+          ContextDirectiveAsFortran());
+    }
+  }
+}
+
 // Check the value of the clause is a constant positive integer.
 template <typename D, typename C, typename PC, std::size_t ClauseEnumSize>
 void DirectiveStructureChecker<D, C, PC,
diff --git a/flang/test/Semantics/OpenACC/acc-routine.f90 b/flang/test/Semantics/OpenACC/acc-routine.f90
index 4dcb849c642c83..42762f537b8bc7 100644
--- a/flang/test/Semantics/OpenACC/acc-routine.f90
+++ b/flang/test/Semantics/OpenACC/acc-routine.f90
@@ -13,3 +13,114 @@ subroutine sub2(a)
 subroutine sub3()
   !$acc routine bind(sub1)
 end subroutine
+
+subroutine sub6()
+  !ERROR: Clause GANG is not allowed if clause GANG appears on the ROUTINE directive
+  !$acc routine gang gang
+
+  !ERROR: Clause GANG is not allowed if clause WORKER appears on the ROUTINE directive
+  !$acc routine worker gang
+
+  !ERROR: Clause GANG is not allowed if clause VECTOR appears on the ROUTINE directive
+  !$acc routine vector gang
+
+  !ERROR: Clause GANG is not allowed if clause SEQ appears on the ROUTINE directive
+  !$acc routine seq gang
+
+  !ERROR: Clause WORKER is not allowed if clause WORKER appears on the ROUTINE directive
+  !$acc routine worker worker
+
+  !ERROR: Clause WORKER is not allowed if clause GANG appears on the ROUTINE directive
+  !$acc routine gang worker
+
+  !ERROR: Clause WORKER is not allowed if clause VECTOR appears on the ROUTINE directive
+  !$acc routine vector worker
+
+  !ERROR: Clause WORKER is not allowed if clause SEQ appears on the ROUTINE directive
+  !$acc routine seq worker
+
+  !ERROR: Clause VECTOR is not allowed if clause VECTOR appears on the ROUTINE directive
+  !$acc routine vector vector
+
+  !ERROR: Clause VECTOR is not allowed if clause GANG appears on the ROUTINE directive
+  !$acc routine gang vector
+
+  !ERROR: Clause VECTOR is not allowed if clause WORKER appears on the ROUTINE directive
+  !$acc routine worker vector
+
+  !ERROR: Clause VECTOR is not allowed if clause SEQ appears on the ROUTINE directive
+  !$acc routine seq vector
+
+  !ERROR: Clause SEQ is not allowed if clause SEQ appears on the ROUTINE directive
+  !$acc routine seq seq
+
+  !ERROR: Clause SEQ is not allowed if clause GANG appears on the ROUTINE directive
+  !$acc routine gang seq
+
+  !ERROR: Clause SEQ is not allowed if clause WORKER appears on the ROUTINE directive
+  !$acc routine worker seq
+
+  !ERROR: Clause SEQ is not allowed if clause VECTOR appears on the ROUTINE directive
+  !$acc routine vector seq
+
+end subroutine
+
+subroutine sub7()
+  !$acc routine device_type(*) gang device_type(host) worker
+
+  !ERROR: Clause SEQ is not allowed if clause GANG appears on the ROUTINE directive
+  !$acc routine device_type(*) gang seq
+
+  !ERROR: Clause WORKER is not allowed if clause GANG appears on the ROUTINE directive
+  !$acc routine device_type(*) gang worker
+
+  !ERROR: Clause GANG is not allowed if clause GANG appears on the ROUTINE directive
+  !$acc routine gang device_type(*) gang
+
+  !ERROR: Clause WORKER is not allowed if clause GANG appears on the ROUTINE directive
+  !$acc routine gang device_type(*) worker
+
+  !ERROR: Clause VECTOR is not allowed if clause GANG appears on the ROUTINE directive
+  !$acc routine gang device_type(*) vector
+
+  !ERROR: Clause SEQ is not allowed if clause GANG appears on the ROUTINE directive
+  !$acc routine gang device_type(*) seq
+
+  !ERROR: Clause WORKER is not allowed if clause WORKER appears on the ROUTINE directive
+  !$acc routine worker device_type(*) worker
+
+  !ERROR: Clause GANG is not allowed if clause WORKER appears on the ROUTINE directive
+  !$acc routine worker device_type(*) gang
+
+  !ERROR: Clause VECTOR is not allowed if clause WORKER appears on the ROUTINE directive
+  !$acc routine worker device_type(*) vector
+
+  !ERROR: Clause SEQ is not allowed if clause WORKER appears on the ROUTINE directive
+  !$acc routine worker device_type(*) seq
+
+  !ERROR: Clause VECTOR is not allowed if clause VECTOR appears on the ROUTINE directive
+  !$acc routine vector device_type(*) vector
+
+  !ERROR: Clause GANG is not allowed if clause VECTOR appears on the ROUTINE directive
+  !$acc routine vector device_type(*) gang
+
+  !ERROR: Clause VECTOR is not allowed if clause VECTOR appears on the ROUTINE directive
+  !$acc routine vector device_type(*) vector
+
+  !ERROR: Clause SEQ is not allowed if clause VECTOR appears on the ROUTINE directive
+  !$acc routine vector device_type(*) seq
+
+  !ERROR: Clause SEQ is not allowed if clause SEQ appears on the ROUTINE directive
+  !$acc routine seq device_type(*) seq
+
+  !ERROR: Clause GANG is not allowed if clause SEQ appears on the ROUTINE directive
+  !$acc routine seq device_type(*) gang
+
+  !ERROR: Clause VECTOR is not allowed if clause SEQ appears on the ROUTINE directive
+  !$acc routine seq device_type(*) vector
+
+  !ERROR: Clause WORKER is not allowed if clause SEQ appears on the ROUTINE directive
+  !$acc routine seq device_type(*) worker
+
+  !$acc routine device_type(host) seq device_type(nvidia) gang device_type(multicore) vector device_type(*) worker
+end subroutine

@llvmbot
Copy link
Collaborator

llvmbot commented Jan 11, 2024

@llvm/pr-subscribers-flang-semantics

Author: Valentin Clement (バレンタイン クレメン) (clementval)

Changes

this patch enforce or fix the enforcement of two restrictions from section 2.15.1:

> Only the gang, worker, vector, seq and bind clauses may follow a device_type clause.

seq was not allowed after device_type with the current implementation.

> Exactly one of the gang, worker, vector, or seq clauses must appear.

This was not properly checked. This patch check correctly for mutually exclusion as described in section 2.4. Mutually exclusive clauses may appear on the same directive if they apply for different device_type.


Full diff: https://github.com/llvm/llvm-project/pull/77802.diff

3 Files Affected:

  • (modified) flang/lib/Semantics/check-acc-structure.cpp (+48-14)
  • (modified) flang/lib/Semantics/check-directive-structure.h (+42)
  • (modified) flang/test/Semantics/OpenACC/acc-routine.f90 (+111)
diff --git a/flang/lib/Semantics/check-acc-structure.cpp b/flang/lib/Semantics/check-acc-structure.cpp
index 4a5798a8a531a4..d69e1c597e2fbf 100644
--- a/flang/lib/Semantics/check-acc-structure.cpp
+++ b/flang/lib/Semantics/check-acc-structure.cpp
@@ -69,7 +69,12 @@ static constexpr inline AccClauseSet updateOnlyAllowedAfterDeviceTypeClauses{
 
 static constexpr inline AccClauseSet routineOnlyAllowedAfterDeviceTypeClauses{
     llvm::acc::Clause::ACCC_bind, llvm::acc::Clause::ACCC_gang,
-    llvm::acc::Clause::ACCC_vector, llvm::acc::Clause::ACCC_worker};
+    llvm::acc::Clause::ACCC_vector, llvm::acc::Clause::ACCC_worker,
+    llvm::acc::Clause::ACCC_seq};
+
+static constexpr inline AccClauseSet routineMutuallyExclusiveClauses{
+    llvm::acc::Clause::ACCC_gang, llvm::acc::Clause::ACCC_worker,
+    llvm::acc::Clause::ACCC_vector, llvm::acc::Clause::ACCC_seq};
 
 bool AccStructureChecker::CheckAllowedModifier(llvm::acc::Clause clause) {
   if (GetContext().directive == llvm::acc::ACCD_enter_data ||
@@ -388,7 +393,6 @@ CHECK_SIMPLE_CLAUSE(NoCreate, ACCC_no_create)
 CHECK_SIMPLE_CLAUSE(Nohost, ACCC_nohost)
 CHECK_SIMPLE_CLAUSE(Private, ACCC_private)
 CHECK_SIMPLE_CLAUSE(Read, ACCC_read)
-CHECK_SIMPLE_CLAUSE(Seq, ACCC_seq)
 CHECK_SIMPLE_CLAUSE(UseDevice, ACCC_use_device)
 CHECK_SIMPLE_CLAUSE(Wait, ACCC_wait)
 CHECK_SIMPLE_CLAUSE(Write, ACCC_write)
@@ -532,18 +536,40 @@ void AccStructureChecker::Enter(const parser::AccClause::DeviceType &d) {
                 .str()),
         ContextDirectiveAsFortran());
   }
+  ResetCrtGroup();
+}
+
+void AccStructureChecker::Enter(const parser::AccClause::Seq &g) {
+  llvm::acc::Clause crtClause = llvm::acc::Clause::ACCC_seq;
+  if (GetContext().directive == llvm::acc::Directive::ACCD_routine) {
+    CheckMutuallyExclusivePerGroup(crtClause,
+        llvm::acc::Clause::ACCC_device_type, routineMutuallyExclusiveClauses);
+  }
+  CheckAllowed(crtClause);
 }
 
 void AccStructureChecker::Enter(const parser::AccClause::Vector &g) {
-  CheckAllowed(llvm::acc::Clause::ACCC_vector);
-  CheckAllowedOncePerGroup(
-      llvm::acc::Clause::ACCC_vector, llvm::acc::Clause::ACCC_device_type);
+  llvm::acc::Clause crtClause = llvm::acc::Clause::ACCC_vector;
+  if (GetContext().directive == llvm::acc::Directive::ACCD_routine) {
+    CheckMutuallyExclusivePerGroup(crtClause,
+        llvm::acc::Clause::ACCC_device_type, routineMutuallyExclusiveClauses);
+  }
+  CheckAllowed(crtClause);
+  if (GetContext().directive != llvm::acc::Directive::ACCD_routine) {
+    CheckAllowedOncePerGroup(crtClause, llvm::acc::Clause::ACCC_device_type);
+  }
 }
 
 void AccStructureChecker::Enter(const parser::AccClause::Worker &g) {
-  CheckAllowed(llvm::acc::Clause::ACCC_worker);
-  CheckAllowedOncePerGroup(
-      llvm::acc::Clause::ACCC_worker, llvm::acc::Clause::ACCC_device_type);
+  llvm::acc::Clause crtClause = llvm::acc::Clause::ACCC_worker;
+  if (GetContext().directive == llvm::acc::Directive::ACCD_routine) {
+    CheckMutuallyExclusivePerGroup(crtClause,
+        llvm::acc::Clause::ACCC_device_type, routineMutuallyExclusiveClauses);
+  }
+  CheckAllowed(crtClause);
+  if (GetContext().directive != llvm::acc::Directive::ACCD_routine) {
+    CheckAllowedOncePerGroup(crtClause, llvm::acc::Clause::ACCC_device_type);
+  }
 }
 
 void AccStructureChecker::Enter(const parser::AccClause::Tile &g) {
@@ -553,24 +579,32 @@ void AccStructureChecker::Enter(const parser::AccClause::Tile &g) {
 }
 
 void AccStructureChecker::Enter(const parser::AccClause::Gang &g) {
-  CheckAllowed(llvm::acc::Clause::ACCC_gang);
-  CheckAllowedOncePerGroup(
-      llvm::acc::Clause::ACCC_gang, llvm::acc::Clause::ACCC_device_type);
+  llvm::acc::Clause crtClause = llvm::acc::Clause::ACCC_gang;
+  if (GetContext().directive == llvm::acc::Directive::ACCD_routine) {
+    CheckMutuallyExclusivePerGroup(crtClause,
+        llvm::acc::Clause::ACCC_device_type, routineMutuallyExclusiveClauses);
+  }
+  CheckAllowed(crtClause);
+  if (GetContext().directive != llvm::acc::Directive::ACCD_routine) {
+    CheckAllowedOncePerGroup(crtClause, llvm::acc::Clause::ACCC_device_type);
+  }
 
   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))
+      if (std::get_if<Fortran::parser::AccGangArg::Num>(&gangArg.u)) {
         hasNum = true;
-      else if (std::get_if<Fortran::parser::AccGangArg::Dim>(&gangArg.u))
+      } else if (std::get_if<Fortran::parser::AccGangArg::Dim>(&gangArg.u)) {
         hasDim = true;
+      }
     }
 
-    if (hasDim && hasNum)
+    if (hasDim && hasNum) {
       context_.Say(GetContext().clauseSource,
           "The num argument is not allowed when dim is specified"_err_en_US);
+    }
   }
 }
 
diff --git a/flang/lib/Semantics/check-directive-structure.h b/flang/lib/Semantics/check-directive-structure.h
index f7a6233a32eddd..829405f99d64c0 100644
--- a/flang/lib/Semantics/check-directive-structure.h
+++ b/flang/lib/Semantics/check-directive-structure.h
@@ -197,6 +197,7 @@ class DirectiveStructureChecker : public virtual BaseChecker {
     const PC *clause{nullptr};
     ClauseMapTy clauseInfo;
     std::list<C> actualClauses;
+    std::list<C> crtGroup;
     Symbol *loopIV{nullptr};
   };
 
@@ -261,6 +262,12 @@ class DirectiveStructureChecker : public virtual BaseChecker {
     GetContext().actualClauses.push_back(type);
   }
 
+  void AddClauseToCrtGroupInContext(C type) {
+    GetContext().crtGroup.push_back(type);
+  }
+
+  void ResetCrtGroup() { GetContext().crtGroup.clear(); }
+
   // Check if the given clause is present in the current context
   const PC *FindClause(C type) { return FindClause(GetContext(), type); }
 
@@ -353,6 +360,9 @@ class DirectiveStructureChecker : public virtual BaseChecker {
   // separator clause appears.
   void CheckAllowedOncePerGroup(C clause, C separator);
 
+  void CheckMutuallyExclusivePerGroup(
+      C clause, C separator, common::EnumSet<C, ClauseEnumSize> set);
+
   void CheckAtLeastOneClause();
 
   void CheckNotAllowedIfClause(
@@ -526,6 +536,7 @@ void DirectiveStructureChecker<D, C, PC, ClauseEnumSize>::CheckAllowed(
   }
   SetContextClauseInfo(clause);
   AddClauseToCrtContext(clause);
+  AddClauseToCrtGroupInContext(clause);
 }
 
 // Enforce restriction where clauses in the given set are not allowed if the
@@ -570,6 +581,37 @@ void DirectiveStructureChecker<D, C, PC,
   }
 }
 
+template <typename D, typename C, typename PC, std::size_t ClauseEnumSize>
+void DirectiveStructureChecker<D, C, PC,
+    ClauseEnumSize>::CheckMutuallyExclusivePerGroup(C clause, C separator,
+    common::EnumSet<C, ClauseEnumSize> set) {
+
+  // Checking of there is any offending clauses before the first separator.
+  for (auto cl : GetContext().actualClauses) {
+    if (cl == separator) {
+      break;
+    }
+    if (set.test(cl)) {
+      context_.Say(GetContext().directiveSource,
+          "Clause %s is not allowed if clause %s appears on the %s directive"_err_en_US,
+          parser::ToUpperCaseLetters(getClauseName(clause).str()),
+          parser::ToUpperCaseLetters(getClauseName(cl).str()),
+          ContextDirectiveAsFortran());
+    }
+  }
+
+  // Checking for mutually exclusive clauses in the current group.
+  for (auto cl : GetContext().crtGroup) {
+    if (set.test(cl)) {
+      context_.Say(GetContext().directiveSource,
+          "Clause %s is not allowed if clause %s appears on the %s directive"_err_en_US,
+          parser::ToUpperCaseLetters(getClauseName(clause).str()),
+          parser::ToUpperCaseLetters(getClauseName(cl).str()),
+          ContextDirectiveAsFortran());
+    }
+  }
+}
+
 // Check the value of the clause is a constant positive integer.
 template <typename D, typename C, typename PC, std::size_t ClauseEnumSize>
 void DirectiveStructureChecker<D, C, PC,
diff --git a/flang/test/Semantics/OpenACC/acc-routine.f90 b/flang/test/Semantics/OpenACC/acc-routine.f90
index 4dcb849c642c83..42762f537b8bc7 100644
--- a/flang/test/Semantics/OpenACC/acc-routine.f90
+++ b/flang/test/Semantics/OpenACC/acc-routine.f90
@@ -13,3 +13,114 @@ subroutine sub2(a)
 subroutine sub3()
   !$acc routine bind(sub1)
 end subroutine
+
+subroutine sub6()
+  !ERROR: Clause GANG is not allowed if clause GANG appears on the ROUTINE directive
+  !$acc routine gang gang
+
+  !ERROR: Clause GANG is not allowed if clause WORKER appears on the ROUTINE directive
+  !$acc routine worker gang
+
+  !ERROR: Clause GANG is not allowed if clause VECTOR appears on the ROUTINE directive
+  !$acc routine vector gang
+
+  !ERROR: Clause GANG is not allowed if clause SEQ appears on the ROUTINE directive
+  !$acc routine seq gang
+
+  !ERROR: Clause WORKER is not allowed if clause WORKER appears on the ROUTINE directive
+  !$acc routine worker worker
+
+  !ERROR: Clause WORKER is not allowed if clause GANG appears on the ROUTINE directive
+  !$acc routine gang worker
+
+  !ERROR: Clause WORKER is not allowed if clause VECTOR appears on the ROUTINE directive
+  !$acc routine vector worker
+
+  !ERROR: Clause WORKER is not allowed if clause SEQ appears on the ROUTINE directive
+  !$acc routine seq worker
+
+  !ERROR: Clause VECTOR is not allowed if clause VECTOR appears on the ROUTINE directive
+  !$acc routine vector vector
+
+  !ERROR: Clause VECTOR is not allowed if clause GANG appears on the ROUTINE directive
+  !$acc routine gang vector
+
+  !ERROR: Clause VECTOR is not allowed if clause WORKER appears on the ROUTINE directive
+  !$acc routine worker vector
+
+  !ERROR: Clause VECTOR is not allowed if clause SEQ appears on the ROUTINE directive
+  !$acc routine seq vector
+
+  !ERROR: Clause SEQ is not allowed if clause SEQ appears on the ROUTINE directive
+  !$acc routine seq seq
+
+  !ERROR: Clause SEQ is not allowed if clause GANG appears on the ROUTINE directive
+  !$acc routine gang seq
+
+  !ERROR: Clause SEQ is not allowed if clause WORKER appears on the ROUTINE directive
+  !$acc routine worker seq
+
+  !ERROR: Clause SEQ is not allowed if clause VECTOR appears on the ROUTINE directive
+  !$acc routine vector seq
+
+end subroutine
+
+subroutine sub7()
+  !$acc routine device_type(*) gang device_type(host) worker
+
+  !ERROR: Clause SEQ is not allowed if clause GANG appears on the ROUTINE directive
+  !$acc routine device_type(*) gang seq
+
+  !ERROR: Clause WORKER is not allowed if clause GANG appears on the ROUTINE directive
+  !$acc routine device_type(*) gang worker
+
+  !ERROR: Clause GANG is not allowed if clause GANG appears on the ROUTINE directive
+  !$acc routine gang device_type(*) gang
+
+  !ERROR: Clause WORKER is not allowed if clause GANG appears on the ROUTINE directive
+  !$acc routine gang device_type(*) worker
+
+  !ERROR: Clause VECTOR is not allowed if clause GANG appears on the ROUTINE directive
+  !$acc routine gang device_type(*) vector
+
+  !ERROR: Clause SEQ is not allowed if clause GANG appears on the ROUTINE directive
+  !$acc routine gang device_type(*) seq
+
+  !ERROR: Clause WORKER is not allowed if clause WORKER appears on the ROUTINE directive
+  !$acc routine worker device_type(*) worker
+
+  !ERROR: Clause GANG is not allowed if clause WORKER appears on the ROUTINE directive
+  !$acc routine worker device_type(*) gang
+
+  !ERROR: Clause VECTOR is not allowed if clause WORKER appears on the ROUTINE directive
+  !$acc routine worker device_type(*) vector
+
+  !ERROR: Clause SEQ is not allowed if clause WORKER appears on the ROUTINE directive
+  !$acc routine worker device_type(*) seq
+
+  !ERROR: Clause VECTOR is not allowed if clause VECTOR appears on the ROUTINE directive
+  !$acc routine vector device_type(*) vector
+
+  !ERROR: Clause GANG is not allowed if clause VECTOR appears on the ROUTINE directive
+  !$acc routine vector device_type(*) gang
+
+  !ERROR: Clause VECTOR is not allowed if clause VECTOR appears on the ROUTINE directive
+  !$acc routine vector device_type(*) vector
+
+  !ERROR: Clause SEQ is not allowed if clause VECTOR appears on the ROUTINE directive
+  !$acc routine vector device_type(*) seq
+
+  !ERROR: Clause SEQ is not allowed if clause SEQ appears on the ROUTINE directive
+  !$acc routine seq device_type(*) seq
+
+  !ERROR: Clause GANG is not allowed if clause SEQ appears on the ROUTINE directive
+  !$acc routine seq device_type(*) gang
+
+  !ERROR: Clause VECTOR is not allowed if clause SEQ appears on the ROUTINE directive
+  !$acc routine seq device_type(*) vector
+
+  !ERROR: Clause WORKER is not allowed if clause SEQ appears on the ROUTINE directive
+  !$acc routine seq device_type(*) worker
+
+  !$acc routine device_type(host) seq device_type(nvidia) gang device_type(multicore) vector device_type(*) worker
+end subroutine

Copy link
Contributor

@jeanPerier jeanPerier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great

Copy link
Contributor

@razvanlupusoru razvanlupusoru left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! Looks good to me.

@clementval clementval merged commit bdfe5d6 into main Jan 11, 2024
6 of 7 checks passed
@clementval clementval deleted the users/clementval/acc_device_type_routine_sema branch January 11, 2024 21:10
@clementval clementval restored the users/clementval/acc_device_type_routine_sema branch January 11, 2024 21:10
@clementval clementval deleted the users/clementval/acc_device_type_routine_sema branch January 11, 2024 21:11
justinfargnoli pushed a commit to justinfargnoli/llvm-project that referenced this pull request Jan 28, 2024
…ine (llvm#77802)

this patch enforce or fix the enforcement of two restrictions from
section 2.15.1:

> Only the gang, worker, vector, seq and bind clauses may follow a
device_type clause.

`seq` was not allowed after `device_type` with the current
implementation.

> Exactly one of the gang, worker, vector, or seq clauses must appear.

This was not properly checked. This patch check correctly for mutually
exclusion as described in section 2.4. Mutually exclusive clauses may
appear on the same directive if they apply for different device_type.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flang:semantics flang Flang issues not falling into any other category openacc
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants