[llvm] r268607 - LTOCodeGenerator: add linkonce(_odr) to "llvm.compiler.used" when present in "MustPreserve" set

Mehdi Amini via llvm-commits llvm-commits at lists.llvm.org
Thu May 5 11:46:07 PDT 2016


> On May 5, 2016, at 11:37 AM, David Blaikie <dblaikie at gmail.com> wrote:
> 
> 
> 
> On Thu, May 5, 2016 at 11:30 AM, Mehdi Amini <mehdi.amini at apple.com <mailto:mehdi.amini at apple.com>> wrote:
> 
>> On May 5, 2016, at 11:25 AM, David Blaikie <dblaikie at gmail.com <mailto:dblaikie at gmail.com>> wrote:
>> 
>> 
>> 
>> On Wed, May 4, 2016 at 10:14 PM, Mehdi Amini via llvm-commits <llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>> wrote:
>> Author: mehdi_amini
>> Date: Thu May  5 00:14:24 2016
>> New Revision: 268607
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=268607&view=rev <http://llvm.org/viewvc/llvm-project?rev=268607&view=rev>
>> Log:
>> LTOCodeGenerator: add linkonce(_odr) to "llvm.compiler.used" when present in "MustPreserve" set
>> 
>> If the linker requested to preserve a linkonce function, we should
>> honor this even if we drop all uses.
>> We explicitely avoid turning them into weak_odr (unlike the first
>> version of this patch in r267644), because the codegen can be
>> different on Darwin: because of `llvm::canBeOmittedFromSymbolTable()`
>> we may emit the symbol as weak_def_can_be_hidden instead of
>> weak_definition.
>> 
>> Do we only do this on Darwin then, and otherwise turn it into weak_odr everywhere else?
> 
> To be sure I understand: you mean we would use llvm.compiler.used on Darwin, and the weak_odr conversion otherwise?
> 
> Yes - this, of course, isn't my wheelhouse, so there may be lots of good reasons not to do this, I'm just curious.
>  
> I'm not fan of special-casing, do you see a drawback with the llvm.compiler.used approach?
> 
> Don't know enough about it - sounds like it might lead to a different linkage in the generated object, which could have an impact on linking? or maybe it all comes out the same in the end?

My intention is to not change the linkage at all. My understanding was that only difference between linkonce and weak is that the former can be dropped if unused in the current module.
A mentioned in the commit message, I discovered that Darwin differs a little bit under some circumstances.

> But it seems like Darwin is the special case here - so it seems strange to do the same odd things to non-Darwin platforms in the name of consistency. But, again, I don't have lots of context here.

True. 
The context is: during LTO the linker ask "MustPreserve(foo)". LibLTO will not turn "foo" into internal linkage. However if "foo" is a linkonce and LTO is able to discard the uses in the LTO module, it will drop it. As a result the linker gets back an object without "foo" even though it asked for it to be "preserved".
To solve this, I'm trying to force the compiler to not drop it. Since the request to preserve it comes from the linker, llvm.compiler.used seems appropriate to express that the compiler should treat the symbol "as if there is a reference to the symbol that it cannot see" (citation from LangRef).

(that was my first patch, and then Duncan suggested to use the weak transformation instead for consistency with gold).

-- 
Mehdi


