On 21 May 2018 at 13:22, Richard Smith <rich...@metafoo.co.uk> wrote:
> On 21 May 2018 at 09:09, Serge Pavlov via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > >> Author: sepavloff >> Date: Mon May 21 09:09:54 2018 >> New Revision: 332847 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=332847&view=rev >> Log: >> [CodeGen] Recognize more cases of zero initialization >> >> If a variable has an initializer, codegen tries to build its value. If >> the variable is large in size, building its value requires substantial >> resources. It causes strange behavior from user viewpoint: compilation >> of huge zero initialized arrays like: >> >> char data_1[2147483648u] = { 0 }; >> >> consumes enormous amount of time and memory. >> >> With this change codegen tries to determine if variable initializer is >> equivalent to zero initializer. In this case variable value is not >> constructed. >> >> This change fixes PR18978. >> >> Differential Revision: https://reviews.llvm.org/D46241 >> >> Removed: >> cfe/trunk/test/SemaCXX/large-array-init.cpp >> Modified: >> cfe/trunk/include/clang/AST/Expr.h >> cfe/trunk/lib/AST/ExprConstant.cpp >> cfe/trunk/lib/CodeGen/CGExprConstant.cpp >> cfe/trunk/test/CodeGen/const-init.c >> cfe/trunk/test/CodeGen/designated-initializers.c >> cfe/trunk/test/CodeGen/union-init2.c >> cfe/trunk/test/CodeGenCXX/cxx11-initializer-aggregate.cpp >> cfe/trunk/test/CodeGenCXX/cxx1z-initializer-aggregate.cpp >> >> Modified: cfe/trunk/include/clang/AST/Expr.h >> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ >> AST/Expr.h?rev=332847&r1=332846&r2=332847&view=diff >> ============================================================ >> ================== >> --- cfe/trunk/include/clang/AST/Expr.h (original) >> +++ cfe/trunk/include/clang/AST/Expr.h Mon May 21 09:09:54 2018 >> @@ -537,6 +537,13 @@ public: >> bool isConstantInitializer(ASTContext &Ctx, bool ForRef, >> const Expr **Culprit = nullptr) const; >> >> + enum SideEffectsKind { >> + SE_NoSideEffects, ///< Strictly evaluate the expression. >> + SE_AllowUndefinedBehavior, ///< Allow UB that we can give a value, >> but not >> + ///< arbitrary unmodeled side effects. >> + SE_AllowSideEffects ///< Allow any unmodeled side effect. >> + }; >> + >> /// EvalStatus is a struct with detailed info about an evaluation in >> progress. >> struct EvalStatus { >> /// Whether the evaluated expression has side effects. >> @@ -565,6 +572,11 @@ public: >> bool hasSideEffects() const { >> return HasSideEffects; >> } >> + >> + bool hasUnacceptableSideEffect(SideEffectsKind SEK) { >> + return (SEK < SE_AllowSideEffects && HasSideEffects) || >> + (SEK < SE_AllowUndefinedBehavior && HasUndefinedBehavior); >> + } >> }; >> >> /// EvalResult is a struct with detailed info about an evaluated >> expression. >> @@ -591,13 +603,6 @@ public: >> /// side-effects. >> bool EvaluateAsBooleanCondition(bool &Result, const ASTContext &Ctx) >> const; >> >> - enum SideEffectsKind { >> - SE_NoSideEffects, ///< Strictly evaluate the expression. >> - SE_AllowUndefinedBehavior, ///< Allow UB that we can give a value, >> but not >> - ///< arbitrary unmodeled side effects. >> - SE_AllowSideEffects ///< Allow any unmodeled side effect. >> - }; >> - >> /// EvaluateAsInt - Return true if this is a constant which we can >> fold and >> /// convert to an integer, using any crazy technique that we want to. >> bool EvaluateAsInt(llvm::APSInt &Result, const ASTContext &Ctx, >> >> Modified: cfe/trunk/lib/AST/ExprConstant.cpp >> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprCo >> nstant.cpp?rev=332847&r1=332846&r2=332847&view=diff >> ============================================================ >> ================== >> --- cfe/trunk/lib/AST/ExprConstant.cpp (original) >> +++ cfe/trunk/lib/AST/ExprConstant.cpp Mon May 21 09:09:54 2018 >> @@ -10312,12 +10312,6 @@ bool Expr::EvaluateAsBooleanCondition(bo >> HandleConversionToBool(Scratch.Val, Result); >> } >> >> -static bool hasUnacceptableSideEffect(Expr::EvalStatus &Result, >> - Expr::SideEffectsKind SEK) { >> - return (SEK < Expr::SE_AllowSideEffects && Result.HasSideEffects) || >> - (SEK < Expr::SE_AllowUndefinedBehavior && >> Result.HasUndefinedBehavior); >> -} >> - >> bool Expr::EvaluateAsInt(APSInt &Result, const ASTContext &Ctx, >> SideEffectsKind AllowSideEffects) const { >> if (!getType()->isIntegralOrEnumerationType()) >> @@ -10325,7 +10319,7 @@ bool Expr::EvaluateAsInt(APSInt &Result, >> >> EvalResult ExprResult; >> if (!EvaluateAsRValue(ExprResult, Ctx) || !ExprResult.Val.isInt() || >> - hasUnacceptableSideEffect(ExprResult, AllowSideEffects)) >> + ExprResult.hasUnacceptableSideEffect(AllowSideEffects)) >> return false; >> >> Result = ExprResult.Val.getInt(); >> @@ -10339,7 +10333,7 @@ bool Expr::EvaluateAsFloat(APFloat &Resu >> >> EvalResult ExprResult; >> if (!EvaluateAsRValue(ExprResult, Ctx) || !ExprResult.Val.isFloat() || >> - hasUnacceptableSideEffect(ExprResult, AllowSideEffects)) >> + ExprResult.hasUnacceptableSideEffect(AllowSideEffects)) >> return false; >> >> Result = ExprResult.Val.getFloat(); >> @@ -10417,7 +10411,7 @@ bool Expr::EvaluateAsInitializer(APValue >> bool Expr::isEvaluatable(const ASTContext &Ctx, SideEffectsKind SEK) >> const { >> EvalResult Result; >> return EvaluateAsRValue(Result, Ctx) && >> - !hasUnacceptableSideEffect(Result, SEK); >> + !Result.hasUnacceptableSideEffect(SEK); >> } >> >> APSInt Expr::EvaluateKnownConstInt(const ASTContext &Ctx, >> >> Modified: cfe/trunk/lib/CodeGen/CGExprConstant.cpp >> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CG >> ExprConstant.cpp?rev=332847&r1=332846&r2=332847&view=diff >> ============================================================ >> ================== >> --- cfe/trunk/lib/CodeGen/CGExprConstant.cpp (original) >> +++ cfe/trunk/lib/CodeGen/CGExprConstant.cpp Mon May 21 09:09:54 2018 >> @@ -1392,20 +1392,40 @@ static QualType getNonMemoryType(CodeGen >> return type; >> } >> >> +/// Checks if the specified initializer is equivalent to zero >> initialization. >> +static bool isZeroInitializer(ConstantEmitter &CE, const Expr *Init) { >> + if (auto *E = dyn_cast_or_null<CXXConstructExpr>(Init)) { >> + CXXConstructorDecl *CD = E->getConstructor(); >> + return CD->isDefaultConstructor() && CD->isTrivial(); >> + } >> + >> + if (auto *IL = dyn_cast_or_null<InitListExpr>(Init)) { >> + for (auto I : IL->inits()) >> + if (!isZeroInitializer(CE, I)) >> + return false; >> + if (const Expr *Filler = IL->getArrayFiller()) >> + return isZeroInitializer(CE, Filler); >> + return true; >> + } >> + >> + QualType InitTy = Init->getType(); >> + if (InitTy->isIntegralOrEnumerationType() || InitTy->isPointerType()) >> { >> + Expr::EvalResult Result; >> + if (Init->EvaluateAsRValue(Result, CE.CGM.getContext()) && >> + !Result.hasUnacceptableSideEffect(Expr::SE_NoSideEffects)) >> > > As I mentioned on the review thread, this is wrong, and you need to call > D->evaluateValue() here instead. > Actually, evaluateValue() isn't quite right here either, because we may have drilled into the initializer already. Here's a simple example of code you miscompile: int f() { static const int &&r = {0}; return r; } Prior to your patch this would be properly initialized; after your patch, due to the incorrect call to EvaluateAsRValue, we "zero-initialize" the reference. Your patch will also result in our re-evaluating the initializers of variables that we've already evaluated. > + return (Result.Val.isInt() && Result.Val.getInt().isNullValue()) || >> + (Result.Val.isLValue() && Result.Val.isNullPointer()); >> + } >> + >> + return false; >> +} >> + >> llvm::Constant *ConstantEmitter::tryEmitPrivateForVarInit(const VarDecl >> &D) { >> // Make a quick check if variable can be default NULL initialized >> // and avoid going through rest of code which may do, for c++11, >> // initialization of memory to all NULLs. >> - if (!D.hasLocalStorage()) { >> - QualType Ty = CGM.getContext().getBaseElementType(D.getType()); >> - if (Ty->isRecordType()) >> - if (const CXXConstructExpr *E = >> - dyn_cast_or_null<CXXConstructExpr>(D.getInit())) { >> - const CXXConstructorDecl *CD = E->getConstructor(); >> - if (CD->isTrivial() && CD->isDefaultConstructor()) >> - return CGM.EmitNullConstant(D.getType()); >> - } >> - } >> + if (!D.hasLocalStorage() && isZeroInitializer(*this, D.getInit())) >> + return CGM.EmitNullConstant(D.getType()); >> >> QualType destType = D.getType(); >> >> >> Modified: cfe/trunk/test/CodeGen/const-init.c >> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ >> const-init.c?rev=332847&r1=332846&r2=332847&view=diff >> ============================================================ >> ================== >> --- cfe/trunk/test/CodeGen/const-init.c (original) >> +++ cfe/trunk/test/CodeGen/const-init.c Mon May 21 09:09:54 2018 >> @@ -167,7 +167,7 @@ void g30() { >> int : 1; >> int x; >> } a = {}; >> - // CHECK: @g30.a = internal global %struct.anon.1 <{ i8 undef, i32 0 >> }>, align 1 >> + // CHECK: @g30.a = internal global %struct.anon.1 zeroinitializer, >> align 1 >> #pragma pack() >> } >> >> >> Modified: cfe/trunk/test/CodeGen/designated-initializers.c >> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ >> designated-initializers.c?rev=332847&r1=332846&r2=332847&view=diff >> ============================================================ >> ================== >> --- cfe/trunk/test/CodeGen/designated-initializers.c (original) >> +++ cfe/trunk/test/CodeGen/designated-initializers.c Mon May 21 09:09:54 >> 2018 >> @@ -8,7 +8,7 @@ struct foo { >> // CHECK: @u = global %union.anon zeroinitializer >> union { int i; float f; } u = { }; >> >> -// CHECK: @u2 = global { i32, [4 x i8] } { i32 0, [4 x i8] undef } >> +// CHECK: @u2 = global %union.anon.0 zeroinitializer >> union { int i; double f; } u2 = { }; >> >> // CHECK: @u3 = global %union.anon.1 zeroinitializer >> >> Modified: cfe/trunk/test/CodeGen/union-init2.c >> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/ >> union-init2.c?rev=332847&r1=332846&r2=332847&view=diff >> ============================================================ >> ================== >> --- cfe/trunk/test/CodeGen/union-init2.c (original) >> +++ cfe/trunk/test/CodeGen/union-init2.c Mon May 21 09:09:54 2018 >> @@ -5,7 +5,7 @@ >> union x {long long b;union x* a;} r = {.a = &r}; >> >> >> -// CHECK: global { [3 x i8], [5 x i8] } { [3 x i8] zeroinitializer, [5 x >> i8] undef } >> +// CHECK: global %union.z zeroinitializer >> union z { >> char a[3]; >> long long b; >> >> Modified: cfe/trunk/test/CodeGenCXX/cxx11-initializer-aggregate.cpp >> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCX >> X/cxx11-initializer-aggregate.cpp?rev=332847&r1=332846&r2= >> 332847&view=diff >> ============================================================ >> ================== >> --- cfe/trunk/test/CodeGenCXX/cxx11-initializer-aggregate.cpp (original) >> +++ cfe/trunk/test/CodeGenCXX/cxx11-initializer-aggregate.cpp Mon May 21 >> 09:09:54 2018 >> @@ -51,3 +51,30 @@ 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}}; >> + >> + // This variable must be initialized elementwise. >> + Filler data_e1[1024] = {}; >> + // CHECK: getelementptr inbounds {{.*}} @_ZN8ZeroInit7data_e1E >> +} >> >> Modified: cfe/trunk/test/CodeGenCXX/cxx1z-initializer-aggregate.cpp >> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCX >> X/cxx1z-initializer-aggregate.cpp?rev=332847&r1=332846&r2= >> 332847&view=diff >> ============================================================ >> ================== >> --- cfe/trunk/test/CodeGenCXX/cxx1z-initializer-aggregate.cpp (original) >> +++ cfe/trunk/test/CodeGenCXX/cxx1z-initializer-aggregate.cpp Mon May 21 >> 09:09:54 2018 >> @@ -17,14 +17,14 @@ namespace Constant { >> >> C c1 = {}; >> C c2 = {1}; >> - // CHECK: @_ZN8Constant2c1E = global { i8 } zeroinitializer, align 1 >> + // CHECK: @_ZN8Constant2c1E = global %"struct.Constant::C" >> zeroinitializer, align 1 >> // CHECK: @_ZN8Constant2c2E = global { i8 } { i8 1 }, align 1 >> >> // Test packing bases into tail padding. >> D d1 = {}; >> D d2 = {1, 2, 3}; >> D d3 = {1}; >> - // CHECK: @_ZN8Constant2d1E = global { i32, i8, i8 } zeroinitializer, >> align 4 >> + // CHECK: @_ZN8Constant2d1E = global %"struct.Constant::D" >> zeroinitializer, align 4 >> // CHECK: @_ZN8Constant2d2E = global { i32, i8, i8 } { i32 1, i8 2, i8 >> 3 }, align 4 >> // CHECK: @_ZN8Constant2d3E = global { i32, i8, i8 } { i32 1, i8 0, i8 >> 0 }, align 4 >> >> >> Removed: cfe/trunk/test/SemaCXX/large-array-init.cpp >> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/ >> large-array-init.cpp?rev=332846&view=auto >> ============================================================ >> ================== >> --- cfe/trunk/test/SemaCXX/large-array-init.cpp (original) >> +++ cfe/trunk/test/SemaCXX/large-array-init.cpp (removed) >> @@ -1,10 +0,0 @@ >> -// RUN: %clang_cc1 -S -o %t.ll -mllvm -debug-only=exprconstant %s 2>&1 | >> \ >> -// RUN: FileCheck %s >> -// REQUIRES: asserts >> - >> -struct S { int i; }; >> - >> -static struct S arr[100000000] = {{ 0 }}; >> -// CHECK: The number of elements to initialize: 1. >> - >> -struct S *foo() { return arr; } >> >> >> _______________________________________________ >> cfe-commits mailing list >> cfe-commits@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >> > >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
