[IRGen] Evaluate constant static variables referenced through member expressions

[arphaman]

Summary

C++ allows us to reference static variables through member expressions. Prior to this patch, non-integer static variables that were referenced using a member expression were always emitted using lvalue loads. The old behaviour introduced an inconsistency between regular uses of static variables and member expressions uses, for example, the following program compiled and linked successfully:

struct Foo {
   constexpr static const char *name = "foo";
};
int main() {
  return Foo::name[0] == 'f';
}

but this program failed to link because "Foo::name" wasn't found:

struct Foo {
   constexpr static const char *name = "foo";
};
int main() {
  Foo f;
  return f.name[0] == 'f';
}

This patch ensures that constant static variables referenced through member expressions are emitted in the same way as ordinary static variable references.

rdar://33942261

[rsmith]

lib/CodeGen/CGExprComplex.cpp

156

If we can (correctly) allow side-effects in the initializer here, why can we not also do so when emitting a DeclRefExpr?

lib/CodeGen/CGExprScalar.cpp

1314–1321

Can we remove this and just use tryEmit... unconditionally?

[arphaman]

• Get rid of the AllowSideEffects argument.
• Get rid of the old integer field evaluation code.

lib/CodeGen/CGExprComplex.cpp

156

I think that DREs can't have side-effects in any case, so I think I can remove that argument.

[rjmccall]

lib/CodeGen/CGExpr.cpp

1319

The side effects here are those associated with the initializer of the referenced declaration, not the DRE itself. Some expressions can be constant-evaluated despite having side-effects because the side-effects occur in an ignored operand, like the LHS of a comma or the base of a MemberExpr that refers to a static member.

We can't allow side effects here because (1) we're not actually collecting the side-effectful expressions to emit and (2) we'd need some permission from the context to decide that we're allowed to do so anyway (with a lambda capture, those side-effects have actually already been emitted, but I'm not convinced that's always true).

On the other hand, I think we need to be able to emit MemberExprs to static members as constants despite the presence of side-effects in their base expressions, and I can't think of any reasonable way to do that except actually making a temporary DeclRefExpr to try to constant-emit instead.

[arphaman]

lib/CodeGen/CGExpr.cpp

1319

Thanks, I think I understand your explanation.

I will try using temporary a DRE instead when a static variable is used in a member expression. That means that the old integer field evaluation code will have to stay though.

[arphaman]

Revert most of the changes and use temporary DREs as suggested by John.

[rjmccall]

lib/CodeGen/CGExprComplex.cpp

166

There's an EmitIgnoredExpr you could use.

Also, I think it would be fine to add a generic tryEmitMemberExprAsConstant that takes a MemberExpr and does this DRE stuff behind the scenes; it's not at all different for the different emitters.

[rjmccall]

lib/CodeGen/CGExprComplex.cpp

166

Well, actually, now I see why it's different for the complex emitter, but I think you could probably extract out a function that forms a complex pair from a constant output without too much trouble.

Also, as long as you're working with this, I think it's likely that the Agg emitter needs to handle this, too. I'm not sure I accept the claim there that constant r-value emission never applies to aggregates, but at the very least you need to handle references just as the DRE case does.

[arphaman]

• Create tryEmitAsConstant for MemberExprs and emitConstant helper for complex values as suggested by John.
• Remove dead constant emission code from CGExprAggregate.cpp
• Emit static variable member expressions as DREs when emitting Lvalues to ensure the IR is consistent between DREs and MEs that reference a constant static variables whose type is a reference to an aggregate

lib/CodeGen/CGExprComplex.cpp

166

It looks like that constant reference code in Agg emitter is dead, so I removed it.

The static constant variables that were references to aggregates were still inconsistent between DREs and MEs, so I now try to emit MEs as DREs in CodeGenFunction's Lvalue emitter now.

[rjmccall]

Looks great, thanks.

lib/CodeGen/CGExpr.cpp

 }
 
 /// Try to emit a reference to the given value without producing it as
 /// an l-value.  This is actually more than an optimization: we can't
 /// produce an l-value for variables that we never actually captured
 /// in a block or lambda, which means const int variables or constexpr
 /// literals or similar.
 CodeGenFunction::ConstantEmission