>  
> Ideally, I'd prefer to get ride of the "weak_def_can_be_hidden" thing we do at the MC level. It is not clear to me why it is not expressed at the IR level, but I haven't had time to give it more thoughts.
> 
> *nod*
> 
> - Dave
>  
> 
> 
> -- 
> Mehdi
> 
> 
> 
>>  
>> 
>> From: Mehdi Amini <mehdi.amini at apple.com <mailto: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=268607&r1=268606&r2=268607&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/LTO/LTOCodeGenerator.cpp?rev=268607&r1=268606&r2=268607&view=diff>
>> ==============================================================================
>> --- llvm/trunk/lib/LTO/LTOCodeGenerator.cpp (original)
>> +++ llvm/trunk/lib/LTO/LTOCodeGenerator.cpp Thu May  5 00:14:24 2016
>> @@ -348,8 +348,73 @@ 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 add it to the
>> +// "llvm.compiler.used" global.
>> +static void preserveDiscardableGVs(
>> +    Module &TheModule,
>> +    llvm::function_ref<bool(const GlobalValue &)> mustPreserveGV) {
>> +  SetVector<Constant *> UsedValuesSet;
>> +  if (GlobalVariable *LLVMUsed =
>> +          TheModule.getGlobalVariable("llvm.compiler.used")) {
>> +    ConstantArray *Inits = cast<ConstantArray>(LLVMUsed->getInitializer());
>> +    for (auto &V : Inits->operands())
>> +      UsedValuesSet.insert(cast<Constant>(&V));
>> +    LLVMUsed->eraseFromParent();
>> +  }
>> +  llvm::Type *i8PTy = llvm::Type::getInt8PtrTy(TheModule.getContext());
>> +  auto mayPreserveGlobal = [&](GlobalValue &GV) {
>> +    if (!GV.isDiscardableIfUnused() || GV.isDeclaration())
>> +      return;
>> +    if (!mustPreserveGV(GV))
>> +      return;
>> +    assert(!GV.hasAvailableExternallyLinkage() && !GV.hasInternalLinkage());
>> +    UsedValuesSet.insert(ConstantExpr::getBitCast(&GV, i8PTy));
>> +  };
>> +  for (auto &GV : TheModule)
>> +    mayPreserveGlobal(GV);
>> +  for (auto &GV : TheModule.globals())
>> +    mayPreserveGlobal(GV);
>> +  for (auto &GV : TheModule.aliases())
>> +    mayPreserveGlobal(GV);
>> +
>> +  if (UsedValuesSet.empty())
>> +    return;
>> +
>> +  llvm::ArrayType *ATy = llvm::ArrayType::get(i8PTy, UsedValuesSet.size());
>> +  auto *LLVMUsed = new llvm::GlobalVariable(
>> +      TheModule, ATy, false, llvm::GlobalValue::AppendingLinkage,
>> +      llvm::ConstantArray::get(ATy, UsedValuesSet.getArrayRef()),
>> +      "llvm.compiler.used");
>> +  LLVMUsed->setSection("llvm.metadata");
>> +}
>> +
>>  void LTOCodeGenerator::applyScopeRestrictions() {
>> -  if (ScopeRestrictionsDone || !ShouldInternalize)
>> +  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)
>>      return;
>> 
>>    if (ShouldRestoreGlobalsLinkage) {
>> @@ -373,22 +438,7 @@ void LTOCodeGenerator::applyScopeRestric
>>    // symbols referenced from asm
>>    UpdateCompilerUsed(*MergedModule, *TargetMach, AsmUndefinedRefs);
>> 
>> -  // 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);
>> +  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=268607&r1=268606&r2=268607&view=diff <http://llvm.org/viewvc/llvm-project/llvm/trunk/test/tools/lto/hide-linkonce-odr.ll?rev=268607&r1=268606&r2=268607&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:24 2016
>> @@ -1,21 +1,28 @@
>>  ; 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
>> +; 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  -exported_symbol _GlobLinkonce
>> 
>>  ; RUN: llvm-dis %t.dylib.lto.opt.bc -o - | FileCheck --check-prefix=IR %s
>> -; check that @a is still a linkonce_odr definition
>> -; IR: define linkonce_odr void @a()
>> +; check that @a is no longer a linkonce_odr definition
>> +; IR-NOT: define linkonce_odr void @a()
>> +; check that @b is appended in llvm.used
>> +; IR: @llvm.compiler.used = appending global [2 x i8*] [i8* bitcast ([1 x i8*]* @GlobLinkonce to i8*), i8* bitcast (void ()* @b to i8*)], section "llvm.metadata"
>> 
>>  ; RUN: llvm-nm %t.dylib | FileCheck --check-prefix=NM %s
>> -; check that the linker can hide @a but not @b
>> +; check that the linker can hide @a but not @b, nor @GlobLinkonce
>> +; NM: 0000000000000f48 S _GlobLinkonce
>>  ; NM: 0000000000000f10 t _a
>>  ; NM: 0000000000000f20 T _b
>>  ; NM: 0000000000000f00 T _c
>> 
>> +
>>  target triple = "x86_64-apple-macosx10.10.0"
>> 
>>  declare void @external()
>> 
>> + at GlobLinkonce = linkonce_odr unnamed_addr constant [1 x i8*] [i8* null], align 8
>> +
>>  define linkonce_odr void @a() noinline {
>> +  %use_of_GlobLinkonce = load [1 x i8*], [1 x i8*] *@GlobLinkonce
>>    call void @external()
>>    ret void
>>  }
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits <http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160505/c75bfb47/attachment.html>


More information about the llvm-commits mailing list