From 1419c89670953b969fcc1c474a258c7475d8e7df Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek Date: Fri, 9 May 2025 07:45:20 -0500 Subject: [PATCH 1/6] [flang][OpenMP] Handle multiple spellings in OmpDirectiveNameParser Collect all spellings from all supported OpenMP versions before parsing. Break up the list of spellings by the initial letter to speed up parsing a little. --- flang/lib/Parser/openmp-parsers.cpp | 49 +++++++++++++++++++++-------- 1 file changed, 36 insertions(+), 13 deletions(-) diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp index 30b5d4f9513a7..29701a616d4b0 100644 --- a/flang/lib/Parser/openmp-parsers.cpp +++ b/flang/lib/Parser/openmp-parsers.cpp @@ -19,6 +19,7 @@ #include "llvm/ADT/ArrayRef.h" #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/StringRef.h" +#include "llvm/ADT/StringSet.h" #include "llvm/Frontend/OpenMP/OMP.h" // OpenMP Directives and Clauses @@ -104,8 +105,13 @@ struct OmpDirectiveNameParser { using Token = TokenStringMatch; std::optional Parse(ParseState &state) const { + if (state.BytesRemaining() == 0) { + return std::nullopt; + } auto begin{state.GetLocation()}; - for (const NameWithId &nid : directives()) { + char next{static_cast(std::tolower(*begin))}; + + for (const NameWithId &nid : directives_starting_with(next)) { if (attempt(Token(nid.first.data())).Parse(state)) { OmpDirectiveName n; n.v = nid.second; @@ -118,30 +124,47 @@ struct OmpDirectiveNameParser { private: using NameWithId = std::pair; + using ConstIterator = std::vector::const_iterator; - llvm::iterator_range directives() const; - void initTokens(NameWithId *) const; + llvm::iterator_range + directives_starting_with(char initial) const; + void initTokens(std::vector[]) const; }; -llvm::iterator_range -OmpDirectiveNameParser::directives() const { - static NameWithId table[llvm::omp::Directive_enumSize]; +llvm::iterator_range +OmpDirectiveNameParser::directives_starting_with(char initial) const { + static const std::vector empty{}; + if (initial < 'a' || initial > 'z') { + return llvm::make_range(std::cbegin(empty), std::cend(empty)); + } + + static std::vector table['z' - 'a' + 1]; [[maybe_unused]] static bool init = (initTokens(table), true); - return llvm::make_range(std::cbegin(table), std::cend(table)); + + int index = initial - 'a'; + return llvm::make_range(std::cbegin(table[index]), std::cend(table[index])); } -void OmpDirectiveNameParser::initTokens(NameWithId *table) const { +void OmpDirectiveNameParser::initTokens(std::vector table[]) const { for (size_t i{0}, e{llvm::omp::Directive_enumSize}; i != e; ++i) { + llvm::StringSet spellings; auto id{static_cast(i)}; - llvm::StringRef name{ - llvm::omp::getOpenMPDirectiveName(id, llvm::omp::FallbackVersion)}; - table[i] = std::make_pair(name.str(), id); + for (unsigned version : llvm::omp::getOpenMPVersions()) { + spellings.insert(llvm::omp::getOpenMPDirectiveName(id, version)); + } + for (auto &[name, _] : spellings) { + char initial{static_cast(std::tolower(name.front()))}; + table[initial - 'a'].emplace_back(name.str(), id); + } } // Sort the table with respect to the directive name length in a descending // order. This is to make sure that longer names are tried first, before // any potential prefix (e.g. "target update" before "target"). - std::sort(table, table + llvm::omp::Directive_enumSize, - [](auto &a, auto &b) { return a.first.size() > b.first.size(); }); + for (int initial{'a'}; initial != 'z' + 1; ++initial) { + llvm::stable_sort(table[initial - 'a'], [](auto &a, auto &b) { + return a.first.size() > b.first.size(); + }); + } } // --- Modifier helpers ----------------------------------------------- From 0e2c062d467d7d23ed88cdf14c2c3de740549ca6 Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek Date: Mon, 7 Jul 2025 13:54:32 -0500 Subject: [PATCH 2/6] [flang][OpenMP] Recognize remaining OpenMP 6.0 spellings in parser Parse OpenMP 6.0 spellings for directives that don't use OmpDirectiveNameParser. --- flang/lib/Parser/openmp-parsers.cpp | 17 +- .../OpenMP/openmp6-directive-spellings.f90 | 252 ++++++++++++++++++ 2 files changed, 263 insertions(+), 6 deletions(-) create mode 100644 flang/test/Parser/OpenMP/openmp6-directive-spellings.f90 diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp index 29701a616d4b0..b25445b0aaef6 100644 --- a/flang/lib/Parser/openmp-parsers.cpp +++ b/flang/lib/Parser/openmp-parsers.cpp @@ -1501,7 +1501,7 @@ TYPE_PARSER( // In this context "TARGET UPDATE" can be parsed as a TARGET directive // followed by an UPDATE clause. This is the only combination at the // moment, exclude it explicitly. - (!"TARGET UPDATE"_sptok) >= + (!("TARGET UPDATE"_sptok || "TARGET_UPDATE"_sptok)) >= construct(first( "MASKED" >> pure(llvm::omp::Directive::OMPD_masked), "MASTER" >> pure(llvm::omp::Directive::OMPD_master), @@ -1514,6 +1514,7 @@ TYPE_PARSER( "SCOPE" >> pure(llvm::omp::Directive::OMPD_scope), "SINGLE" >> pure(llvm::omp::Directive::OMPD_single), "TARGET DATA" >> pure(llvm::omp::Directive::OMPD_target_data), + "TARGET_DATA" >> pure(llvm::omp::Directive::OMPD_target_data), "TARGET PARALLEL" >> pure(llvm::omp::Directive::OMPD_target_parallel), "TARGET TEAMS" >> pure(llvm::omp::Directive::OMPD_target_teams), "TARGET" >> pure(llvm::omp::Directive::OMPD_target), @@ -1534,12 +1535,13 @@ TYPE_PARSER(construct( // OpenMP 5.2: 7.5.4 Declare Variant directive TYPE_PARSER(sourced( - construct(verbatim("DECLARE VARIANT"_tok), + construct( + verbatim("DECLARE VARIANT"_tok) || verbatim("DECLARE_VARIANT"_tok), "(" >> maybe(name / ":"), name / ")", Parser{}))) // 2.16 Declare Reduction Construct TYPE_PARSER(sourced(construct( - verbatim("DECLARE REDUCTION"_tok), + verbatim("DECLARE REDUCTION"_tok) || verbatim("DECLARE_REDUCTION"_tok), "(" >> indirect(Parser{}) / ")", maybe(Parser{})))) @@ -1558,7 +1560,8 @@ TYPE_PARSER( // 2.10.6 Declare Target Construct TYPE_PARSER(sourced(construct( - verbatim("DECLARE TARGET"_tok), Parser{}))) + verbatim("DECLARE TARGET"_tok) || verbatim("DECLARE_TARGET"_tok), + Parser{}))) static OmpMapperSpecifier ConstructOmpMapperSpecifier( std::optional &&mapperName, TypeSpec &&typeSpec, Name &&varName) { @@ -1586,7 +1589,8 @@ TYPE_PARSER(applyFunction(ConstructOmpMapperSpecifier, // OpenMP 5.2: 5.8.8 Declare Mapper Construct TYPE_PARSER(sourced( - construct(verbatim("DECLARE MAPPER"_tok), + construct( + verbatim("DECLARE MAPPER"_tok) || verbatim("DECLARE_MAPPER"_tok), parenthesized(Parser{}), Parser{}))) TYPE_PARSER(construct(Parser{}) || @@ -1633,7 +1637,8 @@ TYPE_PARSER(construct(startOmpLine >> "END ALLOCATORS"_tok)) // 2.8.2 Declare Simd construct TYPE_PARSER( - sourced(construct(verbatim("DECLARE SIMD"_tok), + sourced(construct( + verbatim("DECLARE SIMD"_tok) || verbatim("DECLARE_SIMD"_tok), maybe(parenthesized(name)), Parser{}))) // 2.4 Requires construct diff --git a/flang/test/Parser/OpenMP/openmp6-directive-spellings.f90 b/flang/test/Parser/OpenMP/openmp6-directive-spellings.f90 new file mode 100644 index 0000000000000..6a1d565451220 --- /dev/null +++ b/flang/test/Parser/OpenMP/openmp6-directive-spellings.f90 @@ -0,0 +1,252 @@ +!RUN: %flang_fc1 -fdebug-unparse -fopenmp -fopenmp-version=60 %s -o - | FileCheck --check-prefix=UNPARSE %s +!RUN: %flang_fc1 -fdebug-dump-parse-tree -fopenmp -fopenmp-version=60 %s -o - | FileCheck --check-prefix=PARSE-TREE %s + +! The directives to check: +! cancellation_point +! declare_mapper +! declare_reduction +! declare_simd +! declare_target +! declare_variant +! target_data +! target_enter_data +! target_exit_data +! target_update + +subroutine f00 + implicit none + integer :: i + + !$omp parallel + do i = 1, 10 + !$omp cancellation_point parallel + enddo + !$omp end parallel +end + +!UNPARSE: SUBROUTINE f00 +!UNPARSE: IMPLICIT NONE +!UNPARSE: INTEGER i +!UNPARSE: !$OMP PARALLEL +!UNPARSE: DO i=1_4,10_4 +!UNPARSE: !$OMP CANCELLATION_POINT PARALLEL +!UNPARSE: END DO +!UNPARSE: !$OMP END PARALLEL +!UNPARSE: END SUBROUTINE + +!PARSE-TREE: ExecutionPartConstruct -> ExecutableConstruct -> OpenMPConstruct -> OpenMPStandaloneConstruct -> OpenMPCancellationPointConstruct -> OmpDirectiveSpecification +!PARSE-TREE: | OmpDirectiveName -> llvm::omp::Directive = cancellation point +!PARSE-TREE: | OmpClauseList -> OmpClause -> CancellationConstructType -> OmpCancellationConstructTypeClause +!PARSE-TREE: | | OmpDirectiveName -> llvm::omp::Directive = parallel +!PARSE-TREE: | Flags = None + +subroutine f01 + type :: t + integer :: x + end type + !$omp declare_mapper(t :: v) map(v%x) +end + +!UNPARSE: SUBROUTINE f01 +!UNPARSE: TYPE :: t +!UNPARSE: INTEGER :: x +!UNPARSE: END TYPE +!UNPARSE: !$OMP DECLARE MAPPER (t::v) MAP(v%x) +!UNPARSE: END SUBROUTINE + +!PARSE-TREE: DeclarationConstruct -> SpecificationConstruct -> OpenMPDeclarativeConstruct -> OpenMPDeclareMapperConstruct +!PARSE-TREE: | Verbatim +!PARSE-TREE: | OmpMapperSpecifier +!PARSE-TREE: | | string = 't.omp.default.mapper' +!PARSE-TREE: | | TypeSpec -> DerivedTypeSpec +!PARSE-TREE: | | | Name = 't' +!PARSE-TREE: | | Name = 'v' +!PARSE-TREE: | OmpClauseList -> OmpClause -> Map -> OmpMapClause +!PARSE-TREE: | | OmpObjectList -> OmpObject -> Designator -> DataRef -> StructureComponent +!PARSE-TREE: | | | DataRef -> Name = 'v' +!PARSE-TREE: | | | Name = 'x' +!PARSE-TREE: | | bool = 'true' + +subroutine f02 + type :: t + integer :: x + end type + !$omp declare_reduction(+ : t : omp_out%x = omp_out%x + omp_in%x) +end + +!UNPARSE: SUBROUTINE f02 +!UNPARSE: TYPE :: t +!UNPARSE: INTEGER :: x +!UNPARSE: END TYPE +!UNPARSE: !$OMP DECLARE REDUCTION (+:t: omp_out%x=omp_out%x+omp_in%x +!UNPARSE: ) +!UNPARSE: END SUBROUTINE + +!PARSE-TREE: DeclarationConstruct -> SpecificationConstruct -> OpenMPDeclarativeConstruct -> OpenMPDeclareReductionConstruct +!PARSE-TREE: | Verbatim +!PARSE-TREE: | OmpReductionSpecifier +!PARSE-TREE: | | OmpReductionIdentifier -> DefinedOperator -> IntrinsicOperator = Add +!PARSE-TREE: | | OmpTypeNameList -> OmpTypeSpecifier -> TypeSpec -> DerivedTypeSpec +!PARSE-TREE: | | | Name = 't' +!PARSE-TREE: | | OmpReductionCombiner -> AssignmentStmt = 'omp_out%x=omp_out%x+omp_in%x' +!PARSE-TREE: | | | Variable = 'omp_out%x' +!PARSE-TREE: | | | | Designator -> DataRef -> StructureComponent +!PARSE-TREE: | | | | | DataRef -> Name = 'omp_out' +!PARSE-TREE: | | | | | Name = 'x' +!PARSE-TREE: | | | Expr = 'omp_out%x+omp_in%x' +!PARSE-TREE: | | | | Add +!PARSE-TREE: | | | | | Expr = 'omp_out%x' +!PARSE-TREE: | | | | | | Designator -> DataRef -> StructureComponent +!PARSE-TREE: | | | | | | | DataRef -> Name = 'omp_out' +!PARSE-TREE: | | | | | | | Name = 'x' +!PARSE-TREE: | | | | | Expr = 'omp_in%x' +!PARSE-TREE: | | | | | | Designator -> DataRef -> StructureComponent +!PARSE-TREE: | | | | | | | DataRef -> Name = 'omp_in' +!PARSE-TREE: | | | | | | | Name = 'x' +!PARSE-TREE: | OmpClauseList -> + +subroutine f03 + !$omp declare_simd +end + +!UNPARSE: SUBROUTINE f03 +!UNPARSE: !$OMP DECLARE SIMD +!UNPARSE: END SUBROUTINE + +!PARSE-TREE: OpenMPDeclarativeConstruct -> OpenMPDeclareSimdConstruct +!PARSE-TREE: | Verbatim +!PARSE-TREE: | OmpClauseList -> + +subroutine f04 + !$omp declare_target +end + +!UNPARSE: SUBROUTINE f04 +!UNPARSE: !$OMP DECLARE TARGET +!UNPARSE: END SUBROUTINE + +!PARSE-TREE: OpenMPDeclarativeConstruct -> OpenMPDeclareTargetConstruct +!PARSE-TREE: | Verbatim +!PARSE-TREE: | OmpDeclareTargetSpecifier -> OmpDeclareTargetWithClause -> OmpClauseList -> + +subroutine f05 + implicit none + interface + subroutine g05 + end + end interface + !$omp declare_variant(g05) match(user={condition(.true.)}) +end + +!UNPARSE: SUBROUTINE f05 +!UNPARSE: IMPLICIT NONE +!UNPARSE: INTERFACE +!UNPARSE: SUBROUTINE g05 +!UNPARSE: END SUBROUTINE +!UNPARSE: END INTERFACE +!UNPARSE: !$OMP DECLARE VARIANT (g05) MATCH(USER={CONDITION(.true._4)}) +!UNPARSE: END SUBROUTINE + +!PARSE-TREE: OpenMPDeclarativeConstruct -> OmpDeclareVariantDirective +!PARSE-TREE: | Verbatim +!PARSE-TREE: | Name = 'g05' +!PARSE-TREE: | OmpClauseList -> OmpClause -> Match -> OmpMatchClause -> OmpContextSelectorSpecification -> OmpTraitSetSelector +!PARSE-TREE: | | OmpTraitSetSelectorName -> Value = User +!PARSE-TREE: | | OmpTraitSelector +!PARSE-TREE: | | | OmpTraitSelectorName -> Value = Condition +!PARSE-TREE: | | | Properties +!PARSE-TREE: | | | | OmpTraitProperty -> Scalar -> Expr = '.true._4' +!PARSE-TREE: | | | | | LiteralConstant -> LogicalLiteralConstant +!PARSE-TREE: | | | | | | bool = 'true' + +subroutine f06 + implicit none + integer :: i + !$omp target_data map(tofrom: i) + i = 0 + !$omp end target data +end + +!UNPARSE: SUBROUTINE f06 +!UNPARSE: IMPLICIT NONE +!UNPARSE: INTEGER i +!UNPARSE: !$OMP TARGET DATA MAP(TOFROM: i) +!UNPARSE: i=0_4 +!UNPARSE: !$OMP END TARGET DATA +!UNPARSE: END SUBROUTINE + +!PARSE-TREE: ExecutionPartConstruct -> ExecutableConstruct -> OpenMPConstruct -> OpenMPBlockConstruct +!PARSE-TREE: | OmpBeginBlockDirective +!PARSE-TREE: | | OmpBlockDirective -> llvm::omp::Directive = target data +!PARSE-TREE: | | OmpClauseList -> OmpClause -> Map -> OmpMapClause +!PARSE-TREE: | | | Modifier -> OmpMapType -> Value = Tofrom +!PARSE-TREE: | | | OmpObjectList -> OmpObject -> Designator -> DataRef -> Name = 'i' +!PARSE-TREE: | | | bool = 'true' +!PARSE-TREE: | Block +!PARSE-TREE: | | ExecutionPartConstruct -> ExecutableConstruct -> ActionStmt -> AssignmentStmt = 'i=0_4' +!PARSE-TREE: | | | Variable = 'i' +!PARSE-TREE: | | | | Designator -> DataRef -> Name = 'i' +!PARSE-TREE: | | | Expr = '0_4' +!PARSE-TREE: | | | | LiteralConstant -> IntLiteralConstant = '0' +!PARSE-TREE: | OmpEndBlockDirective +!PARSE-TREE: | | OmpBlockDirective -> llvm::omp::Directive = target data +!PARSE-TREE: | | OmpClauseList -> + +subroutine f07 + implicit none + integer :: i + !$omp target_enter_data map(to: i) +end + +!UNPARSE: SUBROUTINE f07 +!UNPARSE: IMPLICIT NONE +!UNPARSE: INTEGER i +!UNPARSE: !$OMP TARGET_ENTER_DATA MAP(TO: i) +!UNPARSE: END SUBROUTINE + +!PARSE-TREE: ExecutionPartConstruct -> ExecutableConstruct -> OpenMPConstruct -> OpenMPStandaloneConstruct -> OpenMPSimpleStandaloneConstruct -> OmpDirectiveSpecification +!PARSE-TREE: | OmpDirectiveName -> llvm::omp::Directive = target enter data +!PARSE-TREE: | OmpClauseList -> OmpClause -> Map -> OmpMapClause +!PARSE-TREE: | | Modifier -> OmpMapType -> Value = To +!PARSE-TREE: | | OmpObjectList -> OmpObject -> Designator -> DataRef -> Name = 'i' +!PARSE-TREE: | | bool = 'true' +!PARSE-TREE: | Flags = None + +subroutine f08 + implicit none + integer :: i + !$omp target_exit_data map(from: i) +end + +!UNPARSE: SUBROUTINE f08 +!UNPARSE: IMPLICIT NONE +!UNPARSE: INTEGER i +!UNPARSE: !$OMP TARGET_EXIT_DATA MAP(FROM: i) +!UNPARSE: END SUBROUTINE + +!PARSE-TREE: ExecutionPartConstruct -> ExecutableConstruct -> OpenMPConstruct -> OpenMPStandaloneConstruct -> OpenMPSimpleStandaloneConstruct -> OmpDirectiveSpecification +!PARSE-TREE: | OmpDirectiveName -> llvm::omp::Directive = target exit data +!PARSE-TREE: | OmpClauseList -> OmpClause -> Map -> OmpMapClause +!PARSE-TREE: | | Modifier -> OmpMapType -> Value = From +!PARSE-TREE: | | OmpObjectList -> OmpObject -> Designator -> DataRef -> Name = 'i' +!PARSE-TREE: | | bool = 'true' +!PARSE-TREE: | Flags = None + +subroutine f09 + implicit none + integer :: i + !$omp target_update to(i) +end + +!UNPARSE: SUBROUTINE f09 +!UNPARSE: IMPLICIT NONE +!UNPARSE: INTEGER i +!UNPARSE: !$OMP TARGET_UPDATE TO(i) +!UNPARSE: END SUBROUTINE + +!PARSE-TREE: ExecutionPartConstruct -> ExecutableConstruct -> OpenMPConstruct -> OpenMPStandaloneConstruct -> OpenMPSimpleStandaloneConstruct -> OmpDirectiveSpecification +!PARSE-TREE: | OmpDirectiveName -> llvm::omp::Directive = target update +!PARSE-TREE: | OmpClauseList -> OmpClause -> To -> OmpToClause +!PARSE-TREE: | | OmpObjectList -> OmpObject -> Designator -> DataRef -> Name = 'i' +!PARSE-TREE: | | bool = 'true' +!PARSE-TREE: | Flags = None From 1dd6916ae6c7bd360e1c603b0cb9fd3b942233fc Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek Date: Wed, 9 Jul 2025 08:22:33 -0500 Subject: [PATCH 3/6] format --- flang/lib/Parser/openmp-parsers.cpp | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp index 29701a616d4b0..fdf96dffeb8ed 100644 --- a/flang/lib/Parser/openmp-parsers.cpp +++ b/flang/lib/Parser/openmp-parsers.cpp @@ -126,8 +126,8 @@ struct OmpDirectiveNameParser { using NameWithId = std::pair; using ConstIterator = std::vector::const_iterator; - llvm::iterator_range - directives_starting_with(char initial) const; + llvm::iterator_range directives_starting_with( + char initial) const; void initTokens(std::vector[]) const; }; @@ -161,9 +161,8 @@ void OmpDirectiveNameParser::initTokens(std::vector table[]) const { // order. This is to make sure that longer names are tried first, before // any potential prefix (e.g. "target update" before "target"). for (int initial{'a'}; initial != 'z' + 1; ++initial) { - llvm::stable_sort(table[initial - 'a'], [](auto &a, auto &b) { - return a.first.size() > b.first.size(); - }); + llvm::stable_sort(table[initial - 'a'], + [](auto &a, auto &b) { return a.first.size() > b.first.size(); }); } } From a69ffbae3a4eaa3cf4b0505808e693fcf416eb00 Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek Date: Wed, 9 Jul 2025 08:24:58 -0500 Subject: [PATCH 4/6] format --- flang/lib/Parser/openmp-parsers.cpp | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp index e8e1d5ac16e7a..ee88cce288cae 100644 --- a/flang/lib/Parser/openmp-parsers.cpp +++ b/flang/lib/Parser/openmp-parsers.cpp @@ -1533,10 +1533,9 @@ TYPE_PARSER(construct( construct(Parser{}))) // OpenMP 5.2: 7.5.4 Declare Variant directive -TYPE_PARSER(sourced( - construct( - verbatim("DECLARE VARIANT"_tok) || verbatim("DECLARE_VARIANT"_tok), - "(" >> maybe(name / ":"), name / ")", Parser{}))) +TYPE_PARSER(sourced(construct( + verbatim("DECLARE VARIANT"_tok) || verbatim("DECLARE_VARIANT"_tok), + "(" >> maybe(name / ":"), name / ")", Parser{}))) // 2.16 Declare Reduction Construct TYPE_PARSER(sourced(construct( @@ -1587,10 +1586,9 @@ TYPE_PARSER(applyFunction(ConstructOmpMapperSpecifier, maybe(name / ":" / !":"_tok), typeSpec / "::", name)) // OpenMP 5.2: 5.8.8 Declare Mapper Construct -TYPE_PARSER(sourced( - construct( - verbatim("DECLARE MAPPER"_tok) || verbatim("DECLARE_MAPPER"_tok), - parenthesized(Parser{}), Parser{}))) +TYPE_PARSER(sourced(construct( + verbatim("DECLARE MAPPER"_tok) || verbatim("DECLARE_MAPPER"_tok), + parenthesized(Parser{}), Parser{}))) TYPE_PARSER(construct(Parser{}) || construct(Parser{})) @@ -1635,10 +1633,9 @@ TYPE_PARSER(sourced(construct( TYPE_PARSER(construct(startOmpLine >> "END ALLOCATORS"_tok)) // 2.8.2 Declare Simd construct -TYPE_PARSER( - sourced(construct( - verbatim("DECLARE SIMD"_tok) || verbatim("DECLARE_SIMD"_tok), - maybe(parenthesized(name)), Parser{}))) +TYPE_PARSER(sourced(construct( + verbatim("DECLARE SIMD"_tok) || verbatim("DECLARE_SIMD"_tok), + maybe(parenthesized(name)), Parser{}))) // 2.4 Requires construct TYPE_PARSER(sourced(construct( From 7b42bbe446f366fa10e1fb6ecf237169a56400b0 Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek Date: Tue, 8 Jul 2025 18:06:48 -0500 Subject: [PATCH 5/6] [flang][OpenMP] Issue a warning when parsing future directive spelling OpenMP 6.0 introduced alternative spelling for some directives, with the previous spellings still allowed. Warn the user when a new spelling is encountered with OpenMP version set to an older value. --- flang/include/flang/Parser/parse-tree.h | 2 +- flang/lib/Parser/openmp-parsers.cpp | 4 +- flang/lib/Semantics/check-omp-structure.cpp | 181 ++++++++++++++++++ flang/lib/Semantics/check-omp-structure.h | 2 + .../OpenMP/future-directive-spellings.f90 | 92 +++++++++ 5 files changed, 278 insertions(+), 3 deletions(-) create mode 100644 flang/test/Semantics/OpenMP/future-directive-spellings.f90 diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h index 7e752eeb4dfe4..43954ff735361 100644 --- a/flang/include/flang/Parser/parse-tree.h +++ b/flang/include/flang/Parser/parse-tree.h @@ -4612,7 +4612,7 @@ struct OmpDirectiveSpecification { struct OmpMetadirectiveDirective { TUPLE_CLASS_BOILERPLATE(OmpMetadirectiveDirective); - std::tuple t; + std::tuple t; CharBlock source; }; diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp index ee88cce288cae..3016ce4ccd2f8 100644 --- a/flang/lib/Parser/openmp-parsers.cpp +++ b/flang/lib/Parser/openmp-parsers.cpp @@ -1194,7 +1194,7 @@ TYPE_PARSER(sourced(construct( sourced(Parser{})))))) TYPE_PARSER(sourced(construct( - "METADIRECTIVE" >> Parser{}))) + verbatim("METADIRECTIVE"_tok), Parser{}))) // Omp directives enclosing do loop TYPE_PARSER(sourced(construct(first( @@ -1687,7 +1687,7 @@ TYPE_PARSER(sourced(construct( verbatim("ASSUME"_tok), Parser{}))) TYPE_PARSER(sourced(construct( - verbatim(startOmpLine >> "END ASSUME"_tok)))) + startOmpLine >> verbatim("END ASSUME"_tok)))) TYPE_PARSER(sourced( construct(Parser{} / endOmpLine, diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp index aa4cdb9d27019..191f13cdeef65 100644 --- a/flang/lib/Semantics/check-omp-structure.cpp +++ b/flang/lib/Semantics/check-omp-structure.cpp @@ -259,6 +259,55 @@ void OmpStructureChecker::CheckVariableListItem( } } +void OmpStructureChecker::CheckDirectiveSpelling( + parser::CharBlock spelling, llvm::omp::Directive id) { + // Directive names that contain spaces can be spelled in the source without + // any of the spaces. Because of that getOpenMPKind* is not guaranteed to + // work with the source spelling as the argument. + // + // To verify the source spellings, we have to get the spelling for a given + // version, remove spaces and compare it with the source spelling (also + // with spaces removed). + auto removeSpaces = [](llvm::StringRef s) { + std::string n{s.str()}; + for (auto it{n.begin()}; it != n.end();) { + it = isspace(*it) ? n.erase(it) : std::next(it); + } + return n; + }; + + std::string lowerNoWS{removeSpaces( + parser::ToLowerCaseLetters({spelling.begin(), spelling.size()}))}; + llvm::StringRef ref(lowerNoWS); + if (ref.starts_with("end")) { + ref = ref.drop_front(3); + } + + unsigned version{context_.langOptions().OpenMPVersion}; + + // For every "future" version v, check if the check if the corresponding + // spelling of id was introduced later than the current version. If so, + // and if that spelling matches the source spelling, issue a warning. + for (unsigned v : llvm::omp::getOpenMPVersions()) { + if (v <= version) { + continue; + } + llvm::StringRef name{llvm::omp::getOpenMPDirectiveName(id, v)}; + auto [kind, versions]{llvm::omp::getOpenMPDirectiveKindAndVersions(name)}; + assert(kind == id && "Directive kind mismatch"); + + if (static_cast(version) >= versions.Min) { + continue; + } + if (ref == removeSpaces(name)) { + context_.Say(spelling, + "Directive spelling '%s' is introduced in a later OpenMP version, %s"_warn_en_US, + parser::ToUpperCaseLetters(ref), TryVersion(versions.Min)); + break; + } + } +} + void OmpStructureChecker::CheckMultipleOccurrence( semantics::UnorderedSymbolSet &listVars, const std::list &nameList, const parser::CharBlock &item, @@ -436,7 +485,133 @@ void OmpStructureChecker::Leave(const parser::OmpDirectiveSpecification &) { } } +template struct DirectiveSpellingVisitor { + using Directive = llvm::omp::Directive; + + DirectiveSpellingVisitor(Checker &&checker) : checker_(std::move(checker)) {} + + template bool Pre(const T &) { return true; } + template void Post(const T &) {} + + bool Pre(const parser::OmpSectionsDirective &x) { + checker_(x.source, x.v); + return false; + } + bool Pre(const parser::OpenMPDeclarativeAllocate &x) { + checker_(std::get(x.t).source, Directive::OMPD_allocate); + return false; + } + bool Pre(const parser::OmpDispatchDirective &x) { + checker_(std::get(x.t).source, Directive::OMPD_dispatch); + return false; + } + bool Pre(const parser::OmpErrorDirective &x) { + checker_(std::get(x.t).source, Directive::OMPD_error); + return false; + } + bool Pre(const parser::OmpNothingDirective &x) { + checker_(x.source, Directive::OMPD_nothing); + return false; + } + bool Pre(const parser::OpenMPExecutableAllocate &x) { + checker_(std::get(x.t).source, Directive::OMPD_allocate); + return false; + } + bool Pre(const parser::OpenMPAllocatorsConstruct &x) { + checker_( + std::get(x.t).source, Directive::OMPD_allocators); + return false; + } + bool Pre(const parser::OmpAssumeDirective &x) { + checker_(std::get(x.t).source, Directive::OMPD_assume); + return false; + } + bool Pre(const parser::OmpEndAssumeDirective &x) { + checker_(x.v.source, Directive::OMPD_assume); + return false; + } + bool Pre(const parser::OmpCriticalDirective &x) { + checker_(std::get(x.t).source, Directive::OMPD_critical); + return false; + } + bool Pre(const parser::OmpEndCriticalDirective &x) { + checker_(std::get(x.t).source, Directive::OMPD_critical); + return false; + } + bool Pre(const parser::OmpMetadirectiveDirective &x) { + checker_( + std::get(x.t).source, Directive::OMPD_metadirective); + return false; + } + bool Pre(const parser::OpenMPDeclarativeAssumes &x) { + checker_(std::get(x.t).source, Directive::OMPD_assumes); + return false; + } + bool Pre(const parser::OpenMPDeclareMapperConstruct &x) { + checker_( + std::get(x.t).source, Directive::OMPD_declare_mapper); + return false; + } + bool Pre(const parser::OpenMPDeclareReductionConstruct &x) { + checker_(std::get(x.t).source, + Directive::OMPD_declare_reduction); + return false; + } + bool Pre(const parser::OpenMPDeclareSimdConstruct &x) { + checker_( + std::get(x.t).source, Directive::OMPD_declare_simd); + return false; + } + bool Pre(const parser::OpenMPDeclareTargetConstruct &x) { + checker_( + std::get(x.t).source, Directive::OMPD_declare_target); + return false; + } + bool Pre(const parser::OmpDeclareVariantDirective &x) { + checker_(std::get(x.t).source, + Directive::OMPD_declare_variant); + return false; + } + bool Pre(const parser::OpenMPThreadprivate &x) { + checker_( + std::get(x.t).source, Directive::OMPD_threadprivate); + return false; + } + bool Pre(const parser::OpenMPRequiresConstruct &x) { + checker_(std::get(x.t).source, Directive::OMPD_requires); + return false; + } + + bool Pre(const parser::OmpBlockDirective &x) { + checker_(x.source, x.v); + return false; + } + + bool Pre(const parser::OmpLoopDirective &x) { + checker_(x.source, x.v); + return false; + } + + bool Pre(const parser::OmpDirectiveSpecification &x) { + auto &name = std::get(x.t); + checker_(name.source, name.v); + return false; + } + +private: + Checker checker_; +}; + +template +DirectiveSpellingVisitor(T &&) -> DirectiveSpellingVisitor; + void OmpStructureChecker::Enter(const parser::OpenMPConstruct &x) { + DirectiveSpellingVisitor visitor( + [this](parser::CharBlock source, llvm::omp::Directive id) { + return CheckDirectiveSpelling(source, id); + }); + parser::Walk(x, visitor); + // Simd Construct with Ordered Construct Nesting check // We cannot use CurrentDirectiveIsNested() here because // PushContextAndClauseSets() has not been called yet, it is @@ -461,6 +636,12 @@ void OmpStructureChecker::Leave(const parser::OpenMPConstruct &) { } void OmpStructureChecker::Enter(const parser::OpenMPDeclarativeConstruct &x) { + DirectiveSpellingVisitor visitor( + [this](parser::CharBlock source, llvm::omp::Directive id) { + return CheckDirectiveSpelling(source, id); + }); + parser::Walk(x, visitor); + EnterDirectiveNest(DeclarativeNest); } diff --git a/flang/lib/Semantics/check-omp-structure.h b/flang/lib/Semantics/check-omp-structure.h index 2a3853335fd1c..6a877a5d0a7c0 100644 --- a/flang/lib/Semantics/check-omp-structure.h +++ b/flang/lib/Semantics/check-omp-structure.h @@ -163,6 +163,8 @@ class OmpStructureChecker private: bool CheckAllowedClause(llvmOmpClause clause); void CheckVariableListItem(const SymbolSourceMap &symbols); + void CheckDirectiveSpelling( + parser::CharBlock spelling, llvm::omp::Directive id); void CheckMultipleOccurrence(semantics::UnorderedSymbolSet &listVars, const std::list &nameList, const parser::CharBlock &item, const std::string &clauseName); diff --git a/flang/test/Semantics/OpenMP/future-directive-spellings.f90 b/flang/test/Semantics/OpenMP/future-directive-spellings.f90 new file mode 100644 index 0000000000000..ed7c4d6782fbe --- /dev/null +++ b/flang/test/Semantics/OpenMP/future-directive-spellings.f90 @@ -0,0 +1,92 @@ +!RUN: %python %S/../test_errors.py %s %flang -fopenmp -fopenmp-version=52 -Werror + +! The directives to check: +! cancellation_point +! declare_mapper +! declare_reduction +! declare_simd +! declare_target +! declare_variant +! target_data +! target_enter_data +! target_exit_data +! target_update + +subroutine f00 + implicit none + integer :: i + + !$omp parallel + do i = 1, 10 +!WARNING: Directive spelling 'CANCELLATION_POINT' is introduced in a later OpenMP version, try -fopenmp-version=60 + !$omp cancellation_point parallel + enddo + !$omp end parallel +end + +subroutine f01 + type :: t + integer :: x + end type +!WARNING: Directive spelling 'DECLARE_MAPPER' is introduced in a later OpenMP version, try -fopenmp-version=60 + !$omp declare_mapper(t :: v) map(v%x) +end + +subroutine f02 + type :: t + integer :: x + end type +!WARNING: Directive spelling 'DECLARE_REDUCTION' is introduced in a later OpenMP version, try -fopenmp-version=60 + !$omp declare_reduction(+ : t : omp_out%x = omp_out%x + omp_in%x) +end + +subroutine f03 +!WARNING: Directive spelling 'DECLARE_SIMD' is introduced in a later OpenMP version, try -fopenmp-version=60 + !$omp declare_simd +end + +subroutine f04 +!WARNING: Directive spelling 'DECLARE_TARGET' is introduced in a later OpenMP version, try -fopenmp-version=60 + !$omp declare_target +end + +subroutine f05 + implicit none + interface + subroutine g05 + end + end interface +!WARNING: Directive spelling 'DECLARE_VARIANT' is introduced in a later OpenMP version, try -fopenmp-version=60 + !$omp declare_variant(g05) match(user={condition(.true.)}) +end + +subroutine f06 + implicit none + integer :: i +!WARNING: Directive spelling 'TARGET_DATA' is introduced in a later OpenMP version, try -fopenmp-version=60 + !$omp target_data map(tofrom: i) + i = 0 +!WARNING: Directive spelling 'TARGET_DATA' is introduced in a later OpenMP version, try -fopenmp-version=60 + !$omp end target_data +end + +subroutine f07 + implicit none + integer :: i +!WARNING: Directive spelling 'TARGET_ENTER_DATA' is introduced in a later OpenMP version, try -fopenmp-version=60 + !$omp target_enter_data map(to: i) +end + +subroutine f08 + implicit none + integer :: i +!WARNING: Directive spelling 'TARGET_EXIT_DATA' is introduced in a later OpenMP version, try -fopenmp-version=60 + !$omp target_exit_data map(from: i) +end + +subroutine f09 + implicit none + integer :: i +!WARNING: Directive spelling 'TARGET_UPDATE' is introduced in a later OpenMP version, try -fopenmp-version=60 + !$omp target_update to(i) +end From 2db790a01737ec31c5c8ed486e816d4cc7563e22 Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek Date: Thu, 10 Jul 2025 08:58:24 -0500 Subject: [PATCH 6/6] Iterate backwards --- flang/lib/Semantics/check-omp-structure.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp index 191f13cdeef65..89c1565bf66aa 100644 --- a/flang/lib/Semantics/check-omp-structure.cpp +++ b/flang/lib/Semantics/check-omp-structure.cpp @@ -270,8 +270,10 @@ void OmpStructureChecker::CheckDirectiveSpelling( // with spaces removed). auto removeSpaces = [](llvm::StringRef s) { std::string n{s.str()}; - for (auto it{n.begin()}; it != n.end();) { - it = isspace(*it) ? n.erase(it) : std::next(it); + for (size_t idx{n.size()}; idx > 0; --idx) { + if (isspace(n[idx - 1])) { + n.erase(idx - 1, 1); + } } return n; };