r333067 - Revert r333044 "Use zeroinitializer for (trailing zero portion of) large array initializers"
Hans Wennborg via cfe-commits
cfe-commits at lists.llvm.org
Wed May 23 01:24:02 PDT 2018
Author: hans
Date: Wed May 23 01:24:01 2018
New Revision: 333067
URL: http://llvm.org/viewvc/llvm-project?rev=333067&view=rev
Log:
Revert r333044 "Use zeroinitializer for (trailing zero portion of) large array initializers"
It caused asserts, see PR37560.
> Use zeroinitializer for (trailing zero portion of) large array initializers
> more reliably.
>
> Clang has two different ways it emits array constants (from InitListExprs and
> from APValues), and both had some ability to emit zeroinitializer, but neither
> was able to catch all cases where we could use zeroinitializer reliably. In
> particular, emitting from an APValue would fail to notice if all the explicit
> array elements happened to be zero. In addition, for large arrays where only an
> initial portion has an explicit initializer, we would emit the complete
> initializer (which could be huge) rather than emitting only the non-zero
> portion. With this change, when the element would have a suffix of more than 8
> zero elements, we emit the array constant as a packed struct of its initial
> portion followed by a zeroinitializer constant for the trailing zero portion.
>
> In passing, I found a bug where SemaInit would sometimes walk the entire array
> when checking an initializer that only covers the first few elements; that's
> fixed here to unblock testing of the rest.
>
> Differential Revision: https://reviews.llvm.org/D47166
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=333067&r1=333066&r2=333067&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGExprConstant.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGExprConstant.cpp Wed May 23 01:24:01 2018
@@ -635,52 +635,6 @@ static ConstantAddress tryEmitGlobalComp
return ConstantAddress(GV, Align);
}
-static llvm::Constant *
-EmitArrayConstant(llvm::ArrayType *PreferredArrayType,
- 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(PreferredArrayType);
-
- // If there's not many trailing zero elements, just emit an array
- // constant.
- if (NonzeroLength + 8 >= ArrayBound && CommonElementType) {
- Elements.resize(ArrayBound, Filler);
- return llvm::ConstantArray::get(
- llvm::ArrayType::get(CommonElementType, ArrayBound), Elements);
- }
-
- // Add a zeroinitializer array filler if we have trailing zeroes.
- if (unsigned TrailingZeroes = ArrayBound - NonzeroLength) {
- assert(Elements.size() >= NonzeroLength &&
- "missing initializer for non-zero element");
- Elements.resize(NonzeroLength + 1);
- auto *FillerType = PreferredArrayType->getElementType();
- if (TrailingZeroes > 1)
- FillerType = llvm::ArrayType::get(FillerType, TrailingZeroes);
- Elements.back() = llvm::ConstantAggregateZero::get(FillerType);
- }
-
- // 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(PreferredArrayType->getContext(), 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
@@ -880,6 +834,7 @@ public:
llvm::Constant *EmitArrayInitialization(InitListExpr *ILE, QualType T) {
llvm::ArrayType *AType =
cast<llvm::ArrayType>(ConvertType(ILE->getType()));
+ llvm::Type *ElemTy = AType->getElementType();
unsigned NumInitElements = ILE->getNumInits();
unsigned NumElements = AType->getNumElements();
@@ -890,35 +845,55 @@ public:
QualType EltType = CGM.getContext().getAsArrayType(T)->getElementType();
// Initialize remaining array elements.
- llvm::Constant *fillC = nullptr;
- if (Expr *filler = ILE->getArrayFiller()) {
+ llvm::Constant *fillC;
+ if (Expr *filler = ILE->getArrayFiller())
fillC = Emitter.tryEmitAbstractForMemory(filler, EltType);
- if (!fillC)
- return nullptr;
- }
+ 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);
// Copy initializer elements.
SmallVector<llvm::Constant*, 16> Elts;
- if (fillC && fillC->isNullValue())
- Elts.reserve(NumInitableElts + 1);
- else
- Elts.reserve(NumElements);
+ Elts.reserve(std::max(NumInitableElts, NumElements));
- llvm::Type *CommonElementType = nullptr;
+ bool RewriteType = false;
+ bool AllNullValues = true;
for (unsigned i = 0; i < NumInitableElts; ++i) {
Expr *Init = ILE->getInit(i);
llvm::Constant *C = Emitter.tryEmitPrivateForMemory(Init, EltType);
if (!C)
return nullptr;
- if (i == 0)
- CommonElementType = C->getType();
- else if (C->getType() != CommonElementType)
- CommonElementType = nullptr;
+ RewriteType |= (C->getType() != ElemTy);
Elts.push_back(C);
+ if (AllNullValues && !C->isNullValue())
+ AllNullValues = false;
}
- return EmitArrayConstant(AType, CommonElementType, NumElements, Elts,
- fillC);
+ // 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);
}
llvm::Constant *EmitRecordInitialization(InitListExpr *ILE, QualType T) {
@@ -1920,28 +1895,34 @@ llvm::Constant *ConstantEmitter::tryEmit
// Emit array filler, if there is one.
llvm::Constant *Filler = nullptr;
- if (Value.hasArrayFiller()) {
+ if (Value.hasArrayFiller())
Filler = tryEmitAbstractForMemory(Value.getArrayFiller(),
CAT->getElementType());
- if (!Filler)
- return nullptr;
- }
// Emit initializer elements.
llvm::Type *CommonElementType =
CGM.getTypes().ConvertType(CAT->getElementType());
- llvm::ArrayType *PreferredArrayType =
- llvm::ArrayType::get(CommonElementType, NumElements);
- SmallVector<llvm::Constant*, 16> Elts;
- if (Filler && Filler->isNullValue())
- Elts.reserve(NumInitElts + 1);
- else
- Elts.reserve(NumElements);
+ // 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);
+ }
- for (unsigned I = 0; I < NumInitElts; ++I) {
- llvm::Constant *C = tryEmitPrivateForMemory(
- Value.getArrayInitializedElt(I), CAT->getElementType());
+ 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 (!C) return nullptr;
if (I == 0)
@@ -1951,8 +1932,20 @@ llvm::Constant *ConstantEmitter::tryEmit
Elts.push_back(C);
}
- return EmitArrayConstant(PreferredArrayType, CommonElementType, NumElements,
- Elts, Filler);
+ 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);
}
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=333067&r1=333066&r2=333067&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaInit.cpp (original)
+++ cfe/trunk/lib/Sema/SemaInit.cpp Wed May 23 01:24:01 2018
@@ -751,9 +751,6 @@ 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=333067&r1=333066&r2=333067&view=diff
==============================================================================
--- cfe/trunk/test/CodeGen/init.c (original)
+++ cfe/trunk/test/CodeGen/init.c Wed May 23 01:24:01 2018
@@ -72,16 +72,6 @@ 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=333067&r1=333066&r2=333067&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/cxx11-initializer-aggregate.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/cxx11-initializer-aggregate.cpp Wed May 23 01:24:01 2018
@@ -11,13 +11,6 @@ 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 x i32] }> <{ i32 2, [2147483647 x i32] zeroinitializer }> }
- struct B { int n; int arr[1024 * 1024 * 1024 * 2u]; } b = {1, {2}};
-}
-
// CHECK-LABEL: define {{.*}}@_Z3fn1i(
int fn1(int x) {
// CHECK: %[[INITLIST:.*]] = alloca %struct.A
@@ -58,35 +51,3 @@ 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=333067&r1=333066&r2=333067&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/aggregate-initialization.cpp (original)
+++ cfe/trunk/test/SemaCXX/aggregate-initialization.cpp Wed May 23 01:24:01 2018
@@ -180,9 +180,3 @@ 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}};
-}
More information about the cfe-commits
mailing list