Author: gbiv Date: Wed Dec 28 01:27:40 2016 New Revision: 290661 URL: http://llvm.org/viewvc/llvm-project?rev=290661&view=rev Log: [CodeGen] Unique constant CompoundLiterals.
Our newly aggressive constant folding logic makes it possible for CGExprConstant to see the same CompoundLiteralExpr more than once. So, emitting a new GlobalVariable every time we see a CompoundLiteral is no longer correct. We had a similar issue with BlockExprs that was caught while testing said aggressive folding, so I applied the same style of fix (see D26410) here. If we find yet another case where this needs to happen, we should probably refactor this so we don't have a third DenseMap+getter+setter. As a design note: getAddrOfConstantCompoundLiteralIfEmitted is really only intended to be called by ConstExprEmitter::EmitLValue. So, returning a GlobalVariable* instead of a ConstantAddress costs us effectively nothing, and saves us either a few bytes per entry in our map or a bit of code duplication. Modified: cfe/trunk/lib/CodeGen/CGExprConstant.cpp cfe/trunk/lib/CodeGen/CodeGenModule.h cfe/trunk/test/CodeGen/compound-literal.c Modified: cfe/trunk/lib/CodeGen/CGExprConstant.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExprConstant.cpp?rev=290661&r1=290660&r2=290661&view=diff ============================================================================== --- cfe/trunk/lib/CodeGen/CGExprConstant.cpp (original) +++ cfe/trunk/lib/CodeGen/CGExprConstant.cpp Wed Dec 28 01:27:40 2016 @@ -1023,16 +1023,17 @@ public: switch (E->getStmtClass()) { default: break; case Expr::CompoundLiteralExprClass: { - // Note that due to the nature of compound literals, this is guaranteed - // to be the only use of the variable, so we just generate it here. CompoundLiteralExpr *CLE = cast<CompoundLiteralExpr>(E); + CharUnits Align = CGM.getContext().getTypeAlignInChars(E->getType()); + if (llvm::GlobalVariable *Addr = + CGM.getAddrOfConstantCompoundLiteralIfEmitted(CLE)) + return ConstantAddress(Addr, Align); + llvm::Constant* C = CGM.EmitConstantExpr(CLE->getInitializer(), CLE->getType(), CGF); // FIXME: "Leaked" on failure. if (!C) return ConstantAddress::invalid(); - CharUnits Align = CGM.getContext().getTypeAlignInChars(E->getType()); - auto GV = new llvm::GlobalVariable(CGM.getModule(), C->getType(), E->getType().isConstant(CGM.getContext()), llvm::GlobalValue::InternalLinkage, @@ -1040,6 +1041,7 @@ public: llvm::GlobalVariable::NotThreadLocal, CGM.getContext().getTargetAddressSpace(E->getType())); GV->setAlignment(Align.getQuantity()); + CGM.setAddrOfConstantCompoundLiteral(CLE, GV); return ConstantAddress(GV, Align); } case Expr::StringLiteralClass: @@ -1492,6 +1494,18 @@ CodeGenModule::EmitConstantValueForMemor return C; } +llvm::GlobalVariable *CodeGenModule::getAddrOfConstantCompoundLiteralIfEmitted( + const CompoundLiteralExpr *E) { + return EmittedCompoundLiterals.lookup(E); +} + +void CodeGenModule::setAddrOfConstantCompoundLiteral( + const CompoundLiteralExpr *CLE, llvm::GlobalVariable *GV) { + bool Ok = EmittedCompoundLiterals.insert(std::make_pair(CLE, GV)).second; + (void)Ok; + assert(Ok && "CLE has already been emitted!"); +} + ConstantAddress CodeGenModule::GetAddrOfConstantCompoundLiteral(const CompoundLiteralExpr *E) { assert(E->isFileScope() && "not a file-scope compound literal expr"); Modified: cfe/trunk/lib/CodeGen/CodeGenModule.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.h?rev=290661&r1=290660&r2=290661&view=diff ============================================================================== --- cfe/trunk/lib/CodeGen/CodeGenModule.h (original) +++ cfe/trunk/lib/CodeGen/CodeGenModule.h Wed Dec 28 01:27:40 2016 @@ -455,6 +455,10 @@ private: bool isTriviallyRecursive(const FunctionDecl *F); bool shouldEmitFunction(GlobalDecl GD); + /// Map used to be sure we don't emit the same CompoundLiteral twice. + llvm::DenseMap<const CompoundLiteralExpr *, llvm::GlobalVariable *> + EmittedCompoundLiterals; + /// Map of the global blocks we've emitted, so that we don't have to re-emit /// them if the constexpr evaluator gets aggressive. llvm::DenseMap<const BlockExpr *, llvm::Constant *> EmittedGlobalBlocks; @@ -824,6 +828,16 @@ public: /// compound literal expression. ConstantAddress GetAddrOfConstantCompoundLiteral(const CompoundLiteralExpr*E); + /// If it's been emitted already, returns the GlobalVariable corresponding to + /// a compound literal. Otherwise, returns null. + llvm::GlobalVariable * + getAddrOfConstantCompoundLiteralIfEmitted(const CompoundLiteralExpr *E); + + /// Notes that CLE's GlobalVariable is GV. Asserts that CLE isn't already + /// emitted. + void setAddrOfConstantCompoundLiteral(const CompoundLiteralExpr *CLE, + llvm::GlobalVariable *GV); + /// \brief Returns a pointer to a global variable representing a temporary /// with static or thread storage duration. ConstantAddress GetAddrOfGlobalTemporary(const MaterializeTemporaryExpr *E, Modified: cfe/trunk/test/CodeGen/compound-literal.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/compound-literal.c?rev=290661&r1=290660&r2=290661&view=diff ============================================================================== --- cfe/trunk/test/CodeGen/compound-literal.c (original) +++ cfe/trunk/test/CodeGen/compound-literal.c Wed Dec 28 01:27:40 2016 @@ -1,5 +1,10 @@ // RUN: %clang_cc1 -triple x86_64-apple-darwin -emit-llvm %s -o - | FileCheck %s +// Capture the type and name so matching later is cleaner. +struct CompoundTy { int a; }; +// CHECK: @MyCLH = constant [[MY_CLH:[^,]+]] +const struct CompoundTy *const MyCLH = &(struct CompoundTy){3}; + int* a = &(int){1}; struct s {int a, b, c;} * b = &(struct s) {1, 2, 3}; _Complex double * x = &(_Complex double){1.0f}; @@ -66,3 +71,14 @@ struct G g(int x, int y, int z) { // CHECK-NEXT: [[T0:%.*]] = load i48, i48* [[COERCE_TEMP]] // CHECK-NEXT: ret i48 [[T0]] } + +// We had a bug where we'd emit a new GlobalVariable for each time we used a +// const pointer to a variable initialized by a compound literal. +// CHECK-LABEL: define i32 @compareMyCLH() #0 +int compareMyCLH() { + // CHECK: store i8* bitcast ([[MY_CLH]] to i8*) + const void *a = MyCLH; + // CHECK: store i8* bitcast ([[MY_CLH]] to i8*) + const void *b = MyCLH; + return a == b; +} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits