[OpenMP] Allow dynamic condition selector in Metadirective

[robincaloudis]

As reported in #82754, Metadirectives uses default for non-const user condition. This is unexpected as the OpenMP specification 5.1 allows dynamic condition selector within Metadirectives. In contrast static condition selectors work as expected.

Example (from #82754):

int non_const_val = 1; // A non const value makes the `condition` selector dynamic; a const value makes it static
#pragma omp metadirective \
    when( user= {condition(non_const_val > 0)} : parallel num_threads( num_threads ) ) \
    default()
    {
        #pragma omp single
        assert( num_threads == omp_get_num_threads() );
    } 

where as user in user= {...} is called selector set and condition in condition(non_const_val > 0) is called selector (must evaluate to true for the selector to be true).

Some context

As of OpenMP 5.1, dynamic condition selectors seem to be allowed

"[...] Any non-constant expression that is evaluated to determine the suitability of a variant is evaluated according to the data state trait in the dynamic trait set of the OpenMP context. The user selector set is dynamic if the condition selector is present and the expression in the condition selector is not a constant expression; otherwise, it is static. [...]"

Assembled grammar (copied from OpenMP 5.1. specification on Metadirectives and Context Selectors):

This helps a lot when browsing through the code and its vocabulary.

#pragma omp metadirective [clause[ [,] clause] ... ] new-line
// where clause is one of the following:  
when(context-selector-specification: [directive-variant]) 
default([directive-variant])
// where context-selector-specification is
context-selector-specification: 
   trait-set-selector[,trait-set-selector[,...]] 
 // where trait-set-selector is
trait-set-selector: 
   trait-set-selector-name={trait-selector[, trait-selector[, ...]]} 

Specification of Metadirectives in OpenMP 5.1

Closes: #82754

[llvmbot]

@llvm/pr-subscribers-clang

Author: Robin Caloudis (robincaloudis)

Changes

As reported in #82754, Metadirectives uses default for non-const user condition. This is unexpected as the OpenMP specification 5.1 allows dynamic condition selector within Metadirectives. In contrast static condition selector work.

Example:

int non_const_val = 1; // A non const value makes the `condition` selector dynamic; a const value makes it static
#pragma omp metadirective \
    when( user= {condition(non_const_val > 0)} : parallel num_threads( num_threads ) ) \
    default()
    {
        #pragma omp single
        assert( num_threads == omp_get_num_threads() );
    } 

where as user is called selector set and condition is called selector (must evaluate to true for the selector to be true).

Background informations

As of OpenMP 5.1, dynamic condition selectors seem to be allowed

"[...] Any non-constant expression that is evaluated to determine the suitability of a variant is evaluated according to the data state trait in the dynamic trait set of the OpenMP context. The user selector set is dynamic if the condition selector is present and the expression in the condition selector is not a constant expression; otherwise, it is static. [...]"

Assembled grammar (copied from OpenMP 5.1. specification on Metadirectives and Context Selectors):

There is a change that wrong semantics (meaning) are attached against the trait-selector when it is a dynamic condition selector

#pragma omp metadirective [clause[ [,] clause] ... ] new-line
// where clause is one of the following:  
when(context-selector-specification: [directive-variant]) 
default([directive-variant])
// where context-selector-specification is
context-selector-specification: 
   trait-set-selector[,trait-set-selector[,...]] 
 // where trait-set-selector is
trait-set-selector: 
   trait-set-selector-name={trait-selector[, trait-selector[, ...]]} 

Specification of Metadirectives in OpenMP 5.1

Issue: #82754

---

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

1 Files Affected:

• (modified) clang/test/OpenMP/metadirective_ast_print.c (+8)

diff --git a/clang/test/OpenMP/metadirective_ast_print.c b/clang/test/OpenMP/metadirective_ast_print.c
index d9ff7e76452160..afc150610f197b 100644
--- a/clang/test/OpenMP/metadirective_ast_print.c
+++ b/clang/test/OpenMP/metadirective_ast_print.c
@@ -77,6 +77,11 @@ void foo(void) {
                                : parallel) default(nothing)
   for (int i = 0; i < 16; i++)
     ;
+
+  int non_const_val = 1;
+#pragma omp metadirective when(user = {condition(non_const_val > 0)} \
+                               : parallel) default(nothing)
+  bar();
 }
 
 // CHECK: void bar(void);
@@ -107,5 +112,8 @@ void foo(void) {
 // CHECK-AMDGCN-NEXT: for (int i = 0; i < 100; i++)
 // CHECK: for (int i = 0; i < 16; i++)
 // CHECK: for (int i = 0; i < 16; i++)
+// CHECK: int non_const_val = 1;
+// CHECK-NEXT: #pragma omp parallel
+// CHECK-NEXT: bar()
 
 #endif

[github-actions]

✅ With the latest revision this PR passed the Python code formatter.

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[robincaloudis]

From what I understood, I'd say that there is a chance that wrong semantics are attached against the trait-selector when it is a dynamic condition selector, i.e. condition(non_const_val > 0). I'd base my argumentation on the fact that static condition selector do work. @shiltian, what do you think? Any other idea?

Code where selector-set (i.e. user={}) is parsed: https://github.com/llvm/llvm-project/blob/main/clang/lib/Parse/ParseOpenMP.cpp#L2649-L2657

Code where selector(i.e. condition(non_const_val > 0)) is parsed: https://github.com/llvm/llvm-project/blob/main/clang/lib/Parse/ParseOpenMP.cpp#L1185-L1225

[robincaloudis]

Actual behavior: #pragma omp parallel is not in the AST as the default for non-const user condition is used

Expected: #pragma omp parallel should be in the AST as dynamic user conditions that are resolved during runtime are fully specified by OpenMP 5.1

[shiltian]

I'm not familiar with that section of code. Maybe @jdoerfert could give you more insights.