From b3458674c25e62ac911be7e75eaf9f26fae39051 Mon Sep 17 00:00:00 2001 From: Derek Schuff Date: Fri, 8 May 2015 16:47:21 +0000 Subject: [PATCH] Do not emit thunks with available_externally linkage in comdats Functions with available_externally linkage will not be emitted to object files (they will just be undefined symbols), so it does not make sense to put them in comdats. Creates a second overload of maybeSetTrivialComdat that uses the GlobalObject instead of the Decl, and uses that in several places that had the faulty logic. Differential Revision: http://reviews.llvm.org/D9580 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@236879 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/CodeGen/CGDecl.cpp | 3 +-- lib/CodeGen/CGVTT.cpp | 4 +--- lib/CodeGen/CGVTables.cpp | 4 +--- lib/CodeGen/CodeGenModule.cpp | 23 ++++++++++++----------- lib/CodeGen/CodeGenModule.h | 1 + lib/CodeGen/ItaniumCXXABI.cpp | 13 +++++-------- test/CodeGenCXX/thunks.cpp | 27 ++++++++++++++++++++++++++- 7 files changed, 47 insertions(+), 28 deletions(-) diff --git a/lib/CodeGen/CGDecl.cpp b/lib/CodeGen/CGDecl.cpp index fdbf1f62d7..eb9e2e28a9 100644 --- a/lib/CodeGen/CGDecl.cpp +++ b/lib/CodeGen/CGDecl.cpp @@ -210,8 +210,7 @@ llvm::Constant *CodeGenModule::getOrCreateStaticVarDecl( GV->setAlignment(getContext().getDeclAlign(&D).getQuantity()); setGlobalVisibility(GV, &D); - if (supportsCOMDAT() && GV->isWeakForLinker()) - GV->setComdat(TheModule.getOrInsertComdat(GV->getName())); + maybeSetTrivialComdat(*GV); if (D.getTLSKind()) setTLSMode(GV, D); diff --git a/lib/CodeGen/CGVTT.cpp b/lib/CodeGen/CGVTT.cpp index 895afd75af..aca9e7a3e6 100644 --- a/lib/CodeGen/CGVTT.cpp +++ b/lib/CodeGen/CGVTT.cpp @@ -94,8 +94,7 @@ CodeGenVTables::EmitVTTDefinition(llvm::GlobalVariable *VTT, // Set the correct linkage. VTT->setLinkage(Linkage); - if (CGM.supportsCOMDAT() && VTT->isWeakForLinker()) - VTT->setComdat(CGM.getModule().getOrInsertComdat(VTT->getName())); + CGM.maybeSetTrivialComdat(*VTT); // Set the right visibility. CGM.setGlobalVisibility(VTT, RD); @@ -177,4 +176,3 @@ CodeGenVTables::getSecondaryVirtualPointerIndex(const CXXRecordDecl *RD, return I->second; } - diff --git a/lib/CodeGen/CGVTables.cpp b/lib/CodeGen/CGVTables.cpp index 57370a6faa..7359bf4214 100644 --- a/lib/CodeGen/CGVTables.cpp +++ b/lib/CodeGen/CGVTables.cpp @@ -378,9 +378,6 @@ void CodeGenFunction::GenerateThunk(llvm::Function *Fn, // Set the right linkage. CGM.setFunctionLinkage(GD, Fn); - if (CGM.supportsCOMDAT() && Fn->isWeakForLinker()) - Fn->setComdat(CGM.getModule().getOrInsertComdat(Fn->getName())); - // Set the right visibility. const CXXMethodDecl *MD = cast(GD.getDecl()); setThunkVisibility(CGM, MD, Thunk, Fn); @@ -461,6 +458,7 @@ void CodeGenVTables::emitThunk(GlobalDecl GD, const ThunkInfo &Thunk, CGM.getCXXABI().setThunkLinkage(ThunkFn, ForVTable, GD, !Thunk.Return.isEmpty()); } + CGM.maybeSetTrivialComdat(*ThunkFn); } void CodeGenVTables::maybeEmitThunkForVTable(GlobalDecl GD, diff --git a/lib/CodeGen/CodeGenModule.cpp b/lib/CodeGen/CodeGenModule.cpp index d75dcd2213..b3d7b5d4f6 100644 --- a/lib/CodeGen/CodeGenModule.cpp +++ b/lib/CodeGen/CodeGenModule.cpp @@ -1298,8 +1298,7 @@ llvm::Constant *CodeGenModule::GetAddrOfUuidDescriptor( auto *GV = new llvm::GlobalVariable( getModule(), Init->getType(), /*isConstant=*/true, llvm::GlobalValue::LinkOnceODRLinkage, Init, Name); - if (supportsCOMDAT()) - GV->setComdat(TheModule.getOrInsertComdat(GV->getName())); + maybeSetTrivialComdat(*GV); return GV; } @@ -1850,9 +1849,7 @@ CodeGenModule::CreateOrReplaceCXXRuntimeVariable(StringRef Name, OldGV->eraseFromParent(); } - if (supportsCOMDAT() && GV->isWeakForLinker() && - !GV->hasAvailableExternallyLinkage()) - GV->setComdat(TheModule.getOrInsertComdat(GV->getName())); + maybeSetTrivialComdat(*GV); return GV; } @@ -1985,6 +1982,14 @@ void CodeGenModule::maybeSetTrivialComdat(const Decl &D, GO.setComdat(TheModule.getOrInsertComdat(GO.getName())); } +void CodeGenModule::maybeSetTrivialComdat(llvm::GlobalObject &GO) { + if (!supportsCOMDAT()) + return; + if (GO.isWeakForLinker() && !GO.hasAvailableExternallyLinkage()) { + GO.setComdat(getModule().getOrInsertComdat(GO.getName())); + } +} + void CodeGenModule::EmitGlobalVarDefinition(const VarDecl *D) { llvm::Constant *Init = nullptr; QualType ASTTy = D->getType(); @@ -2924,10 +2929,7 @@ GenerateStringLiteral(llvm::Constant *C, llvm::GlobalValue::LinkageTypes LT, nullptr, llvm::GlobalVariable::NotThreadLocal, AddrSpace); GV->setAlignment(Alignment); GV->setUnnamedAddr(true); - if (GV->isWeakForLinker()) { - assert(CGM.supportsCOMDAT() && "Only COFF uses weak string literals"); - GV->setComdat(M.getOrInsertComdat(GV->getName())); - } + CGM.maybeSetTrivialComdat(*GV); return GV; } @@ -3109,8 +3111,7 @@ llvm::Constant *CodeGenModule::GetAddrOfGlobalTemporary( setGlobalVisibility(GV, VD); GV->setAlignment( getContext().getTypeAlignInChars(MaterializedType).getQuantity()); - if (supportsCOMDAT() && GV->isWeakForLinker()) - GV->setComdat(TheModule.getOrInsertComdat(GV->getName())); + maybeSetTrivialComdat(*GV); if (VD->getTLSKind()) setTLSMode(GV, *VD); Slot = GV; diff --git a/lib/CodeGen/CodeGenModule.h b/lib/CodeGen/CodeGenModule.h index feef6c2583..6ce5c31cee 100644 --- a/lib/CodeGen/CodeGenModule.h +++ b/lib/CodeGen/CodeGenModule.h @@ -608,6 +608,7 @@ public: const llvm::Triple &getTriple() const; bool supportsCOMDAT() const; void maybeSetTrivialComdat(const Decl &D, llvm::GlobalObject &GO); + void maybeSetTrivialComdat(llvm::GlobalObject &GO); CGCXXABI &getCXXABI() const { return *ABI; } llvm::LLVMContext &getLLVMContext() { return VMContext; } diff --git a/lib/CodeGen/ItaniumCXXABI.cpp b/lib/CodeGen/ItaniumCXXABI.cpp index e8c28c1265..58786fe2ce 100644 --- a/lib/CodeGen/ItaniumCXXABI.cpp +++ b/lib/CodeGen/ItaniumCXXABI.cpp @@ -1328,8 +1328,7 @@ void ItaniumCXXABI::emitVTableDefinitions(CodeGenVTables &CGVT, // Set the correct linkage. VTable->setLinkage(Linkage); - if (CGM.supportsCOMDAT() && VTable->isWeakForLinker()) - VTable->setComdat(CGM.getModule().getOrInsertComdat(VTable->getName())); + CGM.maybeSetTrivialComdat(*VTable); // Set the right visibility. CGM.setGlobalVisibility(VTable, RD); @@ -1796,8 +1795,8 @@ void ItaniumCXXABI::EmitGuardedInit(CodeGenFunction &CGF, if (!D.isLocalVarDecl() && C) { guard->setComdat(C); CGF.CurFn->setComdat(C); - } else if (CGM.supportsCOMDAT() && guard->isWeakForLinker()) { - guard->setComdat(CGM.getModule().getOrInsertComdat(guard->getName())); + } else { + CGM.maybeSetTrivialComdat(*guard); } CGM.setStaticLocalDeclGuardAddress(&D, guard); @@ -2803,8 +2802,7 @@ llvm::Constant *ItaniumRTTIBuilder::BuildTypeInfo(QualType Ty, bool Force) { new llvm::GlobalVariable(M, Init->getType(), /*Constant=*/true, Linkage, Init, Name); - if (CGM.supportsCOMDAT() && GV->isWeakForLinker()) - GV->setComdat(M.getOrInsertComdat(GV->getName())); + CGM.maybeSetTrivialComdat(*GV); // If there's already an old global variable, replace it with the new one. if (OldGV) { @@ -3598,8 +3596,7 @@ static llvm::Constant *getClangCallTerminateFn(CodeGenModule &CGM) { // we don't want it to turn into an exported symbol. fn->setLinkage(llvm::Function::LinkOnceODRLinkage); fn->setVisibility(llvm::Function::HiddenVisibility); - if (CGM.supportsCOMDAT()) - fn->setComdat(CGM.getModule().getOrInsertComdat(fn->getName())); + CGM.maybeSetTrivialComdat(*fn); // Set up the function. llvm::BasicBlock *entry = diff --git a/test/CodeGenCXX/thunks.cpp b/test/CodeGenCXX/thunks.cpp index 2287d65a28..255017057c 100644 --- a/test/CodeGenCXX/thunks.cpp +++ b/test/CodeGenCXX/thunks.cpp @@ -1,5 +1,5 @@ // RUN: %clang_cc1 %s -triple=x86_64-pc-linux-gnu -munwind-tables -emit-llvm -o - | FileCheck %s -// RUN: %clang_cc1 %s -triple=x86_64-pc-linux-gnu -munwind-tables -emit-llvm -o - -O1 -disable-llvm-optzns | FileCheck %s +// RUN: %clang_cc1 %s -triple=x86_64-pc-linux-gnu -munwind-tables -emit-llvm -o - -O1 -disable-llvm-optzns | FileCheck %s --check-prefix=CHECK --check-prefix=CHECKOPT namespace Test1 { @@ -361,6 +361,28 @@ namespace Test15 { // CHECK: declare void @_ZThn8_N6Test151C1fEiz } +namespace Test16 { + +// Check that the thunk for 'B::f' has available_externally linkage +// and is not in a comdat. + +template +struct A { + virtual void f(); +}; + +template +struct B : virtual A { + virtual void f() { } +}; + +extern template struct B; + +void f(B b) { + b.f(); +} +} + /**** The following has to go at the end of the file ****/ // This is from Test5: @@ -371,4 +393,7 @@ namespace Test15 { // CHECK-LABEL: define linkonce_odr void @_ZN6Test101C3fooEv // CHECK-LABEL: define linkonce_odr void @_ZThn8_N6Test101C3fooEv +// CHECKOPT-LABEL: define available_externally void @_ZTv0_n24_N6Test161BIiE1fEv +// CHECKOPT-NOT: comdat + // CHECK: attributes [[NUW]] = { nounwind uwtable{{.*}} } -- 2.40.0