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}}; -} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits