[clang] b54294e - [clang][ConstantEmitter] have tryEmitPrivate[ForVarInit] try ConstExprEmitter fast-path first
Nick Desaulniers via cfe-commits
cfe-commits at lists.llvm.org
Mon Jul 24 13:51:03 PDT 2023
Author: Nick Desaulniers
Date: 2023-07-24T13:50:45-07:00
New Revision: b54294e2c9596088aaf557b221bcf471745864bc
URL: https://github.com/llvm/llvm-project/commit/b54294e2c9596088aaf557b221bcf471745864bc
DIFF: https://github.com/llvm/llvm-project/commit/b54294e2c9596088aaf557b221bcf471745864bc.diff
LOG: [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
Added:
Modified:
clang/lib/AST/Expr.cpp
clang/lib/AST/ExprConstant.cpp
clang/lib/CodeGen/CGExprConstant.cpp
clang/test/CodeGenCXX/const-init-cxx11.cpp
clang/test/CodeGenCXX/const-init-cxx1y.cpp
clang/test/CodeGenOpenCL/amdgpu-nullptr.cl
Removed:
################################################################################
diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 23aa14784cdaab..6164a419d213fd 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -3312,6 +3312,10 @@ bool Expr::isConstantInitializer(ASTContext &Ctx, bool IsForRef,
// kill the second parameter.
if (IsForRef) {
+ if (auto *EWC = dyn_cast<ExprWithCleanups>(this))
+ return EWC->getSubExpr()->isConstantInitializer(Ctx, true, Culprit);
+ if (auto *MTE = dyn_cast<MaterializeTemporaryExpr>(this))
+ return MTE->getSubExpr()->isConstantInitializer(Ctx, false, Culprit);
EvalResult Result;
if (EvaluateAsLValue(Result, Ctx) && !Result.HasSideEffects)
return true;
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index b5308ed94e5a04..f1c842e261993d 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -8391,8 +8391,8 @@ bool LValueExprEvaluator::VisitMaterializeTemporaryExpr(
E->getSubExpr()->skipRValueSubobjectAdjustments(CommaLHSs, Adjustments);
// If we passed any comma operators, evaluate their LHSs.
- for (unsigned I = 0, N = CommaLHSs.size(); I != N; ++I)
- if (!EvaluateIgnoredValue(Info, CommaLHSs[I]))
+ for (const Expr *E : CommaLHSs)
+ if (!EvaluateIgnoredValue(Info, E))
return false;
// A materialized temporary with static storage duration can appear within the
@@ -8400,6 +8400,8 @@ bool LValueExprEvaluator::VisitMaterializeTemporaryExpr(
// value for use outside this evaluation.
APValue *Value;
if (E->getStorageDuration() == SD_Static) {
+ if (Info.EvalMode == EvalInfo::EM_ConstantFold)
+ return false;
// FIXME: What about SD_Thread?
Value = E->getOrCreateValue(true);
*Value = APValue();
@@ -15475,7 +15477,7 @@ bool Expr::EvaluateAsInitializer(APValue &Value, const ASTContext &Ctx,
EStatus.Diag = &Notes;
EvalInfo Info(Ctx, EStatus,
- (IsConstantInitialization && Ctx.getLangOpts().CPlusPlus11)
+ (IsConstantInitialization && Ctx.getLangOpts().CPlusPlus)
? EvalInfo::EM_ConstantExpression
: EvalInfo::EM_ConstantFold);
Info.setEvaluatingDecl(VD, Value);
diff --git a/clang/lib/CodeGen/CGExprConstant.cpp b/clang/lib/CodeGen/CGExprConstant.cpp
index 99b2ad855d406c..353ee56839f37e 100644
--- a/clang/lib/CodeGen/CGExprConstant.cpp
+++ b/clang/lib/CodeGen/CGExprConstant.cpp
@@ -1215,11 +1215,6 @@ class ConstExprEmitter :
return Visit(E->getSubExpr(), T);
}
- llvm::Constant *VisitMaterializeTemporaryExpr(MaterializeTemporaryExpr *E,
- QualType T) {
- return Visit(E->getSubExpr(), T);
- }
-
llvm::Constant *EmitArrayInitialization(InitListExpr *ILE, QualType T) {
auto *CAT = CGM.getContext().getAsConstantArrayType(ILE->getType());
assert(CAT && "can't emit array init for non-constant-bound array");
@@ -1322,7 +1317,12 @@ class ConstExprEmitter :
assert(CGM.getContext().hasSameUnqualifiedType(Ty, Arg->getType()) &&
"argument to copy ctor is of wrong type");
- return Visit(Arg, Ty);
+ // Look through the temporary; it's just converting the value to an
+ // lvalue to pass it to the constructor.
+ if (auto *MTE = dyn_cast<MaterializeTemporaryExpr>(Arg))
+ return Visit(MTE->getSubExpr(), Ty);
+ // Don't try to support arbitrary lvalue-to-rvalue conversions for now.
+ return nullptr;
}
return CGM.EmitNullConstant(Ty);
@@ -1654,29 +1654,22 @@ llvm::Constant *ConstantEmitter::tryEmitPrivateForVarInit(const VarDecl &D) {
InConstantContext = D.hasConstantInitialization();
QualType destType = D.getType();
+ const Expr *E = D.getInit();
+ assert(E && "No initializer to emit");
+
+ if (!destType->isReferenceType()) {
+ QualType nonMemoryDestType = getNonMemoryType(CGM, destType);
+ if (llvm::Constant *C = ConstExprEmitter(*this).Visit(const_cast<Expr *>(E),
+ nonMemoryDestType))
+ return emitForMemory(C, destType);
+ }
// Try to emit the initializer. Note that this can allow some things that
// are not allowed by tryEmitPrivateForMemory alone.
- if (auto value = D.evaluateValue()) {
+ if (APValue *value = D.evaluateValue())
return tryEmitPrivateForMemory(*value, destType);
- }
-
- // FIXME: Implement C++11 [basic.start.init]p2: if the initializer of a
- // reference is a constant expression, and the reference binds to a temporary,
- // then constant initialization is performed. ConstExprEmitter will
- // incorrectly emit a prvalue constant in this case, and the calling code
- // interprets that as the (pointer) value of the reference, rather than the
- // desired value of the referee.
- if (destType->isReferenceType())
- return nullptr;
-
- const Expr *E = D.getInit();
- assert(E && "No initializer to emit");
- auto nonMemoryDestType = getNonMemoryType(CGM, destType);
- auto C =
- ConstExprEmitter(*this).Visit(const_cast<Expr*>(E), nonMemoryDestType);
- return (C ? emitForMemory(C, destType) : nullptr);
+ return nullptr;
}
llvm::Constant *
@@ -1743,6 +1736,10 @@ llvm::Constant *ConstantEmitter::tryEmitPrivate(const Expr *E,
QualType destType) {
assert(!destType->isVoidType() && "can't emit a void constant");
+ if (llvm::Constant *C =
+ ConstExprEmitter(*this).Visit(const_cast<Expr *>(E), destType))
+ return C;
+
Expr::EvalResult Result;
bool Success = false;
@@ -1752,13 +1749,10 @@ llvm::Constant *ConstantEmitter::tryEmitPrivate(const Expr *E,
else
Success = E->EvaluateAsRValue(Result, CGM.getContext(), InConstantContext);
- llvm::Constant *C;
if (Success && !Result.HasSideEffects)
- C = tryEmitPrivate(Result.Val, destType);
- else
- C = ConstExprEmitter(*this).Visit(const_cast<Expr*>(E), destType);
+ return tryEmitPrivate(Result.Val, destType);
- return C;
+ return nullptr;
}
llvm::Constant *CodeGenModule::getNullPointer(llvm::PointerType *T, QualType QT) {
diff --git a/clang/test/CodeGenCXX/const-init-cxx11.cpp b/clang/test/CodeGenCXX/const-init-cxx11.cpp
index 99d431c3169603..d22d78d2b94edb 100644
--- a/clang/test/CodeGenCXX/const-init-cxx11.cpp
+++ b/clang/test/CodeGenCXX/const-init-cxx11.cpp
@@ -88,7 +88,7 @@ namespace BaseClass {
struct E {};
struct Test2 : X<E,0>, X<E,1>, X<E,2>, X<E,3> {};
- // CHECK: @_ZN9BaseClass2t2E ={{.*}} constant {{.*}} undef
+ // CHECK: @_ZN9BaseClass2t2E ={{.*}} constant {{.*}} zeroinitializer, align 1
extern constexpr Test2 t2 = Test2();
struct __attribute((packed)) PackedD { double y = 2; };
diff --git a/clang/test/CodeGenCXX/const-init-cxx1y.cpp b/clang/test/CodeGenCXX/const-init-cxx1y.cpp
index 5d98b43ca7b395..4ca0a277993e7a 100644
--- a/clang/test/CodeGenCXX/const-init-cxx1y.cpp
+++ b/clang/test/CodeGenCXX/const-init-cxx1y.cpp
@@ -34,8 +34,8 @@ namespace ModifyStaticTemporary {
// 'c.temporary', not the value as modified by the partial evaluation within
// the initialization of 'c.x'.
A c = { 10, (++c.temporary, b.x) };
- // CHECK: @_ZGRN21ModifyStaticTemporary1cE_ = internal global i32 10
// CHECK: @_ZN21ModifyStaticTemporary1cE ={{.*}} global {{.*}} zeroinitializer
+ // CHECK: @_ZGRN21ModifyStaticTemporary1cE_ = internal global i32 10
}
// CHECK: @_ZGRN28VariableTemplateWithConstRef1iIvEE_ = linkonce_odr constant i32 5, align 4
@@ -76,6 +76,19 @@ namespace VariableTemplateWithPack {
S *p = &s<1, 2, 3, 4>;
}
+
+// CHECK: @_ZGR1z_ ={{.*}} global [2 x i32] [i32 10, i32 2]
+// CHECK: @z = global { ptr, i32 } { ptr @_ZGR1z_, i32 10 }
+typedef int v[2];
+struct Z { int &&x, y; };
+Z z = { v{1,2}[0], z.x = 10 };
+
+// CHECK: @_ZGR2z2_ ={{.*}} global %struct.R { i64 10 }
+// @z = {{.}} global %struct.Z { ptr @_ZGR1z_, %struct.R { i64 10 } }
+struct R { mutable long x; };
+struct Z2 { const R &x, y; };
+Z2 z2 = { R{1}, z2.x.x = 10 };
+
// CHECK: __cxa_atexit({{.*}} @_ZN1BD1Ev, {{.*}} @b
// CHECK: define
diff --git a/clang/test/CodeGenOpenCL/amdgpu-nullptr.cl b/clang/test/CodeGenOpenCL/amdgpu-nullptr.cl
index 5d50e37c6b11e9..cb3ca514c584fb 100644
--- a/clang/test/CodeGenOpenCL/amdgpu-nullptr.cl
+++ b/clang/test/CodeGenOpenCL/amdgpu-nullptr.cl
@@ -57,7 +57,7 @@ generic char *generic_p_NULL = NULL;
// CHECK: @fold_generic ={{.*}} local_unnamed_addr addrspace(1) global ptr null, align 8
generic int *fold_generic = (global int*)(generic float*)(private char*)0;
-// CHECK: @fold_priv ={{.*}} local_unnamed_addr addrspace(1) global ptr addrspace(5) addrspacecast (ptr null to ptr addrspace(5)), align 4
+// CHECK: @fold_priv ={{.*}} local_unnamed_addr addrspace(1) global ptr addrspace(5) addrspacecast (ptr addrspace(1) null to ptr addrspace(5)), align 4
private short *fold_priv = (private short*)(generic int*)(global void*)0;
// CHECK: @fold_priv_arith ={{.*}} local_unnamed_addr addrspace(1) global ptr addrspace(5) inttoptr (i32 9 to ptr addrspace(5)), align 4
More information about the cfe-commits
mailing list