-CodeGenFunction::tryEmitAsConstant(DeclRefExpr *refExpr) {
-  ValueDecl *value = refExpr->getDecl();
+CodeGenFunction::tryEmitDeclRefOrMemberExprAsConstant(Expr *E, ValueDecl *Value,
+                                                      bool AllowSideEffects) {
+  assert((isa<DeclRefExpr>(E) || isa<MemberExpr>(E)) && "invalid expr");
 
   // The value needs to be an enum constant or a constant variable.
   ConstantEmissionKind CEK;
-  if (isa<ParmVarDecl>(value)) {
+  if (isa<ParmVarDecl>(Value)) {
     CEK = CEK_None;
-  } else if (auto *var = dyn_cast<VarDecl>(value)) {
+  } else if (auto *var = dyn_cast<VarDecl>(Value)) {
     CEK = checkVarTypeForConstantEmission(var->getType());
-  } else if (isa<EnumConstantDecl>(value)) {
+  } else if (isa<EnumConstantDecl>(Value)) {
     CEK = CEK_AsValueOnly;
   } else {
     CEK = CEK_None;
   }
   if (CEK == CEK_None) return ConstantEmission();
 
   Expr::EvalResult result;
   bool resultIsReference;
   QualType resultType;
 
   // It's best to evaluate all the way as an r-value if that's permitted.
-  if (CEK != CEK_AsReferenceOnly &&
-      refExpr->EvaluateAsRValue(result, getContext())) {
+  if (CEK != CEK_AsReferenceOnly && E->EvaluateAsRValue(result, getContext())) {
     resultIsReference = false;
-    resultType = refExpr->getType();
+    resultType = E->getType();
 
     // Otherwise, try to evaluate as an l-value.
   } else if (CEK != CEK_AsValueOnly &&
-             refExpr->EvaluateAsLValue(result, getContext())) {
+             E->EvaluateAsLValue(result, getContext())) {
     resultIsReference = true;
-    resultType = value->getType();
+    resultType = Value->getType();
 
     // Failure.
   } else {
     return ConstantEmission();
   }
 
   // In any case, if the initializer has side-effects, abandon ship.
-  if (result.HasSideEffects)
+  if (result.HasSideEffects && !AllowSideEffects)
     return ConstantEmission();
 
   // Emit as a constant.
-  auto C = ConstantEmitter(*this).emitAbstract(refExpr->getLocation(),
-                                               result.Val, resultType);
+  auto C = ConstantEmitter(*this).emitAbstract(E->getExprLoc(), result.Val,
+                                               resultType);
 
   // Make sure we emit a debug reference to the global variable.
   // This should probably fire even for
-  if (isa<VarDecl>(value)) {
-    if (!getContext().DeclMustBeEmitted(cast<VarDecl>(value)))
-      EmitDeclRefExprDbgValue(refExpr, result.Val);
+  if (isa<VarDecl>(Value)) {
+    if (!getContext().DeclMustBeEmitted(cast<VarDecl>(Value)))
+      EmitDeclRefOrMemberExprDbgValue(E, result.Val);
   } else {
-    assert(isa<EnumConstantDecl>(value));
-    EmitDeclRefExprDbgValue(refExpr, result.Val);
+    assert(isa<EnumConstantDecl>(Value));
+    EmitDeclRefOrMemberExprDbgValue(E, result.Val);
   }
 
   // If we emitted a reference constant, we need to dereference that.
   if (resultIsReference)
     return ConstantEmission::forReference(C);
 
   return ConstantEmission::forValue(C);
 }

lib/CodeGen/CGExprAgg.cpp

public:
   void VisitDeclRefExpr(DeclRefExpr *E) {
     // For aggregates, we should always be able to emit the variable
     // as an l-value unless it's a reference.  This is due to the fact
     // that we can't actually ever see a normal l2r conversion on an
     // aggregate in C++, and in C there's no language standard
     // actively preventing us from listing variables in the captures
     // list of a block.
     if (E->getDecl()->getType()->isReferenceType()) {
-      if (CodeGenFunction::ConstantEmission result
-            = CGF.tryEmitAsConstant(E)) {
+      if (CodeGenFunction::ConstantEmission result =
+              CGF.tryEmitDeclRefOrMemberExprAsConstant(E, E->getDecl())) {
         EmitFinalDestCopy(E->getType(), result.getReferenceLValue(CGF, E));
         return;
       }
     }
 
     EmitAggLoadOfLValue(E);
   }
 

lib/CodeGen/CGExprComplex.cpp

ComplexPairTy VisitCoyieldExpr(CoyieldExpr *S) {
     return CGF.EmitCoyieldExpr(*S).getComplexVal();
   }
   ComplexPairTy VisitUnaryCoawait(const UnaryOperator *E) {
     return Visit(E->getSubExpr());
   }
 
 
   // l-values.
-  ComplexPairTy VisitDeclRefExpr(DeclRefExpr *E) {
-    if (CodeGenFunction::ConstantEmission result = CGF.tryEmitAsConstant(E)) {
+  Optional<ComplexPairTy>
+  tryEmitDeclRefOrMemberExprAsConstant(Expr *E, ValueDecl *VD,
+                                       bool AllowSideEffects = false) {
+    if (CodeGenFunction::ConstantEmission result =
+            CGF.tryEmitDeclRefOrMemberExprAsConstant(E, VD, AllowSideEffects)) {
       if (result.isReference())
         return EmitLoadOfLValue(result.getReferenceLValue(CGF, E),
                                 E->getExprLoc());
 
       llvm::Constant *pair = result.getValue();
       return ComplexPairTy(pair->getAggregateElement(0U),
                            pair->getAggregateElement(1U));
     }
+    return None;
+  }
+
+  ComplexPairTy VisitDeclRefExpr(DeclRefExpr *E) {
+    auto Constant = tryEmitDeclRefOrMemberExprAsConstant(E, E->getDecl());
+    if (Constant)
+      return *Constant;
     return EmitLoadOfLValue(E);
   }
   ComplexPairTy VisitObjCIvarRefExpr(ObjCIvarRefExpr *E) {
     return EmitLoadOfLValue(E);
   }
   ComplexPairTy VisitObjCMessageExpr(ObjCMessageExpr *E) {
     return CGF.EmitObjCMessageExpr(E).getComplexVal();
   }
   ComplexPairTy VisitArraySubscriptExpr(Expr *E) { return EmitLoadOfLValue(E); }
-  ComplexPairTy VisitMemberExpr(const Expr *E) { return EmitLoadOfLValue(E); }
+  ComplexPairTy VisitMemberExpr(MemberExpr *ME) {
+    auto Constant = tryEmitDeclRefOrMemberExprAsConstant(
+        ME, ME->getMemberDecl(), /*AllowSideEffects=*/true);
+    if (Constant) {
+      if (ME->isArrow())
+        CGF.EmitScalarExpr(ME->getBase());
+      else
+        CGF.EmitLValue(ME->getBase());
+      return *Constant;
+    }
+    return EmitLoadOfLValue(ME);
+  }
   ComplexPairTy VisitOpaqueValueExpr(OpaqueValueExpr *E) {
     if (E->isGLValue())
       return EmitLoadOfLValue(CGF.getOpaqueLValueMapping(E), E->getExprLoc());
     return CGF.getOpaqueRValueMapping(E).getComplexVal();
   }
 
   ComplexPairTy VisitPseudoObjectExpr(PseudoObjectExpr *E) {
     return CGF.EmitPseudoObjectRValue(E).getComplexVal();
   }

lib/CodeGen/CGExprScalar.cpp

Value *VisitOpaqueValueExpr(OpaqueValueExpr *E) {
     if (E->isGLValue())
       return EmitLoadOfLValue(CGF.getOpaqueLValueMapping(E), E->getExprLoc());
 
     // Otherwise, assume the mapping is the scalar directly.
     return CGF.getOpaqueRValueMapping(E).getScalarVal();
   }
 
   // l-values.
-  Value *VisitDeclRefExpr(DeclRefExpr *E) {
-    if (CodeGenFunction::ConstantEmission result = CGF.tryEmitAsConstant(E)) {
+  Value *tryEmitDeclRefOrMemberExprAsConstant(Expr *E, ValueDecl *VD,
+                                              bool AllowSideEffects = false) {
+    if (CodeGenFunction::ConstantEmission result =
+            CGF.tryEmitDeclRefOrMemberExprAsConstant(E, VD, AllowSideEffects)) {
       if (result.isReference())
         return EmitLoadOfLValue(result.getReferenceLValue(CGF, E),
                                 E->getExprLoc());
       return result.getValue();
     }
+    return nullptr;
+  }
+
+  Value *VisitDeclRefExpr(DeclRefExpr *E) {
+    if (Value *V = tryEmitDeclRefOrMemberExprAsConstant(E, E->getDecl()))
+      return V;
     return EmitLoadOfLValue(E);
   }
 
   Value *VisitObjCSelectorExpr(ObjCSelectorExpr *E) {
     return CGF.EmitObjCSelectorExpr(E);
   }
   Value *VisitObjCProtocolExpr(ObjCProtocolExpr *E) {
     return CGF.EmitObjCProtocolExpr(E);
if (isa<llvm::IntegerType>(SrcEltTy)) {
     else
       Res = Builder.CreateFPExt(Src, DstTy, "conv");
   }
 
   return Res;
 }
 
 Value *ScalarExprEmitter::VisitMemberExpr(MemberExpr *E) {
+  Value *V;
+  if (isa<VarDecl>(E->getMemberDecl())) {
+    V = tryEmitDeclRefOrMemberExprAsConstant(E, E->getMemberDecl(),
+                                             /*AllowSideEffects=*/true);
+  } else {
+    // Fields can evaluate to constants if their base can be evaluated.
     llvm::APSInt Value;
-  if (E->EvaluateAsInt(Value, CGF.getContext(), Expr::SE_AllowSideEffects)) {
+    if (E->EvaluateAsInt(Value, CGF.getContext(), Expr::SE_AllowSideEffects))
+      V = Builder.getInt(Value);
+    else
+      V = nullptr;
+  }
+  if (V) {
     if (E->isArrow())
       CGF.EmitScalarExpr(E->getBase());
     else
       EmitLValue(E->getBase());
-    return Builder.getInt(Value);
+    return V;
   }
 
   return EmitLoadOfLValue(E);
 }
 
 Value *ScalarExprEmitter::VisitArraySubscriptExpr(ArraySubscriptExpr *E) {
   TestAndClearIgnoreResultAssign();
 

lib/CodeGen/CodeGenFunction.h

public:
     }
 
     llvm::Constant *getValue() const {
       assert(!isReference());
       return ValueAndIsReference.getPointer();
     }
   };
 
-  ConstantEmission tryEmitAsConstant(DeclRefExpr *refExpr);
+  ConstantEmission
+  tryEmitDeclRefOrMemberExprAsConstant(Expr *E, ValueDecl *Value,
+                                       bool AllowSideEffects = false);
 
   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);
public:
   LValue EmitCXXTypeidLValue(const CXXTypeidExpr *E);
   LValue EmitCXXUuidofLValue(const CXXUuidofExpr *E);
 
   LValue EmitObjCMessageExprLValue(const ObjCMessageExpr *E);
   LValue EmitObjCIvarRefLValue(const ObjCIvarRefExpr *E);
   LValue EmitStmtExprLValue(const StmtExpr *E);
   LValue EmitPointerToDataMemberBinaryExpr(const BinaryOperator *E);
   LValue EmitObjCSelectorLValue(const ObjCSelectorExpr *E);
-  void   EmitDeclRefExprDbgValue(const DeclRefExpr *E, const APValue &Init);
+  void   EmitDeclRefOrMemberExprDbgValue(const Expr *E, const APValue &Init);
 
   //===--------------------------------------------------------------------===//
   //                         Scalar Expression Emission
   //===--------------------------------------------------------------------===//
 
   /// EmitCall - Generate a call of the given function, expecting the given
   /// result type, and using the given argument list which specifies both the
   /// LLVM arguments and the types they were derived from.

lib/CodeGen/CodeGenFunction.cpp

if (getContext().getBuiltinVaListType()->isArrayType())
     return EmitPointerWithAlignment(E);
   return EmitLValue(E).getAddress();
 }
 
 Address CodeGenFunction::EmitMSVAListRef(const Expr *E) {
   return EmitLValue(E).getAddress();
 }
 
-void CodeGenFunction::EmitDeclRefExprDbgValue(const DeclRefExpr *E,
+void CodeGenFunction::EmitDeclRefOrMemberExprDbgValue(const Expr *E,
                                                       const APValue &Init) {
+  const ValueDecl *VD;
+  if (const auto *DRE = dyn_cast<DeclRefExpr>(E))
+    VD = DRE->getDecl();
+  else
+    VD = cast<MemberExpr>(E)->getMemberDecl();
   assert(!Init.isUninit() && "Invalid DeclRefExpr initializer!");
   if (CGDebugInfo *Dbg = getDebugInfo())
     if (CGM.getCodeGenOpts().getDebugInfo() >= codegenoptions::LimitedDebugInfo)
-      Dbg->EmitGlobalVariable(E->getDecl(), Init);
+      Dbg->EmitGlobalVariable(VD, Init);
 }
 
 CodeGenFunction::PeepholeProtection
 CodeGenFunction::protectFromPeepholes(RValue rvalue) {
   // At the moment, the only aggressive peephole we do in IR gen
   // is trunc(zext) folding, but if we add more, we can easily
   // extend this protection.
 

test/CodeGenCXX/member-expr-references-variable.cpp

+// RUN: %clang_cc1 -std=c++11 %s -triple x86_64-apple-darwin10 -emit-llvm -o - | FileCheck %s
+
+struct Struct {
+   constexpr static const char *name = "foo";
+
+   constexpr static __complex float complexValue = 42.0;
+
+   Struct();
+   Struct(int x);
+};
+
+void use(int n, const char *c);
+
+Struct *getPtr();
+
+// CHECK: @[[STR:.*]] = private unnamed_addr constant [4 x i8] c"foo\00", align 1
+
+void scalarStaticVariableInMemberExpr(Struct *ptr, Struct &ref) {
+  use(1, Struct::name);
+// CHECK: call void @_Z3useiPKc(i32 1, i8* getelementptr inbounds ([4 x i8], [4 x i8]* @[[STR]], i32 0, i32 0))
+  Struct s;
+  use(2, s.name);
+// CHECK: call void @_Z3useiPKc(i32 2, i8* getelementptr inbounds ([4 x i8], [4 x i8]* @[[STR]], i32 0, i32 0))
+  use(3, ptr->name);
+// CHECK: load %struct.Struct*, %struct.Struct** %{{.*}}, align 8
+// CHECK: call void @_Z3useiPKc(i32 3, i8* getelementptr inbounds ([4 x i8], [4 x i8]* @[[STR]], i32 0, i32 0))
+  use(4, ref.name);
+// CHECK: load %struct.Struct*, %struct.Struct** %{{.*}}, align 8
+// CHECK: call void @_Z3useiPKc(i32 4, i8* getelementptr inbounds ([4 x i8], [4 x i8]* @[[STR]], i32 0, i32 0))
+  use(5, Struct(2).name);
+// CHECK: call void @_ZN6StructC1Ei(%struct.Struct* %{{.*}}, i32 2)
+// CHECK: call void @_Z3useiPKc(i32 5, i8* getelementptr inbounds ([4 x i8], [4 x i8]* @[[STR]], i32 0, i32 0))
+  use(6, getPtr()->name);
+// CHECK: call %struct.Struct* @_Z6getPtrv()
+// CHECK: call void @_Z3useiPKc(i32 6, i8* getelementptr inbounds ([4 x i8], [4 x i8]* @[[STR]], i32 0, i32 0))
+}
+
+void use(int n, __complex float v);
+
+void complexStaticVariableInMemberExpr(Struct *ptr, Struct &ref) {
+  use(1, Struct::complexValue);
+// CHECK: store float 4.200000e+01, float* %[[coerce0:.*]].{{.*}}, align 4
+// CHECK: store float 0.000000e+00, float* %[[coerce0]].{{.*}}, align 4
+// CHECK: %[[cast0:.*]] = bitcast { float, float }* %[[coerce0]] to <2 x float>*
+// CHECK: %[[vector0:.*]] = load <2 x float>, <2 x float>* %[[cast0]], align 4
+// CHECK: call void @_Z3useiCf(i32 1, <2 x float> %[[vector0]])
+  Struct s;
+  use(2, s.complexValue);
+// CHECK: store float 4.200000e+01, float* %[[coerce1:.*]].{{.*}}, align 4
+// CHECK: store float 0.000000e+00, float* %[[coerce1]].{{.*}}, align 4
+// CHECK: %[[cast1:.*]] = bitcast { float, float }* %[[coerce1]] to <2 x float>*
+// CHECK: %[[vector1:.*]] = load <2 x float>, <2 x float>* %[[cast1]], align 4
+// CHECK: call void @_Z3useiCf(i32 2, <2 x float> %[[vector1]])
+  use(3, ptr->complexValue);
+// CHECK: load %struct.Struct*, %struct.Struct** %{{.*}}, align 8
+// CHECK: store float 4.200000e+01, float* %[[coerce2:.*]].{{.*}}, align 4
+// CHECK: store float 0.000000e+00, float* %[[coerce2]].{{.*}}, align 4
+// CHECK: %[[cast2:.*]] = bitcast { float, float }* %[[coerce2]] to <2 x float>*
+// CHECK: %[[vector2:.*]] = load <2 x float>, <2 x float>* %[[cast2]], align 4
+// CHECK: call void @_Z3useiCf(i32 3, <2 x float> %[[vector2]])
+  use(4, ref.complexValue);
+// CHECK: load %struct.Struct*, %struct.Struct** %{{.*}}, align 8
+// CHECK: store float 4.200000e+01, float* %[[coerce3:.*]].{{.*}}, align 4
+// CHECK: store float 0.000000e+00, float* %[[coerce3]].{{.*}}, align 4
+// CHECK: %[[cast3:.*]] = bitcast { float, float }* %[[coerce3]] to <2 x float>*
+// CHECK: %[[vector3:.*]] = load <2 x float>, <2 x float>* %[[cast3]], align 4
+// CHECK: call void @_Z3useiCf(i32 4, <2 x float> %[[vector3]])
+  use(5, Struct(2).complexValue);
+// CHECK: call void @_ZN6StructC1Ei(%struct.Struct* %{{.*}}, i32 2)
+// CHECK: store float 4.200000e+01, float* %[[coerce4:.*]].{{.*}}, align 4
+// CHECK: store float 0.000000e+00, float* %[[coerce4]].{{.*}}, align 4
+// CHECK: %[[cast4:.*]] = bitcast { float, float }* %[[coerce4]] to <2 x float>*
+// CHECK: %[[vector4:.*]] = load <2 x float>, <2 x float>* %[[cast4]], align 4
+// CHECK: call void @_Z3useiCf(i32 5, <2 x float> %[[vector4]])
+  use(6, getPtr()->complexValue);
+// CHECK: call %struct.Struct* @_Z6getPtrv()
+// CHECK: store float 4.200000e+01, float* %[[coerce5:.*]].{{.*}}, align 4
+// CHECK: store float 0.000000e+00, float* %[[coerce5]].{{.*}}, align 4
+// CHECK: %[[cast5:.*]] = bitcast { float, float }* %[[coerce5]] to <2 x float>*
+// CHECK: %[[vector5:.*]] = load <2 x float>, <2 x float>* %[[cast5]], align 4
+// CHECK: call void @_Z3useiCf(i32 6, <2 x float> %[[vector5]])
+}