[PATCH] D35413: [LLD][ELF] Add DefinedInThunk to SymbolBody to remove need for hash lookup

[Rafael Avila de Espindola via llvm-commits]

The use of the hash table and the proposed new member is so that, for
example, two arm relocations to a thumb function use a single thunk,
correct?

If so the hash table is only needed during the one pass when we are
mutating relocations to point to the thunk.

If the above is correct, a hash table seems better IMHO since it is only
used in a very specific point in the code.

Cheers,
Rafael

Peter Smith via Phabricator <reviews at reviews.llvm.org> writes:

> peter.smith updated this revision to Diff 109959.
> peter.smith added a comment.
>
> Rebased patch, no other changes.
>
>
> https://reviews.llvm.org/D35413
>
> Files:
>   ELF/Relocations.cpp
>   ELF/Relocations.h
>   ELF/Symbols.h
>
>
> Index: ELF/Symbols.h
> ===================================================================
> --- ELF/Symbols.h
> +++ ELF/Symbols.h
> @@ -33,6 +33,8 @@
>  class OutputSection;
>  template <class ELFT> class SharedFile;
>  
> +class Thunk;
> +
>  struct Symbol;
>  
>  // The base class for real symbol classes.
> @@ -89,6 +91,9 @@
>    template <class ELFT> typename ELFT::uint getSize() const;
>    OutputSection *getOutputSection() const;
>  
> +  // The Thunk that this symbol is defined in, nullptr if not defined in Thunk
> +  Thunk *LabeledThunk = nullptr;
> +
>    uint32_t DynsymIndex = 0;
>    uint32_t GotIndex = -1;
>    uint32_t GotPltIndex = -1;
> Index: ELF/Relocations.h
> ===================================================================
> --- ELF/Relocations.h
> +++ ELF/Relocations.h
> @@ -153,10 +153,6 @@
>    // 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 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.
> Index: ELF/Relocations.cpp
> ===================================================================
> --- ELF/Relocations.cpp
> +++ ELF/Relocations.cpp
> @@ -1249,7 +1249,7 @@
>  // was originally to a Thunk, but is no longer in range we revert the
>  // relocation back to its original non-Thunk target.
>  bool ThunkCreator::normalizeExistingThunk(Relocation &Rel, uint64_t Src) {
> -  if (Thunk *ET = Thunks.lookup(Rel.Sym)) {
> +  if (Thunk *ET = Rel.Sym->LabeledThunk) {
>      if (Target->inBranchRange(Rel.Type, Src, Rel.Sym->getVA()))
>        return true;
>      Rel.Sym = &ET->Destination;
> @@ -1328,7 +1328,7 @@
>                else
>                  TS = getISRThunkSec(OS, IS, ISR, Rel.Type, Src);
>                TS->addThunk(T);
> -              Thunks[T->ThunkSym] = T;
> +              T->ThunkSym->LabeledThunk = T;
>              }
>              // Redirect relocation to Thunk, we never go via the PLT to a Thunk
>              Rel.Sym = T->ThunkSym;
>
>
> Index: ELF/Symbols.h
> ===================================================================
> --- ELF/Symbols.h
> +++ ELF/Symbols.h
> @@ -33,6 +33,8 @@
>  class OutputSection;
>  template <class ELFT> class SharedFile;
>  
> +class Thunk;
> +
>  struct Symbol;
>  
>  // The base class for real symbol classes.
> @@ -89,6 +91,9 @@
>    template <class ELFT> typename ELFT::uint getSize() const;
>    OutputSection *getOutputSection() const;
>  
> +  // The Thunk that this symbol is defined in, nullptr if not defined in Thunk
> +  Thunk *LabeledThunk = nullptr;
> +
>    uint32_t DynsymIndex = 0;
>    uint32_t GotIndex = -1;
>    uint32_t GotPltIndex = -1;
> Index: ELF/Relocations.h
> ===================================================================
> --- ELF/Relocations.h
> +++ ELF/Relocations.h
> @@ -153,10 +153,6 @@
>    // 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 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.
> Index: ELF/Relocations.cpp
> ===================================================================
> --- ELF/Relocations.cpp
> +++ ELF/Relocations.cpp
> @@ -1249,7 +1249,7 @@
>  // was originally to a Thunk, but is no longer in range we revert the
>  // relocation back to its original non-Thunk target.
>  bool ThunkCreator::normalizeExistingThunk(Relocation &Rel, uint64_t Src) {
> -  if (Thunk *ET = Thunks.lookup(Rel.Sym)) {
> +  if (Thunk *ET = Rel.Sym->LabeledThunk) {
>      if (Target->inBranchRange(Rel.Type, Src, Rel.Sym->getVA()))
>        return true;
>      Rel.Sym = &ET->Destination;
> @@ -1328,7 +1328,7 @@
>                else
>                  TS = getISRThunkSec(OS, IS, ISR, Rel.Type, Src);
>                TS->addThunk(T);
> -              Thunks[T->ThunkSym] = T;
> +              T->ThunkSym->LabeledThunk = T;
>              }
>              // Redirect relocation to Thunk, we never go via the PLT to a Thunk
>              Rel.Sym = T->ThunkSym;