From 9c821037434dc106543d6905f5519b1129eb904f Mon Sep 17 00:00:00 2001 From: Alexey Bataev Date: Thu, 30 Apr 2015 04:23:23 +0000 Subject: [PATCH] [OPENMP] Allow to use global variables as lcv in loop-based directives. 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. Differential Revision: http://reviews.llvm.org/D9336 llvm-svn: 236201 --- clang/include/clang/Basic/DiagnosticSemaKinds.td | 2 - clang/include/clang/Sema/Sema.h | 6 ++ clang/lib/Parse/ParseStmt.cpp | 6 ++ clang/lib/Sema/SemaOpenMP.cpp | 86 +++++++++++++++++----- clang/test/OpenMP/for_codegen.cpp | 42 ++++++++++- clang/test/OpenMP/for_loop_messages.cpp | 2 - clang/test/OpenMP/for_simd_loop_messages.cpp | 2 - clang/test/OpenMP/parallel_for_loop_messages.cpp | 2 - .../OpenMP/parallel_for_simd_loop_messages.cpp | 2 - clang/test/OpenMP/simd_loop_messages.cpp | 2 - 10 files changed, 119 insertions(+), 33 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index a732724..0cc168f 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -7421,8 +7421,6 @@ def note_omp_implicit_dsa : Note< "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">; diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h index c38226a..d8341b7 100644 --- a/clang/include/clang/Sema/Sema.h +++ b/clang/include/clang/Sema/Sema.h @@ -7424,6 +7424,12 @@ public: /// \brief Called on end of data sharing attribute block. void EndOpenMPDSABlock(Stmt *CurDirective); + /// \brief Check if the current region is an OpenMP loop region and if it is, + /// mark loop control variable, used in \p 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'. diff --git a/clang/lib/Parse/ParseStmt.cpp b/clang/lib/Parse/ParseStmt.cpp index c31216d..055bdea 100644 --- a/clang/lib/Parse/ParseStmt.cpp +++ b/clang/lib/Parse/ParseStmt.cpp @@ -1689,6 +1689,12 @@ StmtResult Parser::ParseForStatement(SourceLocation *TrailingElseLoc) { 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 diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp index 06b9d56..fb3cd9a 100644 --- a/clang/lib/Sema/SemaOpenMP.cpp +++ b/clang/lib/Sema/SemaOpenMP.cpp @@ -82,10 +82,12 @@ private: }; typedef llvm::SmallDenseMap DeclSAMapTy; typedef llvm::SmallDenseMap AlignedMapTy; + typedef llvm::DenseSet LoopControlVariablesSetTy; struct SharingMapTy { DeclSAMapTy SharingMap; AlignedMapTy AlignedMap; + LoopControlVariablesSetTy LCVSet; DefaultDataSharingAttributes DefaultAttr; SourceLocation DefaultAttrLoc; OpenMPDirectiveKind Directive; @@ -93,16 +95,19 @@ private: 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 StackTy; @@ -137,6 +142,12 @@ public: /// 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 @@ public: 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 @@ DeclRefExpr *DSAStackTy::addUniqueAligned(VarDecl *D, DeclRefExpr *NewDE) { 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 @@ bool Sema::IsOpenMPCapturedVar(VarDecl *VD) { 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; @@ -1956,7 +1988,7 @@ public: TestIsStrictOp(false), SubtractStep(false) {} /// \brief Check init-expr for canonical loop form and save loop counter /// variable - #Var and its initialization value - #LB. - bool CheckInit(Stmt *S); + bool CheckInit(Stmt *S, bool EmitDiags = true); /// \brief Check test-expr for canonical form, save upper-bound (#UB), flags /// for less/greater and for strict/non-strict comparison. bool CheckCond(Expr *S); @@ -2096,7 +2128,7 @@ bool OpenMPIterationSpaceChecker::SetStep(Expr *NewStep, bool Subtract) { return false; } -bool OpenMPIterationSpaceChecker::CheckInit(Stmt *S) { +bool OpenMPIterationSpaceChecker::CheckInit(Stmt *S, bool EmitDiags) { // Check init-expr for canonical loop form and save loop counter // variable - #Var and its initialization value - #LB. // OpenMP [2.6] Canonical loop form. init-expr may be one of the following: @@ -2106,7 +2138,9 @@ bool OpenMPIterationSpaceChecker::CheckInit(Stmt *S) { // pointer-type var = lb // if (!S) { - SemaRef.Diag(DefaultLoc, diag::err_omp_loop_not_canonical_init); + if (EmitDiags) { + SemaRef.Diag(DefaultLoc, diag::err_omp_loop_not_canonical_init); + } return true; } InitSrcRange = S->getSourceRange(); @@ -2122,7 +2156,7 @@ bool OpenMPIterationSpaceChecker::CheckInit(Stmt *S) { if (auto Var = dyn_cast_or_null(DS->getSingleDecl())) { if (Var->hasInit()) { // Accept non-canonical init form here but emit ext. warning. - if (Var->getInitStyle() != VarDecl::CInit) + if (Var->getInitStyle() != VarDecl::CInit && EmitDiags) SemaRef.Diag(S->getLocStart(), diag::ext_omp_loop_not_canonical_init) << S->getSourceRange(); @@ -2136,8 +2170,10 @@ bool OpenMPIterationSpaceChecker::CheckInit(Stmt *S) { return SetVarAndLB(dyn_cast(DRE->getDecl()), DRE, CE->getArg(1)); - SemaRef.Diag(S->getLocStart(), diag::err_omp_loop_not_canonical_init) - << S->getSourceRange(); + if (EmitDiags) { + SemaRef.Diag(S->getLocStart(), diag::err_omp_loop_not_canonical_init) + << S->getSourceRange(); + } return true; } @@ -2398,7 +2434,8 @@ Expr *OpenMPIterationSpaceChecker::BuildPreCond(Scope *S, Expr *Cond) const { /// \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 +2471,20 @@ struct LoopIterationSpace { } // 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())) { + OpenMPIterationSpaceChecker ISC(*this, ForLoc); + if (!ISC.CheckInit(Init, /*EmitDiags=*/false)) { + DSAStack->addLoopControlVariable(ISC.GetLoopVar()); + } + 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 +2577,10 @@ static bool CheckOpenMPIterationSpace( // 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 +4253,9 @@ ExprResult Sema::VerifyPositiveIntegerConstantInClause(Expr *E, << E->getSourceRange(); return ExprError(); } + if (CKind == OMPC_collapse) { + DSAStack->setCollapseNumber(Result.getExtValue()); + } return ICE; } diff --git a/clang/test/OpenMP/for_codegen.cpp b/clang/test/OpenMP/for_codegen.cpp index 41cc4c0..31161e2 100644 --- a/clang/test/OpenMP/for_codegen.cpp +++ b/clang/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 @@ void parallel_for(float *a) { // TERM_DEBUG-DAG: [[DBG_LOC_END]] = !DILocation(line: [[@LINE-16]], // TERM_DEBUG-DAG: [[DBG_LOC_CANCEL]] = !DILocation(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 diff --git a/clang/test/OpenMP/for_loop_messages.cpp b/clang/test/OpenMP/for_loop_messages.cpp index cb32484..0b2fa9b 100644 --- a/clang/test/OpenMP/for_loop_messages.cpp +++ b/clang/test/OpenMP/for_loop_messages.cpp @@ -313,7 +313,6 @@ int test_iteration_spaces() { #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]; @@ -321,7 +320,6 @@ int test_iteration_spaces() { #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) diff --git a/clang/test/OpenMP/for_simd_loop_messages.cpp b/clang/test/OpenMP/for_simd_loop_messages.cpp index 403709f..db59d9d 100644 --- a/clang/test/OpenMP/for_simd_loop_messages.cpp +++ b/clang/test/OpenMP/for_simd_loop_messages.cpp @@ -314,7 +314,6 @@ int test_iteration_spaces() { #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]; @@ -322,7 +321,6 @@ int test_iteration_spaces() { #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) diff --git a/clang/test/OpenMP/parallel_for_loop_messages.cpp b/clang/test/OpenMP/parallel_for_loop_messages.cpp index c329997..318f0e6 100644 --- a/clang/test/OpenMP/parallel_for_loop_messages.cpp +++ b/clang/test/OpenMP/parallel_for_loop_messages.cpp @@ -265,14 +265,12 @@ int test_iteration_spaces() { } { -// 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) diff --git a/clang/test/OpenMP/parallel_for_simd_loop_messages.cpp b/clang/test/OpenMP/parallel_for_simd_loop_messages.cpp index 50acb10..43dbbe0 100644 --- a/clang/test/OpenMP/parallel_for_simd_loop_messages.cpp +++ b/clang/test/OpenMP/parallel_for_simd_loop_messages.cpp @@ -266,14 +266,12 @@ int test_iteration_spaces() { } { -// 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) diff --git a/clang/test/OpenMP/simd_loop_messages.cpp b/clang/test/OpenMP/simd_loop_messages.cpp index ce64842..b2c804c 100644 --- a/clang/test/OpenMP/simd_loop_messages.cpp +++ b/clang/test/OpenMP/simd_loop_messages.cpp @@ -260,7 +260,6 @@ int test_iteration_spaces() { #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]; @@ -268,7 +267,6 @@ int test_iteration_spaces() { #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) -- 2.7.4