r236879 - Do not emit thunks with available_externally linkage in comdats

NAKAMURA Takumi geek4civic at gmail.com
Sat May 9 14:21:06 PDT 2015


Excuse me, I have reverted it in r236937.

$ clang++ -std=gnu++11  -target i686-cygwin -O3 -S -o - d.ii -o - |
fgrep ZThn4_N4llvm19RTDyldMemoryManager10findSymbolERKSs | fgrep
.section

Excuse me again, I was lazy not to reduce the testcase.

2015-05-09 1:47 GMT+09:00 Derek Schuff <dschuff at google.com>:
> Author: dschuff
> Date: Fri May  8 11:47:21 2015
> New Revision: 236879
>
> URL: http://llvm.org/viewvc/llvm-project?rev=236879&view=rev
> Log:
> 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
>
> Modified:
>     cfe/trunk/lib/CodeGen/CGDecl.cpp
>     cfe/trunk/lib/CodeGen/CGVTT.cpp
>     cfe/trunk/lib/CodeGen/CGVTables.cpp
>     cfe/trunk/lib/CodeGen/CodeGenModule.cpp
>     cfe/trunk/lib/CodeGen/CodeGenModule.h
>     cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
>     cfe/trunk/test/CodeGenCXX/thunks.cpp
>
> Modified: cfe/trunk/lib/CodeGen/CGDecl.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDecl.cpp?rev=236879&r1=236878&r2=236879&view=diff
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CGDecl.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGDecl.cpp Fri May  8 11:47:21 2015
> @@ -210,8 +210,7 @@ llvm::Constant *CodeGenModule::getOrCrea
>    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);
>
> Modified: cfe/trunk/lib/CodeGen/CGVTT.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGVTT.cpp?rev=236879&r1=236878&r2=236879&view=diff
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CGVTT.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGVTT.cpp Fri May  8 11:47:21 2015
> @@ -94,8 +94,7 @@ CodeGenVTables::EmitVTTDefinition(llvm::
>    // 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::getSecondaryVirtualPoint
>
>    return I->second;
>  }
> -
>
> Modified: cfe/trunk/lib/CodeGen/CGVTables.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGVTables.cpp?rev=236879&r1=236878&r2=236879&view=diff
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CGVTables.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGVTables.cpp Fri May  8 11:47:21 2015
> @@ -378,9 +378,6 @@ void CodeGenFunction::GenerateThunk(llvm
>    // 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<CXXMethodDecl>(GD.getDecl());
>    setThunkVisibility(CGM, MD, Thunk, Fn);
> @@ -461,6 +458,7 @@ void CodeGenVTables::emitThunk(GlobalDec
>      CGM.getCXXABI().setThunkLinkage(ThunkFn, ForVTable, GD,
>                                      !Thunk.Return.isEmpty());
>    }
> +  CGM.maybeSetTrivialComdat(*ThunkFn);
>  }
>
>  void CodeGenVTables::maybeEmitThunkForVTable(GlobalDecl GD,
>
> Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=236879&r1=236878&r2=236879&view=diff
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Fri May  8 11:47:21 2015
> @@ -1298,8 +1298,7 @@ llvm::Constant *CodeGenModule::GetAddrOf
>    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::CreateOrReplaceCXXRuntime
>      OldGV->eraseFromParent();
>    }
>
> -  if (supportsCOMDAT() && GV->isWeakForLinker() &&
> -      !GV->hasAvailableExternallyLinkage())
> -    GV->setComdat(TheModule.getOrInsertComdat(GV->getName()));
> +  maybeSetTrivialComdat(*GV);
>
>    return GV;
>  }
> @@ -1985,6 +1982,14 @@ void CodeGenModule::maybeSetTrivialComda
>    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,
>        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::GetAddrOf
>    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;
>
> Modified: cfe/trunk/lib/CodeGen/CodeGenModule.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.h?rev=236879&r1=236878&r2=236879&view=diff
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CodeGenModule.h (original)
> +++ cfe/trunk/lib/CodeGen/CodeGenModule.h Fri May  8 11:47:21 2015
> @@ -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; }
>
> Modified: cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp?rev=236879&r1=236878&r2=236879&view=diff
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp (original)
> +++ cfe/trunk/lib/CodeGen/ItaniumCXXABI.cpp Fri May  8 11:47:21 2015
> @@ -1328,8 +1328,7 @@ void ItaniumCXXABI::emitVTableDefinition
>    // 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(Code
>      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::Buil
>        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 *getClangCallTermi
>      // 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 =
>
> Modified: cfe/trunk/test/CodeGenCXX/thunks.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/thunks.cpp?rev=236879&r1=236878&r2=236879&view=diff
> ==============================================================================
> --- cfe/trunk/test/CodeGenCXX/thunks.cpp (original)
> +++ cfe/trunk/test/CodeGenCXX/thunks.cpp Fri May  8 11:47:21 2015
> @@ -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 <class C>
> +struct A {
> +  virtual void f();
> +};
> +
> +template <class D>
> +struct B : virtual A<D> {
> +  virtual void f() { }
> +};
> +
> +extern template struct B<int>;
> +
> +void f(B<int> 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{{.*}} }
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
-------------- next part --------------
A non-text attachment was scrubbed...
Name: d.ii.gz
Type: application/x-gzip
Size: 358932 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150510/b3b4d57c/attachment.bin>


More information about the cfe-commits mailing list