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