sfantao updated this revision to Diff 30561.
sfantao added a comment.
Move forced capture of copyin variables to ActOnOpenMPRegionEnd and fix
function names/signatures.
Regression test is not yet included. Will do it once the adopted approach is
acceptable.
http://reviews.llvm.org/D11395
Files:
include/clang/Sema/Sema.h
lib/CodeGen/CGStmtOpenMP.cpp
lib/Sema/SemaExpr.cpp
lib/Sema/SemaOpenMP.cpp
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,13 @@
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;
@@ -669,8 +687,9 @@
bool Sema::isOpenMPPrivateVar(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; }
@@ -1358,15 +1377,29 @@
MarkDeclarationsReferencedInExpr(E);
}
}
- } else if (isParallelOrTaskRegion(DSAStack->getCurrentDirective()) &&
- Clause->getClauseKind() == OMPC_schedule) {
- // Mark all variables in private list clauses as used in inner region.
- // Required for proper codegen of combined directives.
- // TODO: add processing for other clauses.
- if (auto *E = cast_or_null<Expr>(
- cast<OMPScheduleClause>(Clause)->getHelperChunkSize())) {
+ } else if (isParallelOrTaskRegion(DSAStack->getCurrentDirective())) {
+ if (Clause->getClauseKind() == OMPC_schedule) {
+ // Mark all variables in private list clauses as used in inner region.
+ // Required for proper codegen of combined directives.
+ // TODO: add processing for other clauses.
+ if (auto *E = cast_or_null<Expr>(
+ cast<OMPScheduleClause>(Clause)->getHelperChunkSize())) {
MarkDeclarationsReferencedInExpr(E);
}
+ } else if (Clause->getClauseKind() == OMPC_copyin &&
+ getLangOpts().OpenMPUseTLS &&
+ getASTContext().getTargetInfo().isTLSSupported()) {
+ // 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. We only need to do these if supporting TLS in the
+ // OpenMP code generation.
+ for (auto *Expr : cast<OMPCopyinClause>(Clause)->varlists()) {
+ DeclRefExpr *DE = cast<DeclRefExpr>(Expr);
+ tryCaptureVariable(cast<VarDecl>(DE->getDecl()), DE->getLocation(),
+ TryCapture_Implicit);
+ }
+ }
}
}
return ActOnCapturedRegionEnd(S.get());
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) {
+ // 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 (isOpenMPPrivateVar(Var, OpenMPLevel)) {
- Nested = true;
+ 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/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,10 +7700,12 @@
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);
_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
