Skip to content

[clang][OpenMP] Remove compound directives from checkNestingOfRegions #98387

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jul 15, 2024

Conversation

kparzysz
Copy link
Contributor

Express the constraints via constituent directives.

kparzysz added 2 commits July 10, 2024 14:49
The previous check was inconsistent. For example, it would allow
```
#pragma omp target
#pragma omp parallel for
  for (...) {
#pragma omp scan
  }
```
but not
```
#pragma omp target parallel for
  for (...) {
#pragma omp scan
  }
```

Make the check conform to the wording on the specification.
Express the constraints via constituent directives.
@kparzysz kparzysz requested a review from alexey-bataev July 10, 2024 21:08
@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 10, 2024
@llvmbot
Copy link
Member

llvmbot commented Jul 10, 2024

@llvm/pr-subscribers-clang

Author: Krzysztof Parzyszek (kparzysz)

Changes

Express the constraints via constituent directives.


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

1 Files Affected:

  • (modified) clang/lib/Sema/SemaOpenMP.cpp (+53-54)
diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index ef09e53077f47..5861923582eb1 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -4797,6 +4797,12 @@ static bool checkNestingOfRegions(Sema &SemaRef, const DSAStackTy *Stack,
     ShouldBeInTeamsRegion,
     ShouldBeInLoopSimdRegion,
   } Recommend = NoRecommend;
