jyu2 updated this revision to Diff 439195. jyu2 marked 7 inline comments as done. jyu2 retitled this revision from "Generate the capture for field when the field is used with implicit default." to "Generate the capture for field when the field is used in openmp region with implicit default in the member function.". jyu2 edited the summary of this revision. jyu2 added a comment. Herald added subscribers: guansong, yaxunl.
Thanks Alexey for the review. This is address Alexey's comments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D127803/new/ https://reviews.llvm.org/D127803 Files: clang/include/clang/Sema/Sema.h clang/lib/Sema/SemaExprMember.cpp clang/lib/Sema/SemaOpenMP.cpp clang/lib/Sema/TreeTransform.h clang/test/OpenMP/default_firstprivate_ast_print.cpp clang/test/OpenMP/default_private_ast_print.cpp
Index: clang/test/OpenMP/default_private_ast_print.cpp =================================================================== --- clang/test/OpenMP/default_private_ast_print.cpp +++ clang/test/OpenMP/default_private_ast_print.cpp @@ -96,4 +96,57 @@ // DUMP-NEXT: -DeclRefExpr {{.*}} 'a' // DUMP-NEXT: -DeclRefExpr {{.*}} 'yy' } + +void zoo(int); +struct A { + int z; + int f; + A(); + ~A(); + void foo() { +#pragma omp parallel private(z) default(private) + { + z++; + f++; + zoo(z + f); + f++; + } + } + // PRINT: #pragma omp parallel private(this->z) default(private) + // DUMP: -OMPParallelDirective + // DUMP-NEXT: -OMPPrivateClause + // DUMP-NEXT: -DeclRefExpr {{.*}} 'z' + // DUMP-NEXT: -OMPDefaultClause + // DUMP-NEXT: -OMPPrivateClause + // DUMP-NEXT: -DeclRefExpr {{.*}} 'f' + // DUMP: -CXXThisExpr {{.*}} 'A *' implicit this + void bar() { +#pragma omp parallel private(z) default(private) + { +#pragma omp parallel private(z) default(private) + { + z++; + f++; + zoo(z + f); + f++; + } + } + } + // PRINT: #pragma omp parallel private(this->z) default(private) + // PRINT: #pragma omp parallel private(this->z) default(private) + // DUMP: -OMPParallelDirective + // DUMP-NEXT: -OMPPrivateClause + // DUMP-NEXT: -DeclRefExpr {{.*}} 'z' + // DUMP-NEXT: -OMPDefaultClause + // DUMP: -OMPParallelDirective + // DUMP-NEXT: -OMPPrivateClause + // DUMP-NEXT: -DeclRefExpr {{.*}} 'z' + // DUMP-NEXT: -OMPDefaultClause + // DUMP-NEXT: -OMPPrivateClause {{.*}} <implicit> + // DUMP-NEXT: -DeclRefExpr {{.*}} 'f' + // DUMP: -CXXThisExpr + // DUMP: -MemberExpr + // DUMP-NEXT: -CXXThisExpr + // DUMP: -CXXThisExpr +}; #endif // HEADER Index: clang/test/OpenMP/default_firstprivate_ast_print.cpp =================================================================== --- clang/test/OpenMP/default_firstprivate_ast_print.cpp +++ clang/test/OpenMP/default_firstprivate_ast_print.cpp @@ -45,7 +45,8 @@ // PRINT-NEXT: this->targetDev++; // CHECK-NEXT: } // DUMP: -OMPParallelDirective - // DUMP->NEXT: -OMPDefaultClause + // DUMP-NEXT: -OMPDefaultClause + // DUMP-NOT: -OMPFirstprivateClause } // PRINT: template<> void apply<32U>() // PRINT: #pragma omp parallel default(firstprivate) @@ -54,6 +55,8 @@ // CHECK-NEXT: } // DUMP: -OMPParallelDirective // DUMP-NEXT: -OMPDefaultClause + // DUMP-NEXT: -OMPFirstprivateClause + // DUMP-NEXT: -DeclRefExpr {{.*}} 'targetDev' }; void use_template() { @@ -99,4 +102,60 @@ // DUMP-NEXT: -DeclRefExpr {{.*}} 'yy' // DUMP-NEXT: -DeclRefExpr {{.*}} 'y' } +void zoo(int); +struct A { + int z; + int f; + A(); + ~A(); + void foo() { +#pragma omp parallel firstprivate(z) default(firstprivate) + { + z++; + f++; + zoo(z + f); + f++; + } + } + // PRINT: #pragma omp parallel firstprivate(this->z) default(firstprivate) + // DUMP: -OMPParallelDirective + // DUMP-NEXT: -OMPFirstprivateClause + // DUMP-NEXT: -DeclRefExpr {{.*}} 'z' + // DUMP-NEXT: -OMPDefaultClause + // DUMP-NEXT: -OMPFirstprivateClause {{.*}} <implicit> + // DUMP-NEXT: -DeclRefExpr {{.*}} 'f' + // DUMP: -CXXThisExpr {{.*}} 'A *' implicit this + // DUMP-NEXT: -DeclRefExpr {{.*}} 'z' + // DUMP-NEXT: -DeclRefExpr {{.*}} 'f' + void bar() { +#pragma omp parallel firstprivate(z) default(firstprivate) + { +#pragma omp parallel private(z) default(firstprivate) + { + z++; + f++; + zoo(z + f); + f++; + } + } + } + // PRINT: #pragma omp parallel firstprivate(this->z) default(firstprivate) + // PRINT: #pragma omp parallel private(this->z) default(firstprivate) + // DUMP: -OMPParallelDirective + // DUMP-NEXT: -OMPFirstprivateClause + // DUMP-NEXT: -DeclRefExpr {{.*}} 'z' + // DUMP-NEXT: -OMPDefaultClause + // DUMP: -OMPParallelDirective + // DUMP-NEXT: -OMPPrivateClaus + // DUMP-NEXT: -DeclRefExpr {{.*}} 'z' + // DUMP-NEXT: -OMPDefaultClause + // DUMP-NEXT: -OMPFirstprivateClause {{.*}} <implicit> + // DUMP-NEXT: -DeclRefExpr {{.*}} 'f' + // DUMP: -CXXThisExpr {{.*}} 'A *' implicit this + // DUMP-NEXT: -DeclRefExpr {{.*}} 'f' + // DUMP: -MemberExpr {{.*}} + // DUMP-NEXT: -CXXThisExpr + // DUMP: -CXXThisExpr {{.*}} 'A *' implicit this + // DUMP-NEXT: -DeclRefExpr {{.*}} 'z' +}; #endif // HEADER Index: clang/lib/Sema/TreeTransform.h =================================================================== --- clang/lib/Sema/TreeTransform.h +++ clang/lib/Sema/TreeTransform.h @@ -11068,11 +11068,9 @@ return ExprError(); } - if (!getDerived().AlwaysRebuild() && - Base.get() == E->getBase() && - QualifierLoc == E->getQualifierLoc() && - Member == E->getMemberDecl() && - FoundDecl == E->getFoundDecl() && + if (!getDerived().AlwaysRebuild() && !getSema().getLangOpts().OpenMP && + Base.get() == E->getBase() && QualifierLoc == E->getQualifierLoc() && + Member == E->getMemberDecl() && FoundDecl == E->getFoundDecl() && !E->hasExplicitTemplateArgs()) { // Mark it referenced in the new context regardless. Index: clang/lib/Sema/SemaOpenMP.cpp =================================================================== --- clang/lib/Sema/SemaOpenMP.cpp +++ clang/lib/Sema/SemaOpenMP.cpp @@ -196,6 +196,16 @@ llvm::DenseSet<CanonicalDeclPtr<Decl>> UsedInScanDirective; llvm::DenseMap<CanonicalDeclPtr<const Decl>, UsesAllocatorsDeclKind> UsesAllocatorsDecls; + struct ImplicitDefaultFDInfoTy { + const FieldDecl *FD = nullptr; + size_t Sz = -1; + VarDecl *VD = nullptr; + ImplicitDefaultFDInfoTy(const FieldDecl *FD, size_t Sz, VarDecl *VD) + : FD(FD), Sz(Sz), VD(VD) {} + }; + /// List of captuer fields + llvm::SmallVector<ImplicitDefaultFDInfoTy, 8> + ImplicitDefaultFirstprivateFDs; Expr *DeclareMapperVar = nullptr; SharingMapTy(OpenMPDirectiveKind DKind, DeclarationNameInfo Name, Scope *CurScope, SourceLocation Loc) @@ -577,7 +587,9 @@ /// predicate. const DSAVarData hasDSA(ValueDecl *D, - const llvm::function_ref<bool(OpenMPClauseKind, bool)> CPred, + const llvm::function_ref<bool(OpenMPClauseKind, bool, + DefaultDataSharingAttributes)> + CPred, const llvm::function_ref<bool(OpenMPDirectiveKind)> DPred, bool FromParent) const; /// Checks if the specified variables has data-sharing attributes which @@ -1120,6 +1132,50 @@ const SharingMapTy *Top = getTopOfStackOrNull(); return Top ? Top->DeclareMapperVar : nullptr; } + /// get captured field from ImplicitDefaultFirstprivateFDs + VarDecl *getImplicitFDCapExprDecl(const FieldDecl *FD) const { + const_iterator I = begin(); + const_iterator EndI = end(); + size_t Sz = getStackSize(); + for (; I != EndI; ++I) { + if (I->DefaultAttr == DSA_firstprivate || I->DefaultAttr == DSA_private) + break; + Sz--; + } + if (I == EndI) + return nullptr; + for (const auto &IFD : I->ImplicitDefaultFirstprivateFDs) + if (IFD.FD == FD && IFD.Sz == Sz) + return IFD.VD; + return nullptr; + } + /// Check if capture decl is field captured in ImplicitDefaultFirstprivateFDs + bool isImplicitDefaultFirstprivateFD(VarDecl *VD) const { + const_iterator I = begin(); + const_iterator EndI = end(); + for (; I != EndI; ++I) + if (I->DefaultAttr == DSA_firstprivate || I->DefaultAttr == DSA_private) + break; + if (I == EndI) + return false; + for (const auto &IFD : I->ImplicitDefaultFirstprivateFDs) + if (IFD.VD == VD) + return true; + return false; + } + /// Store capture FD info in ImplicitDefaultFirstprivateFDs + void addImplicitDefaultFirstprivateFD(const FieldDecl *FD, VarDecl *VD) { + iterator I = begin(); + const_iterator EndI = end(); + size_t Sz = getStackSize(); + for (; I != EndI; ++I) { + if (I->DefaultAttr == DSA_private || I->DefaultAttr == DSA_firstprivate) { + I->ImplicitDefaultFirstprivateFDs.emplace_back(FD, Sz, VD); + break; + } + Sz--; + } + } }; bool isImplicitTaskingRegion(OpenMPDirectiveKind DKind) { @@ -1815,7 +1871,9 @@ const DSAStackTy::DSAVarData DSAStackTy::hasDSA(ValueDecl *D, - const llvm::function_ref<bool(OpenMPClauseKind, bool)> CPred, + const llvm::function_ref<bool(OpenMPClauseKind, bool, + DefaultDataSharingAttributes)> + CPred, const llvm::function_ref<bool(OpenMPDirectiveKind)> DPred, bool FromParent) const { if (isStackEmpty()) @@ -1831,7 +1889,7 @@ continue; const_iterator NewI = I; DSAVarData DVar = getDSA(NewI, D); - if (I == NewI && CPred(DVar.CKind, DVar.AppliedToPointee)) + if (I == NewI && CPred(DVar.CKind, DVar.AppliedToPointee, I->DefaultAttr)) return DVar; } return {}; @@ -2203,6 +2261,59 @@ false); } +static OMPCapturedExprDecl *buildCaptureDecl(Sema &S, IdentifierInfo *Id, + Expr *CaptureExpr, bool WithInit, + DeclContext *CurContext, + bool AsExpression); + +VarDecl *Sema::isOpenMPFDCaptureDecl( + ValueDecl *D, Expr *Base, bool IsArrow, SourceLocation OpLoc, + const CXXScopeSpec *SS, SourceLocation TemplateKWLoc, ValueDecl *Member, + DeclAccessPair FoundDecl, bool HadMultipleCandidates, + const DeclarationNameInfo &MemberNameInfo, QualType Ty, ExprValueKind VK, + ExprObjectKind OK) { + assert(LangOpts.OpenMP && "OpenMP is not allowed"); + D = getCanonicalDecl(D); + + auto *FD = dyn_cast<FieldDecl>(D); + if (DSAStack->getCurrentDirective() != OMPD_unknown && + (!DSAStack->isClauseParsingMode() || + DSAStack->getParentDirective() != OMPD_unknown)) { + DSAStackTy::DSAVarData DVarPrivate = DSAStack->hasDSA( + D, + [](OpenMPClauseKind C, bool AppliedToPointee, + DefaultDataSharingAttributes DefaultAttr) { + return isOpenMPPrivate(C) && !AppliedToPointee && + (DefaultAttr == DSA_firstprivate || + DefaultAttr == DSA_private); + }, + [](OpenMPDirectiveKind) { return true; }, + DSAStack->isClauseParsingMode()); + if (DVarPrivate.CKind == OMPC_unknown) + return nullptr; + + VarDecl *VD = DSAStack->getImplicitFDCapExprDecl(FD); + if (!VD) { + ExprResult ME = BuildMemberExpr(Base, IsArrow, OpLoc, SS, TemplateKWLoc, + Member, FoundDecl, HadMultipleCandidates, + MemberNameInfo, Ty, VK, OK); + OMPCapturedExprDecl *CD = + buildCaptureDecl(*this, FD->getIdentifier(), ME.get(), + DVarPrivate.CKind == OMPC_private ? false : true, + CurContext->getParent(), + /*AsExpression=*/false); + DeclRefExpr *VDPrivateRefExpr = + buildDeclRefExpr(*this, CD, CD->getType().getNonReferenceType(), + MemberNameInfo.getLoc()); + VD = cast<VarDecl>(VDPrivateRefExpr->getDecl()); + DSAStack->addImplicitDefaultFirstprivateFD(FD, VD); + return VD; + } + return VD; + } + return nullptr; +} + VarDecl *Sema::isOpenMPCapturedDecl(ValueDecl *D, bool CheckScopeInfo, unsigned StopAt) { assert(LangOpts.OpenMP && "OpenMP is not allowed"); @@ -2301,7 +2412,7 @@ // default(none) clause and not used in any clause. DSAStackTy::DSAVarData DVarPrivate = DSAStack->hasDSA( D, - [](OpenMPClauseKind C, bool AppliedToPointee) { + [](OpenMPClauseKind C, bool AppliedToPointee, bool) { return isOpenMPPrivate(C) && !AppliedToPointee; }, [](OpenMPDirectiveKind) { return true; }, @@ -2317,7 +2428,10 @@ (VD && (DSAStack->getDefaultDSA() == DSA_none || DSAStack->getDefaultDSA() == DSA_private || DSAStack->getDefaultDSA() == DSA_firstprivate))) - return VD ? VD : cast<VarDecl>(DVarPrivate.PrivateCopy->getDecl()); + return VD ? VD + : DVarPrivate.PrivateCopy + ? cast<VarDecl>(DVarPrivate.PrivateCopy->getDecl()) + : nullptr; } return nullptr; } @@ -2344,6 +2458,23 @@ OpenMPClauseKind Sema::isOpenMPPrivateDecl(ValueDecl *D, unsigned Level, unsigned CapLevel) const { assert(LangOpts.OpenMP && "OpenMP is not allowed"); + if (DSAStack->getCurrentDirective() != OMPD_unknown && + (!DSAStack->isClauseParsingMode() || + DSAStack->getParentDirective() != OMPD_unknown)) { + DSAStackTy::DSAVarData DVarPrivate = DSAStack->hasDSA( + D, + [](OpenMPClauseKind C, bool AppliedToPointee, + DefaultDataSharingAttributes DefaultAttr) { + return isOpenMPPrivate(C) && !AppliedToPointee && + DefaultAttr == DSA_private; + }, + [](OpenMPDirectiveKind) { return true; }, + DSAStack->isClauseParsingMode()); + if (DVarPrivate.CKind == OMPC_private && isa<OMPCapturedExprDecl>(D) && + DSAStack->isImplicitDefaultFirstprivateFD(cast<VarDecl>(D)) && + !DSAStack->isLoopControlVariable(D).first) + return OMPC_private; + } if (DSAStack->hasExplicitDirective(isOpenMPTaskingDirective, Level)) { bool IsTriviallyCopyable = D->getType().getNonReferenceType().isTriviallyCopyableType(Context) && @@ -3524,7 +3655,8 @@ if (auto *VD = dyn_cast<VarDecl>(E->getDecl())) { // Check the datasharing rules for the expressions in the clauses. if (!CS || (isa<OMPCapturedExprDecl>(VD) && !CS->capturesVariable(VD) && - !Stack->getTopDSA(VD, /*FromParent=*/false).RefExpr)) { + !Stack->getTopDSA(VD, /*FromParent=*/false).RefExpr && + !Stack->isImplicitDefaultFirstprivateFD(VD))) { if (auto *CED = dyn_cast<OMPCapturedExprDecl>(VD)) if (!CED->hasAttr<OMPCaptureNoInitAttr>()) { Visit(CED->getInit()); @@ -3533,10 +3665,12 @@ } else if (VD->isImplicit() || isa<OMPCapturedExprDecl>(VD)) // Do not analyze internal variables and do not enclose them into // implicit clauses. - return; + if (!Stack->isImplicitDefaultFirstprivateFD(VD)) + return; VD = VD->getCanonicalDecl(); // Skip internally declared variables. if (VD->hasLocalStorage() && CS && !CS->capturesVariable(VD) && + !Stack->isImplicitDefaultFirstprivateFD(VD) && !Stack->isImplicitTaskFirstprivate(VD)) return; // Skip allocators in uses_allocators clauses. @@ -4417,6 +4551,7 @@ static OMPCapturedExprDecl *buildCaptureDecl(Sema &S, IdentifierInfo *Id, Expr *CaptureExpr, bool WithInit, + DeclContext *CurContext, bool AsExpression) { assert(CaptureExpr); ASTContext &C = S.getASTContext(); @@ -4435,11 +4570,11 @@ } WithInit = true; } - auto *CED = OMPCapturedExprDecl::Create(C, S.CurContext, Id, Ty, + auto *CED = OMPCapturedExprDecl::Create(C, CurContext, Id, Ty, CaptureExpr->getBeginLoc()); if (!WithInit) CED->addAttr(OMPCaptureNoInitAttr::CreateImplicit(C)); - S.CurContext->addHiddenDecl(CED); + CurContext->addHiddenDecl(CED); Sema::TentativeAnalysisScope Trap(S); S.AddInitializerToDecl(CED, Init, /*DirectInit=*/false); return CED; @@ -4452,6 +4587,7 @@ CD = cast<OMPCapturedExprDecl>(VD); else CD = buildCaptureDecl(S, D->getIdentifier(), CaptureExpr, WithInit, + S.CurContext, /*AsExpression=*/false); return buildDeclRefExpr(S, CD, CD->getType().getNonReferenceType(), CaptureExpr->getExprLoc()); @@ -4462,7 +4598,7 @@ if (!Ref) { OMPCapturedExprDecl *CD = buildCaptureDecl( S, &S.getASTContext().Idents.get(".capture_expr."), CaptureExpr, - /*WithInit=*/true, /*AsExpression=*/true); + /*WithInit=*/true, S.CurContext, /*AsExpression=*/true); Ref = buildDeclRefExpr(S, CD, CD->getType().getNonReferenceType(), CaptureExpr->getExprLoc()); } @@ -17238,6 +17374,8 @@ SourceLocation EndLoc) { SmallVector<Expr *, 8> Vars; SmallVector<Expr *, 8> PrivateCopies; + bool IsImplicitClause = + StartLoc.isInvalid() && LParenLoc.isInvalid() && EndLoc.isInvalid(); for (Expr *RefExpr : VarList) { assert(RefExpr && "NULL expr in OpenMP private clause."); SourceLocation ELoc; @@ -17352,9 +17490,17 @@ *this, VDPrivate, RefExpr->getType().getUnqualifiedType(), ELoc); DeclRefExpr *Ref = nullptr; - if (!VD && !CurContext->isDependentContext()) - Ref = buildCapture(*this, D, SimpleRefExpr, /*WithInit=*/false); - DSAStack->addDSA(D, RefExpr->IgnoreParens(), OMPC_private, Ref); + if (!VD && !CurContext->isDependentContext()) { + auto *FD = dyn_cast<FieldDecl>(D); + VarDecl *VD = FD ? DSAStack->getImplicitFDCapExprDecl(FD) : nullptr; + if (VD) + Ref = buildDeclRefExpr(*this, VD, VD->getType().getNonReferenceType(), + RefExpr->getExprLoc()); + else + Ref = buildCapture(*this, D, SimpleRefExpr, /*WithInit=*/false); + } + if (!IsImplicitClause) + DSAStack->addDSA(D, RefExpr->IgnoreParens(), OMPC_private, Ref); Vars.push_back((VD || CurContext->isDependentContext()) ? RefExpr->IgnoreParens() : Ref); @@ -17627,7 +17773,13 @@ if (TopDVar.CKind == OMPC_lastprivate) { Ref = TopDVar.PrivateCopy; } else { - Ref = buildCapture(*this, D, SimpleRefExpr, /*WithInit=*/true); + auto *FD = dyn_cast<FieldDecl>(D); + VarDecl *VD = FD ? DSAStack->getImplicitFDCapExprDecl(FD) : nullptr; + if (VD) + Ref = buildDeclRefExpr(*this, VD, VD->getType().getNonReferenceType(), + RefExpr->getExprLoc()); + else + Ref = buildCapture(*this, D, SimpleRefExpr, /*WithInit=*/true); if (!isOpenMPCapturedDecl(D)) ExprCaptures.push_back(Ref->getDecl()); } @@ -17898,7 +18050,7 @@ return true; DSAStackTy::DSAVarData DVarPrivate = Stack->hasDSA( VD, - [](OpenMPClauseKind C, bool AppliedToPointee) { + [](OpenMPClauseKind C, bool AppliedToPointee, bool) { return isOpenMPPrivate(C) && !AppliedToPointee; }, [](OpenMPDirectiveKind) { return true; }, Index: clang/lib/Sema/SemaExprMember.cpp =================================================================== --- clang/lib/Sema/SemaExprMember.cpp +++ clang/lib/Sema/SemaExprMember.cpp @@ -1870,6 +1870,13 @@ return getOpenMPCapturedExpr(PrivateCopy, VK, OK, MemberNameInfo.getLoc()); } + if (auto *PrivateCopy = + isOpenMPFDCaptureDecl(Field, Base.get(), IsArrow, OpLoc, &SS, + /*TemplateKWLoc=*/SourceLocation(), Field, + FoundDecl, /*HadMultipleCandidates=*/false, + MemberNameInfo, MemberType, VK, OK)) + return getOpenMPCapturedExpr(PrivateCopy, VK, OK, + MemberNameInfo.getLoc()); } return BuildMemberExpr(Base.get(), IsArrow, OpLoc, &SS, Index: clang/include/clang/Sema/Sema.h =================================================================== --- clang/include/clang/Sema/Sema.h +++ clang/include/clang/Sema/Sema.h @@ -10650,6 +10650,16 @@ /// constructs. VarDecl *isOpenMPCapturedDecl(ValueDecl *D, bool CheckScopeInfo = false, unsigned StopAt = 0); + + VarDecl *isOpenMPFDCaptureDecl(ValueDecl *D, Expr *Base, bool IsArrow, + SourceLocation OpLoc, const CXXScopeSpec *SS, + SourceLocation TemplateKWLoc, + ValueDecl *Member, DeclAccessPair FoundDecl, + bool HadMultipleCandidates, + const DeclarationNameInfo &MemberNameInfo, + QualType Ty, ExprValueKind VK, + ExprObjectKind OK); + ExprResult getOpenMPCapturedExpr(VarDecl *Capture, ExprValueKind VK, ExprObjectKind OK, SourceLocation Loc);
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits