sfantao added inline comments. ================ Comment at: lib/CodeGen/CGStmtOpenMP.cpp:230-246 @@ -229,7 +229,19 @@ if (CopiedVars.insert(VD->getCanonicalDecl()).second) { - // Get the address of the master variable. - auto *MasterAddr = VD->isStaticLocal() - ? CGM.getStaticLocalDeclAddress(VD) - : CGM.GetAddrOfGlobal(VD); + + // Get the address of the master variable. If we are emitting code with + // TLS support, the address is passed from the master as field in the + // captured declaration. + llvm::Value *MasterAddr; + if (getLangOpts().OpenMPUseTLS && + getContext().getTargetInfo().isTLSSupported()) { + assert(CapturedStmtInfo->lookup(VD) && + "Copyin threadprivates should have been captured!"); + DeclRefExpr DRE(const_cast<VarDecl *>(VD), true, (*IRef)->getType(), + VK_LValue, (*IRef)->getExprLoc()); + MasterAddr = EmitLValue(&DRE).getAddress(); + } else { + MasterAddr = VD->isStaticLocal() ? CGM.getStaticLocalDeclAddress(VD) + : CGM.GetAddrOfGlobal(VD); + } // Get the address of the threadprivate variable. auto *PrivateAddr = EmitLValue(*IRef).getAddress(); ---------------- Done with a small modification: the implementation always expect the `CapturedStmtInfo` to contain a valid lookup.
================ Comment at: lib/Sema/SemaOpenMP.cpp:160-667 @@ -159,4 +159,8 @@ + /// \brief Check if the specified variable is a 'copyin' variable for current + /// region. + bool isCopyinVariable(VarDecl *D); + /// \brief Adds explicit data sharing attribute to the specified declaration. void addDSA(VarDecl *D, DeclRefExpr *E, OpenMPClauseKind A); @@ -412,6 +416,12 @@ return Stack.back().LCVSet.count(D) > 0; } +bool DSAStackTy::isCopyinVariable(VarDecl *D){ + assert(Stack.size() > 1 && "Data-sharing attributes stack is empty"); + D = D->getCanonicalDecl(); + DSAInfo I = Stack.back().SharingMap.lookup(D); + return I.Attributes == OMPC_copyin; +} void DSAStackTy::addDSA(VarDecl *D, DeclRefExpr *E, OpenMPClauseKind A) { D = D->getCanonicalDecl(); if (A == OMPC_threadprivate) { @@ -653,4 +663,6 @@ assert(LangOpts.OpenMP && "OpenMP is not allowed"); VD = VD->getCanonicalDecl(); if (DSAStack->getCurrentDirective() != OMPD_unknown) { + if (DSAStack->isCopyinVariable(VD) && !DSAStack->isClauseParsingMode()) + return true; if (DSAStack->isLoopControlVariable(VD) || ---------------- ABataev wrote: > ABataev wrote: > > I'd better write a new function isOpenMPPrivateOrCopyin() function and used > > it instead of isOpenMPPrivate() in Sema::IsOpenMPCapturedVar(). Besides, > > note, that copyin variables must be captured only if TLS mode is on, if > > we're using runtime calls we don't need to capture threadprivates. > If you want to capture copyin variable only in closely nested OpenMP region, > it is enough just to make next changes in Sema::IsOpenMPCapturedVar(): > ... > auto DVarPrivate = DSAStack->getTopDSA(VD, DSAStack->isClauseParsingMode()); > if (DVarPrivate.CKind != OMPC_unknown && (isOpenMPPrivate(DVarPrivate.CKind) > || (getLangOpts().OpenMPTLS && DVarPrivate.CKind == OMPC_copyin)) > ... The TLS check is now implemented in `IsOpenMPCapturedVar`. However I am not combining private with copyin because the action takes is slightly different. ================ Comment at: lib/Sema/SemaOpenMP.cpp:160-667 @@ -159,4 +159,8 @@ + /// \brief Check if the specified variable is a 'copyin' variable for current + /// region. + bool isCopyinVariable(VarDecl *D); + /// \brief Adds explicit data sharing attribute to the specified declaration. void addDSA(VarDecl *D, DeclRefExpr *E, OpenMPClauseKind A); @@ -412,6 +416,12 @@ return Stack.back().LCVSet.count(D) > 0; } +bool DSAStackTy::isCopyinVariable(VarDecl *D){ + assert(Stack.size() > 1 && "Data-sharing attributes stack is empty"); + D = D->getCanonicalDecl(); + DSAInfo I = Stack.back().SharingMap.lookup(D); + return I.Attributes == OMPC_copyin; +} void DSAStackTy::addDSA(VarDecl *D, DeclRefExpr *E, OpenMPClauseKind A) { D = D->getCanonicalDecl(); if (A == OMPC_threadprivate) { @@ -653,4 +663,6 @@ assert(LangOpts.OpenMP && "OpenMP is not allowed"); VD = VD->getCanonicalDecl(); if (DSAStack->getCurrentDirective() != OMPD_unknown) { + if (DSAStack->isCopyinVariable(VD) && !DSAStack->isClauseParsingMode()) + return true; if (DSAStack->isLoopControlVariable(VD) || ---------------- sfantao wrote: > ABataev wrote: > > ABataev wrote: > > > I'd better write a new function isOpenMPPrivateOrCopyin() function and > > > used it instead of isOpenMPPrivate() in Sema::IsOpenMPCapturedVar(). > > > Besides, note, that copyin variables must be captured only if TLS mode is > > > on, if we're using runtime calls we don't need to capture threadprivates. > > If you want to capture copyin variable only in closely nested OpenMP > > region, it is enough just to make next changes in > > Sema::IsOpenMPCapturedVar(): > > ... > > auto DVarPrivate = DSAStack->getTopDSA(VD, DSAStack->isClauseParsingMode()); > > if (DVarPrivate.CKind != OMPC_unknown && > > (isOpenMPPrivate(DVarPrivate.CKind) || (getLangOpts().OpenMPTLS && > > DVarPrivate.CKind == OMPC_copyin)) > > ... > The TLS check is now implemented in `IsOpenMPCapturedVar`. However I am not > combining private with copyin because the action takes is slightly different. Capturing in the closely nested region is not sufficient: declarations have to be captured even if no uses are found. Please, take a look at the last patch and let me know your thoughts. http://reviews.llvm.org/D11395 _______________________________________________ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits