ABataev added inline comments. ================ 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"); ---------------- sfantao wrote: > 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. Try this one: %%% patch Index: SemaOpenMP.cpp =================================================================== --- SemaOpenMP.cpp (revision 243097) +++ SemaOpenMP.cpp (working copy) @@ -120,6 +120,7 @@ /// from current directive. OpenMPClauseKind ClauseKindMode; Sema &SemaRef; + bool ForceCapturing;
typedef SmallVector<SharingMapTy, 8>::reverse_iterator reverse_iterator; @@ -130,7 +131,8 @@ public: explicit DSAStackTy(Sema &S) - : Stack(1), ClauseKindMode(OMPC_unknown), SemaRef(S) {} + : Stack(1), ClauseKindMode(OMPC_unknown), SemaRef(S), + ForceCapturing(false) {} bool isClauseParsingMode() const { return ClauseKindMode != OMPC_unknown; } void setClauseParsingMode(OpenMPClauseKind K) { ClauseKindMode = K; } @@ -146,6 +148,9 @@ Stack.pop_back(); } + void forceVarCapturing(bool V) { ForceCapturing = V; } + bool isForceVarCapturing() const { return ForceCapturing; } + /// \brief If 'aligned' declaration for given variable \a D was not seen yet, /// add it and return NULL; otherwise return previous occurrence's expression /// for diagnostics. @@ -655,7 +660,8 @@ if (DSAStack->getCurrentDirective() != OMPD_unknown) { if (DSAStack->isLoopControlVariable(VD) || (VD->hasLocalStorage() && - isParallelOrTaskRegion(DSAStack->getCurrentDirective()))) + isParallelOrTaskRegion(DSAStack->getCurrentDirective())) || + DSAStack->isForceVarCapturing()) return true; auto DVarPrivate = DSAStack->getTopDSA(VD, DSAStack->isClauseParsingMode()); if (DVarPrivate.CKind != OMPC_unknown && isOpenMPPrivate(DVarPrivate.CKind)) @@ -1351,13 +1357,18 @@ // This is required for proper codegen. for (auto *Clause : Clauses) { if (isOpenMPPrivate(Clause->getClauseKind()) || - Clause->getClauseKind() == OMPC_copyprivate) { + Clause->getClauseKind() == OMPC_copyprivate || + (getLangOpts().OpenMPUseTLS && + getASTContext().getTargetInfo().isTLSSupported() && + Clause->getClauseKind() == OMPC_copyin)) { + DSAStack->forceVarCapturing(Clause->getClauseKind() == OMPC_copyin); // Mark all variables in private list clauses as used in inner region. for (auto *VarRef : Clause->children()) { if (auto *E = cast_or_null<Expr>(VarRef)) { MarkDeclarationsReferencedInExpr(E); } } + DSAStack->forceVarCapturing(/*V=*/false); } else if (isParallelOrTaskRegion(DSAStack->getCurrentDirective()) && Clause->getClauseKind() == OMPC_schedule) { // Mark all variables in private list clauses as used in inner region. %%% http://reviews.llvm.org/D11395 _______________________________________________ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits