koops created this revision.
koops added reviewers: ddpagan, ABataev, jdoerfert, dreachem, tianshilei1992, 
soumitra, RitanyaB, thakis.
Herald added a project: All.
koops requested review of this revision.
Herald added subscribers: cfe-commits, jplehr, sstefan1.
Herald added a project: clang.

https://github.com/llvm/llvm-project/issues/64728
In https://reviews.llvm.org/D144634 : Support for Code Generation of loop bind 
clause,
before mapping a directive to the new directive there is a need to check if the 
parent directive/region is proper for the new mapped directive. e.g.
#pragma omp parallel for
for () {

  #pragma omp loop bind(parallel)
  for() {}

}.

After mapping "omp loop bind(parallel)" to "omp for" the above example becomes:
#pragma omp parallel for
for () {

  #pragma omp for
  for() {}

}.
However, it is illegal to have a "for" region within another "for" region. 
Thanks to David Pagan for pointing this out.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D158266

Files:
  clang/include/clang/Basic/OpenMPKinds.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Basic/OpenMPKinds.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/loop_bind_messages.cpp
  clang/test/PCH/pragma-loop.cpp

Index: clang/test/PCH/pragma-loop.cpp
===================================================================
--- clang/test/PCH/pragma-loop.cpp
+++ clang/test/PCH/pragma-loop.cpp
@@ -116,9 +116,13 @@
 
   inline void run10(int *List, int Length) {
     int i = 0;
-#pragma omp loop bind(teams)
+    int j = 0;
+    #pragma omp teams
     for (int i = 0; i < Length; i++) {
-      List[i] = i;
+      #pragma omp loop bind(teams)
+      for (int j = 0; j < Length; j++) {
+        List[i] = i+j;
+      }
     }
   }
 
Index: clang/test/OpenMP/loop_bind_messages.cpp
===================================================================
--- clang/test/OpenMP/loop_bind_messages.cpp
+++ clang/test/OpenMP/loop_bind_messages.cpp
@@ -4,6 +4,7 @@
 
 #define NNN 50
 int aaa[NNN];
+int aaa2[NNN][NNN];
 
 void parallel_loop() {
   #pragma omp parallel
@@ -15,6 +16,16 @@
    }
 }
 
+void parallel_for_AND_loop_bind() {
+  #pragma omp parallel for
+  for (int i = 0 ; i < NNN ; i++) {
+    #pragma omp loop bind(parallel) // expected-error{{region cannot be closely nested inside 'parallel for' region; perhaps you forget to enclose 'omp loop' directive into a parallel region?}}
+    for (int j = 0 ; j < NNN ; j++) {
+      aaa2[i][j] = i+j;
+    }
+  }
+}
+
 void teams_loop() {
   int var1, var2;
 
@@ -65,12 +76,32 @@
    }
 }
 
+void orphan_loop_teams_bind(){
+  #pragma omp loop bind(teams) // expected-error{{region cannot be closely nested inside 'unknown' region; perhaps you forget to enclose 'omp loop' directive into a teams region?}}
+  for (int i = 0; i < NNN; i++) {
+    aaa[i] = i+i*NNN;
+  }
+}
+
+void parallel_for_with_loop_teams_bind(){
+  #pragma omp parallel for
+  for (int i = 0; i < NNN; i++) {
+    #pragma omp loop bind(teams) // expected-error{{region cannot be closely nested inside 'parallel for' region; perhaps you forget to enclose 'omp loop' directive into a teams region?}}
+    for (int j = 0 ; j < NNN ; j++) {
+      aaa[i] = i+i*NNN;
+    }
+  }
+}
+
 int main(int argc, char *argv[]) {
   parallel_loop();
+  parallel_for_AND_loop_bind();
   teams_loop();
   orphan_loop_with_bind();
   orphan_loop_no_bind();
   teams_loop_reduction();
+  orphan_loop_teams_bind();
+  parallel_for_with_loop_teams_bind();
 }
 
 #endif
Index: clang/lib/Sema/SemaOpenMP.cpp
===================================================================
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -6113,19 +6113,21 @@
                             ArrayRef<OMPClause *> Clauses,
                             OpenMPBindClauseKind BindKind,
                             OpenMPDirectiveKind &Kind,
