Skip to content

Commit b54294e

Browse files
[clang][ConstantEmitter] have tryEmitPrivate[ForVarInit] try ConstExprEmitter fast-path first
As suggested by @efriedma in: https://reviews.llvm.org/D76096#4370369 This should speed up evaluating whether an expression is constant or not, but due to the complexity of these two different implementations, we may start getting different answers for edge cases for which we do not yet have test cases in-tree (or perhaps even performance regressions for some cases). As such, contributors have carte blanche to revert if necessary. For additional historical context about ExprConstant vs CGExprConstant, here's snippets from a private conversation on discord: ndesaulniers: why do we have clang/lib/AST/ExprConstant.cpp and clang/lib/CodeGen/CGExprConstant.cpp? Does clang constant fold during ast walking/creation AND during LLVM codegen? efriedma: originally, clang needed to handle two things: integer constant expressions (the "5" in "int x[5];"), and constant global initializers (the "5" in "int x = 5;"). pre-C++11, the two could be handled mostly separately; so we had the code for integer constants in AST/, and the code for globals in CodeGen/. C++11 constexpr sort of destroyed that separation, though. so now we do both kinds of constant evaluation on the AST, then CGExprConstant translates the result of that evaluation to LLVM IR. but we kept around some bits of the old cgexprconstant to avoid performance/memory usage regressions on large arrays. Reviewed By: efriedma Differential Revision: https://reviews.llvm.org/D151587
1 parent 4942978 commit b54294e

File tree

6 files changed

+48
-35
lines changed

6 files changed

+48
-35
lines changed