+
+  SmallVector<OpenMPDirectiveKind, 4> LeafOrComposite;
+  ArrayRef<OpenMPDirectiveKind> ParentLOC =
+      getLeafOrCompositeConstructs(ParentRegion, LeafOrComposite);
+  OpenMPDirectiveKind EnclosingConstruct = ParentLOC.back();
+
   if (SemaRef.LangOpts.OpenMP >= 51 && Stack->isParentOrderConcurrent() &&
       CurrentRegion != OMPD_simd && CurrentRegion != OMPD_loop &&
       CurrentRegion != OMPD_parallel &&
@@ -4805,6 +4811,7 @@ static bool checkNestingOfRegions(Sema &SemaRef, const DSAStackTy *Stack,
         << getOpenMPDirectiveName(CurrentRegion);
     return true;
   }
+
   if (isOpenMPSimdDirective(ParentRegion) &&
       ((SemaRef.LangOpts.OpenMP <= 45 && CurrentRegion != OMPD_ordered) ||
        (SemaRef.LangOpts.OpenMP >= 50 && CurrentRegion != OMPD_ordered &&
@@ -4828,19 +4835,20 @@ static bool checkNestingOfRegions(Sema &SemaRef, const DSAStackTy *Stack,
         << (SemaRef.LangOpts.OpenMP >= 50 ? 1 : 0);
     return CurrentRegion != OMPD_simd;
   }
-  if (ParentRegion == OMPD_atomic) {
+
+  if (EnclosingConstruct == OMPD_atomic) {
     // OpenMP [2.16, Nesting of Regions]
     // OpenMP constructs may not be nested inside an atomic region.
     SemaRef.Diag(StartLoc, diag::err_omp_prohibited_region_atomic);
     return true;
   }
+
   if (CurrentRegion == OMPD_section) {
     // OpenMP [2.7.2, sections Construct, Restrictions]
     // Orphaned section directives are prohibited. That is, the section
     // directives must appear within the sections construct and must not be
     // encountered elsewhere in the sections region.
-    if (ParentRegion != OMPD_sections &&
-        ParentRegion != OMPD_parallel_sections) {
+    if (EnclosingConstruct != OMPD_sections) {
       SemaRef.Diag(StartLoc, diag::err_omp_orphaned_section_directive)
           << (ParentRegion != OMPD_unknown)
           << getOpenMPDirectiveName(ParentRegion);
@@ -4848,6 +4856,7 @@ static bool checkNestingOfRegions(Sema &SemaRef, const DSAStackTy *Stack,
     }
     return false;
   }
+
   // Allow some constructs (except teams and cancellation constructs) to be
   // orphaned (they could be used in functions, called from OpenMP regions
   // with the required preconditions).
@@ -4856,18 +4865,20 @@ static bool checkNestingOfRegions(Sema &SemaRef, const DSAStackTy *Stack,
       CurrentRegion != OMPD_cancellation_point &&
       CurrentRegion != OMPD_cancel && CurrentRegion != OMPD_scan)
     return false;
+
   // Checks needed for mapping "loop" construct. Please check mapLoopConstruct
   // for a detailed explanation
   if (SemaRef.LangOpts.OpenMP >= 50 && CurrentRegion == OMPD_loop &&
       (BindKind == OMPC_BIND_parallel || BindKind == OMPC_BIND_teams) &&
       (isOpenMPWorksharingDirective(ParentRegion) ||
-       ParentRegion == OMPD_loop)) {
+       EnclosingConstruct == OMPD_loop)) {
     int ErrorMsgNumber = (BindKind == OMPC_BIND_parallel) ? 1 : 4;
     SemaRef.Diag(StartLoc, diag::err_omp_prohibited_region)
         << true << getOpenMPDirectiveName(ParentRegion) << ErrorMsgNumber
         << getOpenMPDirectiveName(CurrentRegion);
     return true;
   }
+
   if (CurrentRegion == OMPD_cancellation_point ||
       CurrentRegion == OMPD_cancel) {
     // OpenMP [2.16, Nesting of Regions]
@@ -4881,27 +4892,17 @@ static bool checkNestingOfRegions(Sema &SemaRef, const DSAStackTy *Stack,
     // construct-type-clause is not taskgroup must be closely nested inside an
     // OpenMP construct that matches the type specified in
     // construct-type-clause.
-    NestingProhibited =
-        !((CancelRegion == OMPD_parallel &&
-           (ParentRegion == OMPD_parallel ||
-            ParentRegion == OMPD_target_parallel)) ||
-          (CancelRegion == OMPD_for &&
-           (ParentRegion == OMPD_for || ParentRegion == OMPD_parallel_for ||
-            ParentRegion == OMPD_target_parallel_for ||
-            ParentRegion == OMPD_distribute_parallel_for ||
-            ParentRegion == OMPD_teams_distribute_parallel_for ||
-            ParentRegion == OMPD_target_teams_distribute_parallel_for)) ||
-          (CancelRegion == OMPD_taskgroup &&
-           (ParentRegion == OMPD_task ||
-            (SemaRef.getLangOpts().OpenMP >= 50 &&
-             (ParentRegion == OMPD_taskloop ||
-              ParentRegion == OMPD_master_taskloop ||
-              ParentRegion == OMPD_masked_taskloop ||
-              ParentRegion == OMPD_parallel_masked_taskloop ||
-              ParentRegion == OMPD_parallel_master_taskloop)))) ||
-          (CancelRegion == OMPD_sections &&
-           (ParentRegion == OMPD_section || ParentRegion == OMPD_sections ||
-            ParentRegion == OMPD_parallel_sections)));
+    ArrayRef<OpenMPDirectiveKind> Leafs = getLeafConstructsOrSelf(ParentRegion);
+    if (CancelRegion == OMPD_taskgroup) {
+      NestingProhibited = EnclosingConstruct != OMPD_task &&
+                          (SemaRef.getLangOpts().OpenMP < 50 ||
+                           EnclosingConstruct != OMPD_taskloop);
+    } else if (CancelRegion == OMPD_sections) {
+      NestingProhibited = EnclosingConstruct != OMPD_section &&
+                          EnclosingConstruct != OMPD_sections;
+    } else {
+      NestingProhibited = CancelRegion != Leafs.back();
+    }
     OrphanSeen = ParentRegion == OMPD_unknown;
   } else if (CurrentRegion == OMPD_master || CurrentRegion == OMPD_masked) {
     // OpenMP 5.1 [2.22, Nesting of Regions]
@@ -4942,13 +4943,12 @@ static bool checkNestingOfRegions(Sema &SemaRef, const DSAStackTy *Stack,
     // OpenMP 5.1 [2.22, Nesting of Regions]
     // A barrier region may not be closely nested inside a worksharing, loop,
     // task, taskloop, critical, ordered, atomic, or masked region.
-    NestingProhibited =
-        isOpenMPWorksharingDirective(ParentRegion) ||
-        isOpenMPGenericLoopDirective(ParentRegion) ||
-        isOpenMPTaskingDirective(ParentRegion) || ParentRegion == OMPD_master ||
-        ParentRegion == OMPD_masked || ParentRegion == OMPD_parallel_master ||
-        ParentRegion == OMPD_parallel_masked || ParentRegion == OMPD_critical ||
-        ParentRegion == OMPD_ordered;
+    NestingProhibited = isOpenMPWorksharingDirective(ParentRegion) ||
+                        isOpenMPGenericLoopDirective(ParentRegion) ||
+                        isOpenMPTaskingDirective(ParentRegion) ||
+                        llvm::is_contained({OMPD_masked, OMPD_master,
+                                            OMPD_critical, OMPD_ordered},
+                                           EnclosingConstruct);
   } else if (isOpenMPWorksharingDirective(CurrentRegion) &&
              !isOpenMPParallelDirective(CurrentRegion) &&
              !isOpenMPTeamsDirective(CurrentRegion)) {
@@ -4956,13 +4956,12 @@ static bool checkNestingOfRegions(Sema &SemaRef, const DSAStackTy *Stack,
     // A loop region that binds to a parallel region or a worksharing region
     // may not be closely nested inside a worksharing, loop, task, taskloop,
     // critical, ordered, atomic, or masked region.
-    NestingProhibited =
-        isOpenMPWorksharingDirective(ParentRegion) ||
-        isOpenMPGenericLoopDirective(ParentRegion) ||
-        isOpenMPTaskingDirective(ParentRegion) || ParentRegion == OMPD_master ||
-        ParentRegion == OMPD_masked || ParentRegion == OMPD_parallel_master ||
-        ParentRegion == OMPD_parallel_masked || ParentRegion == OMPD_critical ||
-        ParentRegion == OMPD_ordered;
+    NestingProhibited = isOpenMPWorksharingDirective(ParentRegion) ||
+                        isOpenMPGenericLoopDirective(ParentRegion) ||
+                        isOpenMPTaskingDirective(ParentRegion) ||
+                        llvm::is_contained({OMPD_masked, OMPD_master,
+                                            OMPD_critical, OMPD_ordered},
+                                           EnclosingConstruct);
     Recommend = ShouldBeInParallelRegion;
   } else if (CurrentRegion == OMPD_ordered) {
     // OpenMP [2.16, Nesting of Regions]
@@ -4973,7 +4972,7 @@ static bool checkNestingOfRegions(Sema &SemaRef, const DSAStackTy *Stack,
     // OpenMP [2.8.1,simd Construct, Restrictions]
     // An ordered construct with the simd clause is the only OpenMP construct
     // that can appear in the simd region.
-    NestingProhibited = ParentRegion == OMPD_critical ||
+    NestingProhibited = EnclosingConstruct == OMPD_critical ||
                         isOpenMPTaskingDirective(ParentRegion) ||
                         !(isOpenMPSimdDirective(ParentRegion) ||
                           Stack->isParentOrderedRegion());
@@ -4983,31 +4982,29 @@ static bool checkNestingOfRegions(Sema &SemaRef, const DSAStackTy *Stack,
     // If specified, a teams construct must be contained within a target
     // construct.
     NestingProhibited =
-        (SemaRef.LangOpts.OpenMP <= 45 && ParentRegion != OMPD_target) ||
-        (SemaRef.LangOpts.OpenMP >= 50 && ParentRegion != OMPD_unknown &&
-         ParentRegion != OMPD_target);
+        (SemaRef.LangOpts.OpenMP <= 45 && EnclosingConstruct != OMPD_target) ||
+        (SemaRef.LangOpts.OpenMP >= 50 && EnclosingConstruct != OMPD_unknown &&
+         EnclosingConstruct != OMPD_target);
     OrphanSeen = ParentRegion == OMPD_unknown;
     Recommend = ShouldBeInTargetRegion;
   } else if (CurrentRegion == OMPD_scan) {
     if (SemaRef.LangOpts.OpenMP >= 50) {
-      SmallVector<OpenMPDirectiveKind, 4> LeafOrComposite;
-      std::ignore = getLeafOrCompositeConstructs(ParentRegion, LeafOrComposite);
       // OpenMP spec 5.0 and 5.1 require scan to be directly enclosed by for,
       // simd, or for simd. This has to take into account combined directives.
       // In 5.2 this seems to be implied by the fact that the specified
       // separated constructs are do, for, and simd.
-      OpenMPDirectiveKind Enclosing = LeafOrComposite.back();
-      NestingProhibited = Enclosing != OMPD_for && Enclosing != OMPD_simd &&
-                          Enclosing != OMPD_for_simd;
+      NestingProhibited = !llvm::is_contained(
+          {OMPD_for, OMPD_simd, OMPD_for_simd}, EnclosingConstruct);
     } else {
       NestingProhibited = true;
     }
     OrphanSeen = ParentRegion == OMPD_unknown;
     Recommend = ShouldBeInLoopSimdRegion;
   }
+
   if (!NestingProhibited && !isOpenMPTargetExecutionDirective(CurrentRegion) &&
       !isOpenMPTargetDataManagementDirective(CurrentRegion) &&
-      (ParentRegion == OMPD_teams || ParentRegion == OMPD_target_teams)) {
+      EnclosingConstruct == OMPD_teams) {
     // OpenMP [5.1, 2.22, Nesting of Regions]
     // distribute, distribute simd, distribute parallel worksharing-loop,
     // distribute parallel worksharing-loop SIMD, loop, parallel regions,
@@ -5024,24 +5021,25 @@ static bool checkNestingOfRegions(Sema &SemaRef, const DSAStackTy *Stack,
                           CurrentRegion == OMPD_atomic);
     Recommend = ShouldBeInParallelRegion;
   }
+
   if (!NestingProhibited && CurrentRegion == OMPD_loop) {
     // OpenMP [5.1, 2.11.7, loop Construct, Restrictions]
     // If the bind clause is present on the loop construct and binding is
     // teams then the corresponding loop region must be strictly nested inside
     // a teams region.
-    NestingProhibited = BindKind == OMPC_BIND_teams &&
-                        ParentRegion != OMPD_teams &&
-                        ParentRegion != OMPD_target_teams;
+    NestingProhibited =
+        BindKind == OMPC_BIND_teams && EnclosingConstruct != OMPD_teams;
     Recommend = ShouldBeInTeamsRegion;
   }
+
   if (!NestingProhibited && isOpenMPNestingDistributeDirective(CurrentRegion)) {
     // OpenMP 4.5 [2.17 Nesting of Regions]
     // The region associated with the distribute construct must be strictly
     // nested inside a teams region
-    NestingProhibited =
-        (ParentRegion != OMPD_teams && ParentRegion != OMPD_target_teams);
+    NestingProhibited = EnclosingConstruct != OMPD_teams;
     Recommend = ShouldBeInTeamsRegion;
   }
+
   if (!NestingProhibited &&
       (isOpenMPTargetExecutionDirective(CurrentRegion) ||
        isOpenMPTargetDataManagementDirective(CurrentRegion))) {
@@ -5061,6 +5059,7 @@ static bool checkNestingOfRegions(Sema &SemaRef, const DSAStackTy *Stack,
         false /* don't skip top directive */);
     CloseNesting = false;
   }
+
   if (NestingProhibited) {
     if (OrphanSeen) {
       SemaRef.Diag(StartLoc, diag::err_omp_orphaned_device_directive)

@@ -4805,6 +4811,7 @@ static bool checkNestingOfRegions(Sema &SemaRef, const DSAStackTy *Stack,
<< getOpenMPDirectiveName(CurrentRegion);
return true;
}

Copy link
Member

Choose a reason for hiding this comment

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

Remove this extra line

Comment on lines 4838 to 4839

if (EnclosingConstruct == OMPD_atomic) {
Copy link
Member

Choose a reason for hiding this comment

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

Remove extra empty line here and in other places

Base automatically changed from users/kparzysz/spr/c06-scan-nesting to main July 11, 2024 13:08
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

@kparzysz kparzysz merged commit 861a8ed into main Jul 15, 2024
7 checks passed
@kparzysz kparzysz deleted the users/kparzysz/spr/c07-region-nesting branch July 15, 2024 13:20
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