Skip to content

Conversation

kparzysz
Copy link
Contributor

@kparzysz kparzysz commented Jul 2, 2024

This removes mentions of specific combined directives.

Also, add a quote from the OpenMP spec to explain the code dealing with the bind clause.

This removes mentions of specific combined directives.

Also, add a quote from the OpenMP spec to explain the code dealing
with the `bind` clause.
@kparzysz kparzysz requested a review from alexey-bataev July 2, 2024 17:19
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:openmp OpenMP related changes to Clang labels Jul 2, 2024
@llvmbot
Copy link
Member

llvmbot commented Jul 2, 2024

@llvm/pr-subscribers-clang

Author: Krzysztof Parzyszek (kparzysz)

Changes

This removes mentions of specific combined directives.

Also, add a quote from the OpenMP spec to explain the code dealing with the bind clause.


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

1 Files Affected:

  • (modified) clang/lib/Sema/SemaOpenMP.cpp (+8-4)
diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index 86666f064f35d..ca7e8acd1c15a 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -6270,16 +6270,20 @@ bool SemaOpenMP::mapLoopConstruct(
     if (BindKind == OMPC_BIND_unknown) {
       // Setting the enclosing teams or parallel construct for the loop
       // directive without bind clause.
+      // [5.0:129:25-28] If the bind clause is not present on the construct and
+      // the loop construct is closely nested inside a teams or parallel
+      // construct, the binding region is the corresponding teams or parallel
+      // region. If none of those conditions hold, the binding region is not
+      // defined.
       BindKind = OMPC_BIND_thread; // Default bind(thread) if binding is unknown
+      auto ParentLeafs = getLeafConstructsOrSelf(ParentDirective);
 
       if (ParentDirective == OMPD_unknown) {
         Diag(DSAStack->getDefaultDSALocation(),
              diag::err_omp_bind_required_on_loop);
-      } else if (ParentDirective == OMPD_parallel ||
-                 ParentDirective == OMPD_target_parallel) {
+      } else if (ParentLeafs.back() == OMPD_parallel) {
         BindKind = OMPC_BIND_parallel;
-      } else if (ParentDirective == OMPD_teams ||
-                 ParentDirective == OMPD_target_teams) {
+      } else if (ParentLeafs.back() == OMPD_teams) {
         BindKind = OMPC_BIND_teams;
       }
     } else {

Copy link
Member

@alexey-bataev alexey-bataev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LG with a nit

BindKind = OMPC_BIND_thread; // Default bind(thread) if binding is unknown
auto ParentLeafs = getLeafConstructsOrSelf(ParentDirective);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expand auto here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@kparzysz kparzysz merged commit acaa026 into llvm:main Jul 3, 2024
@kparzysz kparzysz deleted the users/kparzysz/c04-map-loop-leafs branch July 3, 2024 12:57
cpiaseque pushed a commit to cpiaseque/llvm-project that referenced this pull request Jul 3, 2024
This removes mentions of specific combined directives.

Also, add a quote from the OpenMP spec to explain the code dealing with
the `bind` clause.
kbluck pushed a commit to kbluck/llvm-project that referenced this pull request Jul 6, 2024
This removes mentions of specific combined directives.

Also, add a quote from the OpenMP spec to explain the code dealing with
the `bind` clause.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:openmp OpenMP related changes to Clang clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants