[PATCH] D11395: [OpenMP] Add capture for threadprivate variables used in copyin clause

[Samuel Antao]

sfantao added inline comments.

================
Comment at: include/clang/Sema/Sema.h:7711
@@ -7708,3 +7710,3 @@
   /// is performed.
   bool isOpenMPPrivateVar(VarDecl *VD, unsigned Level);
 
----------------
Done!

================
Comment at: include/clang/Sema/Sema.h:7748-7751
@@ -7745,6 +7747,6 @@
   /// \brief Initialization of captured region for OpenMP region.
   void ActOnOpenMPRegionStart(OpenMPDirectiveKind DKind, Scope *CurScope);
   /// \brief End of OpenMP region.
   ///
   /// \param S Statement associated with the current OpenMP region.
   /// \param Clauses List of clauses for the current OpenMP region.
----------------
Done!

================
Comment at: lib/Sema/SemaOpenMP.cpp:663
@@ -651,3 +662,3 @@
 
-bool Sema::IsOpenMPCapturedVar(VarDecl *VD) {
+bool Sema::IsOpenMPCapturedVar(VarDecl *VD, bool &PropagateCapture) {
   assert(LangOpts.OpenMP && "OpenMP is not allowed");
----------------
That was my initial approach, however that will not do what is required. Let me explain with an example:

```
int A = 0, B = 0, C = 0;
#pragma omp threadprivate(A,B,C)

foo(){
  ++A; ++B; ++C;
  #pragma omp parallel copyin(A,B,C)
  {
    ++B;
    #pragma omp some_other_directive
    {
       ++C;
       bar(); // Uses A 
    }
  }
}
```
If we just change `IsOpenMPCapturedVar` as you say, this is what happens:
 - A will not be captured so bar() will not get the right value of A
 - C will be captured by some_other_directive, and that is not necessary, and will pose an avoidable overhead.
 - Only B would be captured properly.

I am not saying that there is no better way of doing, but after testing several cases this is the best solution I was able to get. Let me know if you have something different to propose or if you need me to clarify anything.

================
Comment at: lib/Sema/SemaOpenMP.cpp:1373
@@ -1353,3 +1372,3 @@
     if (isOpenMPPrivate(Clause->getClauseKind()) ||
         Clause->getClauseKind() == OMPC_copyprivate) {
       // Mark all variables in private list clauses as used in inner region.
----------------
Done!


http://reviews.llvm.org/D11395