[lld] r243496 - [ELF2] Devirtualize SymbolBody.

[Rafael Espíndola via llvm-commits]

I just noticed this commit is *way* worse than I first thought. You
are getting a hard coded string table and doing that for every symbol!

I will fix it, it is just faster that way.

On 28 July 2015 at 18:58, Michael J. Spencer <bigcheesegs at gmail.com> wrote:
> Author: mspencer
> Date: Tue Jul 28 17:58:25 2015
> New Revision: 243496
>
> URL: http://llvm.org/viewvc/llvm-project?rev=243496&view=rev
> Log:
> [ELF2] Devirtualize SymbolBody.
>
> Modified:
>     lld/trunk/ELF/InputFiles.cpp
>     lld/trunk/ELF/Symbols.cpp
>     lld/trunk/ELF/Symbols.h
>
> Modified: lld/trunk/ELF/InputFiles.cpp
> URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/InputFiles.cpp?rev=243496&r1=243495&r2=243496&view=diff
> ==============================================================================
> --- lld/trunk/ELF/InputFiles.cpp (original)
> +++ lld/trunk/ELF/InputFiles.cpp Tue Jul 28 17:58:25 2015
> @@ -65,7 +65,7 @@ SymbolBody *elf2::ObjectFile<ELFT>::crea
>    StringRef Name = *NameOrErr;
>    if (Sym->isUndefined())
>      return new (Alloc) Undefined(Name);
> -  return new (Alloc) DefinedRegular<ELFT>(Name);
> +  return new (Alloc) DefinedRegular<ELFT>(this, Sym);
>  }
>
>  namespace lld {
>
> Modified: lld/trunk/ELF/Symbols.cpp
> URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/Symbols.cpp?rev=243496&r1=243495&r2=243496&view=diff
> ==============================================================================
> --- lld/trunk/ELF/Symbols.cpp (original)
> +++ lld/trunk/ELF/Symbols.cpp Tue Jul 28 17:58:25 2015
> @@ -9,12 +9,30 @@
>
>  #include "Symbols.h"
>  #include "Chunks.h"
> +#include "InputFiles.h"
>
>  using namespace llvm::object;
>
>  using namespace lld;
>  using namespace lld::elf2;
>
> +template <class ELFT>
> +StringRef
> +getSymbolName(const llvm::object::ELFFile<ELFT> *F,
> +  const typename llvm::object::ELFFile<ELFT>::Elf_Sym *S) {
> +  ErrorOr<StringRef> StrTab = F->getStringTableForSymtab(*F->getDotSymtabSec());
> +  if (!StrTab || S->st_name >= StrTab->size())
> +    llvm::report_fatal_error("Invalid string table.");
> +  return StrTab->data() + S->st_name;
> +}
> +
> +template <class ELFT>
> +DefinedRegular<ELFT>::DefinedRegular(ObjectFile<ELFT> *F, const Elf_Sym *S)
> +    : Defined(DefinedRegularKind, getSymbolName<ELFT>(F->getObj(), S)), File(F),
> +      Sym(S) {
> +  IsExternal = S->isExternal();
> +}
> +
>  // Returns 1, 0 or -1 if this symbol should take precedence
>  // over the Other, tie or lose, respectively.
>  int SymbolBody::compare(SymbolBody *Other) {
>
> Modified: lld/trunk/ELF/Symbols.h
> URL: http://llvm.org/viewvc/llvm-project/lld/trunk/ELF/Symbols.h?rev=243496&r1=243495&r2=243496&view=diff
> ==============================================================================
> --- lld/trunk/ELF/Symbols.h (original)
> +++ lld/trunk/ELF/Symbols.h Tue Jul 28 17:58:25 2015
> @@ -21,6 +21,7 @@ using llvm::object::ELFFile;
>  class Chunk;
>  class InputFile;
>  class SymbolBody;
> +template <class ELFT> class ObjectFile;
>
>  // A real symbol object, SymbolBody, is usually accessed indirectly
>  // through a Symbol. There's always one Symbol for each symbol name.
> @@ -40,14 +41,13 @@ public:
>      UndefinedKind,
>    };
>
> -  Kind kind() const { return SymbolKind; }
> -  virtual ~SymbolBody() {}
> +  Kind kind() const { return static_cast<Kind>(SymbolKind); }
>
>    // Returns true if this is an external symbol.
> -  virtual bool isExternal() { return true; }
> +  bool isExternal() const { return true; }
>
>    // Returns the symbol name.
> -  virtual StringRef getName() = 0;
> +  StringRef getName() const { return Name; }
>
>    // A SymbolBody has a backreference to a Symbol. Originally they are
>    // doubly-linked. A backreference will never change. But the pointer
> @@ -64,10 +64,13 @@ public:
>    int compare(SymbolBody *Other);
>
>  protected:
> -  SymbolBody(Kind K) : SymbolKind(K) {}
> +  SymbolBody(Kind K, StringRef N = "")
> +      : SymbolKind(K), IsExternal(true), Name(N) {}
>
> -private:
> -  const Kind SymbolKind;
> +protected:
> +  const unsigned SymbolKind : 8;
> +  unsigned IsExternal : 1;
> +  StringRef Name;
>    Symbol *Backref = nullptr;
>  };
>
> @@ -75,7 +78,7 @@ private:
>  // etc.
>  class Defined : public SymbolBody {
>  public:
> -  Defined(Kind K) : SymbolBody(K) {}
> +  Defined(Kind K, StringRef N = "") : SymbolBody(K, N) {}
>
>    static bool classof(const SymbolBody *S) {
>      Kind K = S->kind();
> @@ -85,31 +88,28 @@ public:
>
>  // Regular defined symbols read from object file symbol tables.
>  template <class ELFT> class DefinedRegular : public Defined {
> +  typedef typename llvm::object::ELFFile<ELFT>::Elf_Sym Elf_Sym;
> +
>  public:
> -  DefinedRegular(StringRef Name) : Defined(DefinedRegularKind), Name(Name) {}
> +  DefinedRegular(ObjectFile<ELFT> *F, const Elf_Sym *S);
>
>    static bool classof(const SymbolBody *S) {
>      return S->kind() == DefinedRegularKind;
>    }
>
> -  StringRef getName() override { return Name; }
> -
>  private:
> -  StringRef Name;
> +  ObjectFile<ELFT> *File;
> +  const Elf_Sym *Sym;
>  };
>
>  // Undefined symbols.
>  class Undefined : public SymbolBody {
>  public:
> -  explicit Undefined(StringRef N) : SymbolBody(UndefinedKind), Name(N) {}
> +  explicit Undefined(StringRef N) : SymbolBody(UndefinedKind, N) {}
>
>    static bool classof(const SymbolBody *S) {
>      return S->kind() == UndefinedKind;
>    }
> -  StringRef getName() override { return Name; }
> -
> -private:
> -  StringRef Name;
>  };
>
>  } // namespace elf2
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits