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] Warn for num_gangs, num_workers and vector_length on acc serial #69622

Merged
merged 3 commits into from
Oct 19, 2023

Conversation

clementval
Copy link
Contributor

For portability with other compilers, just issue a portability warning instead of a hard error when num_gangs, num_workers or vector_length are present on an !$acc serial directive

@llvmbot llvmbot added flang Flang issues not falling into any other category openacc flang:semantics labels Oct 19, 2023
@llvmbot
Copy link
Member

llvmbot commented Oct 19, 2023

@llvm/pr-subscribers-openacc

@llvm/pr-subscribers-flang-semantics

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

Changes

For portability with other compilers, just issue a portability warning instead of a hard error when num_gangs, num_workers or vector_length are present on an !$acc serial directive


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

3 Files Affected:

  • (modified) flang/lib/Semantics/check-acc-structure.cpp (+15-3)
  • (modified) flang/lib/Semantics/check-directive-structure.h (+12-6)
  • (modified) flang/test/Semantics/OpenACC/acc-serial.f90 (+3-3)
diff --git a/flang/lib/Semantics/check-acc-structure.cpp b/flang/lib/Semantics/check-acc-structure.cpp
index ef253586cfa0ed1..fd6c2407de33fc0 100644
--- a/flang/lib/Semantics/check-acc-structure.cpp
+++ b/flang/lib/Semantics/check-acc-structure.cpp
@@ -380,14 +380,12 @@ CHECK_SIMPLE_CLAUSE(IfPresent, ACCC_if_present)
 CHECK_SIMPLE_CLAUSE(Independent, ACCC_independent)
 CHECK_SIMPLE_CLAUSE(NoCreate, ACCC_no_create)
 CHECK_SIMPLE_CLAUSE(Nohost, ACCC_nohost)
-CHECK_SIMPLE_CLAUSE(NumWorkers, ACCC_num_workers)
 CHECK_SIMPLE_CLAUSE(Private, ACCC_private)
 CHECK_SIMPLE_CLAUSE(Read, ACCC_read)
 CHECK_SIMPLE_CLAUSE(Seq, ACCC_seq)
 CHECK_SIMPLE_CLAUSE(Tile, ACCC_tile)
 CHECK_SIMPLE_CLAUSE(UseDevice, ACCC_use_device)
 CHECK_SIMPLE_CLAUSE(Vector, ACCC_vector)
-CHECK_SIMPLE_CLAUSE(VectorLength, ACCC_vector_length)
 CHECK_SIMPLE_CLAUSE(Wait, ACCC_wait)
 CHECK_SIMPLE_CLAUSE(Worker, ACCC_worker)
 CHECK_SIMPLE_CLAUSE(Write, ACCC_write)
@@ -541,13 +539,27 @@ void AccStructureChecker::Enter(const parser::AccClause::Gang &g) {
 }
 
 void AccStructureChecker::Enter(const parser::AccClause::NumGangs &n) {
-  CheckAllowed(llvm::acc::Clause::ACCC_num_gangs);
+  CheckAllowed(llvm::acc::Clause::ACCC_num_gangs,
+      /*warnInsteadOfError=*/GetContext().directive ==
+          llvm::acc::Directive::ACCD_serial);
 
   if (n.v.size() > 3)
     context_.Say(GetContext().clauseSource,
         "NUM_GANGS clause accepts a maximum of 3 arguments"_err_en_US);
 }
 
