Incorrect implicit data-sharing for nested tasks

[smateo]

Summary

There is a minor issue in how the implicit data-sharings for nested tasks are computed.

For the following example:

int x;
#pragma omp task shared(x)
#pragma omp task
x++;

We compute an implicit data-sharing of shared for x in the second task although I think that it should be firstprivate. Below you can find the part of the OpenMP spec that covers this example:

• In a task generating construct, if no default clause is present, a variable for which the data-sharing attribute is not determined by the rules above and that in the enclosing context is determined to be shared by all implicit tasks bound to the current team is shared.
• In a task generating construct, if no default clause is present, a variable for which the data-sharing attribute is not determined by the rules above is firstprivate.

Since each implicit-task has its own copy of x, we shouldn't apply the first rule.

[smateo]

Adding more context

[ABataev]

lib/Sema/SemaOpenMP.cpp

680 ↗

(On Diff #180631)

Better to rename it to isImplicitTaskingRegion

[smateo]

Renaming the isParallelRegion function to isImplicitTaskingRegion.

Shall we rename the isParallelOrTaskRegionto isImplicitOrExplicitTaskingRegion? It is only called 4 times.

Thanks!

[ABataev]

In D56430#1350706, @smateo wrote:

Renaming the isParallelRegion function to isImplicitTaskingRegion.

Shall we rename the isParallelOrTaskRegionto isImplicitOrExplicitTaskingRegion? It is only called 4 times.

Thanks!

Yes, go ahead and do this.

[smateo]

Done! Thanks,

Sergi

[ABataev]

Will you commit this patch? Or I should do it?

[smateo]

I don't have commit access yet. Would you mind to commit it for me? Thanks!,

Sergi

[ABataev]

In D56430#1351212, @smateo wrote:

I don't have commit access yet. Would you mind to commit it for me? Thanks!,

Sergi

Sure, no problems.

cfe/trunk/lib/Sema/SemaOpenMP.cpp

public:
 
   // Return set of mapped classes types
   bool isClassPreviouslyMapped(QualType QT) const {
     const SharingMapTy &StackElem = Stack.back().first.back();
     return StackElem.MappedClassesQualTypes.count(QT) != 0;
   }
 
 };
-bool isParallelOrTaskRegion(OpenMPDirectiveKind DKind) {
-  return isOpenMPParallelDirective(DKind) || isOpenMPTaskingDirective(DKind) ||
-         isOpenMPTeamsDirective(DKind) || DKind == OMPD_unknown;
+
+bool isImplicitTaskingRegion(OpenMPDirectiveKind DKind) {
+  return isOpenMPParallelDirective(DKind) || isOpenMPTeamsDirective(DKind);
+}
+
+bool isImplicitOrExplicitTaskingRegion(OpenMPDirectiveKind DKind) {
+  return isImplicitTaskingRegion(DKind) || isOpenMPTaskingDirective(DKind) || DKind == OMPD_unknown;
 }
 
 } // namespace
 
 static const Expr *getExprAsWritten(const Expr *E) {
   if (const auto *FE = dyn_cast<FullExpr>(E))
     E = FE->getSubExpr();
 
if (isOpenMPTaskingDirective(DVar.DKind)) {
         //  whose data-sharing attribute is not determined by the rules above is
         //  firstprivate.
         DVarTemp = getDSA(I, D);
         if (DVarTemp.CKind != OMPC_shared) {
           DVar.RefExpr = nullptr;
           DVar.CKind = OMPC_firstprivate;
           return DVar;
         }
-      } while (I != E && !isParallelOrTaskRegion(I->Directive));
+      } while (I != E && !isImplicitTaskingRegion(I->Directive));
       DVar.CKind =
           (DVarTemp.CKind == OMPC_unknown) ? OMPC_firstprivate : OMPC_shared;
       return DVar;
     }
   }
   // OpenMP [2.9.1.1, Data-sharing Attribute Rules for Variables Referenced
   // in a Construct, implicitly determined, p.3]
   //  For constructs other than task, if no default clause is present, these
