From 713350593697f52c6e4412794169287b8e0177d9 Mon Sep 17 00:00:00 2001 From: John McCall Date: Sat, 10 Mar 2012 03:05:10 +0000 Subject: [PATCH] Unify the BlockDeclRefExpr and DeclRefExpr paths so that we correctly emit loads of BlockDeclRefExprs even when they don't qualify as ODR-uses. I think I'm adequately convinced that BlockDeclRefExpr can die. llvm-svn: 152479 --- clang/lib/CodeGen/CGExpr.cpp | 116 +++++++++++++++++++++++++ clang/lib/CodeGen/CGExprAgg.cpp | 26 +++++- clang/lib/CodeGen/CGExprComplex.cpp | 20 ++++- clang/lib/CodeGen/CGExprScalar.cpp | 61 ++++--------- clang/lib/CodeGen/CodeGenFunction.h | 30 +++++++ clang/test/CodeGenCXX/blocks-cxx11.cpp | 84 ++++++++++++++++++ 6 files changed, 285 insertions(+), 52 deletions(-) create mode 100644 clang/test/CodeGenCXX/blocks-cxx11.cpp diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp index 9def5543a846..3275327c9be7 100644 --- a/clang/lib/CodeGen/CGExpr.cpp +++ b/clang/lib/CodeGen/CGExpr.cpp @@ -745,6 +745,122 @@ LValue CodeGenFunction::EmitLValue(const Expr *E) { } } +/// Given an object of the given canonical type, can we safely copy a +/// value out of it based on its initializer? +static bool isConstantEmittableObjectType(QualType type) { + assert(type.isCanonical()); + assert(!type->isReferenceType()); + + // Must be const-qualified but non-volatile. + Qualifiers qs = type.getLocalQualifiers(); + if (!qs.hasConst() || qs.hasVolatile()) return false; + + // Otherwise, all object types satisfy this except C++ classes with + // mutable subobjects or non-trivial copy/destroy behavior. + if (const RecordType *RT = dyn_cast(type)) + if (const CXXRecordDecl *RD = dyn_cast(RT->getDecl())) + if (RD->hasMutableFields() || !RD->isTrivial()) + return false; + + return true; +} + +/// Can we constant-emit a load of a reference to a variable of the +/// given type? This is different from predicates like +/// Decl::isUsableInConstantExpressions because we do want it to apply +/// in situations that don't necessarily satisfy the language's rules +/// for this (e.g. C++'s ODR-use rules). For example, we want to able +/// to do this with const float variables even if those variables +/// aren't marked 'constexpr'. +enum ConstantEmissionKind { + CEK_None, + CEK_AsReferenceOnly, + CEK_AsValueOrReference, + CEK_AsValueOnly +}; +static ConstantEmissionKind checkVarTypeForConstantEmission(QualType type) { + type = type.getCanonicalType(); + if (const ReferenceType *ref = dyn_cast(type)) { + if (isConstantEmittableObjectType(ref->getPointeeType())) + return CEK_AsValueOrReference; + return CEK_AsReferenceOnly; + } + if (isConstantEmittableObjectType(type)) + return CEK_AsValueOnly; + return CEK_None; +} + +/// 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(ValueDecl *value, Expr *refExpr) { + // The value needs to be an enum constant or a constant variable. + ConstantEmissionKind CEK; + if (isa(value)) { + CEK = CEK_None; + } else if (VarDecl *var = dyn_cast(value)) { + CEK = checkVarTypeForConstantEmission(var->getType()); + } else if (isa(value)) { + CEK = CEK_AsValueOnly; + } else { + CEK = CEK_None; + } + if (CEK == CEK_None) return ConstantEmission(); + + // We evaluate use an on-stack DeclRefExpr because the constant + // evaluator (quite reasonably) ignores BlockDeclRefExprs. + DeclRefExpr stackRef(value, refExpr->getType(), refExpr->getValueKind(), + refExpr->getExprLoc()); + + // If it's okay to evaluate as a + 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 && + stackRef.EvaluateAsRValue(result, getContext())) { + resultIsReference = false; + resultType = refExpr->getType(); + + // Otherwise, try to evaluate as an l-value. + } else if (CEK != CEK_AsValueOnly && + stackRef.EvaluateAsLValue(result, getContext())) { + resultIsReference = true; + resultType = value->getType(); + + // Failure. + } else { + return ConstantEmission(); + } + + // In any case, if the initializer has side-effects, abandon ship. + if (result.HasSideEffects) + return ConstantEmission(); + + // Emit as a constant. + llvm::Constant *C = CGM.EmitConstantValue(result.Val, resultType, this); + + // Make sure we emit a debug reference to the global variable. + // This should probably fire even for + if (isa(value)) { + if (!getContext().DeclMustBeEmitted(cast(value))) + EmitDeclRefExprDbgValue(&stackRef, C); + } else { + assert(isa(value)); + EmitDeclRefExprDbgValue(&stackRef, C); + } + + // If we emitted a reference constant, we need to dereference that. + if (resultIsReference) + return ConstantEmission::forReference(C); + + return ConstantEmission::forValue(C); +} + llvm::Value *CodeGenFunction::EmitLoadOfScalar(LValue lvalue) { return EmitLoadOfScalar(lvalue.getAddress(), lvalue.isVolatile(), lvalue.getAlignment().getQuantity(), diff --git a/clang/lib/CodeGen/CGExprAgg.cpp b/clang/lib/CodeGen/CGExprAgg.cpp index 899577c4ede9..f7c640adacf5 100644 --- a/clang/lib/CodeGen/CGExprAgg.cpp +++ b/clang/lib/CodeGen/CGExprAgg.cpp @@ -109,7 +109,28 @@ public: } // l-values. - void VisitDeclRefExpr(DeclRefExpr *DRE) { EmitAggLoadOfLValue(DRE); } + void emitDeclRef(ValueDecl *VD, Expr *refExpr) { + // 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 (VD->getType()->isReferenceType()) { + if (CodeGenFunction::ConstantEmission result + = CGF.tryEmitAsConstant(VD, refExpr)) { + EmitFinalDestCopy(refExpr, result.getReferenceLValue(CGF, refExpr)); + return; + } + } + + EmitAggLoadOfLValue(refExpr); + } + void VisitDeclRefExpr(DeclRefExpr *E) { emitDeclRef(E->getDecl(), E); } + void VisitBlockDeclRefExpr(BlockDeclRefExpr *E) { + emitDeclRef(E->getDecl(), E); + } + void VisitMemberExpr(MemberExpr *ME) { EmitAggLoadOfLValue(ME); } void VisitUnaryDeref(UnaryOperator *E) { EmitAggLoadOfLValue(E); } void VisitStringLiteral(StringLiteral *E) { EmitAggLoadOfLValue(E); } @@ -117,9 +138,6 @@ public: void VisitArraySubscriptExpr(ArraySubscriptExpr *E) { EmitAggLoadOfLValue(E); } - void VisitBlockDeclRefExpr(const BlockDeclRefExpr *E) { - EmitAggLoadOfLValue(E); - } void VisitPredefinedExpr(const PredefinedExpr *E) { EmitAggLoadOfLValue(E); } diff --git a/clang/lib/CodeGen/CGExprComplex.cpp b/clang/lib/CodeGen/CGExprComplex.cpp index d2a760054b56..7be88440ff08 100644 --- a/clang/lib/CodeGen/CGExprComplex.cpp +++ b/clang/lib/CodeGen/CGExprComplex.cpp @@ -111,8 +111,24 @@ public: } // l-values. - ComplexPairTy VisitDeclRefExpr(const Expr *E) { return EmitLoadOfLValue(E); } - ComplexPairTy VisitBlockDeclRefExpr(const Expr *E) { return EmitLoadOfLValue(E); } + ComplexPairTy emitDeclRef(ValueDecl *VD, Expr *refExpr) { + if (CodeGenFunction::ConstantEmission result + = CGF.tryEmitAsConstant(VD, refExpr)) { + if (result.isReference()) + return EmitLoadOfLValue(result.getReferenceLValue(CGF, refExpr)); + + llvm::ConstantStruct *pair = + cast(result.getValue()); + return ComplexPairTy(pair->getOperand(0), pair->getOperand(1)); + } + return EmitLoadOfLValue(refExpr); + } + ComplexPairTy VisitDeclRefExpr(DeclRefExpr *E) { + return emitDeclRef(E->getDecl(), E); + } + ComplexPairTy VisitBlockDeclRefExpr(BlockDeclRefExpr *E) { + return emitDeclRef(E->getDecl(), E); + } ComplexPairTy VisitObjCIvarRefExpr(ObjCIvarRefExpr *E) { return EmitLoadOfLValue(E); } diff --git a/clang/lib/CodeGen/CGExprScalar.cpp b/clang/lib/CodeGen/CGExprScalar.cpp index 9da493448aae..a6ba79803d44 100644 --- a/clang/lib/CodeGen/CGExprScalar.cpp +++ b/clang/lib/CodeGen/CGExprScalar.cpp @@ -211,48 +211,24 @@ public: // Otherwise, assume the mapping is the scalar directly. return CGF.getOpaqueRValueMapping(E).getScalarVal(); } - + // l-values. - Value *VisitDeclRefExpr(DeclRefExpr *E) { - VarDecl *VD = dyn_cast(E->getDecl()); - if (!VD && !isa(E->getDecl())) - return EmitLoadOfLValue(E); - if (VD && !VD->isUsableInConstantExpressions(CGF.getContext())) - return EmitLoadOfLValue(E); - - // This is an enumerator or a variable which is usable in constant - // expressions. Try to emit its value instead. - Expr::EvalResult Result; - bool IsReferenceConstant = false; - QualType EvalTy = E->getType(); - if (!E->EvaluateAsRValue(Result, CGF.getContext())) { - // If this is a reference, try to determine what it is bound to. - if (!E->getDecl()->getType()->isReferenceType() || - !E->EvaluateAsLValue(Result, CGF.getContext())) - return EmitLoadOfLValue(E); - - IsReferenceConstant = true; - EvalTy = E->getDecl()->getType(); + Value *emitDeclRef(ValueDecl *VD, Expr *refExpr) { + if (CodeGenFunction::ConstantEmission result + = CGF.tryEmitAsConstant(VD, refExpr)) { + if (result.isReference()) + return EmitLoadOfLValue(result.getReferenceLValue(CGF, refExpr)); + return result.getValue(); } - - assert(!Result.HasSideEffects && "Constant declref with side-effect?!"); - - llvm::Constant *C = CGF.CGM.EmitConstantValue(Result.Val, EvalTy, &CGF); - - // Make sure we emit a debug reference to the global variable. - if (VD) { - if (!CGF.getContext().DeclMustBeEmitted(VD)) - CGF.EmitDeclRefExprDbgValue(E, C); - } else { - assert(isa(E->getDecl())); - CGF.EmitDeclRefExprDbgValue(E, C); - } - - if (IsReferenceConstant) - return EmitLoadOfLValue(CGF.MakeNaturalAlignAddrLValue(C, E->getType())); - - return C; + return EmitLoadOfLValue(refExpr); } + Value *VisitDeclRefExpr(DeclRefExpr *E) { + return emitDeclRef(E->getDecl(), E); + } + Value *VisitBlockDeclRefExpr(BlockDeclRefExpr *E) { + return emitDeclRef(E->getDecl(), E); + } + Value *VisitObjCSelectorExpr(ObjCSelectorExpr *E) { return CGF.EmitObjCSelectorExpr(E); } @@ -304,8 +280,6 @@ public: Value *VisitStmtExpr(const StmtExpr *E); - Value *VisitBlockDeclRefExpr(const BlockDeclRefExpr *E); - // Unary Operators. Value *VisitUnaryPostDec(const UnaryOperator *E) { LValue LV = EmitLValue(E->getSubExpr()); @@ -1272,11 +1246,6 @@ Value *ScalarExprEmitter::VisitStmtExpr(const StmtExpr *E) { .getScalarVal(); } -Value *ScalarExprEmitter::VisitBlockDeclRefExpr(const BlockDeclRefExpr *E) { - LValue LV = CGF.EmitBlockDeclRefLValue(E); - return CGF.EmitLoadOfLValue(LV).getScalarVal(); -} - //===----------------------------------------------------------------------===// // Unary Operators //===----------------------------------------------------------------------===// diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h index 833eee2da4f3..aeeb18c2a791 100644 --- a/clang/lib/CodeGen/CodeGenFunction.h +++ b/clang/lib/CodeGen/CodeGenFunction.h @@ -2107,6 +2107,36 @@ public: LValue EmitMaterializeTemporaryExpr(const MaterializeTemporaryExpr *E); LValue EmitOpaqueValueLValue(const OpaqueValueExpr *e); + class ConstantEmission { + llvm::PointerIntPair ValueAndIsReference; + ConstantEmission(llvm::Constant *C, bool isReference) + : ValueAndIsReference(C, isReference) {} + public: + ConstantEmission() {} + static ConstantEmission forReference(llvm::Constant *C) { + return ConstantEmission(C, true); + } + static ConstantEmission forValue(llvm::Constant *C) { + return ConstantEmission(C, false); + } + + operator bool() const { return ValueAndIsReference.getOpaqueValue() != 0; } + + bool isReference() const { return ValueAndIsReference.getInt(); } + LValue getReferenceLValue(CodeGenFunction &CGF, Expr *refExpr) const { + assert(isReference()); + return CGF.MakeNaturalAlignAddrLValue(ValueAndIsReference.getPointer(), + refExpr->getType()); + } + + llvm::Constant *getValue() const { + assert(!isReference()); + return ValueAndIsReference.getPointer(); + } + }; + + ConstantEmission tryEmitAsConstant(ValueDecl *VD, Expr *refExpr); + RValue EmitPseudoObjectRValue(const PseudoObjectExpr *e, AggValueSlot slot = AggValueSlot::ignored()); LValue EmitPseudoObjectLValue(const PseudoObjectExpr *e); diff --git a/clang/test/CodeGenCXX/blocks-cxx11.cpp b/clang/test/CodeGenCXX/blocks-cxx11.cpp new file mode 100644 index 000000000000..996db1afe69f --- /dev/null +++ b/clang/test/CodeGenCXX/blocks-cxx11.cpp @@ -0,0 +1,84 @@ +// RUN: %clang_cc1 %s -fblocks -triple x86_64-apple-darwin -std=c++11 -emit-llvm -o - | FileCheck %s + +template void takeItByValue(T); +void takeABlock(void (^)()); + +// rdar://problem/11022704 +namespace test_int { + void test() { + const int x = 100; + takeABlock(^{ takeItByValue(x); }); + // CHECK: call void @_Z13takeItByValueIiEvT_(i32 100) + } +} + +namespace test_int_ref { + void test() { + const int y = 200; + const int &x = y; + takeABlock(^{ takeItByValue(x); }); + + // TODO: there's no good reason that this isn't foldable. + // CHECK: call void @_Z13takeItByValueIiEvT_(i32 {{%.*}}) + } +} + +namespace test_float { + void test() { + const float x = 1; + takeABlock(^{ takeItByValue(x); }); + // CHECK: call void @_Z13takeItByValueIfEvT_(float 1.0 + } +} + +namespace test_float_ref { + void test() { + const float y = 100; + const float &x = y; + takeABlock(^{ takeItByValue(x); }); + + // TODO: there's no good reason that this isn't foldable. + // CHECK: call void @_Z13takeItByValueIfEvT_(float {{%.*}}) + } +} + +namespace test_complex_int { + void test() { + constexpr _Complex int x = 500; + takeABlock(^{ takeItByValue(x); }); + // CHECK: store i32 500, + + // CHECK: store i32 500, + // CHECK-NEXT: store i32 0, + // CHECK-NEXT: [[COERCE:%.*]] = bitcast + // CHECK-NEXT: [[CVAL:%.*]] = load i64* [[COERCE]] + // CHECK-NEXT: call void @_Z13takeItByValueICiEvT_(i64 [[CVAL]]) + } +} + +namespace test_complex_int_ref { + void test() { + const _Complex int y = 100; + const _Complex int &x = y; + takeABlock(^{ takeItByValue(x); }); + // CHECK: call void @_Z13takeItByValueICiEvT_(i64 + } +} + +namespace test_complex_int_ref_mutable { + _Complex int y = 100; + void test() { + const _Complex int &x = y; + takeABlock(^{ takeItByValue(x); }); + // CHECK: [[R:%.*]] = load i32* getelementptr inbounds ({ i32, i32 }* @_ZN28test_complex_int_ref_mutable1yE, i32 0, i32 0) + // CHECK-NEXT: [[I:%.*]] = load i32* getelementptr inbounds ({ i32, i32 }* @_ZN28test_complex_int_ref_mutable1yE, i32 0, i32 1) + // CHECK-NEXT: [[RSLOT:%.*]] = getelementptr inbounds { i32, i32 }* [[CSLOT:%.*]], i32 0, i32 0 + // CHECK-NEXT: [[ISLOT:%.*]] = getelementptr inbounds { i32, i32 }* [[CSLOT]], i32 0, i32 1 + // CHECK-NEXT: store i32 [[R]], i32* [[RSLOT]] + // CHECK-NEXT: store i32 [[I]], i32* [[ISLOT]] + // CHECK-NEXT: [[COERCE:%.*]] = bitcast { i32, i32 }* [[CSLOT]] to i64* + // CHECK-NEXT: [[CVAL:%.*]] = load i64* [[COERCE]], + // CHECK-NEXT: call void @_Z13takeItByValueICiEvT_(i64 [[CVAL]]) + } +} +