[CodeGen] Fix assertion on referencing constexpr Obj-C object with ARC.

[vsapsai]

Summary

Failed assertion is

Assertion failed: ((ND->isUsed(false) || !isa<VarDecl>(ND) || !E->getLocation().isValid()) && "Should not use decl without marking it used!"), function EmitDeclRefLValue, file llvm-project/clang/lib/CodeGen/CGExpr.cpp, line 2437.

EmitDeclRefLValue mentions

A DeclRefExpr for a reference initialized by a constant expression can
appear without being odr-used. Directly emit the constant initializer.

The fix is in using the similar approach for non-references of
non-odr-used variables. EmitScalarExpr will try to emit constant if
possible and we can use resulting llvm::Value * without performing
EmitLValue.

rdar://problem/40650504

[rjmccall]

clang/lib/CodeGen/CGObjC.cpp

2459

This switch is just checking what you already computed as SuppressResultRetain. Please just assert in the second case that the qualifier is OCL_Weak.

Also, please stay consistent with the surrounding capitalization of local variables.

2520

Can we test constant-evaluability directly instead of only applying this when the declaration isn't otherwise used?

[vsapsai]

clang/lib/CodeGen/CGObjC.cpp

2459

Not entirely sure I understand the comment about switch correctly. Will change the code according to my understanding.

2520

We can use IsVariableAConstantExpression but it requires removing const from Decl.

Another way to test constant-evaluability is tryEmitAsConstant. It still requires removing const qualifier but overall it is a good approach. It captures the intention well and implementation-wise it is what CGF.EmitScalarExpr is doing anyway. I'll try it and will show how it looks like.

[vsapsai]

• Address review comments.

[vsapsai]

clang/lib/CodeGen/CGObjC.cpp

2520

If we go with tryEmitAsConstant, there is a separate review D53725 to use EmitConstant in more places.

[rjmccall]

clang/lib/CodeGen/CGObjC.cpp

2459

That's exactly what I was asking for, thank you.

[vsapsai]

• Rename EmitConstant to emitScalarConstant.
• Tweak comment to be explicitly about intended IR code, not about Obj-C++ code.

[vsapsai]

Exclude commits tracked in a different review.

[rjmccall]

LGTM.

[vsapsai]

Thanks for the review, John. I'll update the comment with TODO and commit.

clang/lib/CodeGen/CGExpr.cpp

 
 CodeGenFunction::ConstantEmission
 CodeGenFunction::tryEmitAsConstant(const MemberExpr *ME) {
   if (DeclRefExpr *DRE = tryToConvertMemberExprToDeclRefExpr(*this, ME))
     return tryEmitAsConstant(DRE);
   return ConstantEmission();
 }
 
