[flang][OpenMP] Parse METADIRECTIVE in specification part #123397

kparzysz · 2025-01-17T21:11:28Z

Add METADIRECTIVE to the OpenMP declarative constructs as well. Emit a TODO error for both declarative and executable cases.

A trait poperty can be one of serveral alternatives, and each property in a list was parsed as if it could be any of these alternatives independently from other properties. This made the parsing vulnerable to certain ambiguities in the trait grammar (provided in the OpenMP spec). At the same time the OpenMP spec gives the expected types of properties for almost every trait: all properties listed for a given trait are usually of the same type, e.g. names, clauses, etc. Incorporate these restrictions into the parser, and additionally use property extensions as the fallback if the parsing of the expected property type failed. This is intended to allow the parser to succeed, and instead let the semantic-checking code emit a more user-friendly message.

Parse METADIRECTIVE as a standalone executable directive at the moment. This will allow testing the parser code. There is no lowering, not even clause conversion yet. There is also no verification of the allowed values for trait sets, trait properties.

…sz/spr/m03-meta-simple

This implements checks of the validity of context set selectors and trait selectors, plus the types of trait properties. Clause properties are also validated, but not name or extension properties.

Add METADIRECTIVE to the OpenMP declarative constructs as well. Emit a TODO error for both declarative and executable cases.

llvmbot · 2025-01-17T21:12:03Z

@llvm/pr-subscribers-flang-openmp

@llvm/pr-subscribers-flang-parser

Author: Krzysztof Parzyszek (kparzysz)

Changes

Add METADIRECTIVE to the OpenMP declarative constructs as well. Emit a TODO error for both declarative and executable cases.

Full diff: https://github.com/llvm/llvm-project/pull/123397.diff

7 Files Affected:

(modified) flang/include/flang/Parser/parse-tree.h (+2-2)
(modified) flang/lib/Lower/OpenMP/OpenMP.cpp (+7-5)
(modified) flang/lib/Parser/openmp-parsers.cpp (+3-1)
(added) flang/test/Lower/OpenMP/Todo/metadirective-exec.f90 (+9)
(added) flang/test/Lower/OpenMP/Todo/metadirective-spec.f90 (+9)
(modified) flang/test/Parser/OpenMP/metadirective-v50.f90 (+2)
(modified) flang/test/Parser/OpenMP/metadirective.f90 (+53)

diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index 6053ad5dc0f7ad..2e27b6ea7eafa1 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -4537,8 +4537,8 @@ struct OpenMPDeclarativeConstruct {
   CharBlock source;
   std::variant<OpenMPDeclarativeAllocate, OpenMPDeclareMapperConstruct,
       OpenMPDeclareReductionConstruct, OpenMPDeclareSimdConstruct,
-      OpenMPDeclareTargetConstruct, OpenMPThreadprivate,
-      OpenMPRequiresConstruct, OpenMPUtilityConstruct>
+      OpenMPThreadprivate, OpenMPRequiresConstruct, OpenMPUtilityConstruct,
+      OpenMPDeclareTargetConstruct, OmpMetadirectiveDirective>
       u;
 };
 
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index 3a4336fe5b90f9..debab2352abd0a 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -3137,6 +3137,13 @@ static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
   // support the case of threadprivate variable declared in module.
 }
 
+static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
+                   semantics::SemanticsContext &semaCtx,
+                   lower::pft::Evaluation &eval,
+                   const parser::OmpMetadirectiveDirective &meta) {
+  TODO(converter.getCurrentLocation(), "METADIRECTIVE");
+}
+
 static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
                    semantics::SemanticsContext &semaCtx,
                    lower::pft::Evaluation &eval,
@@ -3229,11 +3236,6 @@ static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
   TODO(converter.getCurrentLocation(), "OpenMPDepobjConstruct");
 }
 
-static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
-                   semantics::SemanticsContext &semaCtx,
-                   lower::pft::Evaluation &eval,
-                   const parser::OmpMetadirectiveDirective &construct) {}
-
 static void
 genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
        semantics::SemanticsContext &semaCtx, lower::pft::Evaluation &eval,
diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp
index e3c9292bc5f91e..f5387dcf4b3c3d 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -1283,7 +1283,9 @@ TYPE_PARSER(startOmpLine >>
             construct<OpenMPDeclarativeConstruct>(
                 Parser<OpenMPThreadprivate>{}) ||
             construct<OpenMPDeclarativeConstruct>(
-                Parser<OpenMPUtilityConstruct>{})) /
+                Parser<OpenMPUtilityConstruct>{}) ||
+            construct<OpenMPDeclarativeConstruct>(
+                Parser<OmpMetadirectiveDirective>{})) /
             endOmpLine))
 
 // Block Construct
