[PATCH] D34037: [LLD][ELF] Allow multiple thunks to be added for a symbol.

[Peter Smith via llvm-commits]

compatibleWith I kept to just whether the thunks could be reused.
There is an inBranchRange() function in
https://reviews.llvm.org/D34690 . Hooked up in
https://reviews.llvm.org/D34691 .

I'll rename compatibleWith tomorrow.

Thanks

Peter

On 4 July 2017 at 17:28, Rafael Avila de Espindola
<rafael.espindola at gmail.com> wrote:
> The idea is that compatibleWith should check ranges too?
>
> BTW, I just noticed that compatibleWith should probably be called isCompatibleWith.
>
> LGTM
>
> Cheers,
> Rafael
>
>
> Peter Smith via Phabricator <reviews at reviews.llvm.org> writes:
>
>> peter.smith updated this revision to Diff 102008.
>> peter.smith added a comment.
>>
>> Thanks for the comments. I've updated the diff to return {ET, false} as suggested.
>>
>>
>> https://reviews.llvm.org/D34037
>>
>> Files:
>>   ELF/Relocations.cpp
>>   ELF/Relocations.h
>>
>>
>> Index: ELF/Relocations.h
>> ===================================================================
>> --- ELF/Relocations.h
>> +++ ELF/Relocations.h
>> @@ -144,14 +144,17 @@
>>    std::pair<Thunk *, bool> getThunk(SymbolBody &Body, uint32_t Type);
>>    ThunkSection *addThunkSection(OutputSection *OS,
>>                                  std::vector<InputSection *> *, uint64_t Off);
>> -  // Track Symbols that already have a Thunk
>> -  llvm::DenseMap<SymbolBody *, Thunk *> ThunkedSymbols;
>> +  // Record all the available Thunks for a Symbol
>> +  llvm::DenseMap<SymbolBody *, std::vector<Thunk *>> ThunkedSymbols;
>>
>>    // Find a Thunk from the Thunks symbol definition, we can use this to find
>>    // the Thunk from a relocation to the Thunks symbol definition.
>>    llvm::DenseMap<SymbolBody *, Thunk *> Thunks;
>>
>> -  // Track InputSections that have a ThunkSection placed in front
>> +  // Track InputSections that have an inline ThunkSection placed in front
>> +  // an inline ThunkSection may have control fall through to the section below
>> +  // so we need to make sure that there is only one of them.
>> +  // The Mips LA25 Thunk is an example of an inline ThunkSection.
>>    llvm::DenseMap<InputSection *, ThunkSection *> ThunkedSections;
>>
>>    // All the ThunkSections that we have created, organised by OutputSection
>> Index: ELF/Relocations.cpp
>> ===================================================================
>> --- ELF/Relocations.cpp
>> +++ ELF/Relocations.cpp
>> @@ -1050,10 +1050,17 @@
>>
>>  std::pair<Thunk *, bool> ThunkCreator::getThunk(SymbolBody &Body,
>>                                                  uint32_t Type) {
>> -  auto res = ThunkedSymbols.insert({&Body, nullptr});
>> -  if (res.second || !res.first->second->compatibleWith(Type))
>> -    res.first->second = addThunk(Type, Body);
>> -  return std::make_pair(res.first->second, res.second);
>> +  auto Res = ThunkedSymbols.insert({&Body, std::vector<Thunk *>()});
>> +  if (!Res.second) {
>> +    // Check existing Thunks for Body to see if they can be reused
>> +    for (Thunk *ET : Res.first->second)
>> +      if (ET->compatibleWith(Type))
>> +        return {ET, false};
>> +  }
>> +  // No existing compatible Thunk in range, create a new one
>> +  Thunk *T = addThunk(Type, Body);
>> +  Res.first->second.push_back(T);
>> +  return {T, true};
>>  }
>>
>>  // Call Fn on every executable InputSection accessed via the linker script
>>
>>
>> Index: ELF/Relocations.h
>> ===================================================================
>> --- ELF/Relocations.h
>> +++ ELF/Relocations.h
>> @@ -144,14 +144,17 @@
>>    std::pair<Thunk *, bool> getThunk(SymbolBody &Body, uint32_t Type);
>>    ThunkSection *addThunkSection(OutputSection *OS,
>>                                  std::vector<InputSection *> *, uint64_t Off);
>> -  // Track Symbols that already have a Thunk
>> -  llvm::DenseMap<SymbolBody *, Thunk *> ThunkedSymbols;
>> +  // Record all the available Thunks for a Symbol
>> +  llvm::DenseMap<SymbolBody *, std::vector<Thunk *>> ThunkedSymbols;
>>
>>    // Find a Thunk from the Thunks symbol definition, we can use this to find
>>    // the Thunk from a relocation to the Thunks symbol definition.
>>    llvm::DenseMap<SymbolBody *, Thunk *> Thunks;
>>
>> -  // Track InputSections that have a ThunkSection placed in front
>> +  // Track InputSections that have an inline ThunkSection placed in front
>> +  // an inline ThunkSection may have control fall through to the section below
>> +  // so we need to make sure that there is only one of them.
>> +  // The Mips LA25 Thunk is an example of an inline ThunkSection.
>>    llvm::DenseMap<InputSection *, ThunkSection *> ThunkedSections;
>>
>>    // All the ThunkSections that we have created, organised by OutputSection
>> Index: ELF/Relocations.cpp
>> ===================================================================
>> --- ELF/Relocations.cpp
>> +++ ELF/Relocations.cpp
>> @@ -1050,10 +1050,17 @@
>>
>>  std::pair<Thunk *, bool> ThunkCreator::getThunk(SymbolBody &Body,
>>                                                  uint32_t Type) {
>> -  auto res = ThunkedSymbols.insert({&Body, nullptr});
>> -  if (res.second || !res.first->second->compatibleWith(Type))
>> -    res.first->second = addThunk(Type, Body);
>> -  return std::make_pair(res.first->second, res.second);
>> +  auto Res = ThunkedSymbols.insert({&Body, std::vector<Thunk *>()});
>> +  if (!Res.second) {
>> +    // Check existing Thunks for Body to see if they can be reused
>> +    for (Thunk *ET : Res.first->second)
>> +      if (ET->compatibleWith(Type))
>> +        return {ET, false};
>> +  }
>> +  // No existing compatible Thunk in range, create a new one
>> +  Thunk *T = addThunk(Type, Body);
>> +  Res.first->second.push_back(T);
>> +  return {T, true};
>>  }
>>
>>  // Call Fn on every executable InputSection accessed via the linker script