[OPENMP] getDSA(): handle loop control variables

[Prince781]

Summary

The following example compiles incorrectly since at least clang 8.0.0:

#include <stdio.h>
#include <omp.h>

#define N 100

int main(void) {
    int i;
    int arr[N];

    #pragma omp parallel // shared(i) shared(arr)
    #pragma omp for // private(i)
    for (i = 0; i < N; i++) {
        #pragma omp task // firstprivate(i) shared(arr)
        {
            printf("[thread %2d] i = %d\n", omp_get_thread_num(), i);
            arr[i] = i;
        }
    }

    for (i = 0; i < N; i++) {
        if (arr[i] != i) {
            fprintf(stderr, "FAIL: arr[%d] == %d\n", i, arr[i]);
        }
    }
}

The iteration variable, i, should become private at the omp for construct and then become implicit firstprivate within the task region. What happens instead is that i is never privatized within the task construct. As the task construct is parsed, when a reference to i is determined, the implicit data-sharing attributes are computed incorrectly.

[lebedev.ri]

Needs a test

[Prince781]

Added a lit test.

[ABataev]

Thanks for the report, but the fix is not quite correct.
I'll fix this myself. I just want to push it to 9.0 release branch ASAP.

clang/lib/Sema/SemaOpenMP.cpp

public:
   /// Register specified variable as loop control variable.
   void addLoopControlVariable(const ValueDecl *D, VarDecl *Capture);
   /// Check if the specified variable is a loop control variable for
   /// current region.
   /// \return The index of the loop control variable in the list of associated
   /// for-loops (from outer to inner).
   const LCDeclInfo isLoopControlVariable(const ValueDecl *D) const;
   /// Check if the specified variable is a loop control variable for
+  /// given region.
+  /// \return The index of the loop control variable in the list of associated
+  /// for-loops (from outer to inner).
+  const LCDeclInfo isLoopControlVariable(const ValueDecl *D, const SharingMapTy &Region) const;
+  /// Check if the specified variable is a loop control variable for
   /// parent region.
   /// \return The index of the loop control variable in the list of associated
   /// for-loops (from outer to inner).
   const LCDeclInfo isParentLoopControlVariable(const ValueDecl *D) const;
   /// Get the loop control variable for the I-th loop (or nullptr) in
   /// parent directive.
   const ValueDecl *getParentLoopControlVariable(unsigned I) const;
 
DSAStackTy::DSAVarData DSAStackTy::getDSA(const_iterator &Iter,
   switch (Iter->DefaultAttr) {
   case DSA_shared:
     DVar.CKind = OMPC_shared;
     DVar.ImplicitDSALoc = Iter->DefaultAttrLoc;
     return DVar;
   case DSA_none:
     return DVar;
   case DSA_unspecified:
+    DVar.ImplicitDSALoc = Iter->DefaultAttrLoc;
+    // OpenMP [2.9.1.1, Data-sharing Attribute Rules for Variables Referenced
+    // in a Construct, implicitly determined]
+    //  The loop iteration variable(s) in the associated for-loop(s) of a for or
+    //  parallel for construct is (are) private.
+    // OpenMP 5.0 includes taskloop and distribute directives
+    if (!isOpenMPSimdDirective(DVar.DKind) &&
+        isOpenMPLoopDirective(DVar.DKind) &&
+        isLoopControlVariable(D, *Iter).first) {
+      DVar.CKind = OMPC_private;
+      // TODO: OpenMP 5.0: if (Dvar.DKind == OMPD_loop) DVar.CKind = OMPC_lastprivate;
+      return DVar;
+    }
+
     // OpenMP [2.9.1.1, Data-sharing Attribute Rules for Variables Referenced
     // in a Construct, implicitly determined, p.2]
     //  In a parallel construct, if no default clause is present, these
     //  variables are shared.
-    DVar.ImplicitDSALoc = Iter->DefaultAttrLoc;
     if (isOpenMPParallelDirective(DVar.DKind) ||
         isOpenMPTeamsDirective(DVar.DKind)) {
       DVar.CKind = OMPC_shared;
       return DVar;
     }
 
     // OpenMP [2.9.1.1, Data-sharing Attribute Rules for Variables Referenced
     // in a Construct, implicitly determined, p.4]
void DSAStackTy::addLoopControlVariable(const ValueDecl *D, VarDecl *Capture) {
   SharingMapTy &StackElem = getTopOfStack();
   StackElem.LCVMap.try_emplace(
       D, LCDeclInfo(StackElem.LCVMap.size() + 1, Capture));
 }
 
 const DSAStackTy::LCDeclInfo
 DSAStackTy::isLoopControlVariable(const ValueDecl *D) const {
   assert(!isStackEmpty() && "Data-sharing attributes stack is empty");
+  return isLoopControlVariable(D, getTopOfStack());
+}
+
+const DSAStackTy::LCDeclInfo
+DSAStackTy::isLoopControlVariable(const ValueDecl *D, const SharingMapTy &Region) const {
   D = getCanonicalDecl(D);
-  const SharingMapTy &StackElem = getTopOfStack();
+  const SharingMapTy &StackElem = Region;
   auto It = StackElem.LCVMap.find(D);
   if (It != StackElem.LCVMap.end())
     return It->second;
   return {0, nullptr};
 }
 
 const DSAStackTy::LCDeclInfo
 DSAStackTy::isParentLoopControlVariable(const ValueDecl *D) const {

clang/test/OpenMP/loop_control_var_nested_task.cpp

+// RUN: %clang_cc1 %s -fopenmp -emit-llvm -o - | FileCheck %s
+// CHECK: {{%struct}}.kmp_task_t_with_privates = type { {{.*%struct\.\.kmp_privates.t.*}} }
+// CHECK: define internal void @.omp_task_privates_map.({{.*%struct\.\.kmp_privates.t.*}})
+#define N 100
+int main(void) {
+    // declare this variable outside, so that it will be shared on the outer parallel construct
+    int i;
+    int arr[N];
+
+    #pragma omp parallel // shared(i) shared(arr)
+    #pragma omp for // private(i) - i should be privatized because it is an iteration variable
+    for (i = 0; i < N; i++) {
+        #pragma omp task
+        // CHECK: %.firstpriv.ptr.addr.i = alloca i32*, align 8
+        // CHECK: {{%[0-9]+}} = load i32*, i32** %.firstpriv.ptr.addr.i, align 8
+        arr[i] = i;
+    }
+}