sfantao updated the summary for this revision.
sfantao updated this revision to Diff 30526.
sfantao added a comment.
Add logic to capture variables in the copyin clause even if there is no uses in
the captured region. We need to capture it anyway because there may be uses
that are not visible in compilation unit.
The new logic also prevents the capture declaration from being propagated to
innermost scopes.
This patch correctly fixes the issue, but wanted to make this approach is
approved so that I can go ahead and update the regression tests.
Thanks!
Samuel
http://reviews.llvm.org/D11395
Files:
include/clang/Sema/Sema.h
lib/CodeGen/CGStmtOpenMP.cpp
lib/Parse/ParseOpenMP.cpp
lib/Sema/SemaExpr.cpp
lib/Sema/SemaOpenMP.cpp
lib/Sema/TreeTransform.h
Index: lib/Sema/TreeTransform.h
===================================================================
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -6849,7 +6849,8 @@
return StmtError();
}
getDerived().getSema().ActOnOpenMPRegionStart(D->getDirectiveKind(),
- /*CurScope=*/nullptr);
+ /*CurScope=*/nullptr,
+ Clauses);
StmtResult Body;
{
Sema::CompoundScopeRAII CompoundScope(getSema());
Index: lib/Sema/SemaOpenMP.cpp
===================================================================
--- lib/Sema/SemaOpenMP.cpp
+++ lib/Sema/SemaOpenMP.cpp
@@ -80,6 +80,7 @@
struct DSAInfo {
OpenMPClauseKind Attributes;
DeclRefExpr *RefExpr;
+ DSAInfo() : Attributes(OMPC_unknown), RefExpr(nullptr) {}
};
typedef llvm::SmallDenseMap<VarDecl *, DSAInfo, 64> DeclSAMapTy;
typedef llvm::SmallDenseMap<VarDecl *, DeclRefExpr *, 64> AlignedMapTy;
@@ -156,6 +157,9 @@
/// \brief Check if the specified variable is a loop control variable for
/// current region.
bool isLoopControlVariable(VarDecl *D);
+ /// \brief Check if the specified variable is a 'copyin' variable for the
+ /// 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,13 @@
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();
+ auto L = Stack.back().SharingMap.lookup(D);
+ return L.Attributes == OMPC_copyin;
+}
+
void DSAStackTy::addDSA(VarDecl *D, DeclRefExpr *E, OpenMPClauseKind A) {
D = D->getCanonicalDecl();
if (A == OMPC_threadprivate) {
@@ -649,7 +660,7 @@
#define DSAStack static_cast<DSAStackTy *>(VarDataSharingAttributesStack)
-bool Sema::IsOpenMPCapturedVar(VarDecl *VD) {
+bool Sema::IsOpenMPCapturedVar(VarDecl *VD, bool &PropagateCapture) {
assert(LangOpts.OpenMP && "OpenMP is not allowed");
VD = VD->getCanonicalDecl();
if (DSAStack->getCurrentDirective() != OMPD_unknown) {
@@ -660,6 +671,14 @@
auto DVarPrivate = DSAStack->getTopDSA(VD, DSAStack->isClauseParsingMode());
if (DVarPrivate.CKind != OMPC_unknown && isOpenMPPrivate(DVarPrivate.CKind))
return true;
+
+ if (getLangOpts().OpenMPUseTLS &&
+ getASTContext().getTargetInfo().isTLSSupported() &&
+ !DSAStack->isClauseParsingMode() &&
+ DSAStack->isCopyinVariable(VD)) {
+ PropagateCapture = false;
+ return true;
+ }
DVarPrivate = DSAStack->hasDSA(VD, isOpenMPPrivate, MatchesAlways(),
DSAStack->isClauseParsingMode());
return DVarPrivate.CKind != OMPC_unknown;
@@ -667,10 +686,11 @@
return false;
}
-bool Sema::isOpenMPPrivateVar(VarDecl *VD, unsigned Level) {
+bool Sema::isOpenMPPrivateOrCopyinVar(VarDecl *VD, unsigned Level) {
assert(LangOpts.OpenMP && "OpenMP is not allowed");
- return DSAStack->hasExplicitDSA(
- VD, [](OpenMPClauseKind K) -> bool { return K == OMPC_private; }, Level);
+ return DSAStack->hasExplicitDSA(VD, [](OpenMPClauseKind K) -> bool {
+ return K == OMPC_private || K == OMPC_copyin;
+ }, Level);
}
void Sema::DestroyDataSharingAttributesStack() { delete DSAStack; }
@@ -1145,7 +1165,8 @@
};
} // namespace
-void Sema::ActOnOpenMPRegionStart(OpenMPDirectiveKind DKind, Scope *CurScope) {
+void Sema::ActOnOpenMPRegionStart(OpenMPDirectiveKind DKind, Scope *CurScope,
+ ArrayRef<OMPClause *> Clauses) {
switch (DKind) {
case OMPD_parallel: {
QualType KmpInt32Ty = Context.getIntTypeForBitwidth(32, 1);
@@ -1340,6 +1361,19 @@
case OMPD_unknown:
llvm_unreachable("Unknown OpenMP directive");
}
+
+ // Make sure we capture the declarations used in 'copyin' clause. We can only
+ // do this after the captured region is created. We need to try to captured
+ // those declaration even if they do not have any use in the captured region.
+ if (isOpenMPParallelDirective(DKind)) {
+ for (auto *C : Clauses)
+ if (OMPCopyinClause *CI = dyn_cast<OMPCopyinClause>(C))
+ for (auto *Expr : CI->varlists()) {
+ DeclRefExpr *DE = cast<DeclRefExpr>(Expr);
+ tryCaptureVariable(cast<VarDecl>(DE->getDecl()), DE->getLocation(),
+ TryCapture_Implicit);
+ }
+ }
}
StmtResult Sema::ActOnOpenMPRegionEnd(StmtResult S,
Index: lib/Sema/SemaExpr.cpp
===================================================================
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -12533,7 +12533,8 @@
// By default, capture variables by reference.
bool ByRef = true;
// Using an LValue reference type is consistent with Lambdas (see below).
- if (S.getLangOpts().OpenMP && S.IsOpenMPCapturedVar(Var))
+ bool PropagateCapture = true;
+ if (S.getLangOpts().OpenMP && S.IsOpenMPCapturedVar(Var, PropagateCapture))
DeclRefType = DeclRefType.getUnqualifiedType();
CaptureType = S.Context.getLValueReferenceType(DeclRefType);
Expr *CopyExpr = nullptr;
@@ -12698,8 +12699,9 @@
VarDC = VarDC->getParent();
DeclContext *DC = CurContext;
- const unsigned MaxFunctionScopesIndex = FunctionScopeIndexToStopAt
- ? *FunctionScopeIndexToStopAt : FunctionScopes.size() - 1;
+ unsigned MaxFunctionScopesIndex = FunctionScopeIndexToStopAt
+ ? *FunctionScopeIndexToStopAt
+ : FunctionScopes.size() - 1;
// We need to sync up the Declaration Context with the
// FunctionScopeIndexToStopAt
if (FunctionScopeIndexToStopAt) {
@@ -12718,7 +12720,9 @@
// Capture global variables if it is required to use private copy of this
// variable.
bool IsGlobal = !Var->hasLocalStorage();
- if (IsGlobal && !(LangOpts.OpenMP && IsOpenMPCapturedVar(Var)))
+ bool PropagateCapture = true;
+ if (IsGlobal &&
+ !(LangOpts.OpenMP && IsOpenMPCapturedVar(Var, PropagateCapture)))
return true;
// Walk up the stack to determine whether we can capture the variable,
@@ -12758,18 +12762,33 @@
// Check whether we've already captured it.
- if (isVariableAlreadyCapturedInScopeInfo(CSI, Var, Nested, CaptureType,
- DeclRefType))
+ if (isVariableAlreadyCapturedInScopeInfo(CSI, Var, Nested, CaptureType,
+ DeclRefType)) {
+ // If this capture should not be propagated and was already captured, we
+ // don't need to do anything.
+ if (!PropagateCapture)
+ return true;
break;
+ }
if (getLangOpts().OpenMP) {
if (auto *RSI = dyn_cast<CapturedRegionScopeInfo>(CSI)) {
- // OpenMP private variables should not be captured in outer scope, so
- // just break here.
+
if (RSI->CapRegionKind == CR_OpenMP) {
- if (isOpenMPPrivateVar(Var, OpenMPLevel)) {
- Nested = true;
+ // OpenMP private variables should not be captured in outer scope, so
+ // just break here.
+ // If an OpenMP copyin variable is being captured, PropagateCapture
+ // will be set to false and the capture should happen only in the
+ // scope of the copyin clause.
+ if (isOpenMPPrivateOrCopyinVar(Var, OpenMPLevel)) {
+ Nested = PropagateCapture;
DeclRefType = DeclRefType.getUnqualifiedType();
CaptureType = Context.getLValueReferenceType(DeclRefType);
+
+ if (!PropagateCapture) {
+ --FunctionScopesIndex;
+ MaxFunctionScopesIndex =
+ std::min(MaxFunctionScopesIndex, FunctionScopesIndex + 1);
+ }
break;
}
++OpenMPLevel;
@@ -12981,7 +13000,9 @@
Nested = true;
}
}
- return false;
+ // If PropagateCapture is false, we are not interested in communicating that
+ // a capture was done.
+ return !PropagateCapture;
}
bool Sema::tryCaptureVariable(VarDecl *Var, SourceLocation Loc,
Index: lib/Parse/ParseOpenMP.cpp
===================================================================
--- lib/Parse/ParseOpenMP.cpp
+++ lib/Parse/ParseOpenMP.cpp
@@ -292,7 +292,7 @@
if (HasAssociatedStatement) {
// The body is a block scope like in Lambdas and Blocks.
Sema::CompoundScopeRAII CompoundScope(Actions);
- Actions.ActOnOpenMPRegionStart(DKind, getCurScope());
+ Actions.ActOnOpenMPRegionStart(DKind, getCurScope(), Clauses);
Actions.ActOnStartOfCompoundStmt();
// Parse statement
AssociatedStmt = ParseStatement();
Index: lib/CodeGen/CGStmtOpenMP.cpp
===================================================================
--- lib/CodeGen/CGStmtOpenMP.cpp
+++ lib/CodeGen/CGStmtOpenMP.cpp
@@ -227,10 +227,22 @@
auto *VD = cast<VarDecl>(cast<DeclRefExpr>(*IRef)->getDecl());
QualType Type = VD->getType();
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();
if (CopiedVars.size() == 1) {
Index: include/clang/Sema/Sema.h
===================================================================
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -7700,13 +7700,15 @@
public:
/// \brief Check if the specified variable is used in one of the private
/// clauses (private, firstprivate, lastprivate, reduction etc.) in OpenMP
- /// constructs.
- bool IsOpenMPCapturedVar(VarDecl *VD);
+ /// constructs. \a PropagateCapture is set to false if the capture should not
+ /// be propagated into innermost scopes.
+ bool IsOpenMPCapturedVar(VarDecl *VD, bool &PropagateCapture);
- /// \brief Check if the specified variable is used in 'private' clause.
+ /// \brief Check if the specified variable is used in 'private' or 'copyin'
+ /// clause.
/// \param Level Relative level of nested OpenMP construct for that the check
/// is performed.
- bool isOpenMPPrivateVar(VarDecl *VD, unsigned Level);
+ bool isOpenMPPrivateOrCopyinVar(VarDecl *VD, unsigned Level);
ExprResult PerformOpenMPImplicitIntegerConversion(SourceLocation OpLoc,
Expr *Op);
@@ -7743,7 +7745,10 @@
ArrayRef<Expr *> VarList);
/// \brief Initialization of captured region for OpenMP region.
- void ActOnOpenMPRegionStart(OpenMPDirectiveKind DKind, Scope *CurScope);
+ ///
+ /// \param Clauses List of clauses for the current OpenMP region.
+ void ActOnOpenMPRegionStart(OpenMPDirectiveKind DKind, Scope *CurScope,
+ ArrayRef<OMPClause *> Clauses);
/// \brief End of OpenMP region.
///
/// \param S Statement associated with the current OpenMP region.
_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