const DSAStackTy::DSAVarData DSAStackTy::getTopMostTaskgroupReductionData(
   return DSAVarData();
 }
 
 bool DSAStackTy::isOpenMPLocal(VarDecl *D, iterator Iter) const {
   D = D->getCanonicalDecl();
   if (!isStackEmpty()) {
     iterator I = Iter, E = Stack.back().first.rend();
     Scope *TopScope = nullptr;
-    while (I != E && !isParallelOrTaskRegion(I->Directive) &&
+    while (I != E && !isImplicitOrExplicitTaskingRegion(I->Directive) &&
            !isOpenMPTargetExecutionDirective(I->Directive))
       ++I;
     if (I == E)
       return false;
     TopScope = I->CurScope ? I->CurScope->getParent() : nullptr;
     Scope *CurScope = getCurScope();
     while (CurScope != TopScope && !CurScope->isDeclScope(D))
       CurScope = CurScope->getParent();
DSAStackTy::hasDSA(ValueDecl *D,
   if (isStackEmpty())
     return {};
   D = getCanonicalDecl(D);
   iterator I = Stack.back().first.rbegin();
   iterator EndI = Stack.back().first.rend();
   if (FromParent && I != EndI)
     std::advance(I, 1);
   for (; I != EndI; std::advance(I, 1)) {
-    if (!DPred(I->Directive) && !isParallelOrTaskRegion(I->Directive))
+    if (!DPred(I->Directive) && !isImplicitOrExplicitTaskingRegion(I->Directive))
       continue;
     iterator NewI = I;
     DSAVarData DVar = getDSA(NewI, D);
     if (I == NewI && CPred(DVar.CKind))
       return DVar;
   }
   return {};
 }
VarDecl *Sema::isOpenMPCapturedDecl(ValueDecl *D) {
   }
 
   if (DSAStack->getCurrentDirective() != OMPD_unknown &&
       (!DSAStack->isClauseParsingMode() ||
        DSAStack->getParentDirective() != OMPD_unknown)) {
     auto &&Info = DSAStack->isLoopControlVariable(D);
     if (Info.first ||
         (VD && VD->hasLocalStorage() &&
-         isParallelOrTaskRegion(DSAStack->getCurrentDirective())) ||
+         isImplicitOrExplicitTaskingRegion(DSAStack->getCurrentDirective())) ||
         (VD && DSAStack->isForceVarCapturing()))
       return VD ? VD : Info.second;
     DSAStackTy::DSAVarData DVarPrivate =
         DSAStack->getTopDSA(D, DSAStack->isClauseParsingMode());
     if (DVarPrivate.CKind != OMPC_unknown && isOpenMPPrivate(DVarPrivate.CKind))
       return VD ? VD : cast<VarDecl>(DVarPrivate.PrivateCopy->getDecl());
     DVarPrivate = DSAStack->hasDSA(D, isOpenMPPrivate,
                                    [](OpenMPDirectiveKind) { return true; },
if (auto *VD = dyn_cast<VarDecl>(E->getDecl())) {
 
       SourceLocation ELoc = E->getExprLoc();
       OpenMPDirectiveKind DKind = Stack->getCurrentDirective();
       // The default(none) clause requires that each variable that is referenced
       // in the construct, and does not have a predetermined data-sharing
       // attribute, must have its data-sharing attribute explicitly determined
       // by being listed in a data-sharing attribute clause.
       if (DVar.CKind == OMPC_unknown && Stack->getDefaultDSA() == DSA_none &&
-          isParallelOrTaskRegion(DKind) &&
+          isImplicitOrExplicitTaskingRegion(DKind) &&
           VarsWithInheritedDSA.count(VD) == 0) {
         VarsWithInheritedDSA[VD] = E;
         return;
       }
 
       if (isOpenMPTargetExecutionDirective(DKind) &&
           !Stack->isLoopControlVariable(VD).first) {
         if (!Stack->checkMappableExprComponentListsForDecl(

cfe/trunk/test/OpenMP/task_messages.cpp

 // RUN: %clang_cc1 -verify -fopenmp -ferror-limit 100 -std=c++11 -o - %s
 
 // RUN: %clang_cc1 -verify -fopenmp-simd -ferror-limit 100 -std=c++11 -o - %s
 
 void foo() {
 }
 
 #pragma omp task // expected-error {{unexpected OpenMP directive '#pragma omp task'}}
 
 class S {
-  S(const S &s) { a = s.a + 12; } // expected-note 14 {{implicitly declared private here}}
+  S(const S &s) { a = s.a + 12; } // expected-note 16 {{implicitly declared private here}}
   int a;
 
 public:
   S() : a(0) {}
   S(int a) : a(a) {}
   operator int() { return a; }
   S &operator++() { return *this; }
   S operator+(const S &) { return *this; }
#pragma omp task
   // expected-error@+1 {{calling a private constructor of class 'S'}}
   ++a; // expected-error 2 {{variable 'a' must have explicitly specified data sharing attributes}}
 #pragma omp task
 #pragma omp task
   // expected-error@+1 {{calling a private constructor of class 'S'}}
   ++a; // expected-error {{calling a private constructor of class 'S'}}
 #pragma omp task default(shared)
 #pragma omp task
+  // expected-error@+1 {{calling a private constructor of class 'S'}}
+  ++a;
+#pragma omp parallel shared(a)
+#pragma omp task
+#pragma omp task
+  ++a;
+#pragma omp parallel shared(a)
+#pragma omp task default(shared)
+#pragma omp task
   ++a;
 #pragma omp task
 #pragma omp parallel
   ++a; // expected-error {{calling a private constructor of class 'S'}}
 // expected-error@+2 {{calling a private constructor of class 'S'}}
 #pragma omp task
   ++b;
 #pragma omp task
#pragma omp task
   // expected-error@+1 {{calling a private constructor of class 'S'}}
   ++sa; // expected-error {{variable 'sa' must have explicitly specified data sharing attributes}}
 #pragma omp task
 #pragma omp task
   // expected-error@+1 {{calling a private constructor of class 'S'}}
   ++sa; // expected-error {{calling a private constructor of class 'S'}}
 #pragma omp task default(shared)
 #pragma omp task
+  // expected-error@+1 {{calling a private constructor of class 'S'}}
+  ++sa;
+#pragma omp parallel shared(sa)
+#pragma omp task
+#pragma omp task
+  ++sa;
+#pragma omp parallel shared(sa)
+#pragma omp task default(shared)
+#pragma omp task
   ++sa;
 #pragma omp task
 #pragma omp parallel
   ++sa; // expected-error {{calling a private constructor of class 'S'}}
 // expected-error@+2 {{calling a private constructor of class 'S'}}
 #pragma omp task
   ++sb;
 // expected-error@+2 2 {{calling a private constructor of class 'S'}}