diff --git a/flang/test/Lower/OpenMP/Todo/metadirective-exec.f90 b/flang/test/Lower/OpenMP/Todo/metadirective-exec.f90
new file mode 100644
index 00000000000000..2e160a18966162
--- /dev/null
+++ b/flang/test/Lower/OpenMP/Todo/metadirective-exec.f90
@@ -0,0 +1,9 @@
+!RUN: %not_todo_cmd bbc -emit-hlfir -fopenmp -fopenmp-version=52 -o - %s 2>&1 | FileCheck %s
+!RUN: %not_todo_cmd %flang_fc1 -emit-hlfir -fopenmp -fopenmp-version=52 -o - %s 2>&1 | FileCheck %s
+
+!CHECK: not yet implemented: METADIRECTIVE
+subroutine f00
+  continue
+  !Executable
+  !$omp metadirective when(user={condition(.true.)}: nothing)
+end
diff --git a/flang/test/Lower/OpenMP/Todo/metadirective-spec.f90 b/flang/test/Lower/OpenMP/Todo/metadirective-spec.f90
new file mode 100644
index 00000000000000..a00612a92218a2
--- /dev/null
+++ b/flang/test/Lower/OpenMP/Todo/metadirective-spec.f90
@@ -0,0 +1,9 @@
+!RUN: %not_todo_cmd bbc -emit-hlfir -fopenmp -fopenmp-version=52 -o - %s 2>&1 | FileCheck %s
+!RUN: %not_todo_cmd %flang_fc1 -emit-hlfir -fopenmp -fopenmp-version=52 -o - %s 2>&1 | FileCheck %s
+
+!CHECK: not yet implemented: METADIRECTIVE
+subroutine f00
+  !Specification
+  !$omp metadirective when(user={condition(.true.)}: nothing)
+  implicit none
+end
diff --git a/flang/test/Parser/OpenMP/metadirective-v50.f90 b/flang/test/Parser/OpenMP/metadirective-v50.f90
index 73d5077da3d9f1..d7c3121b8f1b80 100644
--- a/flang/test/Parser/OpenMP/metadirective-v50.f90
+++ b/flang/test/Parser/OpenMP/metadirective-v50.f90
@@ -2,12 +2,14 @@
 !RUN: %flang_fc1 -fdebug-dump-parse-tree -fopenmp -fopenmp-version=50 %s | FileCheck --check-prefix="PARSE-TREE" %s
 
 subroutine f01
+  continue
   !$omp metadirective &
   !$omp & when(user={condition(.true.)}: nothing) &
   !$omp & default(nothing)
 end
 
 !UNPARSE: SUBROUTINE f01
+!UNPARSE:  CONTINUE
 !UNPARSE: !$OMP METADIRECTIVE  WHEN(USER={CONDITION(.true._4)}: NOTHING) DEFAULT(NOTHING)
 !UNPARSE: END SUBROUTINE
 
diff --git a/flang/test/Parser/OpenMP/metadirective.f90 b/flang/test/Parser/OpenMP/metadirective.f90
index 359f0d8be7a65e..4da5c05e65f359 100644
--- a/flang/test/Parser/OpenMP/metadirective.f90
+++ b/flang/test/Parser/OpenMP/metadirective.f90
@@ -2,10 +2,12 @@
 !RUN: %flang_fc1 -fdebug-dump-parse-tree -fopenmp -fopenmp-version=52 %s | FileCheck --check-prefix="PARSE-TREE" %s
 
 subroutine f00
+  continue
   !$omp metadirective when(construct={target, parallel}: nothing)
 end
 
 !UNPARSE: SUBROUTINE f00
+!UNPARSE:  CONTINUE
 !UNPARSE: !$OMP METADIRECTIVE  WHEN(CONSTRUCT={TARGET, PARALLEL}: NOTHING)
 !UNPARSE: END SUBROUTINE
 
@@ -22,10 +24,12 @@ subroutine f00
 !PARSE-TREE: | | | OmpClauseList ->
 
 subroutine f01
+  continue
   !$omp metadirective when(target_device={kind(host), device_num(1)}: nothing)
 end
 
 !UNPARSE: SUBROUTINE f01
+!UNPARSE:  CONTINUE
 !UNPARSE: !$OMP METADIRECTIVE  WHEN(TARGET_DEVICE={KIND(host), DEVICE_NUM(1_4)}: NOTHING)
 !UNPARSE: END SUBROUTINE
 
@@ -47,10 +51,12 @@ subroutine f01
 !PARSE-TREE: | | | OmpClauseList ->
 
 subroutine f02
