From 1dd24e6ab7a70242edfa4139441bfe7753892b4e Mon Sep 17 00:00:00 2001 From: Valentin Clement Date: Tue, 8 Dec 2020 10:36:34 -0500 Subject: [PATCH] [flang][openacc] Add clause validity tests for the update directive Add couple of clause validity tests for the update directive and check for the restriction where at least self, host or device clause must appear on the directive. Reviewed By: sameeranjoshi Differential Revision: https://reviews.llvm.org/D92447 --- flang/include/flang/Parser/dump-parse-tree.h | 1 + flang/include/flang/Parser/parse-tree.h | 6 +++++ flang/lib/Lower/OpenACC.cpp | 19 +++++++++----- flang/lib/Parser/openacc-parsers.cpp | 15 ++++++----- flang/lib/Semantics/check-acc-structure.cpp | 25 +++++++++++++++++- flang/test/Semantics/acc-clause-validity.f90 | 39 +++++++++++++++++++++++++++- llvm/include/llvm/Frontend/OpenACC/ACC.td | 3 +-- 7 files changed, 90 insertions(+), 18 deletions(-) diff --git a/flang/include/flang/Parser/dump-parse-tree.h b/flang/include/flang/Parser/dump-parse-tree.h index 61c5bddc0af..c86c2ec6e66 100644 --- a/flang/include/flang/Parser/dump-parse-tree.h +++ b/flang/include/flang/Parser/dump-parse-tree.h @@ -85,6 +85,7 @@ public: NODE_ENUM(parser::AccReductionOperator, Operator) NODE(parser, AccSizeExpr) NODE(parser, AccSizeExprList) + NODE(parser, AccSelfClause) NODE(parser, AccStandaloneDirective) NODE(parser, AccTileExpr) NODE(parser, AccTileExprList) diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h index 2dcb92b4a72..c990ddd5c91 100644 --- a/flang/include/flang/Parser/parse-tree.h +++ b/flang/include/flang/Parser/parse-tree.h @@ -3873,6 +3873,12 @@ struct AccSizeExprList { WRAPPER_CLASS_BOILERPLATE(AccSizeExprList, std::list); }; +struct AccSelfClause { + UNION_CLASS_BOILERPLATE(AccSelfClause); + std::variant, AccObjectList> u; + CharBlock source; +}; + struct AccGangArgument { TUPLE_CLASS_BOILERPLATE(AccGangArgument); std::tuple, std::optional> t; diff --git a/flang/lib/Lower/OpenACC.cpp b/flang/lib/Lower/OpenACC.cpp index f9a60ca7717..6f45bb623d7 100644 --- a/flang/lib/Lower/OpenACC.cpp +++ b/flang/lib/Lower/OpenACC.cpp @@ -335,13 +335,18 @@ genACCParallelOp(Fortran::lower::AbstractConverter &converter, firOpBuilder.getI1Type(), cond); } else if (const auto *selfClause = std::get_if(&clause.u)) { - if (selfClause->v) { - Value cond = fir::getBase(converter.genExprValue( - *Fortran::semantics::GetExpr(*(selfClause->v)))); - selfCond = firOpBuilder.createConvert(currentLocation, - firOpBuilder.getI1Type(), cond); - } else { - addSelfAttr = true; + const Fortran::parser::AccSelfClause &accSelfClause = selfClause->v; + if (const auto *optCondition = + std::get_if>( + &accSelfClause.u)) { + if (*optCondition) { + Value cond = fir::getBase(converter.genExprValue( + *Fortran::semantics::GetExpr(*optCondition))); + selfCond = firOpBuilder.createConvert(currentLocation, + firOpBuilder.getI1Type(), cond); + } else { + addSelfAttr = true; + } } } else if (const auto *copyClause = std::get_if(&clause.u)) { diff --git a/flang/lib/Parser/openacc-parsers.cpp b/flang/lib/Parser/openacc-parsers.cpp index 510b8027713..bba886f2fbf 100644 --- a/flang/lib/Parser/openacc-parsers.cpp +++ b/flang/lib/Parser/openacc-parsers.cpp @@ -98,13 +98,8 @@ TYPE_PARSER("AUTO" >> construct(construct()) || parenthesized(construct( Parser{} / ":", Parser{})))) || - // SELF clause is either a simple optional condition for compute construct - // or a synonym of the HOST clause for the update directive 2.14.4 holding - // an object list. - "SELF" >> construct(construct( - maybe(parenthesized(scalarLogicalExpr)))) || - construct( - construct(parenthesized(Parser{}))) || + "SELF" >> construct( + construct(Parser{})) || "SEQ" >> construct(construct()) || "TILE" >> construct(construct( parenthesized(Parser{}))) || @@ -176,6 +171,12 @@ TYPE_PARSER(construct( parenthesized(first("NONE" >> pure(AccDefaultClause::Arg::None), "PRESENT" >> pure(AccDefaultClause::Arg::Present))))) +// SELF clause is either a simple optional condition for compute construct +// or a synonym of the HOST clause for the update directive 2.14.4 holding +// an object list. +TYPE_PARSER(construct(parenthesized(Parser{})) || + construct(maybe(parenthesized(scalarLogicalExpr)))) + // Modifier for copyin, copyout, cache and create TYPE_PARSER(construct( first("ZERO:" >> pure(AccDataModifier::Modifier::Zero), diff --git a/flang/lib/Semantics/check-acc-structure.cpp b/flang/lib/Semantics/check-acc-structure.cpp index edcb41eae0c..d6be718aa2c 100644 --- a/flang/lib/Semantics/check-acc-structure.cpp +++ b/flang/lib/Semantics/check-acc-structure.cpp @@ -201,6 +201,8 @@ void AccStructureChecker::Leave(const parser::OpenACCStandaloneConstruct &x) { CheckRequireAtLeastOneOf(); break; case llvm::acc::Directive::ACCD_update: + // Restriction - line 2636 + CheckRequireAtLeastOneOf(); // Restriction - 2301 CheckOnlyAllowedAfter(llvm::acc::Clause::ACCC_device_type, updateOnlyAllowedAfterDeviceTypeClauses); @@ -281,7 +283,6 @@ CHECK_SIMPLE_CLAUSE(Present, ACCC_present) CHECK_SIMPLE_CLAUSE(Private, ACCC_private) CHECK_SIMPLE_CLAUSE(Read, ACCC_read) CHECK_SIMPLE_CLAUSE(Reduction, ACCC_reduction) -CHECK_SIMPLE_CLAUSE(Self, ACCC_self) CHECK_SIMPLE_CLAUSE(Seq, ACCC_seq) CHECK_SIMPLE_CLAUSE(Tile, ACCC_tile) CHECK_SIMPLE_CLAUSE(UseDevice, ACCC_use_device) @@ -346,6 +347,28 @@ void AccStructureChecker::Enter(const parser::AccClause::Copyout &c) { } } +void AccStructureChecker::Enter(const parser::AccClause::Self &x) { + CheckAllowed(llvm::acc::Clause::ACCC_self); + const parser::AccSelfClause &accSelfClause = x.v; + if (GetContext().directive == llvm::acc::Directive::ACCD_update && + std::holds_alternative>( + accSelfClause.u)) { + context_.Say(GetContext().clauseSource, + "SELF clause on the %s directive must have a var-list"_err_en_US, + ContextDirectiveAsFortran()); + } else if (GetContext().directive != llvm::acc::Directive::ACCD_update && + std::holds_alternative(accSelfClause.u)) { + const auto &accObjectList = + std::get(accSelfClause.u); + if (accObjectList.v.size() != 1) { + context_.Say(GetContext().clauseSource, + "SELF clause on the %s directive only accepts optional scalar logical" + " expression"_err_en_US, + ContextDirectiveAsFortran()); + } + } +} + llvm::StringRef AccStructureChecker::getClauseName(llvm::acc::Clause clause) { return llvm::acc::getOpenACCClauseName(clause); } diff --git a/flang/test/Semantics/acc-clause-validity.f90 b/flang/test/Semantics/acc-clause-validity.f90 index 717b5cd3041..22b332841df 100644 --- a/flang/test/Semantics/acc-clause-validity.f90 +++ b/flang/test/Semantics/acc-clause-validity.f90 @@ -173,9 +173,40 @@ program openacc_clause_validity !ERROR: Unmatched PARALLEL directive !$acc end parallel + !ERROR: At least one of DEVICE, HOST, SELF clause must appear on the UPDATE directive + !$acc update + !$acc update self(a, f) host(g) device(h) - !$acc update device(i) device_type(*) async + !$acc update host(aa) async(1) + + !$acc update device(bb) async(async1) + + !ERROR: At most one ASYNC clause can appear on the UPDATE directive + !$acc update host(aa, bb) async(1) async(2) + + !$acc update self(bb, cc(:)) wait(1) + + !ERROR: SELF clause on the UPDATE directive must have a var-list + !$acc update self + + !$acc update device(aa, bb, cc) wait(wait1) + + !$acc update host(aa) host(bb) device(cc) wait(1,2) + + !$acc update device(aa, cc) wait(wait1, wait2) + + !$acc update device(aa) device_type(*) async + + !$acc update host(bb) device_type(*) wait + + !$acc update self(cc) device_type(1,2) async device_type(3) wait + + !ERROR: At most one IF clause can appear on the UPDATE directive + !$acc update device(aa) if(.true.) if(ifCondition) + + !ERROR: At most one IF_PRESENT clause can appear on the UPDATE directive + !$acc update device(bb) if_present if_present !ERROR: Clause IF is not allowed after clause DEVICE_TYPE on the UPDATE directive !$acc update device(i) device_type(*) if(.TRUE.) @@ -205,6 +236,12 @@ program openacc_clause_validity a(i) = 3.14 end do + !ERROR: SELF clause on the PARALLEL LOOP directive only accepts optional scalar logical expression + !$acc parallel loop self(bb, cc(:)) + do i = 1, N + a(i) = 3.14 + end do + !$acc parallel loop self(.true.) do i = 1, N a(i) = 3.14 diff --git a/llvm/include/llvm/Frontend/OpenACC/ACC.td b/llvm/include/llvm/Frontend/OpenACC/ACC.td index 206f82adb1e..4c1d6324929 100644 --- a/llvm/include/llvm/Frontend/OpenACC/ACC.td +++ b/llvm/include/llvm/Frontend/OpenACC/ACC.td @@ -210,8 +210,7 @@ def ACCC_Reduction : Clause<"reduction"> { // 2.5.5 def ACCC_Self : Clause<"self"> { - let flangClassValue = "ScalarLogicalExpr"; - let isValueOptional = true; + let flangClassValue = "AccSelfClause"; } // 2.9.5 -- 2.11.0