From 6eb2e7467c8b6e513ab7f6d6e6788dba67b0326f Mon Sep 17 00:00:00 2001 From: Valentin Clement Date: Thu, 19 Oct 2023 10:54:23 -0700 Subject: [PATCH 1/3] [flang][openacc] Warn for num_gangs, num_workers and vector_length on acc serial --- flang/lib/Semantics/check-acc-structure.cpp | 18 +++++++++++++++--- .../lib/Semantics/check-directive-structure.h | 18 ++++++++++++------ flang/test/Semantics/OpenACC/acc-serial.f90 | 6 +++--- 3 files changed, 30 insertions(+), 12 deletions(-) diff --git a/flang/lib/Semantics/check-acc-structure.cpp b/flang/lib/Semantics/check-acc-structure.cpp index ef253586cfa0e..fd6c2407de33f 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 14a3151e67268..5e86c45a8b777 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 void DirectiveStructureChecker::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 a052ef4e476a8..afcfc00a40ec6 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 From 95df6533462386a2525cb2b824b58fc6d8aee640 Mon Sep 17 00:00:00 2001 From: Valentin Clement Date: Thu, 19 Oct 2023 11:07:18 -0700 Subject: [PATCH 2/3] Also warn on serial loop --- flang/lib/Semantics/check-acc-structure.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/flang/lib/Semantics/check-acc-structure.cpp b/flang/lib/Semantics/check-acc-structure.cpp index fd6c2407de33f..763418ede24d5 100644 --- a/flang/lib/Semantics/check-acc-structure.cpp +++ b/flang/lib/Semantics/check-acc-structure.cpp @@ -541,7 +541,8 @@ void AccStructureChecker::Enter(const parser::AccClause::Gang &g) { void AccStructureChecker::Enter(const parser::AccClause::NumGangs &n) { CheckAllowed(llvm::acc::Clause::ACCC_num_gangs, /*warnInsteadOfError=*/GetContext().directive == - llvm::acc::Directive::ACCD_serial); + llvm::acc::Directive::ACCD_serial || + GetContext().directive == llvm::acc::Directive::ACCD_serial_loop); if (n.v.size() > 3) context_.Say(GetContext().clauseSource, @@ -551,13 +552,15 @@ void AccStructureChecker::Enter(const parser::AccClause::NumGangs &n) { void AccStructureChecker::Enter(const parser::AccClause::NumWorkers &n) { CheckAllowed(llvm::acc::Clause::ACCC_num_workers, /*warnInsteadOfError=*/GetContext().directive == - llvm::acc::Directive::ACCD_serial); + llvm::acc::Directive::ACCD_serial || + GetContext().directive == llvm::acc::Directive::ACCD_serial_loop); } void AccStructureChecker::Enter(const parser::AccClause::VectorLength &n) { CheckAllowed(llvm::acc::Clause::ACCC_vector_length, /*warnInsteadOfError=*/GetContext().directive == - llvm::acc::Directive::ACCD_serial); + llvm::acc::Directive::ACCD_serial || + GetContext().directive == llvm::acc::Directive::ACCD_serial_loop); } void AccStructureChecker::Enter(const parser::AccClause::Reduction &reduction) { From c4d4333e8307b7b969ff334e481ce1ae42984f8d Mon Sep 17 00:00:00 2001 From: Valentin Clement Date: Thu, 19 Oct 2023 11:15:53 -0700 Subject: [PATCH 3/3] Set correct severity --- flang/lib/Semantics/check-directive-structure.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flang/lib/Semantics/check-directive-structure.h b/flang/lib/Semantics/check-directive-structure.h index 5e86c45a8b777..9c3aa47e19e5c 100644 --- a/flang/lib/Semantics/check-directive-structure.h +++ b/flang/lib/Semantics/check-directive-structure.h @@ -459,7 +459,7 @@ void DirectiveStructureChecker::CheckAllowed( !GetContext().requiredClauses.test(clause)) { if (warnInsteadOfError) context_.Say(GetContext().clauseSource, - "%s clause is not allowed on the %s directive and will be ignored"_warn_en_US, + "%s clause is not allowed on the %s directive and will be ignored"_port_en_US, parser::ToUpperCaseLetters(getClauseName(clause).str()), parser::ToUpperCaseLetters(GetContext().directiveSource.ToString())); else