[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

153

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

1312–1319

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

153

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

1323 ↗

(On Diff #111956)

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

1323 ↗

(On Diff #111956)

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

163

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

163

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

163

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/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) {
+  Optional<ComplexPairTy> tryEmitDeclRefExprAsConstant(DeclRefExpr *E) {
     if (CodeGenFunction::ConstantEmission result = CGF.tryEmitAsConstant(E)) {
       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 = tryEmitDeclRefExprAsConstant(E);
+    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) {
+    if (auto *VD = dyn_cast<VarDecl>(ME->getMemberDecl())) {
+      // Try emitting static variable member expressions as constant DREs.
+      DeclRefExpr *DRE = DeclRefExpr::Create(
+          CGF.getContext(), NestedNameSpecifierLoc(), SourceLocation(), VD,
+          /*RefersToEnclosingVariableOrCapture=*/false, ME->getExprLoc(),
+          ME->getType(), ME->getValueKind());
+      auto Constant = tryEmitDeclRefExprAsConstant(DRE);
+      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) {
+  Value *tryEmitDeclRefExprAsConstant(DeclRefExpr *E) {
     if (CodeGenFunction::ConstantEmission result = CGF.tryEmitAsConstant(E)) {
       if (result.isReference())
         return EmitLoadOfLValue(result.getReferenceLValue(CGF, E),
                                 E->getExprLoc());
       return result.getValue();
     }
+    return nullptr;
+  }
+
+  Value *VisitDeclRefExpr(DeclRefExpr *E) {
+    if (Value *V = tryEmitDeclRefExprAsConstant(E))
+      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 *ConstantMemberValue;
+  if (auto *VD = dyn_cast<VarDecl>(E->getMemberDecl())) {
+    // Try emitting static variable member expressions as constant DREs.
+    DeclRefExpr *DRE = DeclRefExpr::Create(
+        CGF.getContext(), NestedNameSpecifierLoc(), SourceLocation(), VD,
+        /*RefersToEnclosingVariableOrCapture=*/false, E->getExprLoc(),
+        E->getType(), E->getValueKind());
+    ConstantMemberValue = tryEmitDeclRefExprAsConstant(DRE);
+  } else {
     llvm::APSInt Value;
-  if (E->EvaluateAsInt(Value, CGF.getContext(), Expr::SE_AllowSideEffects)) {
+    if (E->EvaluateAsInt(Value, CGF.getContext(), Expr::SE_AllowSideEffects))
+      ConstantMemberValue = Builder.getInt(Value);
+    else
+      ConstantMemberValue = nullptr;
+  }
+
+  if (ConstantMemberValue) {
     if (E->isArrow())
       CGF.EmitScalarExpr(E->getBase());
     else
       EmitLValue(E->getBase());
-    return Builder.getInt(Value);
+    return ConstantMemberValue;
   }
 
   return EmitLoadOfLValue(E);
 }
 
 Value *ScalarExprEmitter::VisitArraySubscriptExpr(ArraySubscriptExpr *E) {
   TestAndClearIgnoreResultAssign();
 

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]])
+}