+  continue
   !$omp metadirective when(target_device={kind(any), device_num(7)}: nothing)
 end
 
 !UNPARSE: SUBROUTINE f02
+!UNPARSE:  CONTINUE
 !UNPARSE: !$OMP METADIRECTIVE  WHEN(TARGET_DEVICE={KIND(any), DEVICE_NUM(7_4)}: NOTHING)
 !UNPARSE: END SUBROUTINE
 
@@ -72,11 +78,13 @@ subroutine f02
 !PARSE-TREE: | | | OmpClauseList ->
 
 subroutine f03
+  continue
   !$omp metadirective &
   !$omp & when(implementation={atomic_default_mem_order(acq_rel)}: nothing)
 end
 
 !UNPARSE: SUBROUTINE f03
+!UNPARSE:  CONTINUE
 !UNPARSE: !$OMP METADIRECTIVE  WHEN(IMPLEMENTATION={ATOMIC_DEFAULT_MEM_ORDER(ACQ_REL)}: &
 !UNPARSE: !$OMP&NOTHING)
 !UNPARSE: END SUBROUTINE
@@ -94,11 +102,13 @@ subroutine f03
 !PARSE-TREE: | | | OmpClauseList ->
 
 subroutine f04
+  continue
   !$omp metadirective &
   !$omp when(implementation={extension_trait(haha(1), foo(baz, "bar"(1)))}: nothing)
 end
 
 !UNPARSE: SUBROUTINE f04
+!UNPARSE:  CONTINUE
 !UNPARSE: !$OMP METADIRECTIVE  WHEN(IMPLEMENTATION={extension_trait(haha(1_4), foo(baz,bar(1_4&
 !UNPARSE: !$OMP&)))}: NOTHING)
 !UNPARSE: END SUBROUTINE
@@ -127,6 +137,7 @@ subroutine f04
 
 subroutine f05(x)
   integer :: x
+  continue
   !$omp metadirective &
   !$omp & when(user={condition(score(100): .true.)}: &
   !$omp &    parallel do reduction(+: x)) &
@@ -137,6 +148,7 @@ subroutine f05(x)
 
 !UNPARSE: SUBROUTINE f05 (x)
 !UNPARSE:  INTEGER x
+!UNPARSE:  CONTINUE
 !UNPARSE: !$OMP METADIRECTIVE  WHEN(USER={CONDITION(SCORE(100_4): .true._4)}: PARALLEL DO REDUCTION(+&
 !UNPARSE: !$OMP&: x)) OTHERWISE(NOTHING)
 !UNPARSE:  DO i=1_4,10_4
@@ -165,6 +177,7 @@ subroutine f05(x)
 !PARSE-TREE: | | OmpClauseList ->
 
 subroutine f06
