[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

6 files changed, 48 insertions, 35 deletions

diff --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 23aa147..6164a41 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 b5308ed..f1c842e 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 99b2ad8..353ee56 100644
--- a/clang/lib/CodeGen/CGExprConstant.cpp
+++ b/clang/lib/CodeGen/CGExprConstant.cpp
@@ -1215,11 +1215,6 @@ public:
     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 @@ public:
       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 99d431c..d22d78d 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 5d98b43..4ca0a27 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 5d50e37..cb3ca51 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