r332847 - [CodeGen] Recognize more cases of zero initialization
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Mon May 21 13:41:16 PDT 2018
Reverted in r332886; I added a testcase for the miscompile below
to test/CodeGenCXX/reference-init.cpp
On 21 May 2018 at 13:28, Richard Smith <richard at metafoo.co.uk> wrote:
> On 21 May 2018 at 13:22, Richard Smith <richard at metafoo.co.uk> wrote:
>
>> On 21 May 2018 at 09:09, Serge Pavlov via cfe-commits <
>> cfe-commits at 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/c
>>> onst-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/d
>>> esignated-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/u
>>> nion-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=33
>>> 2847&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=33
>>> 2847&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/l
>>> arge-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 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/20180521/ad6b0343/attachment-0001.html>
More information about the cfe-commits
mailing list