Hi rjmccall, rsmith, hfinkel,
For proper codegen we need to capture variable in the OpenMP region. In
loop-based directives loop control variables are private by default and they
must be captured in this region. There was a problem with capturing of globals,
used as lcv, as they was not marked as private by default.
http://reviews.llvm.org/D9336
Files:
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Sema/Sema.h
lib/Parse/ParseStmt.cpp
lib/Sema/SemaOpenMP.cpp
test/OpenMP/for_codegen.cpp
test/OpenMP/for_loop_messages.cpp
test/OpenMP/for_simd_loop_messages.cpp
test/OpenMP/parallel_for_loop_messages.cpp
test/OpenMP/parallel_for_simd_loop_messages.cpp
test/OpenMP/simd_loop_messages.cpp
EMAIL PREFERENCES
http://reviews.llvm.org/settings/panel/emailpreferences/
Index: test/OpenMP/parallel_for_loop_messages.cpp
===================================================================
--- test/OpenMP/parallel_for_loop_messages.cpp
+++ test/OpenMP/parallel_for_loop_messages.cpp
@@ -265,14 +265,12 @@
}
{
-// expected-error@+2 {{loop iteration variable in the associated loop of 'omp parallel for' directive may not be a variable with global storage without being explicitly marked as private}}
#pragma omp parallel for
for (globalii = 0; globalii < 10; globalii += 1)
c[globalii] = a[globalii];
}
{
-// expected-error@+3 {{loop iteration variable in the associated loop of 'omp parallel for' directive may not be a variable with global storage without being explicitly marked as private}}
#pragma omp parallel for collapse(2)
for (ii = 0; ii < 10; ii += 1)
for (globalii = 0; globalii < 10; globalii += 1)
Index: test/OpenMP/simd_loop_messages.cpp
===================================================================
--- test/OpenMP/simd_loop_messages.cpp
+++ test/OpenMP/simd_loop_messages.cpp
@@ -260,15 +260,13 @@
#pragma omp parallel
{
- // expected-error@+2 {{loop iteration variable in the associated loop of 'omp simd' directive may not be a variable with global storage without being explicitly marked as linear}}
#pragma omp simd
for (globalii = 0; globalii < 10; globalii+=1)
c[globalii] = a[globalii];
}
#pragma omp parallel
{
-// expected-error@+3 {{loop iteration variable in the associated loop of 'omp simd' directive may not be a variable with global storage without being explicitly marked as lastprivate}}
#pragma omp simd collapse(2)
for (ii = 0; ii < 10; ii += 1)
for (globalii = 0; globalii < 10; globalii += 1)
Index: test/OpenMP/parallel_for_simd_loop_messages.cpp
===================================================================
--- test/OpenMP/parallel_for_simd_loop_messages.cpp
+++ test/OpenMP/parallel_for_simd_loop_messages.cpp
@@ -266,14 +266,12 @@
}
{
-// expected-error@+2 {{loop iteration variable in the associated loop of 'omp parallel for simd' directive may not be a variable with global storage without being explicitly marked as linear}}
#pragma omp parallel for simd
for (globalii = 0; globalii < 10; globalii += 1)
c[globalii] = a[globalii];
}
{
-// expected-error@+3 {{loop iteration variable in the associated loop of 'omp parallel for simd' directive may not be a variable with global storage without being explicitly marked as lastprivate}}
#pragma omp parallel for simd collapse(2)
for (ii = 0; ii < 10; ii += 1)
for (globalii = 0; globalii < 10; globalii += 1)
Index: test/OpenMP/for_simd_loop_messages.cpp
===================================================================
--- test/OpenMP/for_simd_loop_messages.cpp
+++ test/OpenMP/for_simd_loop_messages.cpp
@@ -314,15 +314,13 @@
#pragma omp parallel
{
-// expected-error@+2 {{loop iteration variable in the associated loop of 'omp for simd' directive may not be a variable with global storage without being explicitly marked as linear}}
#pragma omp for simd
for (globalii = 0; globalii < 10; globalii += 1)
c[globalii] = a[globalii];
}
#pragma omp parallel
{
-// expected-error@+3 {{loop iteration variable in the associated loop of 'omp for simd' directive may not be a variable with global storage without being explicitly marked as lastprivate}}
#pragma omp for simd collapse(2)
for (ii = 0; ii < 10; ii += 1)
for (globalii = 0; globalii < 10; globalii += 1)
Index: test/OpenMP/for_loop_messages.cpp
===================================================================
--- test/OpenMP/for_loop_messages.cpp
+++ test/OpenMP/for_loop_messages.cpp
@@ -313,15 +313,13 @@
#pragma omp parallel
{
-// expected-error@+2 {{loop iteration variable in the associated loop of 'omp for' directive may not be a variable with global storage without being explicitly marked as private}}
#pragma omp for
for (globalii = 0; globalii < 10; globalii += 1)
c[globalii] = a[globalii];
}
#pragma omp parallel
{
-// expected-error@+3 {{loop iteration variable in the associated loop of 'omp for' directive may not be a variable with global storage without being explicitly marked as private}}
#pragma omp for collapse(2)
for (ii = 0; ii < 10; ii += 1)
for (globalii = 0; globalii < 10; globalii += 1)
Index: test/OpenMP/for_codegen.cpp
===================================================================
--- test/OpenMP/for_codegen.cpp
+++ test/OpenMP/for_codegen.cpp
@@ -8,7 +8,11 @@
#define HEADER
// CHECK: [[IDENT_T_TY:%.+]] = type { i32, i32, i32, i32, i8* }
-// CHECK: [[IMPLICIT_BARRIER_LOC:@.+]] = private unnamed_addr constant %{{.+}} { i32 0, i32 66, i32 0, i32 0, i8*
+// CHECK-DAG: [[IMPLICIT_BARRIER_LOC:@.+]] = private unnamed_addr constant %{{.+}} { i32 0, i32 66, i32 0, i32 0, i8*
+// CHECK-DAG: [[I:@.+]] = global i8 1,
+// CHECK-DAG: [[J:@.+]] = global i8 2,
+// CHECK-DAG: [[K:@.+]] = global i8 3,
+
// CHECK-LABEL: define {{.*void}} @{{.*}}without_schedule_clause{{.*}}(float* {{.+}}, float* {{.+}}, float* {{.+}}, float* {{.+}})
void without_schedule_clause(float *a, float *b, float *c, float *d) {
// CHECK: [[GTID:%.+]] = call i32 @__kmpc_global_thread_num([[IDENT_T_TY]]* [[DEFAULT_LOC:[@%].+]])
@@ -365,5 +369,41 @@
// TERM_DEBUG-DAG: [[DBG_LOC_END]] = !MDLocation(line: [[@LINE-16]],
// TERM_DEBUG-DAG: [[DBG_LOC_CANCEL]] = !MDLocation(line: [[@LINE-17]],
+char i = 1, j = 2, k = 3;
+// CHECK-LABEL: for_with_global_lcv
+void for_with_global_lcv() {
+// CHECK: [[I_ADDR:%.+]] = alloca i8,
+// CHECK: [[J_ADDR:%.+]] = alloca i8,
+
+// CHECK: call void @__kmpc_for_static_init_4(
+// CHECK-NOT: [[I]]
+// CHECK: store i8 %{{.+}}, i8* [[I_ADDR]]
+// CHECK-NOT: [[I]]
+// CHECK: [[I_VAL:%.+]] = load i8, i8* [[I_ADDR]],
+// CHECK-NOT: [[I]]
+// CHECK: store i8 [[I_VAL]], i8* [[K]]
+// CHECK-NOT: [[I]]
+// CHECK: call void @__kmpc_for_static_fini(
+#pragma omp for
+ for (i = 0; i < 2; ++i) {
+ k = i;
+ }
+// CHECK: call void @__kmpc_for_static_init_4(
+// CHECK-NOT: [[J]]
+// CHECK: store i8 %{{.+}}, i8* [[J_ADDR]]
+// CHECK-NOT: [[J]]
+// CHECK: [[J_VAL:%.+]] = load i8, i8* [[J_ADDR]],
+// CHECK-NOT: [[J]]
+// CHECK: store i8 [[J_VAL]], i8* [[K]]
+// CHECK-NOT: [[J]]
+// CHECK: call void @__kmpc_for_static_fini(
+#pragma omp for collapse(2)
+ for (int i = 0; i < 2; ++i)
+ for (j = 0; j < 2; ++j) {
+ k = i;
+ k = j;
+ }
+}
+
#endif // HEADER
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -7421,8 +7421,6 @@
"implicitly determined as %0">;
def err_omp_loop_var_dsa : Error<
"loop iteration variable in the associated loop of 'omp %1' directive may not be %0, predetermined as %2">;
-def err_omp_global_loop_var_dsa : Error<
- "loop iteration variable in the associated loop of 'omp %1' directive may not be a variable with global storage without being explicitly marked as %0">;
def err_omp_not_for : Error<
"%select{statement after '#pragma omp %1' must be a for loop|"
"expected %2 for loops after '#pragma omp %1'%select{|, but found only %4}3}0">;
Index: include/clang/Sema/Sema.h
===================================================================
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -7424,6 +7424,12 @@
/// \brief Called on end of data sharing attribute block.
void EndOpenMPDSABlock(Stmt *CurDirective);
+ /// \brief Checks if the current region is an OpenMP loop region and if it so,
+ /// marks loop control variable, used in \a Init for loop initialization, as
+ /// private by default.
+ /// \param Init First part of the for loop.
+ void ActOnOpenMPLoopInitialization(SourceLocation ForLoc, Stmt *Init);
+
// OpenMP directives and clauses.
/// \brief Called on correct id-expression from the '#pragma omp
/// threadprivate'.
Index: lib/Sema/SemaOpenMP.cpp
===================================================================
--- lib/Sema/SemaOpenMP.cpp
+++ lib/Sema/SemaOpenMP.cpp
@@ -82,27 +82,32 @@
};
typedef llvm::SmallDenseMap<VarDecl *, DSAInfo, 64> DeclSAMapTy;
typedef llvm::SmallDenseMap<VarDecl *, DeclRefExpr *, 64> AlignedMapTy;
+ typedef llvm::DenseSet<VarDecl *> LoopControlVariablesSetTy;
struct SharingMapTy {
DeclSAMapTy SharingMap;
AlignedMapTy AlignedMap;
+ LoopControlVariablesSetTy LCVSet;
DefaultDataSharingAttributes DefaultAttr;
SourceLocation DefaultAttrLoc;
OpenMPDirectiveKind Directive;
DeclarationNameInfo DirectiveName;
Scope *CurScope;
SourceLocation ConstructLoc;
bool OrderedRegion;
+ unsigned CollapseNumber;
SourceLocation InnerTeamsRegionLoc;
SharingMapTy(OpenMPDirectiveKind DKind, DeclarationNameInfo Name,
Scope *CurScope, SourceLocation Loc)
- : SharingMap(), AlignedMap(), DefaultAttr(DSA_unspecified),
+ : SharingMap(), AlignedMap(), LCVSet(), DefaultAttr(DSA_unspecified),
Directive(DKind), DirectiveName(std::move(Name)), CurScope(CurScope),
- ConstructLoc(Loc), OrderedRegion(false), InnerTeamsRegionLoc() {}
+ ConstructLoc(Loc), OrderedRegion(false), CollapseNumber(1),
+ InnerTeamsRegionLoc() {}
SharingMapTy()
- : SharingMap(), AlignedMap(), DefaultAttr(DSA_unspecified),
+ : SharingMap(), AlignedMap(), LCVSet(), DefaultAttr(DSA_unspecified),
Directive(OMPD_unknown), DirectiveName(), CurScope(nullptr),
- ConstructLoc(), OrderedRegion(false), InnerTeamsRegionLoc() {}
+ ConstructLoc(), OrderedRegion(false), CollapseNumber(1),
+ InnerTeamsRegionLoc() {}
};
typedef SmallVector<SharingMapTy, 64> StackTy;
@@ -137,6 +142,12 @@
/// for diagnostics.
DeclRefExpr *addUniqueAligned(VarDecl *D, DeclRefExpr *NewDE);
+ /// \brief Register specified variable as loop control variable.
+ void addLoopControlVariable(VarDecl *D);
+ /// \brief Check if the specified variable is a loop control variable for
+ /// current region.
+ bool isLoopControlVariable(VarDecl *D);
+
/// \brief Adds explicit data sharing attribute to the specified declaration.
void addDSA(VarDecl *D, DeclRefExpr *E, OpenMPClauseKind A);
@@ -209,6 +220,13 @@
return false;
}
+ /// \brief Set collapse value for the region.
+ void setCollapseNumber(unsigned Val) { Stack.back().CollapseNumber = Val; }
+ /// \brief Return collapse value for region.
+ unsigned getCollapseNumber() const {
+ return Stack.back().CollapseNumber;
+ }
+
/// \brief Marks current target region as one with closely nested teams
/// region.
void setParentTeamsRegionLoc(SourceLocation TeamsRegionLoc) {
@@ -356,6 +374,18 @@
return nullptr;
}
+void DSAStackTy::addLoopControlVariable(VarDecl *D) {
+ assert(Stack.size() > 1 && "Data-sharing attributes stack is empty");
+ D = D->getCanonicalDecl();
+ Stack.back().LCVSet.insert(D);
+}
+
+bool DSAStackTy::isLoopControlVariable(VarDecl *D) {
+ assert(Stack.size() > 1 && "Data-sharing attributes stack is empty");
+ D = D->getCanonicalDecl();
+ return Stack.back().LCVSet.count(D) > 0;
+}
+
void DSAStackTy::addDSA(VarDecl *D, DeclRefExpr *E, OpenMPClauseKind A) {
D = D->getCanonicalDecl();
if (A == OMPC_threadprivate) {
@@ -556,6 +586,8 @@
assert(LangOpts.OpenMP && "OpenMP is not allowed");
VD = VD->getCanonicalDecl();
if (DSAStack->getCurrentDirective() != OMPD_unknown) {
+ if (DSAStack->isLoopControlVariable(VD))
+ return true;
auto DVarPrivate = DSAStack->getTopDSA(VD, /*FromParent=*/false);
if (DVarPrivate.CKind != OMPC_unknown && isOpenMPPrivate(DVarPrivate.CKind))
return true;
@@ -2398,7 +2430,8 @@
/// \brief Build reference expression to the counter be used for codegen.
Expr *OpenMPIterationSpaceChecker::BuildCounterVar() const {
return DeclRefExpr::Create(SemaRef.Context, NestedNameSpecifierLoc(),
- GetIncrementSrcRange().getBegin(), Var, false,
+ GetIncrementSrcRange().getBegin(), Var,
+ /*RefersToEnclosingVariableOrCapture=*/true,
DefaultLoc, Var->getType(), VK_LValue);
}
@@ -2434,6 +2467,23 @@
} // namespace
+void Sema::ActOnOpenMPLoopInitialization(SourceLocation ForLoc, Stmt *Init) {
+ assert(getLangOpts().OpenMP && "OpenMP is not active.");
+ assert(Init && "Expected loop in canonical form.");
+ unsigned CollapseIteration = DSAStack->getCollapseNumber();
+ if (CollapseIteration > 0 &&
+ isOpenMPLoopDirective(DSAStack->getCurrentDirective())) {
+ bool Suppress = getDiagnostics().getSuppressAllDiagnostics();
+ getDiagnostics().setSuppressAllDiagnostics(/*Val=*/true);
+ OpenMPIterationSpaceChecker ISC(*this, ForLoc);
+ if (!ISC.CheckInit(Init)) {
+ DSAStack->addLoopControlVariable(ISC.GetLoopVar());
+ }
+ getDiagnostics().setSuppressAllDiagnostics(Suppress);
+ DSAStack->setCollapseNumber(CollapseIteration - 1);
+ }
+}
+
/// \brief Called on a for stmt to check and extract its iteration space
/// for further processing (such as collapsing).
static bool CheckOpenMPIterationSpace(
@@ -2526,18 +2576,10 @@
// Make the loop iteration variable private (for worksharing constructs),
// linear (for simd directives with the only one associated loop) or
// lastprivate (for simd directives with several collapsed loops).
- // FIXME: the next check and error message must be removed once the
- // capturing of global variables in loops is fixed.
if (DVar.CKind == OMPC_unknown)
DVar = DSA.hasDSA(Var, isOpenMPPrivate, MatchesAlways(),
/*FromParent=*/false);
- if (!Var->hasLocalStorage() && DVar.CKind == OMPC_unknown) {
- SemaRef.Diag(Init->getLocStart(), diag::err_omp_global_loop_var_dsa)
- << getOpenMPClauseName(PredeterminedCKind)
- << getOpenMPDirectiveName(DKind);
- HasErrors = true;
- } else
- DSA.addDSA(Var, LoopVarRefExpr, PredeterminedCKind);
+ DSA.addDSA(Var, LoopVarRefExpr, PredeterminedCKind);
}
assert(isOpenMPLoopDirective(DKind) && "DSA for non-loop vars");
@@ -4210,6 +4252,9 @@
<< E->getSourceRange();
return ExprError();
}
+ if (CKind == OMPC_collapse) {
+ DSAStack->setCollapseNumber(Result.getExtValue());
+ }
return ICE;
}
Index: lib/Parse/ParseStmt.cpp
===================================================================
--- lib/Parse/ParseStmt.cpp
+++ lib/Parse/ParseStmt.cpp
@@ -1689,6 +1689,12 @@
FirstPart.get(),
Collection.get(),
T.getCloseLocation());
+ } else {
+ // In OpenMP loop region loop control variable must be captured and be
+ // private. Perform analysis of first part (if any).
+ if (getLangOpts().OpenMP && FirstPart.isUsable()) {
+ Actions.ActOnOpenMPLoopInitialization(ForLoc, FirstPart.get());
+ }
}
// C99 6.8.5p5 - In C99, the body of the for statement is a scope, even if
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
