r333141 - Use zeroinitializer for (trailing zero portion of) large array initializers
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Tue May 29 13:14:52 PDT 2018
On 28 May 2018 at 15:34, David Blaikie via cfe-commits <
cfe-commits at lists.llvm.org> wrote:
> Probably nice to mention in the commit message what the fix was (&
> if/where there was there a test added for it?) so readers don't have to try
> to eyeball diff this commit against the otherone.
>
Fair point. The bug was that we would sometimes use the wrong "common"
element type (when the type actually used for an array element differs from
the IR type that we prefer for that array element type -- this can happen
when emitting a union constant, for example). See the tests
in CodeGenCXX/cxx11-initializer-aggregate.cpp in namespace PR37560 for
examples.
> On Wed, May 23, 2018 at 4:45 PM Richard Smith via cfe-commits <
> cfe-commits at lists.llvm.org> wrote:
>
>> Author: rsmith
>> Date: Wed May 23 16:41:38 2018
>> New Revision: 333141
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=333141&view=rev
>> Log:
>> Use zeroinitializer for (trailing zero portion of) large array
>> initializers
>> more reliably.
>>
>> This re-commits r333044 with a fix for PR37560.
>>
>> Modified:
>> cfe/trunk/lib/CodeGen/CGExprConstant.cpp
>> cfe/trunk/lib/Sema/SemaInit.cpp
>> cfe/trunk/test/CodeGen/init.c
>> cfe/trunk/test/CodeGenCXX/cxx11-initializer-aggregate.cpp
>> cfe/trunk/test/SemaCXX/aggregate-initialization.cpp
>>
>> Modified: cfe/trunk/lib/CodeGen/CGExprConstant.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/
>> CGExprConstant.cpp?rev=333141&r1=333140&r2=333141&view=diff
>> ============================================================
>> ==================
>> --- cfe/trunk/lib/CodeGen/CGExprConstant.cpp (original)
>> +++ cfe/trunk/lib/CodeGen/CGExprConstant.cpp Wed May 23 16:41:38 2018
>> @@ -635,6 +635,60 @@ static ConstantAddress tryEmitGlobalComp
>> return ConstantAddress(GV, Align);
>> }
>>
>> +static llvm::Constant *
>> +EmitArrayConstant(CodeGenModule &CGM, const ConstantArrayType *DestType,
>> + llvm::Type *CommonElementType, unsigned ArrayBound,
>> + SmallVectorImpl<llvm::Constant *> &Elements,
>> + llvm::Constant *Filler) {
>> + // Figure out how long the initial prefix of non-zero elements is.
>> + unsigned NonzeroLength = ArrayBound;
>> + if (Elements.size() < NonzeroLength && Filler->isNullValue())
>> + NonzeroLength = Elements.size();
>> + if (NonzeroLength == Elements.size()) {
>> + while (NonzeroLength > 0 && Elements[NonzeroLength -
>> 1]->isNullValue())
>> + --NonzeroLength;
>> + }
>> +
>> + if (NonzeroLength == 0) {
>> + return llvm::ConstantAggregateZero::get(
>> + CGM.getTypes().ConvertType(QualType(DestType, 0)));
>> + }
>> +
>> + // Add a zeroinitializer array filler if we have lots of trailing
>> zeroes.
>> + unsigned TrailingZeroes = ArrayBound - NonzeroLength;
>> + if (TrailingZeroes >= 8) {
>> + assert(Elements.size() >= NonzeroLength &&
>> + "missing initializer for non-zero element");
>> + Elements.resize(NonzeroLength + 1);
>> + auto *FillerType =
>> + CommonElementType
>> + ? CommonElementType
>> + : CGM.getTypes().ConvertType(DestType->getElementType());
>> + FillerType = llvm::ArrayType::get(FillerType, TrailingZeroes);
>> + Elements.back() = llvm::ConstantAggregateZero::get(FillerType);
>> + CommonElementType = nullptr;
>> + } else if (Elements.size() != ArrayBound) {
>> + // Otherwise pad to the right size with the filler if necessary.
>> + Elements.resize(ArrayBound, Filler);
>> + if (Filler->getType() != CommonElementType)
>> + CommonElementType = nullptr;
>> + }
>> +
>> + // If all elements have the same type, just emit an array constant.
>> + if (CommonElementType)
>> + return llvm::ConstantArray::get(
>> + llvm::ArrayType::get(CommonElementType, ArrayBound), Elements);
>> +
>> + // We have mixed types. Use a packed struct.
>> + llvm::SmallVector<llvm::Type *, 16> Types;
>> + Types.reserve(Elements.size());
>> + for (llvm::Constant *Elt : Elements)
>> + Types.push_back(Elt->getType());
>> + llvm::StructType *SType =
>> + llvm::StructType::get(CGM.getLLVMContext(), Types, true);
>> + return llvm::ConstantStruct::get(SType, Elements);
>> +}
>> +
>> /// This class only needs to handle two cases:
>> /// 1) Literals (this is used by APValue emission to emit literals).
>> /// 2) Arrays, structs and unions (outside C++11 mode, we don't currently
>> @@ -832,68 +886,47 @@ public:
>> }
>>
>> llvm::Constant *EmitArrayInitialization(InitListExpr *ILE, QualType
>> T) {
>> - llvm::ArrayType *AType =
>> - cast<llvm::ArrayType>(ConvertType(ILE->getType()));
>> - llvm::Type *ElemTy = AType->getElementType();
>> + auto *CAT = CGM.getContext().getAsConstantArrayType(ILE->getType());
>> + assert(CAT && "can't emit array init for non-constant-bound array");
>> unsigned NumInitElements = ILE->getNumInits();
>> - unsigned NumElements = AType->getNumElements();
>> + unsigned NumElements = CAT->getSize().getZExtValue();
>>
>> // Initialising an array requires us to automatically
>> // initialise any elements that have not been initialised explicitly
>> unsigned NumInitableElts = std::min(NumInitElements, NumElements);
>>
>> - QualType EltType = CGM.getContext().getAsArrayType(T)->
>> getElementType();
>> + QualType EltType = CAT->getElementType();
>>
>> // Initialize remaining array elements.
>> - llvm::Constant *fillC;
>> - if (Expr *filler = ILE->getArrayFiller())
>> + llvm::Constant *fillC = nullptr;
>> + if (Expr *filler = ILE->getArrayFiller()) {
>> fillC = Emitter.tryEmitAbstractForMemory(filler, EltType);
>> - else
>> - fillC = Emitter.emitNullForMemory(EltType);
>> - if (!fillC)
>> - return nullptr;
>> -
>> - // Try to use a ConstantAggregateZero if we can.
>> - if (fillC->isNullValue() && !NumInitableElts)
>> - return llvm::ConstantAggregateZero::get(AType);
>> + if (!fillC)
>> + return nullptr;
>> + }
>>
>> // Copy initializer elements.
>> SmallVector<llvm::Constant*, 16> Elts;
>> - Elts.reserve(std::max(NumInitableElts, NumElements));
>> + if (fillC && fillC->isNullValue())
>> + Elts.reserve(NumInitableElts + 1);
>> + else
>> + Elts.reserve(NumElements);
>>
>> - bool RewriteType = false;
>> - bool AllNullValues = true;
>> + llvm::Type *CommonElementType = nullptr;
>> for (unsigned i = 0; i < NumInitableElts; ++i) {
>> Expr *Init = ILE->getInit(i);
>> llvm::Constant *C = Emitter.tryEmitPrivateForMemory(Init,
>> EltType);
>> if (!C)
>> return nullptr;
>> - RewriteType |= (C->getType() != ElemTy);
>> + if (i == 0)
>> + CommonElementType = C->getType();
>> + else if (C->getType() != CommonElementType)
>> + CommonElementType = nullptr;
>> Elts.push_back(C);
>> - if (AllNullValues && !C->isNullValue())
>> - AllNullValues = false;
>> }
>>
>> - // If all initializer elements are "zero," then avoid storing
>> NumElements
>> - // instances of the zero representation.
>> - if (AllNullValues)
>> - return llvm::ConstantAggregateZero::get(AType);
>> -
>> - RewriteType |= (fillC->getType() != ElemTy);
>> - Elts.resize(NumElements, fillC);
>> -
>> - if (RewriteType) {
>> - // FIXME: Try to avoid packing the array
>> - std::vector<llvm::Type*> Types;
>> - Types.reserve(Elts.size());
>> - for (unsigned i = 0, e = Elts.size(); i < e; ++i)
>> - Types.push_back(Elts[i]->getType());
>> - llvm::StructType *SType = llvm::StructType::get(AType->
>> getContext(),
>> - Types, true);
>> - return llvm::ConstantStruct::get(SType, Elts);
>> - }
>> -
>> - return llvm::ConstantArray::get(AType, Elts);
>> + return EmitArrayConstant(CGM, CAT, CommonElementType, NumElements,
>> Elts,
>> + fillC);
>> }
>>
>> llvm::Constant *EmitRecordInitialization(InitListExpr *ILE, QualType
>> T) {
>> @@ -1889,40 +1922,31 @@ llvm::Constant *ConstantEmitter::tryEmit
>> case APValue::Union:
>> return ConstStructBuilder::BuildStruct(*this, Value, DestType);
>> case APValue::Array: {
>> - const ArrayType *CAT = CGM.getContext().getAsArrayType(DestType);
>> + const ConstantArrayType *CAT =
>> + CGM.getContext().getAsConstantArrayType(DestType);
>> unsigned NumElements = Value.getArraySize();
>> unsigned NumInitElts = Value.getArrayInitializedElts();
>>
>> // Emit array filler, if there is one.
>> llvm::Constant *Filler = nullptr;
>> - if (Value.hasArrayFiller())
>> + if (Value.hasArrayFiller()) {
>> Filler = tryEmitAbstractForMemory(Value.getArrayFiller(),
>> CAT->getElementType());
>> -
>> - // Emit initializer elements.
>> - llvm::Type *CommonElementType =
>> - CGM.getTypes().ConvertType(CAT->getElementType());
>> -
>> - // Try to use a ConstantAggregateZero if we can.
>> - if (Filler && Filler->isNullValue() && !NumInitElts) {
>> - llvm::ArrayType *AType =
>> - llvm::ArrayType::get(CommonElementType, NumElements);
>> - return llvm::ConstantAggregateZero::get(AType);
>> + if (!Filler)
>> + return nullptr;
>> }
>>
>> + // Emit initializer elements.
>> SmallVector<llvm::Constant*, 16> Elts;
>> - Elts.reserve(NumElements);
>> - for (unsigned I = 0; I < NumElements; ++I) {
>> - llvm::Constant *C = Filler;
>> - if (I < NumInitElts) {
>> - C = tryEmitPrivateForMemory(Value.getArrayInitializedElt(I),
>> - CAT->getElementType());
>> - } else if (!Filler) {
>> - assert(Value.hasArrayFiller() &&
>> - "Missing filler for implicit elements of initializer");
>> - C = tryEmitPrivateForMemory(Value.getArrayFiller(),
>> - CAT->getElementType());
>> - }
>> + if (Filler && Filler->isNullValue())
>> + Elts.reserve(NumInitElts + 1);
>> + else
>> + Elts.reserve(NumElements);
>> +
>> + llvm::Type *CommonElementType = nullptr;
>> + for (unsigned I = 0; I < NumInitElts; ++I) {
>> + llvm::Constant *C = tryEmitPrivateForMemory(
>> + Value.getArrayInitializedElt(I), CAT->getElementType());
>> if (!C) return nullptr;
>>
>> if (I == 0)
>> @@ -1932,20 +1956,8 @@ llvm::Constant *ConstantEmitter::tryEmit
>> Elts.push_back(C);
>> }
>>
>> - if (!CommonElementType) {
>> - // FIXME: Try to avoid packing the array
>> - std::vector<llvm::Type*> Types;
>> - Types.reserve(NumElements);
>> - for (unsigned i = 0, e = Elts.size(); i < e; ++i)
>> - Types.push_back(Elts[i]->getType());
>> - llvm::StructType *SType =
>> - llvm::StructType::get(CGM.getLLVMContext(), Types, true);
>> - return llvm::ConstantStruct::get(SType, Elts);
>> - }
>> -
>> - llvm::ArrayType *AType =
>> - llvm::ArrayType::get(CommonElementType, NumElements);
>> - return llvm::ConstantArray::get(AType, Elts);
>> + return EmitArrayConstant(CGM, CAT, CommonElementType, NumElements,
>> Elts,
>> + Filler);
>> }
>> case APValue::MemberPointer:
>> return CGM.getCXXABI().EmitMemberPointer(Value, DestType);
>>
>> Modified: cfe/trunk/lib/Sema/SemaInit.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/
>> SemaInit.cpp?rev=333141&r1=333140&r2=333141&view=diff
>> ============================================================
>> ==================
>> --- cfe/trunk/lib/Sema/SemaInit.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaInit.cpp Wed May 23 16:41:38 2018
>> @@ -751,6 +751,9 @@ InitListChecker::FillInEmptyInitializati
>> ElementEntity.getKind() == InitializedEntity::EK_VectorElement)
>> ElementEntity.setElementIndex(Init);
>>
>> + if (Init >= NumInits && ILE->hasArrayFiller())
>> + return;
>> +
>> Expr *InitExpr = (Init < NumInits ? ILE->getInit(Init) : nullptr);
>> if (!InitExpr && Init < NumInits && ILE->hasArrayFiller())
>> ILE->setInit(Init, ILE->getArrayFiller());
>>
>> Modified: cfe/trunk/test/CodeGen/init.c
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/
>> CodeGen/init.c?rev=333141&r1=333140&r2=333141&view=diff
>> ============================================================
>> ==================
>> --- cfe/trunk/test/CodeGen/init.c (original)
>> +++ cfe/trunk/test/CodeGen/init.c Wed May 23 16:41:38 2018
>> @@ -72,6 +72,16 @@ struct a7 {
>> struct a7 test7 = { .b = 0, .v = "bar" };
>>
>>
>> +// CHECK-DAG: @huge_array = global {{.*}} <{ i32 1, i32 0, i32 2, i32 0,
>> i32 3, [999999995 x i32] zeroinitializer }>
>> +int huge_array[1000000000] = {1, 0, 2, 0, 3, 0, 0, 0};
>> +
>> +// CHECK-DAG: @huge_struct = global {{.*}} { i32 1, <{ i32, [999999999 x
>> i32] }> <{ i32 2, [999999999 x i32] zeroinitializer }> }
>> +struct Huge {
>> + int a;
>> + int arr[1000 * 1000 * 1000];
>> +} huge_struct = {1, {2, 0, 0, 0}};
>> +
>> +
>> // PR279 comment #3
>> char test8(int X) {
>> char str[100000] = "abc"; // tail should be memset.
>>
>> Modified: cfe/trunk/test/CodeGenCXX/cxx11-initializer-aggregate.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/
>> CodeGenCXX/cxx11-initializer-aggregate.cpp?rev=333141&r1=
>> 333140&r2=333141&view=diff
>> ============================================================
>> ==================
>> --- cfe/trunk/test/CodeGenCXX/cxx11-initializer-aggregate.cpp (original)
>> +++ cfe/trunk/test/CodeGenCXX/cxx11-initializer-aggregate.cpp Wed May 23
>> 16:41:38 2018
>> @@ -11,6 +11,42 @@ namespace NonAggregateCopyInAggregateIni
>> struct C { A &&p; } c{{1}};
>> }
>>
>> +namespace NearlyZeroInit {
>> + // CHECK-DAG: @_ZN14NearlyZeroInit1aE = global {{.*}} <{ i32 1, i32 2,
>> i32 3, [120 x i32] zeroinitializer }>
>> + int a[123] = {1, 2, 3};
>> + // CHECK-DAG: @_ZN14NearlyZeroInit1bE = global {{.*}} { i32 1, <{ i32,
>> [2147483647 <(214)%20748-3647> x i32] }> <{ i32 2, [2147483647 x i32]
>> zeroinitializer }> }
>> + struct B { int n; int arr[1024 * 1024 * 1024 * 2u]; } b = {1, {2}};
>> +}
>> +
>> +namespace PR37560 {
>> + union U {
>> + char x;
>> + int a;
>> + };
>> + // FIXME: [dcl.init]p2, the padding bits of the union object should be
>> + // initialized to 0, not undef, which would allow us to collapse the
>> tail
>> + // of these arrays to zeroinitializer.
>> + // CHECK-DAG: @_ZN7PR375601cE = global <{ { i8, [3 x i8] } }> <{ { i8,
>> [3 x i8] } { i8 0, [3 x i8] undef } }>
>> + U c[1] = {};
>> + // CHECK-DAG: @_ZN7PR375601dE = global {{.*}} <{ { i8, [3 x i8] } { i8
>> 97, [3 x i8] undef }, %"{{[^"]*}}" { i32 123 }, { i8, [3 x i8] } { i8 98,
>> [3 x i8] undef }, { i8, [3 x i8] } { i8 0, [3 x i8] undef },
>> + U d[16] = {'a', {.a = 123}, 'b'};
>> + // CHECK-DAG: @_ZN7PR375601eE = global {{.*}} <{ %"{{[^"]*}}" { i32
>> 123 }, %"{{[^"]*}}" { i32 456 }, { i8, [3 x i8] } { i8 0, [3 x i8] undef },
>> + U e[16] = {{.a = 123}, {.a = 456}};
>> +
>> + union V {
>> + int a;
>> + char x;
>> + };
>> + // CHECK-DAG: @_ZN7PR375601fE = global [1 x %"{{[^"]*}}"]
>> zeroinitializer
>> + V f[1] = {};
>> + // CHECK-DAG: @_ZN7PR375601gE = global {{.*}} <{ { i8, [3 x i8] } { i8
>> 97, [3 x i8] undef }, %"{{[^"]*}}" { i32 123 }, { i8, [3 x i8] } { i8 98,
>> [3 x i8] undef }, [13 x %"{{[^"]*}}"] zeroinitializer }>
>> + V g[16] = {{.x = 'a'}, {.a = 123}, {.x = 'b'}};
>> + // CHECK-DAG: @_ZN7PR375601hE = global {{.*}} <{ %"{{[^"]*}}" { i32
>> 123 }, %"{{[^"]*}}" { i32 456 }, [14 x %"{{[^"]*}}"] zeroinitializer }>
>> + V h[16] = {{.a = 123}, {.a = 456}};
>> + // CHECK-DAG: @_ZN7PR375601iE = global [4 x %"{{[^"]*}}"]
>> [%"{{[^"]*}}" { i32 123 }, %"{{[^"]*}}" { i32 456 }, %"{{[^"]*}}"
>> zeroinitializer, %"{{[^"]*}}" zeroinitializer]
>> + V i[4] = {{.a = 123}, {.a = 456}};
>> +}
>> +
>> // CHECK-LABEL: define {{.*}}@_Z3fn1i(
>> int fn1(int x) {
>> // CHECK: %[[INITLIST:.*]] = alloca %struct.A
>> @@ -51,3 +87,35 @@ namespace NonTrivialInit {
>> // meaningful.
>> B b[30] = {};
>> }
>> +
>> +namespace ZeroInit {
>> + enum { Zero, One };
>> + constexpr int zero() { return 0; }
>> + constexpr int *null() { return nullptr; }
>> + struct Filler {
>> + int x;
>> + Filler();
>> + };
>> + struct S1 {
>> + int x;
>> + };
>> +
>> + // These declarations, if implemented elementwise, require huge
>> + // amout of memory and compiler time.
>> + unsigned char data_1[1024 * 1024 * 1024 * 2u] = { 0 };
>> + unsigned char data_2[1024 * 1024 * 1024 * 2u] = { Zero };
>> + unsigned char data_3[1024][1024][1024] = {{{0}}};
>> + unsigned char data_4[1024 * 1024 * 1024 * 2u] = { zero() };
>> + int *data_5[1024 * 1024 * 512] = { nullptr };
>> + int *data_6[1024 * 1024 * 512] = { null() };
>> + struct S1 data_7[1024 * 1024 * 512] = {{0}};
>> + char data_8[1000 * 1000 * 1000] = {};
>> + int (&&data_9)[1000 * 1000 * 1000] = {0};
>> + unsigned char data_10[1024 * 1024 * 1024 * 2u] = { 1 };
>> + unsigned char data_11[1024 * 1024 * 1024 * 2u] = { One };
>> + unsigned char data_12[1024][1024][1024] = {{{1}}};
>> +
>> + // This variable must be initialized elementwise.
>> + Filler data_e1[1024] = {};
>> + // CHECK: getelementptr inbounds {{.*}} @_ZN8ZeroInit7data_e1E
>> +}
>>
>> Modified: cfe/trunk/test/SemaCXX/aggregate-initialization.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/
>> SemaCXX/aggregate-initialization.cpp?rev=333141&
>> r1=333140&r2=333141&view=diff
>> ============================================================
>> ==================
>> --- cfe/trunk/test/SemaCXX/aggregate-initialization.cpp (original)
>> +++ cfe/trunk/test/SemaCXX/aggregate-initialization.cpp Wed May 23
>> 16:41:38 2018
>> @@ -180,3 +180,9 @@ namespace IdiomaticStdArrayInitDoesNotWa
>>
>> #pragma clang diagnostic pop
>> }
>> +
>> +namespace HugeArraysUseArrayFiller {
>> + // All we're checking here is that initialization completes in a
>> reasonable
>> + // amount of time.
>> + struct A { int n; int arr[1000 * 1000 * 1000]; } a = {1, {2}};
>> +}
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180529/90289059/attachment-0001.html>
More information about the cfe-commits
mailing list