[PATCH] D46241: [CodeGen] Recognize more cases of zero initialization
rsmith added a comment. I've done some more investigation, and I now think this patch is merely working around deficiencies elsewhere. See https://reviews.llvm.org/D47166, which aims to fix those deficiencies more directly. Repository: rC Clang https://reviews.llvm.org/D46241 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46241: [CodeGen] Recognize more cases of zero initialization
rsmith added inline comments.
Comment at: lib/CodeGen/CGExprConstant.cpp:1414-1415
+Expr::EvalResult Result;
+if (Init->EvaluateAsRValue(Result, CE.CGM.getContext()) &&
+!Result.hasUnacceptableSideEffect(Expr::SE_NoSideEffects))
+ return (Result.Val.isInt() && Result.Val.getInt().isNullValue()) ||
sepavloff wrote:
> rsmith wrote:
> > Please call `D.evaluateValue()` here rather than inventing your own
> > evaluation scheme. That way, we'll cache the evaluated initializer on the
> > variable for other uses or reuse the value if we've already evaluated it,
> > and you don't need to worry about the corner cases involved in getting the
> > evaluation right. (As it happens, you're getting some minor details wrong
> > because evaluating an initializer is not quite the same as evaluating an
> > rvalue, but in practice it's not a big deal here.)
>
> Call of `D.evaluateValue()` may result in substantial memory and time
> consumption if the variable value is huge, like in:
> ```
> int char data_1[2147483648u] = { 0 };
> ```
> The idea of this patch is to recognize some cases of zero initialization
> prior to the evaluation of variable initializer. In the example above, value
> would be evaluated only for part of the initializer, namely `0`, which does
> not have an associated variable, so call of `D.evaluateValue()` is not
> possible.
>
As noted, `EvaluateAsRValue` gets some of the details here wrong. I reverted
this in r332886 because of a miscompile due to this fact.
Repository:
rC Clang
https://reviews.llvm.org/D46241
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46241: [CodeGen] Recognize more cases of zero initialization
This revision was automatically updated to reflect the committed changes.
Closed by commit rC332847: [CodeGen] Recognize more cases of zero
initialization (authored by sepavloff, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D46241?vs=146091=147798#toc
Repository:
rC Clang
https://reviews.llvm.org/D46241
Files:
include/clang/AST/Expr.h
lib/AST/ExprConstant.cpp
lib/CodeGen/CGExprConstant.cpp
test/CodeGen/const-init.c
test/CodeGen/designated-initializers.c
test/CodeGen/union-init2.c
test/CodeGenCXX/cxx11-initializer-aggregate.cpp
test/CodeGenCXX/cxx1z-initializer-aggregate.cpp
test/SemaCXX/large-array-init.cpp
Index: lib/CodeGen/CGExprConstant.cpp
===
--- lib/CodeGen/CGExprConstant.cpp
+++ lib/CodeGen/CGExprConstant.cpp
@@ -1392,20 +1392,40 @@
return type;
}
+/// Checks if the specified initializer is equivalent to zero initialization.
+static bool isZeroInitializer(ConstantEmitter , const Expr *Init) {
+ if (auto *E = dyn_cast_or_null(Init)) {
+CXXConstructorDecl *CD = E->getConstructor();
+return CD->isDefaultConstructor() && CD->isTrivial();
+ }
+
+ if (auto *IL = dyn_cast_or_null(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))
+ return (Result.Val.isInt() && Result.Val.getInt().isNullValue()) ||
+ (Result.Val.isLValue() && Result.Val.isNullPointer());
+ }
+
+ return false;
+}
+
llvm::Constant *ConstantEmitter::tryEmitPrivateForVarInit(const VarDecl ) {
// 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(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();
Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -10312,20 +10312,14 @@
HandleConversionToBool(Scratch.Val, Result);
}
-static bool hasUnacceptableSideEffect(Expr::EvalStatus ,
- Expr::SideEffectsKind SEK) {
- return (SEK < Expr::SE_AllowSideEffects && Result.HasSideEffects) ||
- (SEK < Expr::SE_AllowUndefinedBehavior && Result.HasUndefinedBehavior);
-}
-
bool Expr::EvaluateAsInt(APSInt , const ASTContext ,
SideEffectsKind AllowSideEffects) const {
if (!getType()->isIntegralOrEnumerationType())
return false;
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 @@
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::isEvaluatable(const ASTContext , SideEffectsKind SEK) const {
EvalResult Result;
return EvaluateAsRValue(Result, Ctx) &&
- !hasUnacceptableSideEffect(Result, SEK);
+ !Result.hasUnacceptableSideEffect(SEK);
}
APSInt Expr::EvaluateKnownConstInt(const ASTContext ,
Index: include/clang/AST/Expr.h
===
--- include/clang/AST/Expr.h
+++ include/clang/AST/Expr.h
@@ -537,6 +537,13 @@
bool isConstantInitializer(ASTContext , 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.
+ };
+
///
[PATCH] D46241: [CodeGen] Recognize more cases of zero initialization
sepavloff added a comment. @rsmith Do yo think this patch is OK to apply? Repository: rC Clang https://reviews.llvm.org/D46241 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46241: [CodeGen] Recognize more cases of zero initialization
sepavloff added inline comments.
Comment at: lib/CodeGen/CGExprConstant.cpp:1414-1415
+Expr::EvalResult Result;
+if (Init->EvaluateAsRValue(Result, CE.CGM.getContext()) &&
+!Result.hasUnacceptableSideEffect(Expr::SE_NoSideEffects))
+ return (Result.Val.isInt() && Result.Val.getInt().isNullValue()) ||
rsmith wrote:
> Please call `D.evaluateValue()` here rather than inventing your own
> evaluation scheme. That way, we'll cache the evaluated initializer on the
> variable for other uses or reuse the value if we've already evaluated it, and
> you don't need to worry about the corner cases involved in getting the
> evaluation right. (As it happens, you're getting some minor details wrong
> because evaluating an initializer is not quite the same as evaluating an
> rvalue, but in practice it's not a big deal here.)
Call of `D.evaluateValue()` may result in substantial memory and time
consumption if the variable value is huge, like in:
```
int char data_1[2147483648u] = { 0 };
```
The idea of this patch is to recognize some cases of zero initialization prior
to the evaluation of variable initializer. In the example above, value would be
evaluated only for part of the initializer, namely `0`, which does not have an
associated variable, so call of `D.evaluateValue()` is not possible.
Repository:
rC Clang
https://reviews.llvm.org/D46241
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46241: [CodeGen] Recognize more cases of zero initialization
rsmith added inline comments. Comment at: lib/CodeGen/CGExprConstant.cpp:1414-1415 +Expr::EvalResult Result; +if (Init->EvaluateAsRValue(Result, CE.CGM.getContext()) && +!Result.hasUnacceptableSideEffect(Expr::SE_NoSideEffects)) + return (Result.Val.isInt() && Result.Val.getInt().isNullValue()) || Please call `D.evaluateValue()` here rather than inventing your own evaluation scheme. That way, we'll cache the evaluated initializer on the variable for other uses or reuse the value if we've already evaluated it, and you don't need to worry about the corner cases involved in getting the evaluation right. (As it happens, you're getting some minor details wrong because evaluating an initializer is not quite the same as evaluating an rvalue, but in practice it's not a big deal here.) Repository: rC Clang https://reviews.llvm.org/D46241 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46241: [CodeGen] Recognize more cases of zero initialization
rjmccall accepted this revision. rjmccall added a comment. This revision is now accepted and ready to land. Thanks, LGTM. Repository: rC Clang https://reviews.llvm.org/D46241 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46241: [CodeGen] Recognize more cases of zero initialization
sepavloff updated this revision to Diff 146091.
sepavloff added a comment.
Updated patch
Try evaluating initializer value only if the initializer type is
integral or pointer. It avoids calculation of large value, which
then is discarded.
Repository:
rC Clang
https://reviews.llvm.org/D46241
Files:
include/clang/AST/Expr.h
lib/AST/ExprConstant.cpp
lib/CodeGen/CGExprConstant.cpp
test/CodeGen/const-init.c
test/CodeGen/designated-initializers.c
test/CodeGen/union-init2.c
test/CodeGenCXX/cxx11-initializer-aggregate.cpp
test/CodeGenCXX/cxx1z-initializer-aggregate.cpp
test/SemaCXX/large-array-init.cpp
Index: test/SemaCXX/large-array-init.cpp
===
--- test/SemaCXX/large-array-init.cpp
+++ /dev/null
@@ -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[1] = {{ 0 }};
-// CHECK: The number of elements to initialize: 1.
-
-struct S *foo() { return arr; }
Index: test/CodeGenCXX/cxx1z-initializer-aggregate.cpp
===
--- test/CodeGenCXX/cxx1z-initializer-aggregate.cpp
+++ test/CodeGenCXX/cxx1z-initializer-aggregate.cpp
@@ -17,14 +17,14 @@
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
Index: test/CodeGenCXX/cxx11-initializer-aggregate.cpp
===
--- test/CodeGenCXX/cxx11-initializer-aggregate.cpp
+++ test/CodeGenCXX/cxx11-initializer-aggregate.cpp
@@ -51,3 +51,30 @@
// 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
+}
Index: test/CodeGen/union-init2.c
===
--- test/CodeGen/union-init2.c
+++ test/CodeGen/union-init2.c
@@ -5,7 +5,7 @@
union x {long long b;union x* a;} r = {.a = };
-// 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;
Index: test/CodeGen/designated-initializers.c
===
--- test/CodeGen/designated-initializers.c
+++ test/CodeGen/designated-initializers.c
@@ -8,7 +8,7 @@
// 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
Index: test/CodeGen/const-init.c
===
--- test/CodeGen/const-init.c
+++ test/CodeGen/const-init.c
@@ -167,7 +167,7 @@
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()
}
Index: lib/CodeGen/CGExprConstant.cpp
===
--- lib/CodeGen/CGExprConstant.cpp
+++ lib/CodeGen/CGExprConstant.cpp
@@ -1392,20 +1392,40 @@
return type;
}
+/// Checks if the specified initializer is equivalent to zero initialization.
+static bool isZeroInitializer(ConstantEmitter , const Expr *Init) {
+ if (auto *E = dyn_cast_or_null(Init)) {
+CXXConstructorDecl *CD = E->getConstructor();
+return
[PATCH] D46241: [CodeGen] Recognize more cases of zero initialization
rjmccall added inline comments.
Comment at: lib/CodeGen/CGExprConstant.cpp:1413
+ } else if (!Init->isEvaluatable(CE.CGM.getContext())) {
+return false;
+ } else if (InitTy->hasPointerRepresentation()) {
sepavloff wrote:
> rsmith wrote:
> > sepavloff wrote:
> > > rjmccall wrote:
> > > > Aren't the null-pointer and integer-constant-expression checks below
> > > > already checking this? Also, `isEvaluatable` actually computes the
> > > > full value internally (as an `APValue`), so if you're worried about the
> > > > memory and compile-time effects of producing such a value, you really
> > > > shouldn't call it.
> > > >
> > > > You could reasonably move this entire function to be a method on `Expr`
> > > > that takes an `ASTContext`.
> > > Comment for `EvaluateAsRValue` says that it tries calculate expression
> > > agressively. Indeed, for the code:
> > > ```
> > > decltype(nullptr) null();
> > > int *p = null();
> > > ```
> > > compiler ignores potential side effect of `null()` and removes the call,
> > > leaving only zero initialization. `isNullPointerConstant` behaves
> > > similarly.
> > Nonetheless, it looks like this function could evaluate `Init` up to three
> > times, which seems unreasonable. Instead of the checks based on trying to
> > evaluate the initializer (`isNullPointerConstant` +
> > `isIntegerConstantExpr`), how about calling `VarDecl::evaluateValue()`
> > (which will return a potentially pre-computed and cached initializer value)
> > and checking if the result is a zero constant?
> >
> > In fact, `tryEmitPrivateForVarInit` already does most of that for you, and
> > the right place to make this change is probably in
> > `tryEmitPrivateForMemory`, where you can test to see if the `APValue` is
> > zero-initialized and produce a `zeroinitializer` if so. As a side-benefit,
> > putting the change there will mean we'll also start using `zeroinitializer`
> > for zero-initialized subobjects of objects that have non-zero pieces.
> An important point in this patch is that CodeGen tries to find out, if the
> initializer can be replaced with zeroinitializer, *prior* to the evaluation
> of it. For huge arrays the evaluation consumes huge amount of memory and time
> and it must be avoided.
>
> With this patch CodeGen may evaluate parts of the initializer, if it is
> represented by `InitListExpr`. It may cause redundant calculation, for
> instance if the check for zero initialization failed but the initializer is
> constant. To avoid this redundancy we could cache result of evaluation in
> instances of `Expr` and then use the partial values in the evaluation of the
> initializer. The simple use case solved by this patch probably is not a
> sufficient justification for such redesign.
I really think you should have some sort of type restriction in front of this
so that you don't end up creating a huge APValue only to throw it away because
it's not an int or a pointer. It's quite possible to have something like an
array initializer in a nested position that's not within an InitListExpr , e.g.
due to compound literals or std::initializer_list.
Repository:
rC Clang
https://reviews.llvm.org/D46241
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46241: [CodeGen] Recognize more cases of zero initialization
sepavloff added a comment.
> Hmm. In C++, a static object which isn't constant-initialized is
> zero-initialized, which is required to set any padding bits to zero (N4640
> [dcl.init]p6) in both structs and unions. In C, a static object which doesn't
> have an initializer also has all any padding bits set to zero (N1548
> 6.7.9p10). Now, this object has an initializer (that acts as a
> constant-initializer in C++), so those rules don't directly apply; but I
> would argue that the intent of the standards is not that padding bits become
> undefined when you happen to have an initializer. So I actually think the
> zeroinitializer emission is more correct.
Using undefined values instead of zero initialization was implemented in
https://reviews.llvm.org/rL101535. There is no much info about the reason of
the implementation. Clang uses undefined values for padding bits, in particular
in unions, when the first member is not widest. The code:
union C1 {
char sel;
double dval;
};
union C1 val_1 = { 0 };
produces:
@val_1 = dso_local global { i8, [7 x i8] } { i8 0, [7 x i8] undef }, align 8
Another case is unnamed bit fields.
struct C2 {
int : 4;
int x;
};
struct C2 val_2 = { 0 };
produces:
@val_2 = dso_local global { [4 x i8], i32 } { [4 x i8] undef, i32 0 }, align 4
Strictly speaking, this IR does not mean violation of the standard, but it can
modify code generation in some cases. If we decided to use zeroinitializer in
this case, we probably would need to revise using undefined values in
initializers, otherwise similar declarations like:
union C1 val_1a = { 0 };
union C1 val_1b = { 1 };
would produce different IR representations, with and without undefined values.
The test `SemaCXX/large-array-init.cpp` is removed in this change. This test
was added in https://reviews.llvm.org/rL325120 to solve
https://bugs.llvm.org/show_bug.cgi?id=18978, which describes the same problem,
as solved by this patch. This patch presents more efficient solution, with it
the tests compiles 50 time faster. If r325120 does not solve additional
problems, it can be reverted.
Repository:
rC Clang
https://reviews.llvm.org/D46241
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46241: [CodeGen] Recognize more cases of zero initialization
sepavloff updated this revision to Diff 145735.
sepavloff added a comment.
Added treatment of structures/unions
Repository:
rC Clang
https://reviews.llvm.org/D46241
Files:
include/clang/AST/Expr.h
lib/AST/ExprConstant.cpp
lib/CodeGen/CGExprConstant.cpp
test/CodeGen/const-init.c
test/CodeGen/designated-initializers.c
test/CodeGen/union-init2.c
test/CodeGenCXX/cxx11-initializer-aggregate.cpp
test/CodeGenCXX/cxx1z-initializer-aggregate.cpp
test/SemaCXX/large-array-init.cpp
Index: test/SemaCXX/large-array-init.cpp
===
--- test/SemaCXX/large-array-init.cpp
+++ /dev/null
@@ -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[1] = {{ 0 }};
-// CHECK: The number of elements to initialize: 1.
-
-struct S *foo() { return arr; }
Index: test/CodeGenCXX/cxx1z-initializer-aggregate.cpp
===
--- test/CodeGenCXX/cxx1z-initializer-aggregate.cpp
+++ test/CodeGenCXX/cxx1z-initializer-aggregate.cpp
@@ -17,14 +17,14 @@
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
Index: test/CodeGenCXX/cxx11-initializer-aggregate.cpp
===
--- test/CodeGenCXX/cxx11-initializer-aggregate.cpp
+++ test/CodeGenCXX/cxx11-initializer-aggregate.cpp
@@ -51,3 +51,30 @@
// 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
+}
Index: test/CodeGen/union-init2.c
===
--- test/CodeGen/union-init2.c
+++ test/CodeGen/union-init2.c
@@ -5,7 +5,7 @@
union x {long long b;union x* a;} r = {.a = };
-// 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;
Index: test/CodeGen/designated-initializers.c
===
--- test/CodeGen/designated-initializers.c
+++ test/CodeGen/designated-initializers.c
@@ -8,7 +8,7 @@
// 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
Index: test/CodeGen/const-init.c
===
--- test/CodeGen/const-init.c
+++ test/CodeGen/const-init.c
@@ -167,7 +167,7 @@
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()
}
Index: lib/CodeGen/CGExprConstant.cpp
===
--- lib/CodeGen/CGExprConstant.cpp
+++ lib/CodeGen/CGExprConstant.cpp
@@ -1392,20 +1392,37 @@
return type;
}
+/// Checks if the specified initializer is equivalent to zero initialization.
+static bool isZeroInitializer(ConstantEmitter , const Expr *Init) {
+ if (auto *E = dyn_cast_or_null(Init)) {
+CXXConstructorDecl *CD = E->getConstructor();
+return CD->isDefaultConstructor() && CD->isTrivial();
+ }
+
+ if (auto *IL = dyn_cast_or_null(Init)) {
+for (auto I : IL->inits())
+ if
[PATCH] D46241: [CodeGen] Recognize more cases of zero initialization
rjmccall added inline comments.
Comment at: lib/CodeGen/CGExprConstant.cpp:1403
+ if (auto *IL = dyn_cast_or_null(Init)) {
+if (InitTy->isConstantArrayType()) {
+ for (auto I : IL->inits())
sepavloff wrote:
> rjmccall wrote:
> > Do you actually care if it's an array initialization instead of a
> > struct/enum initialization?
> If this code is enabled for for records too, some tests start to fail. For
> instance, the code:
> ```
> union { int i; double f; } u2 = { };
> ```
> produces output:
> ```
> %union.anon = type { double }
> @u2 = global %union.anon zeroinitializer, align 4
> ```
> while previously it produced:
> ```
> @u2 = global { i32, [4 x i8] } { i32 0, [4 x i8] undef }, align 4
> ```
> The latter looks more correct.
Hmm. In C++, a static object which isn't constant-initialized is
zero-initialized, which is required to set any padding bits to zero (N4640
[dcl.init]p6) in both structs and unions. In C, a static object which doesn't
have an initializer also has all any padding bits set to zero (N1548 6.7.9p10).
Now, this object has an initializer (that acts as a constant-initializer in
C++), so those rules don't directly apply; but I would argue that the intent of
the standards is not that padding bits become undefined when you happen to have
an initializer. So I actually think the `zeroinitializer` emission is more
correct.
Repository:
rC Clang
https://reviews.llvm.org/D46241
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46241: [CodeGen] Recognize more cases of zero initialization
sepavloff added inline comments.
Comment at: lib/CodeGen/CGExprConstant.cpp:1413
+ } else if (!Init->isEvaluatable(CE.CGM.getContext())) {
+return false;
+ } else if (InitTy->hasPointerRepresentation()) {
rsmith wrote:
> sepavloff wrote:
> > rjmccall wrote:
> > > Aren't the null-pointer and integer-constant-expression checks below
> > > already checking this? Also, `isEvaluatable` actually computes the full
> > > value internally (as an `APValue`), so if you're worried about the memory
> > > and compile-time effects of producing such a value, you really shouldn't
> > > call it.
> > >
> > > You could reasonably move this entire function to be a method on `Expr`
> > > that takes an `ASTContext`.
> > Comment for `EvaluateAsRValue` says that it tries calculate expression
> > agressively. Indeed, for the code:
> > ```
> > decltype(nullptr) null();
> > int *p = null();
> > ```
> > compiler ignores potential side effect of `null()` and removes the call,
> > leaving only zero initialization. `isNullPointerConstant` behaves similarly.
> Nonetheless, it looks like this function could evaluate `Init` up to three
> times, which seems unreasonable. Instead of the checks based on trying to
> evaluate the initializer (`isNullPointerConstant` + `isIntegerConstantExpr`),
> how about calling `VarDecl::evaluateValue()` (which will return a potentially
> pre-computed and cached initializer value) and checking if the result is a
> zero constant?
>
> In fact, `tryEmitPrivateForVarInit` already does most of that for you, and
> the right place to make this change is probably in `tryEmitPrivateForMemory`,
> where you can test to see if the `APValue` is zero-initialized and produce a
> `zeroinitializer` if so. As a side-benefit, putting the change there will
> mean we'll also start using `zeroinitializer` for zero-initialized subobjects
> of objects that have non-zero pieces.
An important point in this patch is that CodeGen tries to find out, if the
initializer can be replaced with zeroinitializer, *prior* to the evaluation of
it. For huge arrays the evaluation consumes huge amount of memory and time and
it must be avoided.
With this patch CodeGen may evaluate parts of the initializer, if it is
represented by `InitListExpr`. It may cause redundant calculation, for instance
if the check for zero initialization failed but the initializer is constant. To
avoid this redundancy we could cache result of evaluation in instances of
`Expr` and then use the partial values in the evaluation of the initializer.
The simple use case solved by this patch probably is not a sufficient
justification for such redesign.
Repository:
rC Clang
https://reviews.llvm.org/D46241
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46241: [CodeGen] Recognize more cases of zero initialization
sepavloff updated this revision to Diff 145445.
sepavloff marked an inline comment as done.
sepavloff added a comment.
Avoid redundant initializer calculation
Repository:
rC Clang
https://reviews.llvm.org/D46241
Files:
include/clang/AST/Expr.h
lib/AST/ExprConstant.cpp
lib/CodeGen/CGExprConstant.cpp
test/CodeGenCXX/cxx11-initializer-aggregate.cpp
Index: test/CodeGenCXX/cxx11-initializer-aggregate.cpp
===
--- test/CodeGenCXX/cxx11-initializer-aggregate.cpp
+++ test/CodeGenCXX/cxx11-initializer-aggregate.cpp
@@ -51,3 +51,27 @@
// meaningful.
B b[30] = {};
}
+
+namespace ZeroInit {
+ enum { Zero, One };
+ constexpr int zero() { return 0; }
+ constexpr int *null() { return nullptr; }
+ struct Filler {
+int x;
+Filler();
+ };
+
+ // 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() };
+
+
+ // This variable must be initialized elementwise.
+ Filler data_e1[1024] = {};
+ // CHECK: getelementptr inbounds {{.*}} @_ZN8ZeroInit7data_e1E
+}
Index: lib/CodeGen/CGExprConstant.cpp
===
--- lib/CodeGen/CGExprConstant.cpp
+++ lib/CodeGen/CGExprConstant.cpp
@@ -1392,20 +1392,40 @@
return type;
}
+/// Checks if the specified initializer is equivalent to zero initialization.
+static bool isZeroInitializer(ConstantEmitter , const Expr *Init) {
+ QualType InitTy = Init->getType().getCanonicalType();
+ if (auto *E = dyn_cast_or_null(Init)) {
+CXXConstructorDecl *CD = E->getConstructor();
+return CD->isDefaultConstructor() && CD->isTrivial();
+ }
+ if (auto *IL = dyn_cast_or_null(Init)) {
+if (InitTy->isConstantArrayType()) {
+ for (auto I : IL->inits())
+if (!isZeroInitializer(CE, I))
+ return false;
+ if (const Expr *Filler = IL->getArrayFiller()) {
+return isZeroInitializer(CE, Filler);
+ }
+ return true;
+}
+ } else {
+Expr::EvalResult Result;
+if (Init->EvaluateAsRValue(Result, CE.CGM.getContext()) &&
+!Result.hasUnacceptableSideEffect(Expr::SE_NoSideEffects))
+ return (Result.Val.isInt() && Result.Val.getInt().isNullValue()) ||
+ (Result.Val.isLValue() && Result.Val.isNullPointer());
+ }
+
+ return false;
+}
+
llvm::Constant *ConstantEmitter::tryEmitPrivateForVarInit(const VarDecl ) {
// 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(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();
Index: lib/AST/ExprConstant.cpp
===
--- lib/AST/ExprConstant.cpp
+++ lib/AST/ExprConstant.cpp
@@ -10224,20 +10224,14 @@
HandleConversionToBool(Scratch.Val, Result);
}
-static bool hasUnacceptableSideEffect(Expr::EvalStatus ,
- Expr::SideEffectsKind SEK) {
- return (SEK < Expr::SE_AllowSideEffects && Result.HasSideEffects) ||
- (SEK < Expr::SE_AllowUndefinedBehavior && Result.HasUndefinedBehavior);
-}
-
bool Expr::EvaluateAsInt(APSInt , const ASTContext ,
SideEffectsKind AllowSideEffects) const {
if (!getType()->isIntegralOrEnumerationType())
return false;
EvalResult ExprResult;
if (!EvaluateAsRValue(ExprResult, Ctx) || !ExprResult.Val.isInt() ||
- hasUnacceptableSideEffect(ExprResult, AllowSideEffects))
+ ExprResult.hasUnacceptableSideEffect(AllowSideEffects))
return false;
Result = ExprResult.Val.getInt();
@@ -10251,7 +10245,7 @@
EvalResult ExprResult;
if (!EvaluateAsRValue(ExprResult, Ctx) || !ExprResult.Val.isFloat() ||
- hasUnacceptableSideEffect(ExprResult, AllowSideEffects))
+ ExprResult.hasUnacceptableSideEffect(AllowSideEffects))
return false;
Result = ExprResult.Val.getFloat();
@@ -10317,7 +10311,7 @@
bool Expr::isEvaluatable(const ASTContext , SideEffectsKind SEK) const {
EvalResult Result;
return EvaluateAsRValue(Result, Ctx) &&
-
[PATCH] D46241: [CodeGen] Recognize more cases of zero initialization
rsmith added inline comments.
Comment at: lib/CodeGen/CGExprConstant.cpp:1413
+ } else if (!Init->isEvaluatable(CE.CGM.getContext())) {
+return false;
+ } else if (InitTy->hasPointerRepresentation()) {
sepavloff wrote:
> rjmccall wrote:
> > Aren't the null-pointer and integer-constant-expression checks below
> > already checking this? Also, `isEvaluatable` actually computes the full
> > value internally (as an `APValue`), so if you're worried about the memory
> > and compile-time effects of producing such a value, you really shouldn't
> > call it.
> >
> > You could reasonably move this entire function to be a method on `Expr`
> > that takes an `ASTContext`.
> Comment for `EvaluateAsRValue` says that it tries calculate expression
> agressively. Indeed, for the code:
> ```
> decltype(nullptr) null();
> int *p = null();
> ```
> compiler ignores potential side effect of `null()` and removes the call,
> leaving only zero initialization. `isNullPointerConstant` behaves similarly.
Nonetheless, it looks like this function could evaluate `Init` up to three
times, which seems unreasonable. Instead of the checks based on trying to
evaluate the initializer (`isNullPointerConstant` + `isIntegerConstantExpr`),
how about calling `VarDecl::evaluateValue()` (which will return a potentially
pre-computed and cached initializer value) and checking if the result is a zero
constant?
In fact, `tryEmitPrivateForVarInit` already does most of that for you, and the
right place to make this change is probably in `tryEmitPrivateForMemory`, where
you can test to see if the `APValue` is zero-initialized and produce a
`zeroinitializer` if so. As a side-benefit, putting the change there will mean
we'll also start using `zeroinitializer` for zero-initialized subobjects of
objects that have non-zero pieces.
Repository:
rC Clang
https://reviews.llvm.org/D46241
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46241: [CodeGen] Recognize more cases of zero initialization
sepavloff marked an inline comment as done.
sepavloff added inline comments.
Comment at: lib/CodeGen/CGExprConstant.cpp:1403
+ if (auto *IL = dyn_cast_or_null(Init)) {
+if (InitTy->isConstantArrayType()) {
+ for (auto I : IL->inits())
rjmccall wrote:
> Do you actually care if it's an array initialization instead of a struct/enum
> initialization?
If this code is enabled for for records too, some tests start to fail. For
instance, the code:
```
union { int i; double f; } u2 = { };
```
produces output:
```
%union.anon = type { double }
@u2 = global %union.anon zeroinitializer, align 4
```
while previously it produced:
```
@u2 = global { i32, [4 x i8] } { i32 0, [4 x i8] undef }, align 4
```
The latter looks more correct.
Comment at: lib/CodeGen/CGExprConstant.cpp:1413
+ } else if (!Init->isEvaluatable(CE.CGM.getContext())) {
+return false;
+ } else if (InitTy->hasPointerRepresentation()) {
rjmccall wrote:
> Aren't the null-pointer and integer-constant-expression checks below already
> checking this? Also, `isEvaluatable` actually computes the full value
> internally (as an `APValue`), so if you're worried about the memory and
> compile-time effects of producing such a value, you really shouldn't call it.
>
> You could reasonably move this entire function to be a method on `Expr` that
> takes an `ASTContext`.
Comment for `EvaluateAsRValue` says that it tries calculate expression
agressively. Indeed, for the code:
```
decltype(nullptr) null();
int *p = null();
```
compiler ignores potential side effect of `null()` and removes the call,
leaving only zero initialization. `isNullPointerConstant` behaves similarly.
Comment at: lib/CodeGen/CGExprConstant.cpp:1417
+if (Init->EvaluateAsRValue(ResVal, CE.CGM.getContext()))
+ return ResVal.Val.isLValue() && ResVal.Val.isNullPointer();
+ } else {
rjmccall wrote:
> There's a `isNullPointerConstant` method (you should use
> `NPC_NeverValueDependent`).
It make code more readable. Thank you!
Repository:
rC Clang
https://reviews.llvm.org/D46241
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46241: [CodeGen] Recognize more cases of zero initialization
sepavloff updated this revision to Diff 145049.
sepavloff marked an inline comment as done.
sepavloff added a comment.
Small simplification
Repository:
rC Clang
https://reviews.llvm.org/D46241
Files:
lib/CodeGen/CGExprConstant.cpp
test/CodeGenCXX/cxx11-initializer-aggregate.cpp
Index: test/CodeGenCXX/cxx11-initializer-aggregate.cpp
===
--- test/CodeGenCXX/cxx11-initializer-aggregate.cpp
+++ test/CodeGenCXX/cxx11-initializer-aggregate.cpp
@@ -51,3 +51,27 @@
// meaningful.
B b[30] = {};
}
+
+namespace ZeroInit {
+ enum { Zero, One };
+ constexpr int zero() { return 0; }
+ constexpr int *null() { return nullptr; }
+ struct Filler {
+int x;
+Filler();
+ };
+
+ // 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() };
+
+
+ // This variable must be initialized elementwise.
+ Filler data_e1[1024] = {};
+ // CHECK: getelementptr inbounds {{.*}} @_ZN8ZeroInit7data_e1E
+}
Index: lib/CodeGen/CGExprConstant.cpp
===
--- lib/CodeGen/CGExprConstant.cpp
+++ lib/CodeGen/CGExprConstant.cpp
@@ -1392,20 +1392,43 @@
return type;
}
+/// Checks if the specified initializer is equivalent to zero initialization.
+static bool isZeroInitializer(ConstantEmitter , const Expr *Init) {
+ QualType InitTy = Init->getType().getCanonicalType();
+ if (auto *E = dyn_cast_or_null(Init)) {
+CXXConstructorDecl *CD = E->getConstructor();
+return CD->isDefaultConstructor() && CD->isTrivial();
+ }
+ if (auto *IL = dyn_cast_or_null(Init)) {
+if (InitTy->isConstantArrayType()) {
+ for (auto I : IL->inits())
+if (!isZeroInitializer(CE, I))
+ return false;
+ if (const Expr *Filler = IL->getArrayFiller()) {
+return isZeroInitializer(CE, Filler);
+ }
+ return true;
+}
+ } else if (!Init->isEvaluatable(CE.CGM.getContext())) {
+return false;
+ } else if (Init->isNullPointerConstant(CE.CGM.getContext(),
+ Expr::NPC_NeverValueDependent)) {
+return true;
+ } else {
+llvm::APSInt Value;
+if (Init->isIntegerConstantExpr(Value, CE.CGM.getContext()))
+ return Value.isNullValue();
+ }
+
+ return false;
+}
+
llvm::Constant *ConstantEmitter::tryEmitPrivateForVarInit(const VarDecl ) {
// 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(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();
Index: test/CodeGenCXX/cxx11-initializer-aggregate.cpp
===
--- test/CodeGenCXX/cxx11-initializer-aggregate.cpp
+++ test/CodeGenCXX/cxx11-initializer-aggregate.cpp
@@ -51,3 +51,27 @@
// meaningful.
B b[30] = {};
}
+
+namespace ZeroInit {
+ enum { Zero, One };
+ constexpr int zero() { return 0; }
+ constexpr int *null() { return nullptr; }
+ struct Filler {
+int x;
+Filler();
+ };
+
+ // 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() };
+
+
+ // This variable must be initialized elementwise.
+ Filler data_e1[1024] = {};
+ // CHECK: getelementptr inbounds {{.*}} @_ZN8ZeroInit7data_e1E
+}
Index: lib/CodeGen/CGExprConstant.cpp
===
--- lib/CodeGen/CGExprConstant.cpp
+++ lib/CodeGen/CGExprConstant.cpp
@@ -1392,20 +1392,43 @@
return type;
}
+/// Checks if the specified initializer is equivalent to zero initialization.
+static bool isZeroInitializer(ConstantEmitter , const Expr *Init) {
+ QualType InitTy = Init->getType().getCanonicalType();
+ if (auto *E =
[PATCH] D46241: [CodeGen] Recognize more cases of zero initialization
rjmccall added inline comments.
Comment at: lib/CodeGen/CGExprConstant.cpp:1403
+ if (auto *IL = dyn_cast_or_null(Init)) {
+if (InitTy->isConstantArrayType()) {
+ for (auto I : IL->inits())
Do you actually care if it's an array initialization instead of a struct/enum
initialization?
Comment at: lib/CodeGen/CGExprConstant.cpp:1413
+ } else if (!Init->isEvaluatable(CE.CGM.getContext())) {
+return false;
+ } else if (InitTy->hasPointerRepresentation()) {
Aren't the null-pointer and integer-constant-expression checks below already
checking this? Also, `isEvaluatable` actually computes the full value
internally (as an `APValue`), so if you're worried about the memory and
compile-time effects of producing such a value, you really shouldn't call it.
You could reasonably move this entire function to be a method on `Expr` that
takes an `ASTContext`.
Comment at: lib/CodeGen/CGExprConstant.cpp:1417
+if (Init->EvaluateAsRValue(ResVal, CE.CGM.getContext()))
+ return ResVal.Val.isLValue() && ResVal.Val.isNullPointer();
+ } else {
There's a `isNullPointerConstant` method (you should use
`NPC_NeverValueDependent`).
Repository:
rC Clang
https://reviews.llvm.org/D46241
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46241: [CodeGen] Recognize more cases of zero initialization
sepavloff marked 3 inline comments as done.
sepavloff added inline comments.
Comment at: lib/CodeGen/CGExprConstant.cpp:1414
+ return true;
+}
+ }
rjmccall wrote:
> You should check the array fill expression here; for now, the test case would
> be something like `StructWithNonTrivialDefaultInit sArr[10] = {};`.
Indeed, this is the missing case. Thank you!
Repository:
rC Clang
https://reviews.llvm.org/D46241
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46241: [CodeGen] Recognize more cases of zero initialization
sepavloff updated this revision to Diff 144898.
sepavloff added a comment.
Addressed review notes.
Repository:
rC Clang
https://reviews.llvm.org/D46241
Files:
lib/CodeGen/CGExprConstant.cpp
test/CodeGenCXX/cxx11-initializer-aggregate.cpp
Index: test/CodeGenCXX/cxx11-initializer-aggregate.cpp
===
--- test/CodeGenCXX/cxx11-initializer-aggregate.cpp
+++ test/CodeGenCXX/cxx11-initializer-aggregate.cpp
@@ -51,3 +51,27 @@
// meaningful.
B b[30] = {};
}
+
+namespace ZeroInit {
+ enum { Zero, One };
+ constexpr int zero() { return 0; }
+ constexpr int *null() { return nullptr; }
+ struct Filler {
+int x;
+Filler();
+ };
+
+ // 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() };
+
+
+ // This variable must be initialized elementwise.
+ Filler data_e1[1024] = {};
+ // CHECK: getelementptr inbounds {{.*}} @_ZN8ZeroInit7data_e1E
+}
Index: lib/CodeGen/CGExprConstant.cpp
===
--- lib/CodeGen/CGExprConstant.cpp
+++ lib/CodeGen/CGExprConstant.cpp
@@ -1392,20 +1392,44 @@
return type;
}
+/// Checks if the specified initializer is equivalent to zero initialization.
+static bool isZeroInitializer(ConstantEmitter , const Expr *Init) {
+ QualType InitTy = Init->getType().getCanonicalType();
+ if (auto *E = dyn_cast_or_null(Init)) {
+CXXConstructorDecl *CD = E->getConstructor();
+return CD->isDefaultConstructor() && CD->isTrivial();
+ }
+ if (auto *IL = dyn_cast_or_null(Init)) {
+if (InitTy->isConstantArrayType()) {
+ for (auto I : IL->inits())
+if (!isZeroInitializer(CE, I))
+ return false;
+ if (const Expr *Filler = IL->getArrayFiller()) {
+return isZeroInitializer(CE, Filler);
+ }
+ return true;
+}
+ } else if (!Init->isEvaluatable(CE.CGM.getContext())) {
+return false;
+ } else if (InitTy->hasPointerRepresentation()) {
+Expr::EvalResult ResVal;
+if (Init->EvaluateAsRValue(ResVal, CE.CGM.getContext()))
+ return ResVal.Val.isLValue() && ResVal.Val.isNullPointer();
+ } else {
+llvm::APSInt Value;
+if (Init->isIntegerConstantExpr(Value, CE.CGM.getContext()))
+ return Value.isNullValue();
+ }
+
+ return false;
+}
+
llvm::Constant *ConstantEmitter::tryEmitPrivateForVarInit(const VarDecl ) {
// 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(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();
Index: test/CodeGenCXX/cxx11-initializer-aggregate.cpp
===
--- test/CodeGenCXX/cxx11-initializer-aggregate.cpp
+++ test/CodeGenCXX/cxx11-initializer-aggregate.cpp
@@ -51,3 +51,27 @@
// meaningful.
B b[30] = {};
}
+
+namespace ZeroInit {
+ enum { Zero, One };
+ constexpr int zero() { return 0; }
+ constexpr int *null() { return nullptr; }
+ struct Filler {
+int x;
+Filler();
+ };
+
+ // 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() };
+
+
+ // This variable must be initialized elementwise.
+ Filler data_e1[1024] = {};
+ // CHECK: getelementptr inbounds {{.*}} @_ZN8ZeroInit7data_e1E
+}
Index: lib/CodeGen/CGExprConstant.cpp
===
--- lib/CodeGen/CGExprConstant.cpp
+++ lib/CodeGen/CGExprConstant.cpp
@@ -1392,20 +1392,44 @@
return type;
}
+/// Checks if the specified initializer is equivalent to zero initialization.
+static bool isZeroInitializer(ConstantEmitter , const Expr *Init) {
+ QualType InitTy = Init->getType().getCanonicalType();
+ if (auto
[PATCH] D46241: [CodeGen] Recognize more cases of zero initialization
rjmccall added inline comments.
Comment at: lib/CodeGen/CGExprConstant.cpp:1395
+static bool isZeroInitializer(ConstantEmitter , const Expr *Init) {
+ QualType InitTy = Init->getType().getCanonicalType();
You should have a comment here clarifying that this is checking whether the
initializer is equivalent to a C++ zero initialization, not checking whether
the initializer produces a zero bit-pattern.
Comment at: lib/CodeGen/CGExprConstant.cpp:1414
+ return true;
+}
+ }
You should check the array fill expression here; for now, the test case would
be something like `StructWithNonTrivialDefaultInit sArr[10] = {};`.
Comment at: lib/CodeGen/CGExprConstant.cpp:1415
+}
+ }
+ return false;
I would suggest doing your primary switch over the form of Init instead of its
type, and you can just have an isIntegerConstantExpr check at the end.
You should also check for a null pointer expression.
Repository:
rC Clang
https://reviews.llvm.org/D46241
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D46241: [CodeGen] Recognize more cases of zero initialization
sepavloff created this revision.
sepavloff added reviewers: rjmccall, rsmith.
If a variable has initializer, codegen tries to build its value. It
causes strange behavior from user viewpoint: compilation of huge zero
initialized arrays like:
int char data_1[2147483648u] = { 0 };
consumes enormous amount of time and memory.
With this change compiler recognizes more patterns when variable is
initialized with zeros and elementwise treatment can be avoided.
Repository:
rC Clang
https://reviews.llvm.org/D46241
Files:
lib/CodeGen/CGExprConstant.cpp
test/CodeGenCXX/cxx11-initializer-aggregate.cpp
Index: test/CodeGenCXX/cxx11-initializer-aggregate.cpp
===
--- test/CodeGenCXX/cxx11-initializer-aggregate.cpp
+++ test/CodeGenCXX/cxx11-initializer-aggregate.cpp
@@ -51,3 +51,15 @@
// meaningful.
B b[30] = {};
}
+
+namespace ZeroInit {
+ enum { Zero, One };
+ constexpr int zero() { return 0; }
+
+ // 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() };
+}
Index: lib/CodeGen/CGExprConstant.cpp
===
--- lib/CodeGen/CGExprConstant.cpp
+++ lib/CodeGen/CGExprConstant.cpp
@@ -1392,20 +1392,36 @@
return type;
}
+static bool isZeroInitializer(ConstantEmitter , const Expr *Init) {
+ QualType InitTy = Init->getType().getCanonicalType();
+ QualType BaseTy = CE.CGM.getContext().getBaseElementType(InitTy);
+ if (BaseTy->isRecordType()) {
+if (auto *E = dyn_cast_or_null(Init)) {
+CXXConstructorDecl *CD = E->getConstructor();
+if (CD->isDefaultConstructor() && CD->isTrivial())
+return true;
+}
+ } else if (InitTy->isIntegerType()) {
+llvm::APSInt Value;
+if (Init->isIntegerConstantExpr(Value, CE.CGM.getContext()))
+ return Value.isNullValue();
+ } else if (InitTy->isConstantArrayType()) {
+if (auto *IL = dyn_cast_or_null(Init)) {
+ for (auto I : IL->inits())
+if (!isZeroInitializer(CE, I))
+ return false;
+ return true;
+}
+ }
+ return false;
+}
+
llvm::Constant *ConstantEmitter::tryEmitPrivateForVarInit(const VarDecl ) {
// 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(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();
Index: test/CodeGenCXX/cxx11-initializer-aggregate.cpp
===
--- test/CodeGenCXX/cxx11-initializer-aggregate.cpp
+++ test/CodeGenCXX/cxx11-initializer-aggregate.cpp
@@ -51,3 +51,15 @@
// meaningful.
B b[30] = {};
}
+
+namespace ZeroInit {
+ enum { Zero, One };
+ constexpr int zero() { return 0; }
+
+ // 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() };
+}
Index: lib/CodeGen/CGExprConstant.cpp
===
--- lib/CodeGen/CGExprConstant.cpp
+++ lib/CodeGen/CGExprConstant.cpp
@@ -1392,20 +1392,36 @@
return type;
}
+static bool isZeroInitializer(ConstantEmitter , const Expr *Init) {
+ QualType InitTy = Init->getType().getCanonicalType();
+ QualType BaseTy = CE.CGM.getContext().getBaseElementType(InitTy);
+ if (BaseTy->isRecordType()) {
+if (auto *E = dyn_cast_or_null(Init)) {
+CXXConstructorDecl *CD = E->getConstructor();
+if (CD->isDefaultConstructor() && CD->isTrivial())
+return true;
+}
+ } else if (InitTy->isIntegerType()) {
+llvm::APSInt Value;
+if (Init->isIntegerConstantExpr(Value, CE.CGM.getContext()))
+ return Value.isNullValue();
+ } else if (InitTy->isConstantArrayType()) {
+if (auto *IL = dyn_cast_or_null(Init)) {
+ for (auto I : IL->inits())
+if (!isZeroInitializer(CE, I))
+ return false;
+ return true;
+}
+ }
+ return false;
+}
+
llvm::Constant
