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

[Rafael Avila de Espindola via llvm-commits]

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