maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

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).

llvm-svn: 362179
This commit is contained in:
Richard Smith 2019-05-31 00:45:10 +00:00
parent 2fdd95c1c8
commit 0621a8f353
8 changed files with 145 additions and 71 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
clang/include/clang/Sema/ScopeInfo.h
View File

@ -557,8 +557,8 @@ class Capture {
public:
Capture(VarDecl *Var, bool Block, bool ByRef, bool IsNested,
SourceLocation Loc, SourceLocation EllipsisLoc, QualType CaptureType,
Expr *Cpy, bool Invalid)
: CapturedVar(Var), InitExpr(Cpy), Loc(Loc), EllipsisLoc(EllipsisLoc),
bool Invalid)
: CapturedVar(Var), Loc(Loc), EllipsisLoc(EllipsisLoc),
CaptureType(CaptureType),
Kind(Block ? Cap_Block : ByRef ? Cap_ByRef : Cap_ByCopy),
Nested(IsNested), CapturesThis(false), ODRUsed(false),
@ -593,6 +593,9 @@ public:
bool isInvalid() const { return Invalid; }
/// Determine whether this capture is an init-capture.
bool isInitCapture() const;
bool isODRUsed() const { return ODRUsed; }
bool isNonODRUsed() const { return NonODRUsed; }
void markUsed(bool IsODRUse) {
@ -624,8 +627,8 @@ public:
/// that would store this capture.
QualType getCaptureType() const { return CaptureType; }
Expr *getInitExpr() const {
assert(!isVLATypeCapture() && "no init expression for type capture");
Expr *getThisInitExpr() const {
assert(isThisCapture() && "no 'this' init expression for non-this capture");
return InitExpr;
}
};
@ -665,9 +668,9 @@ public:
void addCapture(VarDecl *Var, bool isBlock, bool isByref, bool isNested,
SourceLocation Loc, SourceLocation EllipsisLoc,
QualType CaptureType, Expr *Cpy, bool Invalid) {
QualType CaptureType, bool Invalid) {
Captures.push_back(Capture(Var, isBlock, isByref, isNested, Loc,
EllipsisLoc, CaptureType, Cpy, Invalid));
EllipsisLoc, CaptureType, Invalid));
CaptureMap[Var] = Captures.size();
}

20
clang/lib/Sema/ScopeInfo.cpp
View File

@ -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() &&

2
clang/lib/Sema/SemaDecl.cpp
View File

@ -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(),

19
clang/lib/Sema/SemaExpr.cpp
View File

@ -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;
}

29
clang/lib/Sema/SemaLambda.cpp
View File

@ -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:

80
clang/lib/Sema/SemaOpenMP.cpp
View File

@ -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;

46
clang/lib/Sema/SemaStmt.cpp
View File

@ -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;
}

5
clang/test/OpenMP/for_lastprivate_codegen.cpp
View File

@ -193,10 +193,13 @@ int main() {
// LAMBDA-LABEL: @main
// LAMBDA: alloca [[SS_TY]],
// LAMBDA: alloca [[CAP_TY:%.+]],
// FIXME: The outer lambda should not capture 'sivar'; that capture is not
// used for anything.
// LAMBDA: store {{.*}}@_ZZ4mainE5sivar,
// LAMBDA: call void [[OUTER_LAMBDA:@.+]]([[CAP_TY]]*
[&]() {
// LAMBDA: define{{.*}} internal{{.*}} void [[OUTER_LAMBDA]](
// LAMBDA: call void {{.+}} @__kmpc_fork_call({{.+}}, i32 1, {{.+}}* [[OMP_REGION:@.+]] to {{.+}}, i32* %{{.+}})
// LAMBDA: call void {{.+}} @__kmpc_fork_call({{.+}}, i32 1, {{.+}}* [[OMP_REGION:@.+]] to {{.+}}, i32* @_ZZ4mainE5sivar)
#pragma omp parallel
#pragma omp for lastprivate(g, g1, sivar)
for (int i = 0; i < 2; ++i) {