[clang] 3488e8c - [OPENMP]Fix PR46347: several ordered directives in a single region.

[Alexey Bataev via cfe-commits]

Author: Alexey Bataev
Date: 2020-06-16T15:53:14-04:00
New Revision: 3488e8c21cec8bac7dabd8f6b7c642dbace31d65

URL: https://github.com/llvm/llvm-project/commit/3488e8c21cec8bac7dabd8f6b7c642dbace31d65
DIFF: https://github.com/llvm/llvm-project/commit/3488e8c21cec8bac7dabd8f6b7c642dbace31d65.diff

LOG: [OPENMP]Fix PR46347: several ordered directives in a single region.

Summary:
According to OpenMP, During execution of an iteration of a worksharing-loop or a loop nest within a worksharing-loop, simd, or worksharing-loop SIMD region, a thread must not execute more than one ordered region corresponding to an ordered construct without a depend clause.
Need to report an error in this case.

Reviewers: jdoerfert

Subscribers: yaxunl, guansong, sstefan1, cfe-commits, caomhin

Tags: #clang

Differential Revision: https://reviews.llvm.org/D81951

Added: 
    

Modified: 
    clang/include/clang/Basic/DiagnosticSemaKinds.td
    clang/lib/Sema/SemaOpenMP.cpp
    clang/test/OpenMP/ordered_messages.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index b0a6184906c2..52fc40771228 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -9937,10 +9937,10 @@ def err_omp_prohibited_region_critical_same_name : Error<
   "cannot nest 'critical' regions having the same name %0">;
 def note_omp_previous_critical_region : Note<
   "previous 'critical' region starts here">;
-def err_omp_several_scan_directives_in_region : Error<
-  "exactly one 'scan' directive must appear in the loop body of an enclosing directive">;
-def note_omp_previous_scan_directive : Note<
-  "previous 'scan' directive used here">;
+def err_omp_several_directives_in_region : Error<
+  "exactly one '%0' directive must appear in the loop body of an enclosing directive">;
+def note_omp_previous_directive : Note<
+  "previous '%0' directive used here">;
 def err_omp_sections_not_compound_stmt : Error<
   "the statement for '#pragma omp sections' must be a compound statement">;
 def err_omp_parallel_sections_not_compound_stmt : Error<

diff  --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index 68b3ba930c46..f2b29e6356c1 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -169,6 +169,7 @@ class DSAStackTy {
     bool LoopStart = false;
     bool BodyComplete = false;
     SourceLocation PrevScanLocation;
+    SourceLocation PrevOrderedLocation;
     SourceLocation InnerTeamsRegionLoc;
     /// Reference to the taskgroup task_reduction reference expression.
     Expr *TaskgroupReductionRef = nullptr;
@@ -848,6 +849,21 @@ class DSAStackTy {
     const SharingMapTy *Top = getSecondOnStackOrNull();
     return Top ? Top->PrevScanLocation : SourceLocation();
   }
+  /// Mark that parent region already has ordered directive.
+  void setParentHasOrderedDirective(SourceLocation Loc) {
+    if (SharingMapTy *Parent = getSecondOnStackOrNull())
+      Parent->PrevOrderedLocation = Loc;
+  }
+  /// Return true if current region has inner ordered construct.
+  bool doesParentHasOrderedDirective() const {
+    const SharingMapTy *Top = getSecondOnStackOrNull();
+    return Top ? Top->PrevOrderedLocation.isValid() : false;
+  }
+  /// Returns the location of the previously specified ordered directive.
+  SourceLocation getParentOrderedDirectiveLoc() const {
+    const SharingMapTy *Top = getSecondOnStackOrNull();
+    return Top ? Top->PrevOrderedLocation : SourceLocation();
+  }
 
   /// Set collapse value for the region.
   void setAssociatedLoops(unsigned Val) {
@@ -9187,9 +9203,10 @@ StmtResult Sema::ActOnOpenMPScanDirective(ArrayRef<OMPClause *> Clauses,
   // Check that only one instance of scan directives is used in the same outer
   // region.
   if (DSAStack->doesParentHasScanDirective()) {
-    Diag(StartLoc, diag::err_omp_several_scan_directives_in_region);
+    Diag(StartLoc, diag::err_omp_several_directives_in_region) << "scan";
     Diag(DSAStack->getParentScanDirectiveLoc(),
-         diag::note_omp_previous_scan_directive);
+         diag::note_omp_previous_directive)
+        << "scan";
     return StmtError();
   }
   DSAStack->setParentHasScanDirective(StartLoc);
@@ -9265,6 +9282,22 @@ StmtResult Sema::ActOnOpenMPOrderedDirective(ArrayRef<OMPClause *> Clauses,
   if ((!AStmt && !DependFound) || ErrorFound)
     return StmtError();
 
+  // OpenMP 5.0, 2.17.9, ordered Construct, Restrictions.
+  // During execution of an iteration of a worksharing-loop or a loop nest
+  // within a worksharing-loop, simd, or worksharing-loop SIMD region, a thread
+  // must not execute more than one ordered region corresponding to an ordered
+  // construct without a depend clause.
+  if (!DependFound) {
+    if (DSAStack->doesParentHasOrderedDirective()) {
+      Diag(StartLoc, diag::err_omp_several_directives_in_region) << "ordered";
+      Diag(DSAStack->getParentOrderedDirectiveLoc(),
+           diag::note_omp_previous_directive)
+          << "ordered";
+      return StmtError();
+    }
+    DSAStack->setParentHasOrderedDirective(StartLoc);
+  }
+
   if (AStmt) {
     assert(isa<CapturedStmt>(AStmt) && "Captured statement expected");
 

diff  --git a/clang/test/OpenMP/ordered_messages.cpp b/clang/test/OpenMP/ordered_messages.cpp
index 01846bfb0144..544c97d36ab3 100644
--- a/clang/test/OpenMP/ordered_messages.cpp
+++ b/clang/test/OpenMP/ordered_messages.cpp
@@ -61,6 +61,17 @@ T foo() {
       foo();
     }
   }
+  #pragma omp for ordered
+  for (int i = 0; i < 10; ++i) {
+    #pragma omp ordered // expected-note {{previous 'ordered' directive used here}}
+    {
+      foo();
+    }
+    #pragma omp ordered // expected-error {{exactly one 'ordered' directive must appear in the loop body of an enclosing directive}}
+    {
+      foo();
+    }
+  }
   #pragma omp ordered simd simd // expected-error {{directive '#pragma omp ordered' cannot contain more than one 'simd' clause}}
   {
     foo();
@@ -79,7 +90,18 @@ T foo() {
       foo();
     }
   }
-  #pragma omp for simd
+  #pragma omp simd
+  for (int i = 0; i < 10; ++i) {
+#pragma omp ordered simd // expected-note {{previous 'ordered' directive used here}}
+    {
+      foo();
+    }
+#pragma omp ordered simd // expected-error {{exactly one 'ordered' directive must appear in the loop body of an enclosing directive}}
+    {
+      foo();
+    }
+  }
+#pragma omp for simd
   for (int i = 0; i < 10; ++i) {
     #pragma omp ordered // expected-error {{OpenMP constructs may not be nested inside a simd region}}
     {