+void AccStructureChecker::Enter(const parser::AccClause::NumWorkers &n) {
+  CheckAllowed(llvm::acc::Clause::ACCC_num_workers,
+      /*warnInsteadOfError=*/GetContext().directive ==
+          llvm::acc::Directive::ACCD_serial);
+}
+
+void AccStructureChecker::Enter(const parser::AccClause::VectorLength &n) {
+  CheckAllowed(llvm::acc::Clause::ACCC_vector_length,
+      /*warnInsteadOfError=*/GetContext().directive ==
+          llvm::acc::Directive::ACCD_serial);
+}
+
 void AccStructureChecker::Enter(const parser::AccClause::Reduction &reduction) {
   CheckAllowed(llvm::acc::Clause::ACCC_reduction);
 
diff --git a/flang/lib/Semantics/check-directive-structure.h b/flang/lib/Semantics/check-directive-structure.h
index 14a3151e672685e..5e86c45a8b7771e 100644
--- a/flang/lib/Semantics/check-directive-structure.h
+++ b/flang/lib/Semantics/check-directive-structure.h
@@ -333,7 +333,7 @@ class DirectiveStructureChecker : public virtual BaseChecker {
 
   void CheckRequireAtLeastOneOf(bool warnInsteadOfError = false);
 
-  void CheckAllowed(C clause);
+  void CheckAllowed(C clause, bool warnInsteadOfError = false);
 
   void CheckAtLeastOneClause();
 
@@ -452,15 +452,21 @@ std::string DirectiveStructureChecker<D, C, PC,
 // Check that clauses present on the directive are allowed clauses.
 template <typename D, typename C, typename PC, std::size_t ClauseEnumSize>
 void DirectiveStructureChecker<D, C, PC, ClauseEnumSize>::CheckAllowed(
-    C clause) {
+    C clause, bool warnInsteadOfError) {
   if (!GetContext().allowedClauses.test(clause) &&
       !GetContext().allowedOnceClauses.test(clause) &&
       !GetContext().allowedExclusiveClauses.test(clause) &&
       !GetContext().requiredClauses.test(clause)) {
-    context_.Say(GetContext().clauseSource,
-        "%s clause is not allowed on the %s directive"_err_en_US,
-        parser::ToUpperCaseLetters(getClauseName(clause).str()),
-        parser::ToUpperCaseLetters(GetContext().directiveSource.ToString()));
+    if (warnInsteadOfError)
+      context_.Say(GetContext().clauseSource,
+          "%s clause is not allowed on the %s directive and will be ignored"_warn_en_US,
+          parser::ToUpperCaseLetters(getClauseName(clause).str()),
+          parser::ToUpperCaseLetters(GetContext().directiveSource.ToString()));
+    else
+      context_.Say(GetContext().clauseSource,
+          "%s clause is not allowed on the %s directive"_err_en_US,
+          parser::ToUpperCaseLetters(getClauseName(clause).str()),
+          parser::ToUpperCaseLetters(GetContext().directiveSource.ToString()));
     return;
   }
   if ((GetContext().allowedOnceClauses.test(clause) ||
diff --git a/flang/test/Semantics/OpenACC/acc-serial.f90 b/flang/test/Semantics/OpenACC/acc-serial.f90
index a052ef4e476a885..afcfc00a40ec64d 100644
--- a/flang/test/Semantics/OpenACC/acc-serial.f90
+++ b/flang/test/Semantics/OpenACC/acc-serial.f90
@@ -77,15 +77,15 @@ program openacc_serial_validity
   !$acc serial wait(wait1) wait(wait2)
   !$acc end serial
 
-  !ERROR: NUM_GANGS clause is not allowed on the SERIAL directive
+  !PORTABILITY: NUM_GANGS clause is not allowed on the SERIAL directive and will be ignored
   !$acc serial num_gangs(8)
   !$acc end serial
 
-  !ERROR: NUM_WORKERS clause is not allowed on the SERIAL directive
+  !PORTABILITY: NUM_WORKERS clause is not allowed on the SERIAL directive and will be ignored
   !$acc serial num_workers(8)
   !$acc end serial
 
-  !ERROR: VECTOR_LENGTH clause is not allowed on the SERIAL directive
+  !PORTABILITY: VECTOR_LENGTH clause is not allowed on the SERIAL directive and will be ignored
   !$acc serial vector_length(128)
   !$acc end serial
 

Copy link
Contributor

@vzakhari vzakhari left a comment

Choose a reason for hiding this comment

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

LGTM

@clementval clementval merged commit 621a271 into llvm:main Oct 19, 2023
2 checks passed
@clementval clementval deleted the acc_serial_warn branch October 19, 2023 21:03
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.

3 participants