+  continue
   ! Two trait set selectors
   !$omp metadirective &
   !$omp & when(implementation={vendor("amd")}, &
@@ -172,6 +185,7 @@ subroutine f06
 end
 
 !UNPARSE: SUBROUTINE f06
+!UNPARSE:  CONTINUE
 !UNPARSE: !$OMP METADIRECTIVE  WHEN(IMPLEMENTATION={VENDOR(amd)}, USER={CONDITION(.true._4)}: NO&
 !UNPARSE: !$OMP&THING)
 !UNPARSE: END SUBROUTINE
@@ -196,3 +210,42 @@ subroutine f06
 !PARSE-TREE: | | | llvm::omp::Directive = nothing
 !PARSE-TREE: | | | OmpClauseList ->
 
+subroutine f07
+  ! Declarative metadirective
+  !$omp metadirective &
+  !$omp & when(implementation={vendor("amd")}: declare simd) &
+  !$omp & when(user={condition(.true.)}: declare target) &
+  !$omp & otherwise(nothing)
+end
+
+!UNPARSE: SUBROUTINE f07
+!UNPARSE: !$OMP METADIRECTIVE  WHEN(IMPLEMENTATION={VENDOR(amd)}: DECLARE SIMD) WHEN(USE&
+!UNPARSE: !$OMP&R={CONDITION(.true._4)}: DECLARE TARGET) OTHERWISE(NOTHING)
+!UNPARSE: END SUBROUTINE
+
+!PARSE-TREE: OpenMPDeclarativeConstruct -> OmpMetadirectiveDirective
+!PARSE-TREE: | OmpClauseList -> OmpClause -> When -> OmpWhenClause
+!PARSE-TREE: | | Modifier -> OmpContextSelectorSpecification -> OmpTraitSetSelector
+!PARSE-TREE: | | | OmpTraitSetSelectorName -> Value = Implementation
+!PARSE-TREE: | | | OmpTraitSelector
+!PARSE-TREE: | | | | OmpTraitSelectorName -> Value = Vendor
+!PARSE-TREE: | | | | Properties
+!PARSE-TREE: | | | | | OmpTraitProperty -> OmpTraitPropertyName -> string = 'amd'
+!PARSE-TREE: | | OmpDirectiveSpecification
+!PARSE-TREE: | | | llvm::omp::Directive = declare simd
+!PARSE-TREE: | | | OmpClauseList ->
+!PARSE-TREE: | OmpClause -> When -> OmpWhenClause
+!PARSE-TREE: | | Modifier -> 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'
+!PARSE-TREE: | | OmpDirectiveSpecification
+!PARSE-TREE: | | | llvm::omp::Directive = declare target
+!PARSE-TREE: | | | OmpClauseList ->
+!PARSE-TREE: | OmpClause -> Otherwise -> OmpOtherwiseClause -> OmpDirectiveSpecification
+!PARSE-TREE: | | llvm::omp::Directive = nothing
+!PARSE-TREE: | | OmpClauseList ->

kiranchandramohan

LG.

The specification part has to be emitted in module files. But this is not necessary for producing the TODOs.

tblah

LGTM, thanks!

kiranchandramohan

LG.

Declarative directives have to be propagated to module files but for the purpose of generating TODOs, this is not required.

Add METADIRECTIVE to the OpenMP declarative constructs as well. Emit a TODO error for both declarative and executable cases.

kparzysz added 14 commits January 14, 2025 06:50

format/comment

c44051d

Merge branch 'users/kparzysz/spr/m02-context-parse' into users/kparzy…

909b251

…sz/spr/m03-meta-simple

add "sourced"

13740f9

Merge branch 'users/kparzysz/spr/m02-context-parse' into users/kparzy…

3d88aad

…sz/spr/m03-meta-simple

fix parser

527315f

Merge branch 'main' into users/kparzysz/spr/m02-context-parse

d68017e

Merge branch 'users/kparzysz/spr/m02-context-parse' into users/kparzy…

078ab71

…sz/spr/m03-meta-simple

add missing visitor for OmpMetadirectiveDirective

4c5b3ce

support for default

205a760

[flang][OpenMP] Semantic checks for context selectors

109df33

This implements checks of the validity of context set selectors and trait selectors, plus the types of trait properties. Clause properties are also validated, but not name or extension properties.

format

14fe4f2

[flang][OpenMP] Parse METADIRECTIVE in specification part

7fa026c

Add METADIRECTIVE to the OpenMP declarative constructs as well. Emit a TODO error for both declarative and executable cases.

kparzysz requested review from tblah, kiranchandramohan and Leporacanthicus January 17, 2025 21:11

llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir flang:openmp flang:parser labels Jan 17, 2025

fix example

7c2a3be

kiranchandramohan approved these changes Jan 23, 2025

View reviewed changes

kparzysz mentioned this pull request Jan 24, 2025

[Flang][OpenMP] Important issues to be fixed prior to removing experimental status of OpenMP #110008

Open

85 tasks

tblah approved these changes Feb 3, 2025

View reviewed changes

kiranchandramohan approved these changes Feb 3, 2025

View reviewed changes

Base automatically changed from users/kparzysz/spr/m04-sema-checks to main February 3, 2025 15:48

kparzysz added 2 commits February 3, 2025 10:00

Merge branch 'main' into users/kparzysz/spr/m05-spec-todo

cfaa7c5

Remove duplicated function

b38fb1d

kparzysz merged commit 6dfe20d into main Feb 3, 2025
8 checks passed

kparzysz deleted the users/kparzysz/spr/m05-spec-todo branch February 3, 2025 17:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[flang][OpenMP] Parse METADIRECTIVE in specification part #123397

[flang][OpenMP] Parse METADIRECTIVE in specification part #123397

Uh oh!

kparzysz commented Jan 17, 2025

Uh oh!

llvmbot commented Jan 17, 2025 •

edited

Loading

Uh oh!

kiranchandramohan left a comment

Uh oh!

tblah left a comment

Uh oh!

kiranchandramohan left a comment

Uh oh!

Uh oh!

Uh oh!

[flang][OpenMP] Parse METADIRECTIVE in specification part #123397

[flang][OpenMP] Parse METADIRECTIVE in specification part #123397

Uh oh!

Conversation

kparzysz commented Jan 17, 2025

Uh oh!

llvmbot commented Jan 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kiranchandramohan left a comment

Choose a reason for hiding this comment

Uh oh!

tblah left a comment

Choose a reason for hiding this comment

Uh oh!

kiranchandramohan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

llvmbot commented Jan 17, 2025 •

edited

Loading