clang/lib/AST/Expr.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3312,6 +3312,10 @@ bool Expr::isConstantInitializer(ASTContext &Ctx, bool IsForRef,
33123312
// kill the second parameter.
33133313

33143314
if (IsForRef) {
3315+
if (auto *EWC = dyn_cast<ExprWithCleanups>(this))
3316+
return EWC->getSubExpr()->isConstantInitializer(Ctx, true, Culprit);
3317+
if (auto *MTE = dyn_cast<MaterializeTemporaryExpr>(this))
3318+
return MTE->getSubExpr()->isConstantInitializer(Ctx, false, Culprit);
33153319
EvalResult Result;
33163320
if (EvaluateAsLValue(Result, Ctx) && !Result.HasSideEffects)
33173321
return true;

clang/lib/AST/ExprConstant.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8391,15 +8391,17 @@ bool LValueExprEvaluator::VisitMaterializeTemporaryExpr(
83918391
E->getSubExpr()->skipRValueSubobjectAdjustments(CommaLHSs, Adjustments);
83928392

83938393
// If we passed any comma operators, evaluate their LHSs.
8394-
for (unsigned I = 0, N = CommaLHSs.size(); I != N; ++I)
8395-
if (!EvaluateIgnoredValue(Info, CommaLHSs[I]))
8394+
for (const Expr *E : CommaLHSs)
8395+
if (!EvaluateIgnoredValue(Info, E))
83968396
return false;
83978397

83988398
// A materialized temporary with static storage duration can appear within the
83998399
// result of a constant expression evaluation, so we need to preserve its
84008400
// value for use outside this evaluation.
84018401
APValue *Value;
84028402
if (E->getStorageDuration() == SD_Static) {
8403+
if (Info.EvalMode == EvalInfo::EM_ConstantFold)
8404+
return false;
84038405
// FIXME: What about SD_Thread?
84048406
Value = E->getOrCreateValue(true);
84058407
*Value = APValue();
@@ -15475,7 +15477,7 @@ bool Expr::EvaluateAsInitializer(APValue &Value, const ASTContext &Ctx,
1547515477
EStatus.Diag = &Notes;
1547615478

1547715479
EvalInfo Info(Ctx, EStatus,
15478-
(IsConstantInitialization && Ctx.getLangOpts().CPlusPlus11)
15480+
(IsConstantInitialization && Ctx.getLangOpts().CPlusPlus)
1547915481
? EvalInfo::EM_ConstantExpression
1548015482
: EvalInfo::EM_ConstantFold);
1548115483
Info.setEvaluatingDecl(VD, Value);

clang/lib/CodeGen/CGExprConstant.cpp

Lines changed: 23 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1215,11 +1215,6 @@ class ConstExprEmitter :
12151215
return Visit(E->getSubExpr(), T);
12161216
}
12171217

1218-
llvm::Constant *VisitMaterializeTemporaryExpr(MaterializeTemporaryExpr *E,
1219-
QualType T) {
1220-
return Visit(E->getSubExpr(), T);
1221-
}
1222-
12231218
llvm::Constant *EmitArrayInitialization(InitListExpr *ILE, QualType T) {
12241219
auto *CAT = CGM.getContext().getAsConstantArrayType(ILE->getType());
12251220
assert(CAT && "can't emit array init for non-constant-bound array");
@@ -1322,7 +1317,12 @@ class ConstExprEmitter :
13221317
assert(CGM.getContext().hasSameUnqualifiedType(Ty, Arg->getType()) &&
13231318
"argument to copy ctor is of wrong type");
13241319

1325-
return Visit(Arg, Ty);
1320+
// Look through the temporary; it's just converting the value to an
1321+
// lvalue to pass it to the constructor.
1322+
if (auto *MTE = dyn_cast<MaterializeTemporaryExpr>(Arg))
1323+
return Visit(MTE->getSubExpr(), Ty);
1324+
// Don't try to support arbitrary lvalue-to-rvalue conversions for now.
1325+
return nullptr;
13261326
}
13271327

13281328
return CGM.EmitNullConstant(Ty);
@@ -1654,29 +1654,22 @@ llvm::Constant *ConstantEmitter::tryEmitPrivateForVarInit(const VarDecl &D) {
16541654
InConstantContext = D.hasConstantInitialization();
16551655

16561656
QualType destType = D.getType();
1657+
const Expr *E = D.getInit();
1658+
assert(E && "No initializer to emit");
1659+
1660+
if (!destType->isReferenceType()) {
1661+
QualType nonMemoryDestType = getNonMemoryType(CGM, destType);
1662+
if (llvm::Constant *C = ConstExprEmitter(*this).Visit(const_cast<Expr *>(E),
1663+
nonMemoryDestType))
1664+
return emitForMemory(C, destType);
1665+
}
16571666

16581667
// Try to emit the initializer. Note that this can allow some things that
16591668
// are not allowed by tryEmitPrivateForMemory alone.
1660-
if (auto value = D.evaluateValue()) {
1669+
if (APValue *value = D.evaluateValue())
16611670
return tryEmitPrivateForMemory(*value, destType);
1662-
}
1663-
1664-
// FIXME: Implement C++11 [basic.start.init]p2: if the initializer of a
1665-
// reference is a constant expression, and the reference binds to a temporary,
1666-
// then constant initialization is performed. ConstExprEmitter will
1667-
// incorrectly emit a prvalue constant in this case, and the calling code
1668-
// interprets that as the (pointer) value of the reference, rather than the
1669-
// desired value of the referee.
1670-
if (destType->isReferenceType())
1671-
return nullptr;
1672-
1673-
const Expr *E = D.getInit();
1674-
assert(E && "No initializer to emit");
16751671

1676-
auto nonMemoryDestType = getNonMemoryType(CGM, destType);
1677-
auto C =
1678-
ConstExprEmitter(*this).Visit(const_cast<Expr*>(E), nonMemoryDestType);
1679-
return (C ? emitForMemory(C, destType) : nullptr);
1672+
return nullptr;
16801673
}
16811674

16821675
llvm::Constant *
@@ -1743,6 +1736,10 @@ llvm::Constant *ConstantEmitter::tryEmitPrivate(const Expr *E,
17431736
QualType destType) {
17441737
assert(!destType->isVoidType() && "can't emit a void constant");
17451738

1739+
if (llvm::Constant *C =
1740+
ConstExprEmitter(*this).Visit(const_cast<Expr *>(E), destType))
1741+
return C;
1742+
17461743
Expr::EvalResult Result;
17471744

17481745
bool Success = false;
@@ -1752,13 +1749,10 @@ llvm::Constant *ConstantEmitter::tryEmitPrivate(const Expr *E,
17521749
else
17531750
Success = E->EvaluateAsRValue(Result, CGM.getContext(), InConstantContext);
17541751

1755-
llvm::Constant *C;
17561752
if (Success && !Result.HasSideEffects)
1757-
C = tryEmitPrivate(Result.Val, destType);
1758-
else
1759-
C = ConstExprEmitter(*this).Visit(const_cast<Expr*>(E), destType);
1753+
return tryEmitPrivate(Result.Val, destType);
17601754

1761-
return C;
1755+
return nullptr;
17621756
}
17631757

17641758
llvm::Constant *CodeGenModule::getNullPointer(llvm::PointerType *T, QualType QT) {

clang/test/CodeGenCXX/const-init-cxx11.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ namespace BaseClass {
8888

8989
struct E {};
9090
struct Test2 : X<E,0>, X<E,1>, X<E,2>, X<E,3> {};
91-
// CHECK: @_ZN9BaseClass2t2E ={{.*}} constant {{.*}} undef
91+
// CHECK: @_ZN9BaseClass2t2E ={{.*}} constant {{.*}} zeroinitializer, align 1
9292
extern constexpr Test2 t2 = Test2();
9393

9494
struct __attribute((packed)) PackedD { double y = 2; };

clang/test/CodeGenCXX/const-init-cxx1y.cpp

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,8 @@ namespace ModifyStaticTemporary {
3434
// 'c.temporary', not the value as modified by the partial evaluation within
3535
// the initialization of 'c.x'.
3636
A c = { 10, (++c.temporary, b.x) };
37-
// CHECK: @_ZGRN21ModifyStaticTemporary1cE_ = internal global i32 10
3837
// CHECK: @_ZN21ModifyStaticTemporary1cE ={{.*}} global {{.*}} zeroinitializer
38+
// CHECK: @_ZGRN21ModifyStaticTemporary1cE_ = internal global i32 10
3939
}
4040

4141
// CHECK: @_ZGRN28VariableTemplateWithConstRef1iIvEE_ = linkonce_odr constant i32 5, align 4
@@ -76,6 +76,19 @@ namespace VariableTemplateWithPack {
7676
S *p = &s<1, 2, 3, 4>;
7777
}
7878

79+
80+
// CHECK: @_ZGR1z_ ={{.*}} global [2 x i32] [i32 10, i32 2]
81+
// CHECK: @z = global { ptr, i32 } { ptr @_ZGR1z_, i32 10 }
82+
typedef int v[2];
83+
struct Z { int &&x, y; };
84+
Z z = { v{1,2}[0], z.x = 10 };
85+
86+
// CHECK: @_ZGR2z2_ ={{.*}} global %struct.R { i64 10 }
87+
// @z = {{.}} global %struct.Z { ptr @_ZGR1z_, %struct.R { i64 10 } }
88+
struct R { mutable long x; };
89+
struct Z2 { const R &x, y; };
90+
Z2 z2 = { R{1}, z2.x.x = 10 };
91+
7992
// CHECK: __cxa_atexit({{.*}} @_ZN1BD1Ev, {{.*}} @b
8093

8194
// CHECK: define

clang/test/CodeGenOpenCL/amdgpu-nullptr.cl

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ generic char *generic_p_NULL = NULL;
5757
// CHECK: @fold_generic ={{.*}} local_unnamed_addr addrspace(1) global ptr null, align 8
5858
generic int *fold_generic = (global int*)(generic float*)(private char*)0;
5959

60-
// CHECK: @fold_priv ={{.*}} local_unnamed_addr addrspace(1) global ptr addrspace(5) addrspacecast (ptr null to ptr addrspace(5)), align 4
60+
// CHECK: @fold_priv ={{.*}} local_unnamed_addr addrspace(1) global ptr addrspace(5) addrspacecast (ptr addrspace(1) null to ptr addrspace(5)), align 4
6161
private short *fold_priv = (private short*)(generic int*)(global void*)0;
6262

6363
// CHECK: @fold_priv_arith ={{.*}} local_unnamed_addr addrspace(1) global ptr addrspace(5) inttoptr (i32 9 to ptr addrspace(5)), align 4

0 commit comments

Comments
 (0)