Object: Simplify the IRObjectFile symbol iterator implementation.

[pcc]

Summary

Change the IRObjectFile symbol iterator to be a pointer into a vector of
PointerUnions representing either IR symbols or asm symbols. While at it,
teach IRObjectFile to handle ifuncs.

This change is in preparation for a future change for supporting multiple
modules in an IRObjectFile. Although it causes an increase in memory
consumption, we can deal with that issue separately by introducing a bitcode
symbol table.

[pcc]

Add test

[pcc]

• Remove ifunc support

[mehdi_amini]

LGTM.

llvm/include/llvm/Object/IRObjectFile.h

32

I haven't tracked how the AsmSymbol are passed in, but couldn't we keep a StringRef instead of a std::string?
(I know it is not directly related to your patch)

[pcc]

llvm/include/llvm/Object/IRObjectFile.h

32

These strings are owned by the StringMap in RecordStreamer, so we can't.

llvm/include/llvm/Object/IRObjectFile.h

 //===- IRObjectFile.h - LLVM IR object file implementation ------*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This file declares the IRObjectFile template class.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_OBJECT_IROBJECTFILE_H
 #define LLVM_OBJECT_IROBJECTFILE_H
 
+#include "llvm/ADT/PointerUnion.h"
 #include "llvm/Object/SymbolicFile.h"
 
 namespace llvm {
 class Mangler;
 class Module;
 class GlobalValue;
 class Triple;
 
 namespace object {
 class ObjectFile;
 
 class IRObjectFile : public SymbolicFile {
   std::unique_ptr<Module> M;
   std::unique_ptr<Mangler> Mang;
-  std::vector<std::pair<std::string, uint32_t>> AsmSymbols;
+  typedef std::pair<std::string, uint32_t> AsmSymbol;
+  SpecificBumpPtrAllocator<AsmSymbol> AsmSymbols;
+
+  typedef PointerUnion<GlobalValue *, AsmSymbol *> Sym;
+  std::vector<Sym> SymTab;
+  static Sym getSym(DataRefImpl &Symb) {
+    return *reinterpret_cast<Sym *>(Symb.p);
+  }
 
 public:
   IRObjectFile(MemoryBufferRef Object, std::unique_ptr<Module> M);
   ~IRObjectFile() override;
   void moveSymbolNext(DataRefImpl &Symb) const override;
   std::error_code printSymbolName(raw_ostream &OS,
                                   DataRefImpl Symb) const override;
   uint32_t getSymbolFlags(DataRefImpl Symb) const override;

llvm/lib/Object/IRObjectFile.cpp

 #include "llvm/Support/TargetRegistry.h"
 #include "llvm/Support/raw_ostream.h"
 using namespace llvm;
 using namespace object;
 
 IRObjectFile::IRObjectFile(MemoryBufferRef Object, std::unique_ptr<Module> Mod)
     : SymbolicFile(Binary::ID_IR, Object), M(std::move(Mod)) {
   Mang.reset(new Mangler());
+
+  for (Function &F : *M)
+    SymTab.push_back(&F);
+  for (GlobalVariable &GV : M->globals())
+    SymTab.push_back(&GV);
+  for (GlobalAlias &GA : M->aliases())
+    SymTab.push_back(&GA);
+
   CollectAsmUndefinedRefs(Triple(M->getTargetTriple()), M->getModuleInlineAsm(),
                           [this](StringRef Name, BasicSymbolRef::Flags Flags) {
-                            AsmSymbols.emplace_back(Name, std::move(Flags));
+                            SymTab.push_back(new (AsmSymbols.Allocate())
+                                                 AsmSymbol(Name, Flags));
                           });
 }
 
 // Parse inline ASM and collect the list of symbols that are not defined in
 // the current module. This is inspired from IRObjectFile.
 void IRObjectFile::CollectAsmUndefinedRefs(
     const Triple &TT, StringRef InlineAsm,
     function_ref<void(StringRef, BasicSymbolRef::Flags)> AsmUndefinedRefs) {
for (auto &KV : Streamer) {
     case RecordStreamer::UndefinedWeak:
       Res |= BasicSymbolRef::SF_Weak;
       Res |= BasicSymbolRef::SF_Undefined;
     }
     AsmUndefinedRefs(Key, BasicSymbolRef::Flags(Res));
   }
 }
 
-IRObjectFile::~IRObjectFile() {
- }
-
-static GlobalValue *getGV(DataRefImpl &Symb) {
-  if ((Symb.p & 3) == 3)
-    return nullptr;
-
-  return reinterpret_cast<GlobalValue*>(Symb.p & ~uintptr_t(3));
-}
-
-static uintptr_t skipEmpty(Module::const_alias_iterator I, const Module &M) {
-  if (I == M.alias_end())
-    return 3;
-  const GlobalValue *GV = &*I;
-  return reinterpret_cast<uintptr_t>(GV) | 2;
-}
-
-static uintptr_t skipEmpty(Module::const_global_iterator I, const Module &M) {
-  if (I == M.global_end())
-    return skipEmpty(M.alias_begin(), M);
-  const GlobalValue *GV = &*I;
-  return reinterpret_cast<uintptr_t>(GV) | 1;
-}
-
-static uintptr_t skipEmpty(Module::const_iterator I, const Module &M) {
-  if (I == M.end())
-    return skipEmpty(M.global_begin(), M);
-  const GlobalValue *GV = &*I;
-  return reinterpret_cast<uintptr_t>(GV) | 0;
-}
-
-static unsigned getAsmSymIndex(DataRefImpl Symb) {
-  assert((Symb.p & uintptr_t(3)) == 3);
-  uintptr_t Index = Symb.p & ~uintptr_t(3);
-  Index >>= 2;
-  return Index;
-}
+IRObjectFile::~IRObjectFile() {}
 
 void IRObjectFile::moveSymbolNext(DataRefImpl &Symb) const {
-  const GlobalValue *GV = getGV(Symb);
-  uintptr_t Res;
-
-  switch (Symb.p & 3) {
-  case 0: {
-    Module::const_iterator Iter(static_cast<const Function*>(GV));
-    ++Iter;
-    Res = skipEmpty(Iter, *M);
-    break;
-  }
-  case 1: {
-    Module::const_global_iterator Iter(static_cast<const GlobalVariable*>(GV));
-    ++Iter;
-    Res = skipEmpty(Iter, *M);
-    break;
-  }
-  case 2: {
-    Module::const_alias_iterator Iter(static_cast<const GlobalAlias*>(GV));
-    ++Iter;
-    Res = skipEmpty(Iter, *M);
-    break;
-  }
-  case 3: {
-    unsigned Index = getAsmSymIndex(Symb);
-    assert(Index < AsmSymbols.size());
-    ++Index;
-    Res = (Index << 2) | 3;
-    break;
-  }
-  default:
-    llvm_unreachable("unreachable case");
-  }
-
-  Symb.p = Res;
+  Symb.p += sizeof(Sym);
 }
 
 std::error_code IRObjectFile::printSymbolName(raw_ostream &OS,
                                               DataRefImpl Symb) const {
-  const GlobalValue *GV = getGV(Symb);
-  if (!GV) {
-    unsigned Index = getAsmSymIndex(Symb);
-    assert(Index <= AsmSymbols.size());
-    OS << AsmSymbols[Index].first;
+  Sym S = getSym(Symb);
+  if (S.is<AsmSymbol *>()) {
+    OS << S.get<AsmSymbol *>()->first;
     return std::error_code();
   }
 
+  auto *GV = S.get<GlobalValue *>();
   if (GV->hasDLLImportStorageClass())
     OS << "__imp_";
 
   if (Mang)
     Mang->getNameWithPrefix(OS, GV, false);
   else
     OS << GV->getName();
 
   return std::error_code();
 }
 
 uint32_t IRObjectFile::getSymbolFlags(DataRefImpl Symb) const {
-  const GlobalValue *GV = getGV(Symb);
+  Sym S = getSym(Symb);
+  if (S.is<AsmSymbol *>())
+    return S.get<AsmSymbol *>()->second;
 
-  if (!GV) {
-    unsigned Index = getAsmSymIndex(Symb);
-    assert(Index <= AsmSymbols.size());
-    return AsmSymbols[Index].second;
-  }
+  auto *GV = S.get<GlobalValue *>();
 
   uint32_t Res = BasicSymbolRef::SF_None;
   if (GV->isDeclarationForLinker())
     Res |= BasicSymbolRef::SF_Undefined;
   else if (GV->hasHiddenVisibility() && !GV->hasLocalLinkage())
     Res |= BasicSymbolRef::SF_Hidden;
   if (const GlobalVariable *GVar = dyn_cast<GlobalVariable>(GV)) {
     if (GVar->isConstant())
uint32_t IRObjectFile::getSymbolFlags(DataRefImpl Symb) const {
   else if (auto *Var = dyn_cast<GlobalVariable>(GV)) {
     if (Var->getSection() == "llvm.metadata")
       Res |= BasicSymbolRef::SF_FormatSpecific;
   }
 
   return Res;
 }
 
-GlobalValue *IRObjectFile::getSymbolGV(DataRefImpl Symb) { return getGV(Symb); }
+GlobalValue *IRObjectFile::getSymbolGV(DataRefImpl Symb) {
+  return getSym(Symb).dyn_cast<GlobalValue *>();
+}
 
 std::unique_ptr<Module> IRObjectFile::takeModule() { return std::move(M); }
 
 basic_symbol_iterator IRObjectFile::symbol_begin() const {
-  Module::const_iterator I = M->begin();
   DataRefImpl Ret;
-  Ret.p = skipEmpty(I, *M);
+  Ret.p = reinterpret_cast<uintptr_t>(SymTab.data());
   return basic_symbol_iterator(BasicSymbolRef(Ret, this));
 }
 
 basic_symbol_iterator IRObjectFile::symbol_end() const {
   DataRefImpl Ret;
-  uint64_t NumAsm = AsmSymbols.size();
-  NumAsm <<= 2;
-  Ret.p = 3 | NumAsm;
+  Ret.p = reinterpret_cast<uintptr_t>(SymTab.data() + SymTab.size());
   return basic_symbol_iterator(BasicSymbolRef(Ret, this));
 }
 
 ErrorOr<MemoryBufferRef> IRObjectFile::findBitcodeInObject(const ObjectFile &Obj) {
   for (const SectionRef &Sec : Obj.sections()) {
     if (Sec.isBitcode()) {
       StringRef SecContents;
       if (std::error_code EC = Sec.getContents(SecContents))