[llvm] r268606 - Revert "LTOCodeGenerator: turns linkonce(_odr) into weak_(odr) when present "MustPreserve" set"

[Rafael Espíndola via llvm-commits]

Interesting.

To fix https://llvm.org/bugs/show_bug.cgi?id=27553 I intend to add an
attribute that says that as symbol can be omitted. I guess as long as
that is computed early, this transformation could just preserve that
attribute.

Cheers,
Rafael


On 5 May 2016 at 01:14, Mehdi Amini via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
> Author: mehdi_amini
> Date: Thu May  5 00:14:20 2016
> New Revision: 268606
>
> URL: http://llvm.org/viewvc/llvm-project?rev=268606&view=rev
> Log:
> Revert "LTOCodeGenerator: turns linkonce(_odr) into weak_(odr) when present "MustPreserve" set"
>
> This reverts commit r267644. Turning linkonce_odr into weak_odr is
> a sementic change on Darwin: because of
> `llvm::canBeOmittedFromSymbolTable()` we may emit the symbol as
> weak_def_can_be_hidden instead of weak_definition.
>
> From: Mehdi Amini <mehdi.amini at apple.com>
>
> Modified:
>     llvm/trunk/lib/LTO/LTOCodeGenerator.cpp
>     llvm/trunk/test/tools/lto/hide-linkonce-odr.ll
>
> Modified: llvm/trunk/lib/LTO/LTOCodeGenerator.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/LTO/LTOCodeGenerator.cpp?rev=268606&r1=268605&r2=268606&view=diff
> ==============================================================================
> --- llvm/trunk/lib/LTO/LTOCodeGenerator.cpp (original)
> +++ llvm/trunk/lib/LTO/LTOCodeGenerator.cpp Thu May  5 00:14:20 2016
> @@ -348,62 +348,14 @@ std::unique_ptr<TargetMachine> LTOCodeGe
>                                   RelocModel, CodeModel::Default, CGOptLevel));
>  }
>
> -// If a linkonce global is present in the MustPreserveSymbols, we need to make
> -// sure we honor this. To force the compiler to not drop it, we turn its linkage
> -// to the weak equivalent.
> -static void
> -preserveDiscardableGVs(Module &TheModule,
> -                       function_ref<bool(const GlobalValue &)> mustPreserveGV) {
> -  auto mayPreserveGlobal = [&](GlobalValue &GV) {
> -    if (!GV.isDiscardableIfUnused() || GV.isDeclaration())
> -      return;
> -    if (!mustPreserveGV(GV))
> -      return;
> -    if (GV.hasAvailableExternallyLinkage() || GV.hasLocalLinkage())
> -      report_fatal_error("The linker asked LTO to preserve a symbol with an"
> -                         "unexpected linkage");
> -    GV.setLinkage(GlobalValue::getWeakLinkage(GV.hasLinkOnceODRLinkage()));
> -  };
> -
> -  for (auto &GV : TheModule)
> -    mayPreserveGlobal(GV);
> -  for (auto &GV : TheModule.globals())
> -    mayPreserveGlobal(GV);
> -  for (auto &GV : TheModule.aliases())
> -    mayPreserveGlobal(GV);
> -}
> -
>  void LTOCodeGenerator::applyScopeRestrictions() {
> -  if (ScopeRestrictionsDone)
> -    return;
> -
> -  // Declare a callback for the internalize pass that will ask for every
> -  // candidate GlobalValue if it can be internalized or not.
> -  SmallString<64> MangledName;
> -  auto mustPreserveGV = [&](const GlobalValue &GV) -> bool {
> -    // Unnamed globals can't be mangled, but they can't be preserved either.
> -    if (!GV.hasName())
> -      return false;
> -
> -    // Need to mangle the GV as the "MustPreserveSymbols" StringSet is filled
> -    // with the linker supplied name, which on Darwin includes a leading
> -    // underscore.
> -    MangledName.clear();
> -    MangledName.reserve(GV.getName().size() + 1);
> -    Mangler::getNameWithPrefix(MangledName, GV.getName(),
> -                               MergedModule->getDataLayout());
> -    return MustPreserveSymbols.count(MangledName);
> -  };
> -
> -  // Preserve linkonce value on linker request
> -  preserveDiscardableGVs(*MergedModule, mustPreserveGV);
> -
> -  if (!ShouldInternalize)
> +  if (ScopeRestrictionsDone || !ShouldInternalize)
>      return;
>
>    if (ShouldRestoreGlobalsLinkage) {
>      // Record the linkage type of non-local symbols so they can be restored
> -    // prior to module splitting.
> +    // prior
> +    // to module splitting.
>      auto RecordLinkage = [&](const GlobalValue &GV) {
>        if (!GV.hasAvailableExternallyLinkage() && !GV.hasLocalLinkage() &&
>            GV.hasName())
> @@ -421,7 +373,22 @@ void LTOCodeGenerator::applyScopeRestric
>    // symbols referenced from asm
>    UpdateCompilerUsed(*MergedModule, *TargetMach, AsmUndefinedRefs);
>
> -  internalizeModule(*MergedModule, mustPreserveGV);
> +  // Declare a callback for the internalize pass that will ask for every
> +  // candidate GlobalValue if it can be internalized or not.
> +  Mangler Mangler;
> +  SmallString<64> MangledName;
> +  auto MustPreserveGV = [&](const GlobalValue &GV) -> bool {
> +    // Need to mangle the GV as the "MustPreserveSymbols" StringSet is filled
> +    // with the linker supplied name, which on Darwin includes a leading
> +    // underscore.
> +    MangledName.clear();
> +    MangledName.reserve(GV.getName().size() + 1);
> +    Mangler::getNameWithPrefix(MangledName, GV.getName(),
> +                               MergedModule->getDataLayout());
> +    return MustPreserveSymbols.count(MangledName);
> +  };
> +
> +  internalizeModule(*MergedModule, MustPreserveGV);
>
>    ScopeRestrictionsDone = true;
>  }
>
> Modified: llvm/trunk/test/tools/lto/hide-linkonce-odr.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/lto/hide-linkonce-odr.ll?rev=268606&r1=268605&r2=268606&view=diff
> ==============================================================================
> --- llvm/trunk/test/tools/lto/hide-linkonce-odr.ll (original)
> +++ llvm/trunk/test/tools/lto/hide-linkonce-odr.ll Thu May  5 00:14:20 2016
> @@ -1,11 +1,9 @@
>  ; RUN: llvm-as %s -o %t.o
> -; RUN: %ld64 -lto_library %llvmshlibdir/libLTO.dylib -dylib -arch x86_64 -macosx_version_min 10.10.0 -lSystem -o %t.dylib %t.o -save-temps  -undefined dynamic_lookup -exported_symbol _c -exported_symbol _b
> +; RUN: %ld64 -lto_library %llvmshlibdir/libLTO.dylib -dylib -arch x86_64 -macosx_version_min 10.10.0 -lSystem -o %t.dylib %t.o -save-temps  -undefined dynamic_lookup
>
>  ; RUN: llvm-dis %t.dylib.lto.opt.bc -o - | FileCheck --check-prefix=IR %s
> -; check that @a is no longer a linkonce_odr definition
> -; IR-NOT: define linkonce_odr void @a()
> -; check that @b is turned into weak because it is exported
> -; IR: define weak_odr void @b() #1 {
> +; check that @a is still a linkonce_odr definition
> +; IR: define linkonce_odr void @a()
>
>  ; RUN: llvm-nm %t.dylib | FileCheck --check-prefix=NM %s
>  ; check that the linker can hide @a but not @b
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits