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

[Alexey Bataev]

ABataev added a comment.

Samuel, thanks for the review. But please, provide full diff as described in http://llvm.org/docs/Phabricator.html. Sometimes it is important to see full context.


================
Comment at: include/clang/Sema/Sema.h:7711
@@ -7708,3 +7710,3 @@
   /// is performed.
-  bool isOpenMPPrivateVar(VarDecl *VD, unsigned Level);
+  bool isOpenMPPrivateOrCopyinVar(VarDecl *VD, unsigned Level);
 
----------------
Copyin variables are private also, so don't need to modify the name of the function

================
Comment at: include/clang/Sema/Sema.h:7748-7751
@@ -7745,3 +7747,6 @@
   /// \brief Initialization of captured region for OpenMP region.
-  void ActOnOpenMPRegionStart(OpenMPDirectiveKind DKind, Scope *CurScope);
+  ///
+  /// \param Clauses List of clauses for the current OpenMP region.
+  void ActOnOpenMPRegionStart(OpenMPDirectiveKind DKind, Scope *CurScope,
+                              ArrayRef<OMPClause *> Clauses);
   /// \brief End of OpenMP region.
----------------
Clauses arg is not required, capture copyin variables in Sema::ActOnOpenMPRegionEnd() function.

================
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");
----------------
I don't think that you need so many changes to capture copyin variables. I think the only thing that must be done is just to modify Sema::IsOpenMPCapturedVar() function and instead of isOpenMPPrivate() function use special function that combines isOpenMPPrivate() and check for OMPC_copyin. But the check for copyin must be done only if TLS mode is on

================
Comment at: lib/Sema/SemaOpenMP.cpp:1373
@@ +1372,3 @@
+          DeclRefExpr *DE = cast<DeclRefExpr>(Expr);
+          tryCaptureVariable(cast<VarDecl>(DE->getDecl()), DE->getLocation(),
+                             TryCapture_Implicit);
----------------
This must be done in Sema::ActOnOpenMPRegionEnd() and only if TLS mode is on.


http://reviews.llvm.org/D11395