From 53b8d641ed71d7fb837a20293b566463bd623f92 Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek Date: Thu, 10 Jul 2025 20:08:28 -0500 Subject: [PATCH] [flang][OpenMP] Avoid unnecessary parsing of OpenMP constructs When parsing a specification part, the parser will look ahead to see if the next construct is an executable construct. In doing so it will invoke OpenMPConstruct parser, whereas the only necessary thing to check would be the directive alone. --- flang/include/flang/Parser/parse-tree.h | 10 +++++ flang/lib/Parser/openmp-parsers.cpp | 9 +++- flang/lib/Parser/parse-tree.cpp | 47 ++++++++++++++++++++ flang/lib/Parser/program-parsers.cpp | 2 +- flang/lib/Parser/type-parsers.h | 1 + flang/test/Parser/OpenMP/fail-construct1.f90 | 2 +- 6 files changed, 68 insertions(+), 3 deletions(-) diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h index ab2dde7d5dfbe..cc1d032f94d4a 100644 --- a/flang/include/flang/Parser/parse-tree.h +++ b/flang/include/flang/Parser/parse-tree.h @@ -3481,6 +3481,9 @@ struct OmpDirectiveName { // This allows "construct(Verbatim(""))". OmpDirectiveName(const Verbatim &name); using WrapperTrait = std::true_type; + + bool IsExecutionPart() const; // Is allowed in the execution part + CharBlock source; llvm::omp::Directive v{llvm::omp::Directive::OMPD_unknown}; }; @@ -5028,6 +5031,13 @@ struct OpenMPLoopConstruct { t; }; +// Lookahead class to identify execution-part OpenMP constructs without +// parsing the entire OpenMP construct. +struct OpenMPExecDirective { + WRAPPER_CLASS_BOILERPLATE(OpenMPExecDirective, OmpDirectiveName); + CharBlock source; +}; + struct OpenMPConstruct { UNION_CLASS_BOILERPLATE(OpenMPConstruct); std::variant( scalarIntExpr)) TYPE_PARSER( - construct(designator) || construct("/" >> name / "/")) + construct(designator) || "/" >> construct(name) / "/") // OMP 5.0 2.19.4.5 LASTPRIVATE ([lastprivate-modifier :] list) TYPE_PARSER(construct( @@ -1757,6 +1757,13 @@ TYPE_PARSER(construct( Parser{} / endOmpLine, Parser{}, Parser{} / endOmpLine)) +static bool IsExecutionPart(const OmpDirectiveName &name) { + return name.IsExecutionPart(); +} + +TYPE_PARSER(construct( + startOmpLine >> predicated(Parser{}, IsExecutionPart))) + TYPE_CONTEXT_PARSER("OpenMP construct"_en_US, startOmpLine >> withMessage("expected OpenMP construct"_err_en_US, diff --git a/flang/lib/Parser/parse-tree.cpp b/flang/lib/Parser/parse-tree.cpp index 824612e49293f..7b46c9f469b06 100644 --- a/flang/lib/Parser/parse-tree.cpp +++ b/flang/lib/Parser/parse-tree.cpp @@ -368,6 +368,53 @@ llvm::omp::Clause OmpClause::Id() const { return std::visit([](auto &&s) { return getClauseIdForClass(s); }, u); } +bool OmpDirectiveName::IsExecutionPart() const { + // Can the directive appear in the execution part of the program. + llvm::omp::Directive id{v}; + switch (llvm::omp::getDirectiveCategory(id)) { + case llvm::omp::Category::Executable: + return true; + case llvm::omp::Category::Declarative: + switch (id) { + case llvm::omp::Directive::OMPD_allocate: + return true; + default: + return false; + } + break; + case llvm::omp::Category::Informational: + switch (id) { + case llvm::omp::Directive::OMPD_assume: + return true; + default: + return false; + } + break; + case llvm::omp::Category::Meta: + return true; + case llvm::omp::Category::Subsidiary: + switch (id) { + // TODO: case llvm::omp::Directive::OMPD_task_iteration: + case llvm::omp::Directive::OMPD_section: + case llvm::omp::Directive::OMPD_scan: + return true; + default: + return false; + } + break; + case llvm::omp::Category::Utility: + switch (id) { + case llvm::omp::Directive::OMPD_error: + case llvm::omp::Directive::OMPD_nothing: + return true; + default: + return false; + } + break; + } + return false; +} + const OmpArgumentList &OmpDirectiveSpecification::Arguments() const { static OmpArgumentList empty{decltype(OmpArgumentList::v){}}; if (auto &arguments = std::get>(t)) { diff --git a/flang/lib/Parser/program-parsers.cpp b/flang/lib/Parser/program-parsers.cpp index d6e7f11297187..5f4e62ffdbbf2 100644 --- a/flang/lib/Parser/program-parsers.cpp +++ b/flang/lib/Parser/program-parsers.cpp @@ -108,7 +108,7 @@ constexpr auto actionStmtLookAhead{first(actionStmt >> ok, "ALLOCATE ("_tok, "CALL" >> name >> "("_tok, "GO TO"_tok, "OPEN ("_tok, "PRINT"_tok / space / !"("_tok, "READ ("_tok, "WRITE ("_tok)}; constexpr auto execPartLookAhead{first(actionStmtLookAhead >> ok, - openaccConstruct >> ok, openmpConstruct >> ok, "ASSOCIATE ("_tok, + openaccConstruct >> ok, openmpExecDirective >> ok, "ASSOCIATE ("_tok, "BLOCK"_tok, "SELECT"_tok, "CHANGE TEAM"_sptok, "CRITICAL"_tok, "DO"_tok, "IF ("_tok, "WHERE ("_tok, "FORALL ("_tok, "!$CUF"_tok)}; constexpr auto declErrorRecovery{ diff --git a/flang/lib/Parser/type-parsers.h b/flang/lib/Parser/type-parsers.h index 8e91ede093fc4..3900c5a86c874 100644 --- a/flang/lib/Parser/type-parsers.h +++ b/flang/lib/Parser/type-parsers.h @@ -137,6 +137,7 @@ constexpr Parser compilerDirective; constexpr Parser openaccConstruct; constexpr Parser openaccDeclarativeConstruct; constexpr Parser openmpConstruct; +constexpr Parser openmpExecDirective; constexpr Parser openmpDeclarativeConstruct; constexpr Parser ompEndLoopDirective; constexpr Parser intrinsicVectorTypeSpec; // Extension diff --git a/flang/test/Parser/OpenMP/fail-construct1.f90 b/flang/test/Parser/OpenMP/fail-construct1.f90 index f0ee22125cee0..f0b3f7438ae58 100644 --- a/flang/test/Parser/OpenMP/fail-construct1.f90 +++ b/flang/test/Parser/OpenMP/fail-construct1.f90 @@ -1,5 +1,5 @@ ! RUN: not %flang_fc1 -fsyntax-only -fopenmp %s 2>&1 | FileCheck %s -! CHECK: error: expected OpenMP construct !$omp parallel +! CHECK: error: expected '!$OMP ' end