diff options
author | Richard Smith <richard-llvm@metafoo.co.uk> | 2019-05-31 00:45:10 +0000 |
---|---|---|
committer | Richard Smith <richard-llvm@metafoo.co.uk> | 2019-05-31 00:45:10 +0000 |
commit | 4c6199e026e6ff47812f39c9802313b23cd26107 (patch) | |
tree | 73d695752aa2d1f891f98245dc5670141c59a2a5 /lib | |
parent | 00cfd3ff27bf1b2d323717402403086af1dc3237 (diff) | |
download | clang-4c6199e026e6ff47812f39c9802313b23cd26107.tar.gz |
Defer capture initialization for captured regions until after we've left
the captured region scope.
This removes a case where we would build expressions (and mark
declarations odr-used) in the wrong scope.
Remove the now-unused 'capture initializer' field on sema::Capture
(except for 'this' captures, which still need to be cleaned up).
No functionality change intended (except that we now very slightly more
precisely determine whether we need to use a capture or not when another
captured region encloses an OpenMP captured region).
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@362179 91177308-0d34-0410-b5e6-96231b3b80d8
Diffstat (limited to 'lib')
-rw-r--r-- | lib/Sema/ScopeInfo.cpp | 20 | ||||
-rw-r--r-- | lib/Sema/SemaDecl.cpp | 2 | ||||
-rw-r--r-- | lib/Sema/SemaExpr.cpp | 19 | ||||
-rw-r--r-- | lib/Sema/SemaLambda.cpp | 29 | ||||
-rw-r--r-- | lib/Sema/SemaOpenMP.cpp | 80 | ||||
-rw-r--r-- | lib/Sema/SemaStmt.cpp | 46 |
6 files changed, 132 insertions, 64 deletions
diff --git a/lib/Sema/ScopeInfo.cpp b/lib/Sema/ScopeInfo.cpp index dd309a2811..e84e592a48 100644 --- a/lib/Sema/ScopeInfo.cpp +++ b/lib/Sema/ScopeInfo.cpp @@ -112,13 +112,6 @@ FunctionScopeInfo::WeakObjectProfileTy::getBaseInfo(const Expr *E) { return BaseInfoTy(D, IsExact); } -bool CapturingScopeInfo::isVLATypeCaptured(const VariableArrayType *VAT) const { - for (auto &Cap : Captures) - if (Cap.isVLATypeCapture() && Cap.getCapturedVLAType() == VAT) - return true; - return false; -} - FunctionScopeInfo::WeakObjectProfileTy::WeakObjectProfileTy( const ObjCPropertyRefExpr *PropE) : Base(nullptr, true), Property(getBestPropertyDecl(PropE)) { @@ -223,6 +216,19 @@ void FunctionScopeInfo::markSafeWeakUse(const Expr *E) { ThisUse->markSafe(); } +bool Capture::isInitCapture() const { + // Note that a nested capture of an init-capture is not itself an + // init-capture. + return !isNested() && isVariableCapture() && getVariable()->isInitCapture(); +} + +bool CapturingScopeInfo::isVLATypeCaptured(const VariableArrayType *VAT) const { + for (auto &Cap : Captures) + if (Cap.isVLATypeCapture() && Cap.getCapturedVLAType() == VAT) + return true; + return false; +} + void LambdaScopeInfo::getPotentialVariableCapture(unsigned Idx, VarDecl *&VD, Expr *&E) const { assert(Idx < getNumPotentialVariableCaptures() && diff --git a/lib/Sema/SemaDecl.cpp b/lib/Sema/SemaDecl.cpp index fbc410f014..759eb531c5 100644 --- a/lib/Sema/SemaDecl.cpp +++ b/lib/Sema/SemaDecl.cpp @@ -12939,7 +12939,7 @@ static void RebuildLambdaScopeInfo(CXXMethodDecl *CallOperator, /*RefersToEnclosingVariableOrCapture*/true, C.getLocation(), /*EllipsisLoc*/C.isPackExpansion() ? C.getEllipsisLoc() : SourceLocation(), - CaptureType, /*Expr*/ nullptr, /*Invalid*/false); + CaptureType, /*Invalid*/false); } else if (C.capturesThis()) { LSI->addThisCapture(/*Nested*/ false, C.getLocation(), I->getType(), diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp index 7a86e885bd..5746a102b7 100644 --- a/lib/Sema/SemaExpr.cpp +++ b/lib/Sema/SemaExpr.cpp @@ -15326,7 +15326,7 @@ static bool captureInBlock(BlockScopeInfo *BSI, VarDecl *Var, // Actually capture the variable. if (BuildAndDiagnose) BSI->addCapture(Var, HasBlocksAttr, ByRef, Nested, Loc, SourceLocation(), - CaptureType, nullptr, Invalid); + CaptureType, Invalid); return !Invalid; } @@ -15360,22 +15360,10 @@ static bool captureInCapturedRegion(CapturedRegionScopeInfo *RSI, else CaptureType = DeclRefType; - Expr *CopyExpr = nullptr; - if (BuildAndDiagnose) { - // The current implementation assumes that all variables are captured - // by references. Since there is no capture by copy, no expression - // evaluation will be needed. - CopyExpr = new (S.Context) DeclRefExpr( - S.Context, Var, RefersToCapturedVariable, DeclRefType, VK_LValue, Loc); - Var->setReferenced(true); - Var->markUsed(S.Context); - } - // Actually capture the variable. if (BuildAndDiagnose) RSI->addCapture(Var, /*isBlock*/ false, ByRef, RefersToCapturedVariable, - Loc, SourceLocation(), CaptureType, CopyExpr, - Invalid); + Loc, SourceLocation(), CaptureType, Invalid); return !Invalid; } @@ -15474,8 +15462,7 @@ static bool captureInLambda(LambdaScopeInfo *LSI, // Add the capture. if (BuildAndDiagnose) LSI->addCapture(Var, /*IsBlock=*/false, ByRef, RefersToCapturedVariable, - Loc, EllipsisLoc, CaptureType, /*CopyExpr=*/nullptr, - Invalid); + Loc, EllipsisLoc, CaptureType, Invalid); return !Invalid; } diff --git a/lib/Sema/SemaLambda.cpp b/lib/Sema/SemaLambda.cpp index 6d487cc832..a17a3da67f 100644 --- a/lib/Sema/SemaLambda.cpp +++ b/lib/Sema/SemaLambda.cpp @@ -844,9 +844,10 @@ VarDecl *Sema::createLambdaInitCaptureVarDecl(SourceLocation Loc, } void Sema::addInitCapture(LambdaScopeInfo *LSI, VarDecl *Var) { + assert(Var->isInitCapture() && "init capture flag should be set"); LSI->addCapture(Var, /*isBlock*/false, Var->getType()->isReferenceType(), /*isNested*/false, Var->getLocation(), SourceLocation(), - Var->getType(), Var->getInit(), /*Invalid*/false); + Var->getType(), /*Invalid*/false); } void Sema::ActOnStartOfLambdaDefinition(LambdaIntroducer &Intro, @@ -1488,8 +1489,8 @@ mapImplicitCaptureStyle(CapturingScopeInfo::ImplicitCaptureStyle ICS) { } bool Sema::CaptureHasSideEffects(const Capture &From) { - if (!From.isVLATypeCapture()) { - Expr *Init = From.getInitExpr(); + if (From.isInitCapture()) { + Expr *Init = From.getVariable()->getInit(); if (Init && Init->HasSideEffects(Context)) return true; } @@ -1637,7 +1638,7 @@ ExprResult Sema::BuildLambdaExpr(SourceLocation StartLoc, SourceLocation EndLoc, if (!CurContext->isDependentContext() && !IsImplicit && !From.isODRUsed()) { // Initialized captures that are non-ODR used may not be eliminated. bool NonODRUsedInitCapture = - IsGenericLambda && From.isNonODRUsed() && From.getInitExpr(); + IsGenericLambda && From.isNonODRUsed() && From.isInitCapture(); if (!NonODRUsedInitCapture) { bool IsLast = (I + 1) == LSI->NumExplicitCaptures; SourceRange FixItRange; @@ -1682,7 +1683,7 @@ ExprResult Sema::BuildLambdaExpr(SourceLocation StartLoc, SourceLocation EndLoc, Captures.push_back( LambdaCapture(From.getLocation(), IsImplicit, From.isCopyCapture() ? LCK_StarThis : LCK_This)); - CaptureInits.push_back(From.getInitExpr()); + CaptureInits.push_back(From.getThisInitExpr()); continue; } if (From.isVLATypeCapture()) { @@ -1696,15 +1697,15 @@ ExprResult Sema::BuildLambdaExpr(SourceLocation StartLoc, SourceLocation EndLoc, LambdaCaptureKind Kind = From.isCopyCapture() ? LCK_ByCopy : LCK_ByRef; Captures.push_back(LambdaCapture(From.getLocation(), IsImplicit, Kind, Var, From.getEllipsisLoc())); - Expr *Init = From.getInitExpr(); - if (!Init) { - auto InitResult = performLambdaVarCaptureInitialization( - *this, From, Field, CaptureDefaultLoc, IsImplicit); - if (InitResult.isInvalid()) - return ExprError(); - Init = InitResult.get(); - } - CaptureInits.push_back(Init); + + ExprResult Init = + From.isInitCapture() + ? Var->getInit() + : performLambdaVarCaptureInitialization( + *this, From, Field, CaptureDefaultLoc, IsImplicit); + if (Init.isInvalid()) + return ExprError(); + CaptureInits.push_back(Init.get()); } // C++11 [expr.prim.lambda]p6: diff --git a/lib/Sema/SemaOpenMP.cpp b/lib/Sema/SemaOpenMP.cpp index 5a6b49961f..7e75a98070 100644 --- a/lib/Sema/SemaOpenMP.cpp +++ b/lib/Sema/SemaOpenMP.cpp @@ -143,6 +143,7 @@ private: bool NowaitRegion = false; bool CancelRegion = false; bool LoopStart = false; + bool BodyComplete = false; SourceLocation InnerTeamsRegionLoc; /// Reference to the taskgroup task_reduction reference expression. Expr *TaskgroupReductionRef = nullptr; @@ -172,19 +173,22 @@ private: /// captured by reference. bool ForceCaptureByReferenceInTargetExecutable = false; CriticalsWithHintsTy Criticals; + unsigned IgnoredStackElements = 0; /// Iterators over the stack iterate in order from innermost to outermost /// directive. using const_iterator = StackTy::const_reverse_iterator; const_iterator begin() const { - return Stack.empty() ? const_iterator() : Stack.back().first.rbegin(); + return Stack.empty() ? const_iterator() + : Stack.back().first.rbegin() + IgnoredStackElements; } const_iterator end() const { return Stack.empty() ? const_iterator() : Stack.back().first.rend(); } using iterator = StackTy::reverse_iterator; iterator begin() { - return Stack.empty() ? iterator() : Stack.back().first.rbegin(); + return Stack.empty() ? iterator() + : Stack.back().first.rbegin() + IgnoredStackElements; } iterator end() { return Stack.empty() ? iterator() : Stack.back().first.rend(); @@ -195,16 +199,18 @@ private: bool isStackEmpty() const { return Stack.empty() || Stack.back().second != CurrentNonCapturingFunctionScope || - Stack.back().first.empty(); + Stack.back().first.size() <= IgnoredStackElements; } size_t getStackSize() const { - return isStackEmpty() ? 0 : Stack.back().first.size(); + return isStackEmpty() ? 0 + : Stack.back().first.size() - IgnoredStackElements; } SharingMapTy *getTopOfStackOrNull() { - if (isStackEmpty()) + size_t Size = getStackSize(); + if (Size == 0) return nullptr; - return &Stack.back().first.back(); + return &Stack.back().first[Size - 1]; } const SharingMapTy *getTopOfStackOrNull() const { return const_cast<DSAStackTy&>(*this).getTopOfStackOrNull(); @@ -280,6 +286,14 @@ public: } void setClauseParsingMode(OpenMPClauseKind K) { ClauseKindMode = K; } + bool isBodyComplete() const { + const SharingMapTy *Top = getTopOfStackOrNull(); + return Top && Top->BodyComplete; + } + void setBodyComplete() { + getTopOfStack().BodyComplete = true; + } + bool isForceVarCapturing() const { return ForceCapturing; } void setForceVarCapturing(bool V) { ForceCapturing = V; } @@ -292,6 +306,8 @@ public: void push(OpenMPDirectiveKind DKind, const DeclarationNameInfo &DirName, Scope *CurScope, SourceLocation Loc) { + assert(!IgnoredStackElements && + "cannot change stack while ignoring elements"); if (Stack.empty() || Stack.back().second != CurrentNonCapturingFunctionScope) Stack.emplace_back(StackTy(), CurrentNonCapturingFunctionScope); @@ -300,11 +316,39 @@ public: } void pop() { + assert(!IgnoredStackElements && + "cannot change stack while ignoring elements"); assert(!Stack.back().first.empty() && "Data-sharing attributes stack is empty!"); Stack.back().first.pop_back(); } + /// RAII object to temporarily leave the scope of a directive when we want to + /// logically operate in its parent. + class ParentDirectiveScope { + DSAStackTy &Self; + bool Active; + public: + ParentDirectiveScope(DSAStackTy &Self, bool Activate) + : Self(Self), Active(false) { + if (Activate) + enable(); + } + ~ParentDirectiveScope() { disable(); } + void disable() { + if (Active) { + --Self.IgnoredStackElements; + Active = false; + } + } + void enable() { + if (!Active) { + ++Self.IgnoredStackElements; + Active = true; + } + } + }; + /// Marks that we're started loop parsing. void loopInit() { assert(isOpenMPLoopDirective(getCurrentDirective()) && @@ -334,12 +378,16 @@ public: } /// Start new OpenMP region stack in new non-capturing function. void pushFunction() { + assert(!IgnoredStackElements && + "cannot change stack while ignoring elements"); const FunctionScopeInfo *CurFnScope = SemaRef.getCurFunction(); assert(!isa<CapturingScopeInfo>(CurFnScope)); CurrentNonCapturingFunctionScope = CurFnScope; } /// Pop region stack for non-capturing function. void popFunction(const FunctionScopeInfo *OldFSI) { + assert(!IgnoredStackElements && + "cannot change stack while ignoring elements"); if (!Stack.empty() && Stack.back().second == OldFSI) { assert(Stack.back().first.empty()); Stack.pop_back(); @@ -1711,13 +1759,20 @@ VarDecl *Sema::isOpenMPCapturedDecl(ValueDecl *D, bool CheckScopeInfo, assert(LangOpts.OpenMP && "OpenMP is not allowed"); D = getCanonicalDecl(D); + // If we want to determine whether the variable should be captured from the + // perspective of the current capturing scope, and we've already left all the + // capturing scopes of the top directive on the stack, check from the + // perspective of its parent directive (if any) instead. + DSAStackTy::ParentDirectiveScope InParentDirectiveRAII( + *DSAStack, CheckScopeInfo && DSAStack->isBodyComplete()); + // If we are attempting to capture a global variable in a directive with // 'target' we return true so that this global is also mapped to the device. // auto *VD = dyn_cast<VarDecl>(D); - if (VD && !VD->hasLocalStorage()) { - if (isInOpenMPDeclareTargetContext() && - (getCurCapturedRegion() || getCurBlock() || getCurLambda())) { + if (VD && !VD->hasLocalStorage() && + (getCurCapturedRegion() || getCurBlock() || getCurLambda())) { + if (isInOpenMPDeclareTargetContext()) { // Try to mark variable as declare target if it is used in capturing // regions. if (!OMPDeclareTargetDeclAttr::isDeclareTargetDeclaration(VD)) @@ -1734,6 +1789,7 @@ VarDecl *Sema::isOpenMPCapturedDecl(ValueDecl *D, bool CheckScopeInfo, } // Capture variables captured by reference in lambdas for target-based // directives. + // FIXME: Triggering capture from here is completely inappropriate. if (VD && !DSAStack->isClauseParsingMode()) { if (const auto *RD = VD->getType() .getCanonicalType() @@ -1742,6 +1798,7 @@ VarDecl *Sema::isOpenMPCapturedDecl(ValueDecl *D, bool CheckScopeInfo, bool SavedForceCaptureByReferenceInTargetExecutable = DSAStack->isForceCaptureByReferenceInTargetExecutable(); DSAStack->setForceCaptureByReferenceInTargetExecutable(/*V=*/true); + InParentDirectiveRAII.disable(); if (RD->isLambda()) { llvm::DenseMap<const VarDecl *, FieldDecl *> Captures; FieldDecl *ThisCapture; @@ -1771,6 +1828,8 @@ VarDecl *Sema::isOpenMPCapturedDecl(ValueDecl *D, bool CheckScopeInfo, } } } + if (CheckScopeInfo && DSAStack->isBodyComplete()) + InParentDirectiveRAII.enable(); DSAStack->setForceCaptureByReferenceInTargetExecutable( SavedForceCaptureByReferenceInTargetExecutable); } @@ -3392,6 +3451,7 @@ StmtResult Sema::ActOnOpenMPRegionEnd(StmtResult S, return StmtError(); } StmtResult SR = S; + unsigned CompletedRegions = 0; for (OpenMPDirectiveKind ThisCaptureRegion : llvm::reverse(CaptureRegions)) { // Mark all variables in private list clauses as used in inner region. // Required for proper codegen of combined directives. @@ -3413,6 +3473,8 @@ StmtResult Sema::ActOnOpenMPRegionEnd(StmtResult S, } } } + if (++CompletedRegions == CaptureRegions.size()) + DSAStack->setBodyComplete(); SR = ActOnCapturedRegionEnd(SR.get()); } return SR; diff --git a/lib/Sema/SemaStmt.cpp b/lib/Sema/SemaStmt.cpp index 357e257abe..7a9a801b18 100644 --- a/lib/Sema/SemaStmt.cpp +++ b/lib/Sema/SemaStmt.cpp @@ -4223,7 +4223,7 @@ Sema::CreateCapturedStmtRecordDecl(CapturedDecl *&CD, SourceLocation Loc, return RD; } -static void +static bool buildCapturedStmtCaptureList(Sema &S, CapturedRegionScopeInfo *RSI, SmallVectorImpl<CapturedStmt::Capture> &Captures, SmallVectorImpl<Expr *> &CaptureInits) { @@ -4237,7 +4237,7 @@ buildCapturedStmtCaptureList(Sema &S, CapturedRegionScopeInfo *RSI, if (Cap.isThisCapture()) { Captures.push_back(CapturedStmt::Capture(Cap.getLocation(), CapturedStmt::VCK_This)); - CaptureInits.push_back(Cap.getInitExpr()); + CaptureInits.push_back(Cap.getThisInitExpr()); continue; } else if (Cap.isVLATypeCapture()) { Captures.push_back( @@ -4248,13 +4248,25 @@ buildCapturedStmtCaptureList(Sema &S, CapturedRegionScopeInfo *RSI, if (S.getLangOpts().OpenMP && RSI->CapRegionKind == CR_OpenMP) S.setOpenMPCaptureKind(Field, Cap.getVariable(), RSI->OpenMPLevel); - Captures.push_back(CapturedStmt::Capture(Cap.getLocation(), + + VarDecl *Var = Cap.getVariable(); + SourceLocation Loc = Cap.getLocation(); + + // FIXME: For a non-reference capture, we need to build an expression to + // perform a copy here! + ExprResult Init = S.BuildDeclarationNameExpr( + CXXScopeSpec(), DeclarationNameInfo(Var->getDeclName(), Loc), Var); + if (Init.isInvalid()) + return true; + + Captures.push_back(CapturedStmt::Capture(Loc, Cap.isReferenceCapture() ? CapturedStmt::VCK_ByRef : CapturedStmt::VCK_ByCopy, - Cap.getVariable())); - CaptureInits.push_back(Cap.getInitExpr()); + Var)); + CaptureInits.push_back(Init.get()); } + return false; } void Sema::ActOnCapturedRegionStart(SourceLocation Loc, Scope *CurScope, @@ -4347,25 +4359,31 @@ void Sema::ActOnCapturedRegionStart(SourceLocation Loc, Scope *CurScope, void Sema::ActOnCapturedRegionError() { DiscardCleanupsInEvaluationContext(); PopExpressionEvaluationContext(); + PopDeclContext(); + PoppedFunctionScopePtr ScopeRAII = PopFunctionScopeInfo(); + CapturedRegionScopeInfo *RSI = cast<CapturedRegionScopeInfo>(ScopeRAII.get()); - CapturedRegionScopeInfo *RSI = getCurCapturedRegion(); RecordDecl *Record = RSI->TheRecordDecl; Record->setInvalidDecl(); SmallVector<Decl*, 4> Fields(Record->fields()); ActOnFields(/*Scope=*/nullptr, Record->getLocation(), Record, Fields, SourceLocation(), SourceLocation(), ParsedAttributesView()); - - PopDeclContext(); - PopFunctionScopeInfo(); } StmtResult Sema::ActOnCapturedRegionEnd(Stmt *S) { - CapturedRegionScopeInfo *RSI = getCurCapturedRegion(); + // Leave the captured scope before we start creating captures in the + // enclosing scope. + DiscardCleanupsInEvaluationContext(); + PopExpressionEvaluationContext(); + PopDeclContext(); + PoppedFunctionScopePtr ScopeRAII = PopFunctionScopeInfo(); + CapturedRegionScopeInfo *RSI = cast<CapturedRegionScopeInfo>(ScopeRAII.get()); SmallVector<CapturedStmt::Capture, 4> Captures; SmallVector<Expr *, 4> CaptureInits; - buildCapturedStmtCaptureList(*this, RSI, Captures, CaptureInits); + if (buildCapturedStmtCaptureList(*this, RSI, Captures, CaptureInits)) + return StmtError(); CapturedDecl *CD = RSI->TheCapturedDecl; RecordDecl *RD = RSI->TheRecordDecl; @@ -4377,11 +4395,5 @@ StmtResult Sema::ActOnCapturedRegionEnd(Stmt *S) { CD->setBody(Res->getCapturedStmt()); RD->completeDefinition(); - DiscardCleanupsInEvaluationContext(); - PopExpressionEvaluationContext(); - - PopDeclContext(); - PopFunctionScopeInfo(); - return Res; } |