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

Pete Cooper via llvm-commits llvm-commits at lists.llvm.org
Thu May 5 11:26:32 PDT 2016


Great.  Thanks Mehdi.

Pete
> On May 5, 2016, at 11:21 AM, Mehdi Amini <mehdi.amini at apple.com> wrote:
> 
> Let's see if r268658 solves it!
> 
> -- 
> Mehdi
> 
>> On May 5, 2016, at 11:15 AM, Pete Cooper <peter_cooper at apple.com> wrote:
>> 
>> 
>>> On May 5, 2016, at 11:12 AM, Mehdi Amini <mehdi.amini at apple.com> wrote:
>>> 
>>> Looks like my code's broken somehow. Investigating…
>> Thanks!
>> 
>> Pete
>>> 
>>> -- 
>>> Mehdi
>>> 
>>>> On May 5, 2016, at 11:09 AM, Pete Cooper <peter_cooper at apple.com> wrote:
>>>> 
>>>> Hi Rafael, Mehdi
>>>> 
>>>> This is sort of related, so apologies for stealing this thread…
>>>> 
>>>> I’m seeing an LTO failure on the bots which is related to these changes.  Unfortunately the prior builds also had similar issues so i’m not sure where the real problem started (one of these commits may have just moved around the assert we trigger when refactoring).
>>>> 
>>>> Any ideas?  You both know this code much more than I do.  The latest failure is in the link below, with the build before it showing the previous failure.
>>>> 
>>>> http://lab.llvm.org:8080/green/job/clang-stage2-configure-Rlto_build/8875/console
>>>> 
>>>> Cheers,
>>>> Pete
>>>>> On May 5, 2016, at 6:21 AM, Rafael Espíndola <rafael.espindola at gmail.com> wrote:
>>>>> 
>>>>> 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
>>>> 
>>> 
>> 
> 



More information about the llvm-commits mailing list