r321386 - [OPENMP] Captured arguments of the capturable clauses by value.

Maxim Kuvyrkov via cfe-commits cfe-commits at lists.llvm.org
Sun Dec 24 05:27:17 PST 2017


> On Dec 23, 2017, at 12:01 AM, Alexey Bataev via cfe-commits <cfe-commits at lists.llvm.org> wrote:
> 
> Author: abataev
> Date: Fri Dec 22 13:01:52 2017
> New Revision: 321386
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=321386&view=rev
> Log:
> [OPENMP] Captured arguments of the capturable clauses by value.
> 
> If the clause is applied to the combined construct and has captured
> expression, try to capture this expression by value rather than by
> reference.
> 
> Modified:
>    cfe/trunk/lib/Sema/SemaOpenMP.cpp
>    cfe/trunk/test/OpenMP/parallel_for_codegen.cpp
>    cfe/trunk/test/OpenMP/teams_distribute_parallel_for_num_threads_codegen.cpp
>    cfe/trunk/test/OpenMP/teams_distribute_parallel_for_simd_num_threads_codegen.cpp

Hi Alexey,

Changes to the teams_distribute_parallel_* tests break 32-bit ARM buildbots, e.g., http://lab.llvm.org:8011/builders/clang-cmake-armv7-a15/builds/14264 .  Would you please investigate?

Thanks,

--
Maxim Kuvyrkov
www.linaro.org


