<div dir="ltr">Reverted in r332886; I added a testcase for the miscompile below to test/CodeGenCXX/reference-init.cpp<br></div><div class="gmail_extra"><br><div class="gmail_quote">On 21 May 2018 at 13:28, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><div class="h5">On 21 May 2018 at 13:22, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><div class="m_4798190679242068737gmail-h5">On 21 May 2018 at 09:09, Serge Pavlov via cfe-commits <span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Author: sepavloff<br>
Date: Mon May 21 09:09:54 2018<br>
New Revision: 332847<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=332847&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject?rev=332847&view=rev</a><br>
Log:<br>
[CodeGen] Recognize more cases of zero initialization<br>
<br>
If a variable has an initializer, codegen tries to build its value. If<br>
the variable is large in size, building its value requires substantial<br>
resources. It causes strange behavior from user viewpoint: compilation<br>
of huge zero initialized arrays like:<br>
<br>
char data_1[2147483648u] = { 0 };<br>
<br>
consumes enormous amount of time and memory.<br>
<br>
With this change codegen tries to determine if variable initializer is<br>
equivalent to zero initializer. In this case variable value is not<br>
constructed.<br>
<br>
This change fixes PR18978.<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D46241" rel="noreferrer" target="_blank">https://reviews.llvm.org/D4624<wbr>1</a><br>
<br>
Removed:<br>
cfe/trunk/test/SemaCXX/large-a<wbr>rray-init.cpp<br>
Modified:<br>
cfe/trunk/include/clang/AST/Ex<wbr>pr.h<br>
cfe/trunk/lib/AST/ExprConstant<wbr>.cpp<br>
cfe/trunk/lib/CodeGen/CGExprCo<wbr>nstant.cpp<br>
cfe/trunk/test/CodeGen/const-i<wbr>nit.c<br>
cfe/trunk/test/CodeGen/designa<wbr>ted-initializers.c<br>
cfe/trunk/test/CodeGen/union-i<wbr>nit2.c<br>
cfe/trunk/test/CodeGenCXX/cxx1<wbr>1-initializer-aggregate.cpp<br>
cfe/trunk/test/CodeGenCXX/cxx1<wbr>z-initializer-aggregate.cpp<br>
<br>
Modified: cfe/trunk/include/clang/AST/Ex<wbr>pr.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=332847&r1=332846&r2=332847&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/cfe/trunk/include/clang/<wbr>AST/Expr.h?rev=332847&r1=33284<wbr>6&r2=332847&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/include/clang/AST/Ex<wbr>pr.h (original)<br>
+++ cfe/trunk/include/clang/AST/Ex<wbr>pr.h Mon May 21 09:09:54 2018<br>
@@ -537,6 +537,13 @@ public:<br>
bool isConstantInitializer(ASTConte<wbr>xt &Ctx, bool ForRef,<br>
const Expr **Culprit = nullptr) const;<br>
<br>
+ enum SideEffectsKind {<br>
+ SE_NoSideEffects, ///< Strictly evaluate the expression.<br>
+ SE_AllowUndefinedBehavior, ///< Allow UB that we can give a value, but not<br>
+ ///< arbitrary unmodeled side effects.<br>
+ SE_AllowSideEffects ///< Allow any unmodeled side effect.<br>
+ };<br>
+<br>
/// EvalStatus is a struct with detailed info about an evaluation in progress.<br>
struct EvalStatus {<br>
/// Whether the evaluated expression has side effects.<br>
@@ -565,6 +572,11 @@ public:<br>
bool hasSideEffects() const {<br>
return HasSideEffects;<br>
}<br>
+<br>
+ bool hasUnacceptableSideEffect(Side<wbr>EffectsKind SEK) {<br>
+ return (SEK < SE_AllowSideEffects && HasSideEffects) ||<br>
+ (SEK < SE_AllowUndefinedBehavior && HasUndefinedBehavior);<br>
+ }<br>
};<br>
<br>
/// EvalResult is a struct with detailed info about an evaluated expression.<br>
@@ -591,13 +603,6 @@ public:<br>
/// side-effects.<br>
bool EvaluateAsBooleanCondition(boo<wbr>l &Result, const ASTContext &Ctx) const;<br>
<br>
- enum SideEffectsKind {<br>
- SE_NoSideEffects, ///< Strictly evaluate the expression.<br>
- SE_AllowUndefinedBehavior, ///< Allow UB that we can give a value, but not<br>
- ///< arbitrary unmodeled side effects.<br>
- SE_AllowSideEffects ///< Allow any unmodeled side effect.<br>
- };<br>
-<br>
/// EvaluateAsInt - Return true if this is a constant which we can fold and<br>
/// convert to an integer, using any crazy technique that we want to.<br>
bool EvaluateAsInt(llvm::APSInt &Result, const ASTContext &Ctx,<br>
<br>
Modified: cfe/trunk/lib/AST/ExprConstant<wbr>.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=332847&r1=332846&r2=332847&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/cfe/trunk/lib/AST/ExprCo<wbr>nstant.cpp?rev=332847&r1=33284<wbr>6&r2=332847&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/lib/AST/ExprConstant<wbr>.cpp (original)<br>
+++ cfe/trunk/lib/AST/ExprConstant<wbr>.cpp Mon May 21 09:09:54 2018<br>
@@ -10312,12 +10312,6 @@ bool Expr::EvaluateAsBooleanConditi<wbr>on(bo<br>
HandleConversionToBool(Scratch<wbr>.Val, Result);<br>
}<br>
<br>
-static bool hasUnacceptableSideEffect(Expr<wbr>::EvalStatus &Result,<br>
- Expr::SideEffectsKind SEK) {<br>
- return (SEK < Expr::SE_AllowSideEffects && Result.HasSideEffects) ||<br>
- (SEK < Expr::SE_AllowUndefinedBehavio<wbr>r && Result.HasUndefinedBehavior);<br>
-}<br>
-<br>
bool Expr::EvaluateAsInt(APSInt &Result, const ASTContext &Ctx,<br>
SideEffectsKind AllowSideEffects) const {<br>
if (!getType()->isIntegralOrEnume<wbr>rationType())<br>
@@ -10325,7 +10319,7 @@ bool Expr::EvaluateAsInt(APSInt &Result,<br>
<br>
EvalResult ExprResult;<br>
if (!EvaluateAsRValue(ExprResult, Ctx) || !ExprResult.Val.isInt() ||<br>
- hasUnacceptableSideEffect(Expr<wbr>Result, AllowSideEffects))<br>
+ ExprResult.hasUnacceptableSide<wbr>Effect(AllowSideEffects))<br>
return false;<br>
<br>
Result = ExprResult.Val.getInt();<br>
@@ -10339,7 +10333,7 @@ bool Expr::EvaluateAsFloat(APFloat &Resu<br>
<br>
EvalResult ExprResult;<br>
if (!EvaluateAsRValue(ExprResult, Ctx) || !ExprResult.Val.isFloat() ||<br>
- hasUnacceptableSideEffect(Expr<wbr>Result, AllowSideEffects))<br>
+ ExprResult.hasUnacceptableSide<wbr>Effect(AllowSideEffects))<br>
return false;<br>
<br>
Result = ExprResult.Val.getFloat();<br>
@@ -10417,7 +10411,7 @@ bool Expr::EvaluateAsInitializer(AP<wbr>Value<br>
bool Expr::isEvaluatable(const ASTContext &Ctx, SideEffectsKind SEK) const {<br>
EvalResult Result;<br>
return EvaluateAsRValue(Result, Ctx) &&<br>
- !hasUnacceptableSideEffect(Re<wbr>sult, SEK);<br>
+ !Result.hasUnacceptableSideEf<wbr>fect(SEK);<br>
}<br>
<br>
APSInt Expr::EvaluateKnownConstInt(co<wbr>nst ASTContext &Ctx,<br>
<br>
Modified: cfe/trunk/lib/CodeGen/CGExprCo<wbr>nstant.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprConstant.cpp?rev=332847&r1=332846&r2=332847&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/cfe/trunk/lib/CodeGen/CG<wbr>ExprConstant.cpp?rev=332847&r1<wbr>=332846&r2=332847&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/lib/CodeGen/CGExprCo<wbr>nstant.cpp (original)<br>
+++ cfe/trunk/lib/CodeGen/CGExprCo<wbr>nstant.cpp Mon May 21 09:09:54 2018<br>
@@ -1392,20 +1392,40 @@ static QualType getNonMemoryType(CodeGen<br>
return type;<br>
}<br>
<br>
+/// Checks if the specified initializer is equivalent to zero initialization.<br>
+static bool isZeroInitializer(ConstantEmit<wbr>ter &CE, const Expr *Init) {<br>
+ if (auto *E = dyn_cast_or_null<CXXConstructE<wbr>xpr>(Init)) {<br>
+ CXXConstructorDecl *CD = E->getConstructor();<br>
+ return CD->isDefaultConstructor() && CD->isTrivial();<br>
+ }<br>
+<br>
+ if (auto *IL = dyn_cast_or_null<InitListExpr><wbr>(Init)) {<br>
+ for (auto I : IL->inits())<br>
+ if (!isZeroInitializer(CE, I))<br>
+ return false;<br>
+ if (const Expr *Filler = IL->getArrayFiller())<br>
+ return isZeroInitializer(CE, Filler);<br>
+ return true;<br>
+ }<br>
+<br>
+ QualType InitTy = Init->getType();<br>
+ if (InitTy->isIntegralOrEnumerati<wbr>onType() || InitTy->isPointerType()) {<br>
+ Expr::EvalResult Result;<br>
+ if (Init->EvaluateAsRValue(Result<wbr>, CE.CGM.getContext()) &&<br>
+ !Result.hasUnacceptableSideEff<wbr>ect(Expr::SE_NoSideEffects))<br></blockquote><div><br></div></div></div><div>As I mentioned on the review thread, this is wrong, and you need to call D->evaluateValue() here instead.</div></div></div></div></blockquote><div><br></div></div></div><div>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:</div><div><br></div><div> int f() { static const int &&r = {0}; return r; }<br></div><div><br></div><div>Prior to your patch this would be properly initialized; after your patch, due to the incorrect call to EvaluateAsRValue, we "zero-initialize" the reference.</div><div><br></div><div>Your patch will also result in our re-evaluating the initializers of variables that we've already evaluated.</div><div><div class="h5"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><div class="m_4798190679242068737gmail-h5"><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+ return (Result.Val.isInt() && Result.Val.getInt().isNullValu<wbr>e()) ||<br>
+ (Result.Val.isLValue() && Result.Val.isNullPointer());<br>
+ }<br>
+<br>
+ return false;<br>
+}<br>
+<br>
llvm::Constant *ConstantEmitter::tryEmitPriva<wbr>teForVarInit(const VarDecl &D) {<br>
// Make a quick check if variable can be default NULL initialized<br>
// and avoid going through rest of code which may do, for c++11,<br>
// initialization of memory to all NULLs.<br>
- if (!D.hasLocalStorage()) {<br>
- QualType Ty = CGM.getContext().getBaseElemen<wbr>tType(D.getType());<br>
- if (Ty->isRecordType())<br>
- if (const CXXConstructExpr *E =<br>
- dyn_cast_or_null<CXXConstructE<wbr>xpr>(D.getInit())) {<br>
- const CXXConstructorDecl *CD = E->getConstructor();<br>
- if (CD->isTrivial() && CD->isDefaultConstructor())<br>
- return CGM.EmitNullConstant(D.getType<wbr>());<br>
- }<br>
- }<br>
+ if (!D.hasLocalStorage() && isZeroInitializer(*this, D.getInit()))<br>
+ return CGM.EmitNullConstant(D.getType<wbr>());<br>
<br>
QualType destType = D.getType();<br>
<br>
<br>
Modified: cfe/trunk/test/CodeGen/const-i<wbr>nit.c<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/const-init.c?rev=332847&r1=332846&r2=332847&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/cfe/trunk/test/CodeGen/c<wbr>onst-init.c?rev=332847&r1=3328<wbr>46&r2=332847&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/test/CodeGen/const-i<wbr>nit.c (original)<br>
+++ cfe/trunk/test/CodeGen/const-i<wbr>nit.c Mon May 21 09:09:54 2018<br>
@@ -167,7 +167,7 @@ void g30() {<br>
int : 1;<br>
int x;<br>
} a = {};<br>
- // CHECK: @g30.a = internal global %struct.anon.1 <{ i8 undef, i32 0 }>, align 1<br>
+ // CHECK: @g30.a = internal global %struct.anon.1 zeroinitializer, align 1<br>
#pragma pack()<br>
}<br>
<br>
<br>
Modified: cfe/trunk/test/CodeGen/designa<wbr>ted-initializers.c<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/designated-initializers.c?rev=332847&r1=332846&r2=332847&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/cfe/trunk/test/CodeGen/d<wbr>esignated-initializers.c?rev=3<wbr>32847&r1=332846&r2=332847&view<wbr>=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/test/CodeGen/designa<wbr>ted-initializers.c (original)<br>
+++ cfe/trunk/test/CodeGen/designa<wbr>ted-initializers.c Mon May 21 09:09:54 2018<br>
@@ -8,7 +8,7 @@ struct foo {<br>
// CHECK: @u = global %union.anon zeroinitializer<br>
union { int i; float f; } u = { };<br>
<br>
-// CHECK: @u2 = global { i32, [4 x i8] } { i32 0, [4 x i8] undef }<br>
+// CHECK: @u2 = global %union.anon.0 zeroinitializer<br>
union { int i; double f; } u2 = { };<br>
<br>
// CHECK: @u3 = global %union.anon.1 zeroinitializer<br>
<br>
Modified: cfe/trunk/test/CodeGen/union-i<wbr>nit2.c<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/union-init2.c?rev=332847&r1=332846&r2=332847&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/cfe/trunk/test/CodeGen/u<wbr>nion-init2.c?rev=332847&r1=332<wbr>846&r2=332847&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/test/CodeGen/union-i<wbr>nit2.c (original)<br>
+++ cfe/trunk/test/CodeGen/union-i<wbr>nit2.c Mon May 21 09:09:54 2018<br>
@@ -5,7 +5,7 @@<br>
union x {long long b;union x* a;} r = {.a = &r};<br>
<br>
<br>
-// CHECK: global { [3 x i8], [5 x i8] } { [3 x i8] zeroinitializer, [5 x i8] undef }<br>
+// CHECK: global %union.z zeroinitializer<br>
union z {<br>
char a[3];<br>
long long b;<br>
<br>
Modified: cfe/trunk/test/CodeGenCXX/cxx1<wbr>1-initializer-aggregate.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/cxx11-initializer-aggregate.cpp?rev=332847&r1=332846&r2=332847&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/cfe/trunk/test/CodeGenCX<wbr>X/cxx11-initializer-aggregate.<wbr>cpp?rev=332847&r1=332846&r2=33<wbr>2847&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/test/CodeGenCXX/cxx1<wbr>1-initializer-aggregate.cpp (original)<br>
+++ cfe/trunk/test/CodeGenCXX/cxx1<wbr>1-initializer-aggregate.cpp Mon May 21 09:09:54 2018<br>
@@ -51,3 +51,30 @@ 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>
+<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/CodeGenCXX/cxx1<wbr>z-initializer-aggregate.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/cxx1z-initializer-aggregate.cpp?rev=332847&r1=332846&r2=332847&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/cfe/trunk/test/CodeGenCX<wbr>X/cxx1z-initializer-aggregate.<wbr>cpp?rev=332847&r1=332846&r2=33<wbr>2847&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/test/CodeGenCXX/cxx1<wbr>z-initializer-aggregate.cpp (original)<br>
+++ cfe/trunk/test/CodeGenCXX/cxx1<wbr>z-initializer-aggregate.cpp Mon May 21 09:09:54 2018<br>
@@ -17,14 +17,14 @@ namespace Constant {<br>
<br>
C c1 = {};<br>
C c2 = {1};<br>
- // CHECK: @_ZN8Constant2c1E = global { i8 } zeroinitializer, align 1<br>
+ // CHECK: @_ZN8Constant2c1E = global %"struct.Constant::C" zeroinitializer, align 1<br>
// CHECK: @_ZN8Constant2c2E = global { i8 } { i8 1 }, align 1<br>
<br>
// Test packing bases into tail padding.<br>
D d1 = {};<br>
D d2 = {1, 2, 3};<br>
D d3 = {1};<br>
- // CHECK: @_ZN8Constant2d1E = global { i32, i8, i8 } zeroinitializer, align 4<br>
+ // CHECK: @_ZN8Constant2d1E = global %"struct.Constant::D" zeroinitializer, align 4<br>
// CHECK: @_ZN8Constant2d2E = global { i32, i8, i8 } { i32 1, i8 2, i8 3 }, align 4<br>
// CHECK: @_ZN8Constant2d3E = global { i32, i8, i8 } { i32 1, i8 0, i8 0 }, align 4<br>
<br>
<br>
Removed: cfe/trunk/test/SemaCXX/large-a<wbr>rray-init.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/large-array-init.cpp?rev=332846&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/cfe/trunk/test/SemaCXX/l<wbr>arge-array-init.cpp?rev=332846<wbr>&view=auto</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/test/SemaCXX/large-a<wbr>rray-init.cpp (original)<br>
+++ cfe/trunk/test/SemaCXX/large-a<wbr>rray-init.cpp (removed)<br>
@@ -1,10 +0,0 @@<br>
-// RUN: %clang_cc1 -S -o %t.ll -mllvm -debug-only=exprconstant %s 2>&1 | \<br>
-// RUN: FileCheck %s<br>
-// REQUIRES: asserts<br>
-<br>
-struct S { int i; };<br>
-<br>
-static struct S arr[100000000] = {{ 0 }};<br>
-// CHECK: The number of elements to initialize: 1.<br>
-<br>
-struct S *foo() { return arr; }<br>
<br>
<br>
______________________________<wbr>_________________<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/<wbr>mailman/listinfo/cfe-commits</a><br>
</blockquote></div></div></div><br></div></div>
</blockquote></div></div></div><br></div></div>
</blockquote></div><br></div>