+llvm::Value *CodeGenFunction::emitScalarConstant(
+    const CodeGenFunction::ConstantEmission &Constant, Expr *E) {
+  assert(Constant && "not a constant");
+  if (Constant.isReference())
+    return EmitLoadOfLValue(Constant.getReferenceLValue(*this, E),
+                            E->getExprLoc())
+        .getScalarVal();
+  return Constant.getValue();
+}
+
 llvm::Value *CodeGenFunction::EmitLoadOfScalar(LValue lvalue,
                                                SourceLocation Loc) {
   return EmitLoadOfScalar(lvalue.getAddress(), lvalue.isVolatile(),
                           lvalue.getType(), Loc, lvalue.getBaseInfo(),
                           lvalue.getTBAAInfo(), lvalue.isNontemporal());
 }
 
 static bool hasBooleanRepresentation(QualType Ty) {

clang/lib/CodeGen/CGExprScalar.cpp

Value *VisitOpaqueValueExpr(OpaqueValueExpr *E) {
     if (E->isGLValue())
       return EmitLoadOfLValue(CGF.getOrCreateOpaqueLValueMapping(E),
                               E->getExprLoc());
 
     // Otherwise, assume the mapping is the scalar directly.
     return CGF.getOrCreateOpaqueRValueMapping(E).getScalarVal();
   }
 
-  Value *emitConstant(const CodeGenFunction::ConstantEmission &Constant,
-                      Expr *E) {
-    assert(Constant && "not a constant");
-    if (Constant.isReference())
-      return EmitLoadOfLValue(Constant.getReferenceLValue(CGF, E),
-                              E->getExprLoc());
-    return Constant.getValue();
-  }
-
   // l-values.
   Value *VisitDeclRefExpr(DeclRefExpr *E) {
     if (CodeGenFunction::ConstantEmission Constant = CGF.tryEmitAsConstant(E))
-      return emitConstant(Constant, E);
+      return CGF.emitScalarConstant(Constant, E);
     return EmitLoadOfLValue(E);
   }
 
   Value *VisitObjCSelectorExpr(ObjCSelectorExpr *E) {
     return CGF.EmitObjCSelectorExpr(E);
   }
   Value *VisitObjCProtocolExpr(ObjCProtocolExpr *E) {
     return CGF.EmitObjCProtocolExpr(E);
Value *ScalarExprEmitter::VisitConvertVectorExpr(ConvertVectorExpr *E) {
   }
 
   return Res;
 }
 
 Value *ScalarExprEmitter::VisitMemberExpr(MemberExpr *E) {
   if (CodeGenFunction::ConstantEmission Constant = CGF.tryEmitAsConstant(E)) {
     CGF.EmitIgnoredExpr(E->getBase());
-    return emitConstant(Constant, E);
+    return CGF.emitScalarConstant(Constant, E);
   } else {
     llvm::APSInt Value;
     if (E->EvaluateAsInt(Value, CGF.getContext(), Expr::SE_AllowSideEffects)) {
       CGF.EmitIgnoredExpr(E->getBase());
       return Builder.getInt(Value);
     }
   }
 

clang/lib/CodeGen/CGObjC.cpp

 
 void CodeGenFunction::EmitObjCAutoreleasePoolCleanup(llvm::Value *Ptr) {
   if (CGM.getLangOpts().ObjCAutoRefCount)
     EHStack.pushCleanup<CallObjCAutoreleasePoolObject>(NormalCleanup, Ptr);
   else
     EHStack.pushCleanup<CallObjCMRRAutoreleasePoolObject>(NormalCleanup, Ptr);
 }
 
-static TryEmitResult tryEmitARCRetainLoadOfScalar(CodeGenFunction &CGF,
-                                                  LValue lvalue,
-                                                  QualType type) {
-  switch (type.getObjCLifetime()) {
+static bool shouldRetainObjCLifetime(Qualifiers::ObjCLifetime lifetime) {
+  switch (lifetime) {
   case Qualifiers::OCL_None:
   case Qualifiers::OCL_ExplicitNone:
   case Qualifiers::OCL_Strong:
   case Qualifiers::OCL_Autoreleasing:
-    return TryEmitResult(CGF.EmitLoadOfLValue(lvalue,
-                                              SourceLocation()).getScalarVal(),
-                         false);
+    return true;
 
   case Qualifiers::OCL_Weak:
-    return TryEmitResult(CGF.EmitARCLoadWeakRetained(lvalue.getAddress()),
-                         true);
+    return false;
   }
 
   llvm_unreachable("impossible lifetime!");
 }
 
 static TryEmitResult tryEmitARCRetainLoadOfScalar(CodeGenFunction &CGF,
+                                                  LValue lvalue,
+                                                  QualType type) {
+  llvm::Value *result;
+  bool shouldRetain = shouldRetainObjCLifetime(type.getObjCLifetime());
+  if (shouldRetain) {
+    result = CGF.EmitLoadOfLValue(lvalue, SourceLocation()).getScalarVal();
+  } else {
+    assert(type.getObjCLifetime() == Qualifiers::OCL_Weak);
+    result = CGF.EmitARCLoadWeakRetained(lvalue.getAddress());
+  }
+  return TryEmitResult(result, !shouldRetain);
+}
+
+static TryEmitResult tryEmitARCRetainLoadOfScalar(CodeGenFunction &CGF,
                                                   const Expr *e) {
   e = e->IgnoreParens();
   QualType type = e->getType();
 
   // If we're loading retained from a __strong xvalue, we can avoid
   // an extra retain/release pair by zeroing out the source of this
   // "move" operation.
   if (e->isXValue() &&
static TryEmitResult tryEmitARCRetainLoadOfScalar(CodeGenFunction &CGF,
   // result of the call to objc_storeWeak instead of reloading.
   if (CGF.getLangOpts().CPlusPlus &&
       !type.isVolatileQualified() &&
       type.getObjCLifetime() == Qualifiers::OCL_Weak &&
       isa<BinaryOperator>(e) &&
       cast<BinaryOperator>(e)->getOpcode() == BO_Assign)
     return TryEmitResult(CGF.EmitScalarExpr(e), false);
 
+  // TODO: update comment when settled on implementation.
+  // DeclRefExpr referencing non-odr-used variable might be constant-evaluated
+  // and in this case we don't have l-value. Emit code for scalar expression
+  // instead of emitting LValue and loading it.
+  if (const auto *decl_expr = dyn_cast<DeclRefExpr>(e)) {
+    auto *DRE = const_cast<DeclRefExpr *>(decl_expr);
+    if (CodeGenFunction::ConstantEmission constant = CGF.tryEmitAsConstant(DRE))
+      return TryEmitResult(CGF.emitScalarConstant(constant, DRE),
+                           !shouldRetainObjCLifetime(type.getObjCLifetime()));
+  }
+
   return tryEmitARCRetainLoadOfScalar(CGF, CGF.EmitLValue(e), type);
 }
 
 typedef llvm::function_ref<llvm::Value *(CodeGenFunction &CGF,
                                          llvm::Value *value)>
   ValueTransform;
 
 /// Insert code immediately after a call.

clang/lib/CodeGen/CodeGenFunction.h

public:
     llvm::Constant *getValue() const {
       assert(!isReference());
       return ValueAndIsReference.getPointer();
     }
   };
 
   ConstantEmission tryEmitAsConstant(DeclRefExpr *refExpr);
   ConstantEmission tryEmitAsConstant(const MemberExpr *ME);
+  llvm::Value *emitScalarConstant(const ConstantEmission &Constant, Expr *E);
 
   RValue EmitPseudoObjectRValue(const PseudoObjectExpr *e,
                                 AggValueSlot slot = AggValueSlot::ignored());
   LValue EmitPseudoObjectLValue(const PseudoObjectExpr *e);
 
   llvm::Value *EmitIvarOffset(const ObjCInterfaceDecl *Interface,
                               const ObjCIvarDecl *Ivar);
   LValue EmitLValueForField(LValue Base, const FieldDecl* Field);

clang/test/CodeGenObjCXX/arc-constexpr.mm

-// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -emit-llvm -fobjc-arc -o - -std=c++11 %s | FileCheck %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -emit-llvm -fobjc-arc -fobjc-runtime-has-weak -o - -std=c++11 %s | FileCheck %s
 
 // CHECK: %[[TYPE:[a-z0-9]+]] = type opaque
 // CHECK: @[[CFSTRING:[a-z0-9_]+]] = private global %struct.__NSConstantString_tag
+@class NSString;
 
-// CHECK: define void @_Z5test1v
+// CHECK-LABEL: define void @_Z5test1v
 // CHECK:   %[[ALLOCA:[A-Z]+]] = alloca %[[TYPE]]*
 // CHECK:   %[[V0:[0-9]+]] = call i8* @objc_retain(i8* bitcast (%struct.__NSConstantString_tag* @[[CFSTRING]]
 // CHECK:   %[[V1:[0-9]+]] = bitcast i8* %[[V0]] to %[[TYPE]]*
 // CHECK:   store %[[TYPE]]* %[[V1]], %[[TYPE]]** %[[ALLOCA]]
 // CHECK:   %[[V2:[0-9]+]] = bitcast %[[TYPE]]** %[[ALLOCA]]
 // CHECK:   call void @objc_storeStrong(i8** %[[V2]], i8* null)
-
-@class NSString;
-
 void test1() {
   constexpr NSString *S = @"abc";
 }
+
+// CHECK-LABEL: define void @_Z5test2v
+// CHECK:      %[[CONST:[a-zA-Z]+]] = alloca %[[TYPE]]*
+// CHECK:      %[[REF_CONST:[a-zA-Z]+]] = alloca %[[TYPE]]*
+// CHECK:      %[[V0:[0-9]+]] = call i8* @objc_retain(i8* bitcast (%struct.__NSConstantString_tag* @[[CFSTRING]]
+// CHECK-NEXT: %[[V1:[0-9]+]] = bitcast i8* %[[V0]] to %[[TYPE]]*
+// CHECK-NEXT: store %[[TYPE]]* %[[V1]], %[[TYPE]]** %[[CONST]]
+// CHECK:      %[[V2:[0-9]+]] = call i8* @objc_retain(i8* bitcast (%struct.__NSConstantString_tag* @[[CFSTRING]]
+// CHECK-NEXT: %[[V3:[0-9]+]] = bitcast i8* %[[V2]] to %[[TYPE]]*
+// CHECK-NEXT: store %[[TYPE]]* %[[V3]], %[[TYPE]]** %[[REF_CONST]]
+// CHECK:      %[[V4:[0-9]+]] = bitcast %[[TYPE]]** %[[REF_CONST]]
+// CHECK-NEXT: call void @objc_storeStrong(i8** %[[V4]], i8* null)
+// CHECK:      %[[V5:[0-9]+]] = bitcast %[[TYPE]]** %[[CONST]]
+// CHECK-NEXT: call void @objc_storeStrong(i8** %[[V5]], i8* null)
+void test2() {
+  constexpr NSString *Const = @"abc";
+  // In IR RefConst should be initialized with Const initializer instead of
+  // reading from variable.
+  NSString* RefConst = Const;
+}
+
+// CHECK-LABEL: define void @_Z5test3v
+// CHECK:      %[[WEAK_CONST:[a-zA-Z]+]] = alloca %[[TYPE]]*
+// CHECK:      %[[REF_WEAK_CONST:[a-zA-Z]+]] = alloca %[[TYPE]]*
+// CHECK:      %[[V0:[0-9]+]] = bitcast %[[TYPE]]** %[[WEAK_CONST]]
+// CHECK-NEXT: %[[V1:[0-9]+]] = call i8* @objc_initWeak(i8** %[[V0]], i8* bitcast (%struct.__NSConstantString_tag* @[[CFSTRING]]
+// CHECK:      store %[[TYPE]]* bitcast (%struct.__NSConstantString_tag* @[[CFSTRING]] to %[[TYPE]]*), %[[TYPE]]** %[[REF_WEAK_CONST]]
+// CHECK:      %[[V2:[0-9]+]] = bitcast %[[TYPE]]** %[[REF_WEAK_CONST]]
+// CHECK-NEXT: call void @objc_storeStrong(i8** %[[V2]], i8* null)
+// CHECK:      %[[V3:[0-9]+]] = bitcast %[[TYPE]]** %[[WEAK_CONST]]
+// CHECK-NEXT: call void @objc_destroyWeak(i8** %[[V3]])
+void test3() {
+  __weak constexpr NSString *WeakConst = @"abc";
+  NSString* RefWeakConst = WeakConst;
+}