> 
> Modified: cfe/trunk/lib/Sema/SemaOpenMP.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOpenMP.cpp?rev=321386&r1=321385&r2=321386&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaOpenMP.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaOpenMP.cpp Fri Dec 22 13:01:52 2017
> @@ -1290,9 +1290,14 @@ bool Sema::IsOpenMPCapturedByRef(ValueDe
>   }
> 
>   if (IsByRef && Ty.getNonReferenceType()->isScalarType()) {
> -    IsByRef = !DSAStack->hasExplicitDSA(
> -        D, [](OpenMPClauseKind K) -> bool { return K == OMPC_firstprivate; },
> -        Level, /*NotLastprivate=*/true);
> +    IsByRef =
> +        !DSAStack->hasExplicitDSA(
> +            D,
> +            [](OpenMPClauseKind K) -> bool { return K == OMPC_firstprivate; },
> +            Level, /*NotLastprivate=*/true) &&
> +        // If the variable is artificial and must be captured by value - try to
> +        // capture by value.
> +        !(isa<OMPCapturedExprDecl>(D) && D->hasAttr<OMPCaptureKindAttr>());
>   }
> 
>   // When passing data by copy, we need to make sure it fits the uintptr size
> @@ -2321,10 +2326,11 @@ static OMPCapturedExprDecl *buildCapture
>   ASTContext &C = S.getASTContext();
>   Expr *Init = AsExpression ? CaptureExpr : CaptureExpr->IgnoreImpCasts();
>   QualType Ty = Init->getType();
> +  Attr *OMPCaptureKind = nullptr;
>   if (CaptureExpr->getObjectKind() == OK_Ordinary && CaptureExpr->isGLValue()) {
> -    if (S.getLangOpts().CPlusPlus)
> +    if (S.getLangOpts().CPlusPlus) {
>       Ty = C.getLValueReferenceType(Ty);
> -    else {
> +    } else {
>       Ty = C.getPointerType(Ty);
>       ExprResult Res =
>           S.CreateBuiltinUnaryOp(CaptureExpr->getExprLoc(), UO_AddrOf, Init);
> @@ -2333,11 +2339,16 @@ static OMPCapturedExprDecl *buildCapture
>       Init = Res.get();
>     }
>     WithInit = true;
> +  } else if (AsExpression) {
> +    // This variable must be captured by value.
> +    OMPCaptureKind = OMPCaptureKindAttr::CreateImplicit(C, OMPC_unknown);
>   }
>   auto *CED = OMPCapturedExprDecl::Create(C, S.CurContext, Id, Ty,
>                                           CaptureExpr->getLocStart());
>   if (!WithInit)
>     CED->addAttr(OMPCaptureNoInitAttr::CreateImplicit(C, SourceRange()));
> +  if (OMPCaptureKind)
> +    CED->addAttr(OMPCaptureKind);
>   S.CurContext->addHiddenDecl(CED);
>   S.AddInitializerToDecl(CED, Init, /*DirectInit=*/false);
>   return CED;
> @@ -2346,31 +2357,34 @@ static OMPCapturedExprDecl *buildCapture
> static DeclRefExpr *buildCapture(Sema &S, ValueDecl *D, Expr *CaptureExpr,
>                                  bool WithInit) {
>   OMPCapturedExprDecl *CD;
> -  if (auto *VD = S.IsOpenMPCapturedDecl(D))
> +  if (auto *VD = S.IsOpenMPCapturedDecl(D)) {
>     CD = cast<OMPCapturedExprDecl>(VD);
> -  else
> +  } else {
>     CD = buildCaptureDecl(S, D->getIdentifier(), CaptureExpr, WithInit,
>                           /*AsExpression=*/false);
> +  }
>   return buildDeclRefExpr(S, CD, CD->getType().getNonReferenceType(),
>                           CaptureExpr->getExprLoc());
> }
> 
> static ExprResult buildCapture(Sema &S, Expr *CaptureExpr, DeclRefExpr *&Ref) {
> +  CaptureExpr = S.DefaultLvalueConversion(CaptureExpr).get();
>   if (!Ref) {
> -    auto *CD =
> -        buildCaptureDecl(S, &S.getASTContext().Idents.get(".capture_expr."),
> -                         CaptureExpr, /*WithInit=*/true, /*AsExpression=*/true);
> +    OMPCapturedExprDecl *CD = buildCaptureDecl(
> +        S, &S.getASTContext().Idents.get(".capture_expr."), CaptureExpr,
> +        /*WithInit=*/true, /*AsExpression=*/true);
>     Ref = buildDeclRefExpr(S, CD, CD->getType().getNonReferenceType(),
>                            CaptureExpr->getExprLoc());
>   }
>   ExprResult Res = Ref;
>   if (!S.getLangOpts().CPlusPlus &&
>       CaptureExpr->getObjectKind() == OK_Ordinary && CaptureExpr->isGLValue() &&
> -      Ref->getType()->isPointerType())
> +      Ref->getType()->isPointerType()) {
>     Res = S.CreateBuiltinUnaryOp(CaptureExpr->getExprLoc(), UO_Deref, Ref);
> -  if (!Res.isUsable())
> -    return ExprError();
> -  return CaptureExpr->isGLValue() ? Res : S.DefaultLvalueConversion(Res.get());
> +    if (!Res.isUsable())
> +      return ExprError();
> +  }
> +  return S.DefaultLvalueConversion(Res.get());
> }
> 
> namespace {
> @@ -8117,12 +8131,13 @@ OMPClause *Sema::ActOnOpenMPIfClause(Ope
>     if (Val.isInvalid())
>       return nullptr;
> 
> -    ValExpr = MakeFullExpr(Val.get()).get();
> +    ValExpr = Val.get();
> 
>     OpenMPDirectiveKind DKind = DSAStack->getCurrentDirective();
>     CaptureRegion =
>         getOpenMPCaptureRegionForClause(DKind, OMPC_if, NameModifier);
>     if (CaptureRegion != OMPD_unknown && !CurContext->isDependentContext()) {
> +      ValExpr = MakeFullExpr(ValExpr).get();
>       llvm::MapVector<Expr *, DeclRefExpr *> Captures;
>       ValExpr = tryBuildCapture(*this, ValExpr, Captures).get();
>       HelperValStmt = buildPreInits(Context, Captures);
> @@ -8239,6 +8254,7 @@ OMPClause *Sema::ActOnOpenMPNumThreadsCl
>   OpenMPDirectiveKind CaptureRegion =
>       getOpenMPCaptureRegionForClause(DKind, OMPC_num_threads);
>   if (CaptureRegion != OMPD_unknown && !CurContext->isDependentContext()) {
> +    ValExpr = MakeFullExpr(ValExpr).get();
>     llvm::MapVector<Expr *, DeclRefExpr *> Captures;
>     ValExpr = tryBuildCapture(*this, ValExpr, Captures).get();
>     HelperValStmt = buildPreInits(Context, Captures);
> @@ -8666,6 +8682,7 @@ OMPClause *Sema::ActOnOpenMPScheduleClau
>                      DSAStack->getCurrentDirective(), OMPC_schedule) !=
>                      OMPD_unknown &&
>                  !CurContext->isDependentContext()) {
> +        ValExpr = MakeFullExpr(ValExpr).get();
>         llvm::MapVector<Expr *, DeclRefExpr *> Captures;
>         ValExpr = tryBuildCapture(*this, ValExpr, Captures).get();
>         HelperValStmt = buildPreInits(Context, Captures);
> @@ -11355,6 +11372,7 @@ OMPClause *Sema::ActOnOpenMPDeviceClause
>   OpenMPDirectiveKind CaptureRegion =
>       getOpenMPCaptureRegionForClause(DKind, OMPC_device);
>   if (CaptureRegion != OMPD_unknown && !CurContext->isDependentContext()) {
> +    ValExpr = MakeFullExpr(ValExpr).get();
>     llvm::MapVector<Expr *, DeclRefExpr *> Captures;
>     ValExpr = tryBuildCapture(*this, ValExpr, Captures).get();
>     HelperValStmt = buildPreInits(Context, Captures);
> @@ -12378,6 +12396,7 @@ OMPClause *Sema::ActOnOpenMPNumTeamsClau
>   OpenMPDirectiveKind CaptureRegion =
>       getOpenMPCaptureRegionForClause(DKind, OMPC_num_teams);
>   if (CaptureRegion != OMPD_unknown && !CurContext->isDependentContext()) {
> +    ValExpr = MakeFullExpr(ValExpr).get();
>     llvm::MapVector<Expr *, DeclRefExpr *> Captures;
>     ValExpr = tryBuildCapture(*this, ValExpr, Captures).get();
>     HelperValStmt = buildPreInits(Context, Captures);
> @@ -12404,6 +12423,7 @@ OMPClause *Sema::ActOnOpenMPThreadLimitC
>   OpenMPDirectiveKind CaptureRegion =
>       getOpenMPCaptureRegionForClause(DKind, OMPC_thread_limit);
>   if (CaptureRegion != OMPD_unknown && !CurContext->isDependentContext()) {
> +    ValExpr = MakeFullExpr(ValExpr).get();
>     llvm::MapVector<Expr *, DeclRefExpr *> Captures;
>     ValExpr = tryBuildCapture(*this, ValExpr, Captures).get();
>     HelperValStmt = buildPreInits(Context, Captures);
> @@ -12514,6 +12534,7 @@ OMPClause *Sema::ActOnOpenMPDistSchedule
>                      DSAStack->getCurrentDirective(), OMPC_dist_schedule) !=
>                      OMPD_unknown &&
>                  !CurContext->isDependentContext()) {
> +        ValExpr = MakeFullExpr(ValExpr).get();
>         llvm::MapVector<Expr *, DeclRefExpr *> Captures;
>         ValExpr = tryBuildCapture(*this, ValExpr, Captures).get();
>         HelperValStmt = buildPreInits(Context, Captures);
> 
> Modified: cfe/trunk/test/OpenMP/parallel_for_codegen.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/parallel_for_codegen.cpp?rev=321386&r1=321385&r2=321386&view=diff
> ==============================================================================
> --- cfe/trunk/test/OpenMP/parallel_for_codegen.cpp (original)
> +++ cfe/trunk/test/OpenMP/parallel_for_codegen.cpp Fri Dec 22 13:01:52 2017
> @@ -15,10 +15,12 @@ void with_var_schedule() {
>   double a = 5;
> // CHECK: [[CHUNK_SIZE:%.+]] = fptosi double %{{.+}}to i8
> // CHECK: store i8 %{{.+}}, i8* [[CHUNK:%.+]],
> -// CHECK: call void {{.+}} @__kmpc_fork_call({{.+}}, i8* [[CHUNK]])
> +// CHECK: [[VAL:%.+]] = load i8, i8* [[CHUNK]],
> +// CHECK: store i8 [[VAL]], i8*
> +// CHECK: [[CHUNK:%.+]] = load i64, i64* %
> +// CHECK: call void {{.+}} @__kmpc_fork_call({{.+}}, i64 [[CHUNK]])
> 
> -// CHECK: [[CHUNK:%.+]] = load i8*, i8** %
> -// CHECK: [[CHUNK_VAL:%.+]] = load i8, i8* [[CHUNK]],
> +// CHECK: [[CHUNK_VAL:%.+]] = load i8, i8* %
> // CHECK: [[CHUNK_SIZE:%.+]] = sext i8 [[CHUNK_VAL]] to i64
> // CHECK: call void @__kmpc_for_static_init_8u([[IDENT_T_TY]]* [[LOOP_LOC]], i32 [[GTID:%[^,]+]], i32 33, i32* [[IS_LAST:%[^,]+]], i64* [[OMP_LB:%[^,]+]], i64* [[OMP_UB:%[^,]+]], i64* [[OMP_ST:%[^,]+]], i64 1, i64 [[CHUNK_SIZE]])
> // CHECK: call void @__kmpc_for_static_fini([[IDENT_T_TY]]* [[LOOP_LOC]], i32 [[GTID]])
> 
> Modified: cfe/trunk/test/OpenMP/teams_distribute_parallel_for_num_threads_codegen.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/teams_distribute_parallel_for_num_threads_codegen.cpp?rev=321386&r1=321385&r2=321386&view=diff
> ==============================================================================
> --- cfe/trunk/test/OpenMP/teams_distribute_parallel_for_num_threads_codegen.cpp (original)
> +++ cfe/trunk/test/OpenMP/teams_distribute_parallel_for_num_threads_codegen.cpp Fri Dec 22 13:01:52 2017
> @@ -60,8 +60,8 @@ int main() {
> #pragma omp teams distribute parallel for num_threads(a)
>   for (int i = 0; i < 100; i++) {
>     // CHECK: define{{.+}} void [[OMP_TEAMS_OUTLINED_1]](
> -    // CHECK-DAG: [[A_ADDR:%.+]] = alloca i8*,
> -    // CHECK-DAG: [[A_REF:%.+]] = load i8*, i8** [[A_ADDR]],
> +    // CHECK-DAG: [[A_ADDR:%.+]] = alloca i64,
> +    // CHECK-DAG: [[A_REF:%.+]] = bitcast i64* [[A_ADDR]] to i8*
>     // CHECK-DAG: [[A_VAL:%.+]] = load i8, i8* [[A_REF]],
>     // CHECK-DAG: [[A_EXT:%.+]] = sext i8 [[A_VAL]] to {{.+}}
>     // CHECK: call {{.*}}void @__kmpc_push_num_threads([[IDENT_T_TY]]* [[DEF_LOC_2]], i32 {{.+}}, i32 [[A_EXT]])
> @@ -110,9 +110,9 @@ int main() {
> // CHECK: call {{.*}}void {{.+}} @__kmpc_fork_teams({{.+}}, i32 {{.+}}, {{.+}}* [[T_OMP_TEAMS_OUTLINED_3:@.+]] to {{.+}})
> 
> // CHECK: define{{.+}} void [[T_OMP_TEAMS_OUTLINED_3]]({{.+}}, {{.+}}, {{.+}} [[NUM_TH_CPT_IN:%.+]])
> -// CHECK: [[NUM_TH_CPT:%.+]] = alloca i8*,
> +// CHECK: [[NUM_TH_CPT:%.+]] = alloca i64,
> // CHECK: store {{.+}} [[NUM_TH_CPT_IN]], {{.+}} [[NUM_TH_CPT]],
> -// CHECK: [[NUM_TH_REF:%.+]] = load{{.+}}, {{.+}} [[NUM_TH_CPT]],
> +// CHECK: [[NUM_TH_REF:%.+]] = bitcast i64* [[NUM_TH_CPT]] to i8*
> // CHECK-DAG:   [[NUM_TH_VAL:%.+]] = load {{.+}}, {{.+}} [[NUM_TH_REF]],
> // CHECK-DAG:   [[NUM_TH_SEXT:%.+]] = sext i8 [[NUM_TH_VAL]] to {{.+}}
> // CHECK:       call {{.*}}void @__kmpc_push_num_threads([[IDENT_T_TY]]* [[DEF_LOC_2]], i32 {{.+}}, i32 [[NUM_TH_SEXT]])
> 
> Modified: cfe/trunk/test/OpenMP/teams_distribute_parallel_for_simd_num_threads_codegen.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/teams_distribute_parallel_for_simd_num_threads_codegen.cpp?rev=321386&r1=321385&r2=321386&view=diff
> ==============================================================================
> --- cfe/trunk/test/OpenMP/teams_distribute_parallel_for_simd_num_threads_codegen.cpp (original)
> +++ cfe/trunk/test/OpenMP/teams_distribute_parallel_for_simd_num_threads_codegen.cpp Fri Dec 22 13:01:52 2017
> @@ -60,8 +60,8 @@ int main() {
> #pragma omp teams distribute parallel for simd num_threads(a)
>   for (int i = 0; i < 100; i++) {
>     // CHECK: define{{.+}} void [[OMP_TEAMS_OUTLINED_1]](
> -    // CHECK-DAG: [[A_ADDR:%.+]] = alloca i8*,
> -    // CHECK-DAG: [[A_REF:%.+]] = load i8*, i8** [[A_ADDR]],
> +    // CHECK-DAG: [[A_ADDR:%.+]] = alloca i64,
> +    // CHECK-DAG: [[A_REF:%.+]] = bitcast i64* [[A_ADDR]] to i8*
>     // CHECK-DAG: [[A_VAL:%.+]] = load i8, i8* [[A_REF]],
>     // CHECK-DAG: [[A_EXT:%.+]] = sext i8 [[A_VAL]] to {{.+}}
>     // CHECK: call {{.*}}void @__kmpc_push_num_threads([[IDENT_T_TY]]* [[DEF_LOC_2]], i32 {{.+}}, i32 [[A_EXT]])
> @@ -110,9 +110,9 @@ int main() {
> // CHECK: call {{.*}}void {{.+}} @__kmpc_fork_teams({{.+}}, i32 {{.+}}, {{.+}}* [[T_OMP_TEAMS_OUTLINED_3:@.+]] to {{.+}})
> 
> // CHECK: define{{.+}} void [[T_OMP_TEAMS_OUTLINED_3]]({{.+}}, {{.+}}, {{.+}} [[NUM_TH_CPT_IN:%.+]])
> -// CHECK: [[NUM_TH_CPT:%.+]] = alloca i8*,
> +// CHECK: [[NUM_TH_CPT:%.+]] = alloca i64,
> // CHECK: store {{.+}} [[NUM_TH_CPT_IN]], {{.+}} [[NUM_TH_CPT]],
> -// CHECK: [[NUM_TH_REF:%.+]] = load{{.+}}, {{.+}} [[NUM_TH_CPT]],
> +// CHECK: [[NUM_TH_REF:%.+]] = bitcast i64* [[NUM_TH_CPT]] to i8*
> // CHECK-DAG:   [[NUM_TH_VAL:%.+]] = load {{.+}}, {{.+}} [[NUM_TH_REF]],
> // CHECK-DAG:   [[NUM_TH_SEXT:%.+]] = sext i8 [[NUM_TH_VAL]] to {{.+}}
> // CHECK:       call {{.*}}void @__kmpc_push_num_threads([[IDENT_T_TY]]* [[DEF_LOC_2]], i32 {{.+}}, i32 [[NUM_TH_SEXT]])
> 
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits



More information about the cfe-commits mailing list