From 16dc7b68c4d90cd3abac7015c27030f3d6c710f9 Mon Sep 17 00:00:00 2001 From: Alexey Bataev Date: Wed, 20 May 2015 03:46:04 +0000 Subject: [PATCH] Fix for aggregate copying of variable length arrays. Patch fixes codegen for aggregate copying of VLAs. Currently method CodeGenFunction::EmitAggregateCopy() does not support copying of VLAs. Patch checks if the size of the type is 0, then checks if the type is actually a variable-length array. Then it calculates total length for this array and calculates total size of the array in bytes: * aligned_sizeof(ElementType) (if copy assignment is requested). If simple copying is requested, size is calculated like: * aligned_sizeof(ElementType) - aligned_sizeof(ElementType) + sizeof(ElementType). memcpy() is used with this calculated size of the VLA. Differential Revision: http://reviews.llvm.org/D9851 llvm-svn: 237768 --- clang/lib/CodeGen/CGExprAgg.cpp | 43 +++++++++++++++------- clang/lib/Sema/SemaOpenMP.cpp | 3 +- .../test/OpenMP/parallel_firstprivate_codegen.cpp | 13 ++++++- 3 files changed, 43 insertions(+), 16 deletions(-) diff --git a/clang/lib/CodeGen/CGExprAgg.cpp b/clang/lib/CodeGen/CGExprAgg.cpp index 2ad5ec3..77e4464e 100644 --- a/clang/lib/CodeGen/CGExprAgg.cpp +++ b/clang/lib/CodeGen/CGExprAgg.cpp @@ -1447,7 +1447,34 @@ void CodeGenFunction::EmitAggregateCopy(llvm::Value *DestPtr, if (alignment.isZero()) alignment = TypeInfo.second; - // FIXME: Handle variable sized types. + llvm::Value *SizeVal = nullptr; + if (TypeInfo.first.isZero()) { + // But note that getTypeInfo returns 0 for a VLA. + if (auto *VAT = dyn_cast_or_null( + getContext().getAsArrayType(Ty))) { + QualType BaseEltTy; + SizeVal = emitArrayLength(VAT, BaseEltTy, DestPtr); + TypeInfo = getContext().getTypeInfoDataSizeInChars(BaseEltTy); + std::pair LastElementTypeInfo; + if (!isAssignment) + LastElementTypeInfo = getContext().getTypeInfoInChars(BaseEltTy); + assert(!TypeInfo.first.isZero()); + SizeVal = Builder.CreateNUWMul( + SizeVal, + llvm::ConstantInt::get(SizeTy, TypeInfo.first.getQuantity())); + if (!isAssignment) { + SizeVal = Builder.CreateNUWSub( + SizeVal, + llvm::ConstantInt::get(SizeTy, TypeInfo.first.getQuantity())); + SizeVal = Builder.CreateNUWAdd( + SizeVal, llvm::ConstantInt::get( + SizeTy, LastElementTypeInfo.first.getQuantity())); + } + } + } + if (!SizeVal) { + SizeVal = llvm::ConstantInt::get(SizeTy, TypeInfo.first.getQuantity()); + } // FIXME: If we have a volatile struct, the optimizer can remove what might // appear to be `extra' memory ops: @@ -1478,9 +1505,6 @@ void CodeGenFunction::EmitAggregateCopy(llvm::Value *DestPtr, } else if (const RecordType *RecordTy = Ty->getAs()) { RecordDecl *Record = RecordTy->getDecl(); if (Record->hasObjectMember()) { - CharUnits size = TypeInfo.first; - llvm::Type *SizeTy = ConvertType(getContext().getSizeType()); - llvm::Value *SizeVal = llvm::ConstantInt::get(SizeTy, size.getQuantity()); CGM.getObjCRuntime().EmitGCMemmoveCollectable(*this, DestPtr, SrcPtr, SizeVal); return; @@ -1489,10 +1513,6 @@ void CodeGenFunction::EmitAggregateCopy(llvm::Value *DestPtr, QualType BaseType = getContext().getBaseElementType(Ty); if (const RecordType *RecordTy = BaseType->getAs()) { if (RecordTy->getDecl()->hasObjectMember()) { - CharUnits size = TypeInfo.first; - llvm::Type *SizeTy = ConvertType(getContext().getSizeType()); - llvm::Value *SizeVal = - llvm::ConstantInt::get(SizeTy, size.getQuantity()); CGM.getObjCRuntime().EmitGCMemmoveCollectable(*this, DestPtr, SrcPtr, SizeVal); return; @@ -1505,9 +1525,6 @@ void CodeGenFunction::EmitAggregateCopy(llvm::Value *DestPtr, // the optimizer wishes to expand it in to scalar memory operations. llvm::MDNode *TBAAStructTag = CGM.getTBAAStructInfo(Ty); - Builder.CreateMemCpy(DestPtr, SrcPtr, - llvm::ConstantInt::get(IntPtrTy, - TypeInfo.first.getQuantity()), - alignment.getQuantity(), isVolatile, - /*TBAATag=*/nullptr, TBAAStructTag); + Builder.CreateMemCpy(DestPtr, SrcPtr, SizeVal, alignment.getQuantity(), + isVolatile, /*TBAATag=*/nullptr, TBAAStructTag); } diff --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp index 126d6fa..c7d0c14 100644 --- a/clang/lib/Sema/SemaOpenMP.cpp +++ b/clang/lib/Sema/SemaOpenMP.cpp @@ -1286,7 +1286,8 @@ StmtResult Sema::ActOnOpenMPRegionEnd(StmtResult S, } // This is required for proper codegen. for (auto *Clause : Clauses) { - if (isOpenMPPrivate(Clause->getClauseKind())) { + if (isOpenMPPrivate(Clause->getClauseKind()) || + Clause->getClauseKind() == OMPC_copyprivate) { // Mark all variables in private list clauses as used in inner region. for (auto *VarRef : Clause->children()) { if (auto *E = cast_or_null(VarRef)) { diff --git a/clang/test/OpenMP/parallel_firstprivate_codegen.cpp b/clang/test/OpenMP/parallel_firstprivate_codegen.cpp index 64a1294..24522ba 100644 --- a/clang/test/OpenMP/parallel_firstprivate_codegen.cpp +++ b/clang/test/OpenMP/parallel_firstprivate_codegen.cpp @@ -250,11 +250,20 @@ struct St { ~St() {} }; -void array_func(int a[3], St s[2]) { +void array_func(int a[3], St s[2], int n, long double vla1[n]) { + double vla2[n]; // ARRAY: @__kmpc_fork_call( // ARRAY: call void @llvm.memcpy.p0i8.p0i8.i64(i8* %{{.+}}, i8* %{{.+}}, i64 12, i32 4, i1 false) // ARRAY: call void @_ZN2StC1ERKS_(%struct.St* %{{.+}}, %struct.St* dereferenceable(8) %{{.+}} -#pragma omp parallel firstprivate(a, s) +// ARRAY: call i8* @llvm.stacksave() +// ARRAY: [[SIZE:%.+]] = mul nuw i64 %{{.+}}, 16 +// ARRAY: call void @llvm.memcpy.p0i8.p0i8.i64(i8* %{{.+}}, i8* %{{.+}}, i64 [[SIZE]], i32 16, i1 false) +// ARRAY: [[SIZE:%.+]] = mul nuw i64 %{{.+}}, 8 +// ARRAY: call void @llvm.memcpy.p0i8.p0i8.i64(i8* %{{.+}}, i8* %{{.+}}, i64 [[SIZE]], i32 8, i1 false) +// ARRAY: call void @llvm.stackrestore(i8* +// ARRAY: call void @_ZN2StD1Ev(%struct.St* %{{.+}}) +// ARRAY: br i1 +#pragma omp parallel firstprivate(a, s, vla1, vla2) ; } #endif -- 2.7.4