-                            OpenMPDirectiveKind &PrevMappedDirective) {
+                            OpenMPDirectiveKind &PrevMappedDirective,
+                            SourceLocation StartLoc, SourceLocation EndLoc) {
 
   bool UseClausesWithoutBind = false;
 
   // Restricting to "#pragma omp loop bind"
   if (getLangOpts().OpenMP >= 50 && Kind == OMPD_loop) {
+
+    const OpenMPDirectiveKind ParentDirective = DSAStack->getParentDirective();
+
     if (BindKind == OMPC_BIND_unknown) {
       // Setting the enclosing teams or parallel construct for the loop
       // directive without bind clause.
       BindKind = OMPC_BIND_thread; // Default bind(thread) if binding is unknown
 
-      const OpenMPDirectiveKind ParentDirective =
-          DSAStack->getParentDirective();
       if (ParentDirective == OMPD_unknown) {
         Diag(DSAStack->getDefaultDSALocation(),
              diag::err_omp_bind_required_on_loop);
@@ -6158,12 +6160,26 @@
 
     switch (BindKind) {
     case OMPC_BIND_parallel:
+      if (isOpenMPForDirective(ParentDirective)) {
+        // "omp for" within a "for" region is not permitted.
+        Diag(StartLoc, diag::err_omp_prohibited_region)
+            << true << getOpenMPDirectiveName(ParentDirective) << 1
+            << getOpenMPDirectiveName(Kind);
+      }
       Kind = OMPD_for;
       DSAStack->setCurrentDirective(OMPD_for);
       DSAStack->setMappedDirective(OMPD_loop);
       PrevMappedDirective = OMPD_loop;
       break;
     case OMPC_BIND_teams:
+      if (!isOpenMPTeamsDirective(ParentDirective)) {
+        // A "loop" region that binds to a teams region must be strictly nested
+        // inside a teams region.
+        // A "distribute" region must be strictly nested inside a teams region.
+        Diag(StartLoc, diag::err_omp_prohibited_region)
+            << true << getOpenMPDirectiveName(ParentDirective) << 4
+            << getOpenMPDirectiveName(Kind);
+      }
       Kind = OMPD_distribute;
       DSAStack->setCurrentDirective(OMPD_distribute);
       DSAStack->setMappedDirective(OMPD_loop);
@@ -6217,8 +6233,9 @@
   llvm::SmallVector<OMPClause *> ClausesWithoutBind;
   bool UseClausesWithoutBind = false;
 
-  UseClausesWithoutBind = mapLoopConstruct(ClausesWithoutBind, Clauses,
-                                           BindKind, Kind, PrevMappedDirective);
+  UseClausesWithoutBind =
+      mapLoopConstruct(ClausesWithoutBind, Clauses, BindKind, Kind,
+                       PrevMappedDirective, StartLoc, EndLoc);
 
   llvm::SmallVector<OMPClause *, 8> ClausesWithImplicit;
   VarsWithInheritedDSAType VarsWithInheritedDSA;
Index: clang/lib/Basic/OpenMPKinds.cpp
===================================================================
--- clang/lib/Basic/OpenMPKinds.cpp
+++ clang/lib/Basic/OpenMPKinds.cpp
@@ -563,6 +563,18 @@
   llvm_unreachable("Invalid OpenMP simple clause kind");
 }
 
+bool clang::isOpenMPForDirective(OpenMPDirectiveKind DKind) {
+  return DKind == OMPD_for || DKind == OMPD_for_simd ||
+         DKind == OMPD_parallel_for || DKind == OMPD_parallel_for_simd ||
+         DKind == OMPD_target_parallel_for ||
+         DKind == OMPD_distribute_parallel_for ||
+         DKind == OMPD_distribute_parallel_for_simd ||
+         DKind == OMPD_target_parallel_for_simd ||
+         DKind == OMPD_target_teams_distribute_parallel_for ||
+         DKind == OMPD_target_teams_distribute_parallel_for_simd ||
+         DKind == OMPD_parallel_loop || DKind == OMPD_target_parallel_loop;
+}
+
 bool clang::isOpenMPLoopDirective(OpenMPDirectiveKind DKind) {
   return DKind == OMPD_simd || DKind == OMPD_for || DKind == OMPD_for_simd ||
          DKind == OMPD_parallel_for || DKind == OMPD_parallel_for_simd ||
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -11210,7 +11210,8 @@
                         ArrayRef<OMPClause *> Clauses,
                         OpenMPBindClauseKind BindKind,
                         OpenMPDirectiveKind &Kind,
-                        OpenMPDirectiveKind &PrevMappedDirective);
+                        OpenMPDirectiveKind &PrevMappedDirective,
+                        SourceLocation StartLoc, SourceLocation EndLoc);
 
 public:
   /// The declarator \p D defines a function in the scope \p S which is nested
Index: clang/include/clang/Basic/OpenMPKinds.h
===================================================================
--- clang/include/clang/Basic/OpenMPKinds.h
+++ clang/include/clang/Basic/OpenMPKinds.h
@@ -243,6 +243,13 @@
 /// or 'omp for' directive, otherwise - false.
 bool isOpenMPLoopDirective(OpenMPDirectiveKind DKind);
 
+/// Checks if the specified directive is a directive with an associated
+/// for loop construct.
+/// \param DKind Specified directive.
+/// \return true - the directive is a for-associated directive like
+/// or 'omp for' directive, otherwise - false.
+bool isOpenMPForDirective(OpenMPDirectiveKind DKind);
+
 /// Checks if the specified directive is a worksharing directive.
 /// \param DKind Specified directive.
 /// \return true - the directive is a worksharing directive like 'omp for',
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to