From bdfe5d69dee7baa0a54cddca8a99e0fdaeded4a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Valentin=20Clement=20=28=E3=83=90=E3=83=AC=E3=83=B3?= =?UTF-8?q?=E3=82=BF=E3=82=A4=E3=83=B3=20=E3=82=AF=E3=83=AC=E3=83=A1?= =?UTF-8?q?=E3=83=B3=29?= Date: Thu, 11 Jan 2024 13:10:53 -0800 Subject: [PATCH] [flang][openacc] Apply mutually exclusive clauses restriction to routine (#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. --- flang/lib/Semantics/check-acc-structure.cpp | 62 +++++++--- .../lib/Semantics/check-directive-structure.h | 42 +++++++ flang/test/Semantics/OpenACC/acc-routine.f90 | 111 ++++++++++++++++++ 3 files changed, 201 insertions(+), 14 deletions(-) diff --git a/flang/lib/Semantics/check-acc-structure.cpp b/flang/lib/Semantics/check-acc-structure.cpp index 4a5798a8a531..d69e1c597e2f 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(&gangArg.u)) + if (std::get_if(&gangArg.u)) { hasNum = true; - else if (std::get_if(&gangArg.u)) + } else if (std::get_if(&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 f7a6233a32ed..829405f99d64 100644 --- a/flang/lib/Semantics/check-directive-structure.h +++ b/flang/lib/Semantics/check-directive-structure.h @@ -197,6 +197,7 @@ protected: const PC *clause{nullptr}; ClauseMapTy clauseInfo; std::list actualClauses; + std::list crtGroup; Symbol *loopIV{nullptr}; }; @@ -261,6 +262,12 @@ protected: 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 @@ protected: // separator clause appears. void CheckAllowedOncePerGroup(C clause, C separator); + void CheckMutuallyExclusivePerGroup( + C clause, C separator, common::EnumSet set); + void CheckAtLeastOneClause(); void CheckNotAllowedIfClause( @@ -526,6 +536,7 @@ void DirectiveStructureChecker::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 +void DirectiveStructureChecker::CheckMutuallyExclusivePerGroup(C clause, C separator, + common::EnumSet 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 void DirectiveStructureChecker