[MC] Eliminate two symbol-related hash maps (llvm#95464)

Previously, a symbol insertion requires (at least) three hash table
operations:

- Lookup/create entry in Symbols (main symbol table)
- Lookup NextUniqueID to deduplicate identical temporary labels
- Add entry to UsedNames, which is also used to serve as storage for the
symbol name in the MCSymbol.

All three lookups are done with the same name, so combining these into a
single table reduces the number of lookups to one. Thus, a pointer to a
symbol table entry can be passed to createSymbol to avoid a duplicate
lookup of the same name.

The new symbol table entry value is placed in a separate header to avoid
including MCContext in MCSymbol or vice versa.

Author: aengelke

bolt/lib/Passes/BinaryPasses.cpp

@@ -636,7 +636,9 @@ Error LowerAnnotations::runOnFunctions(BinaryContext &BC) {
 Error CleanMCState::runOnFunctions(BinaryContext &BC) {
   MCContext &Ctx = *BC.Ctx;
   for (const auto &SymMapEntry : Ctx.getSymbols()) {
-    const MCSymbol *S = SymMapEntry.second;
+    const MCSymbol *S = SymMapEntry.getValue().Symbol;
+    if (!S)
+      continue;
     if (S->isDefined()) {
       LLVM_DEBUG(dbgs() << "BOLT-DEBUG: Symbol \"" << S->getName()
                         << "\" is already defined\n");

llvm/include/llvm/MC/MCContext.h

@@ -21,6 +21,7 @@
 #include "llvm/MC/MCDwarf.h"
 #include "llvm/MC/MCPseudoProbe.h"
 #include "llvm/MC/MCSection.h"
+#include "llvm/MC/MCSymbolTableEntry.h"
 #include "llvm/MC/SectionKind.h"
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/Compiler.h"
@@ -81,7 +82,7 @@ struct WasmSignature;
 ///
 class MCContext {
 public:
-  using SymbolTable = StringMap<MCSymbol *, BumpPtrAllocator &>;
+  using SymbolTable = StringMap<MCSymbolTableValue, BumpPtrAllocator &>;
   using DiagHandlerTy =
       std::function<void(const SMDiagnostic &, bool, const SourceMgr &,
                          std::vector<const MDNode *> &)>;
@@ -147,7 +148,7 @@ class MCContext {
 
   SpecificBumpPtrAllocator<wasm::WasmSignature> WasmSignatureAllocator;
 
-  /// Bindings of names to symbols.
+  /// Bindings of names to symbol table values.
   SymbolTable Symbols;
 
   /// A mapping from a local label number and an instance count to a symbol.
@@ -158,18 +159,8 @@ class MCContext {
   /// We have three labels represented by the pairs (1, 0), (2, 0) and (1, 1)
   DenseMap<std::pair<unsigned, unsigned>, MCSymbol *> LocalSymbols;
 
-  /// Keeps tracks of names that were used both for used declared and
-  /// artificial symbols. The value is "true" if the name has been used for a
-  /// non-section symbol (there can be at most one of those, plus an unlimited
-  /// number of section symbols with the same name).
-  StringMap<bool, BumpPtrAllocator &> UsedNames;
-
   /// Keeps track of labels that are used in inline assembly.
-  SymbolTable InlineAsmUsedLabelNames;
-
-  /// The next ID to dole out to an unnamed assembler temporary symbol with
-  /// a given prefix.
-  StringMap<unsigned> NextID;
+  StringMap<MCSymbol *, BumpPtrAllocator &> InlineAsmUsedLabelNames;
 
   /// Instances of directional local labels.
   DenseMap<unsigned, MCLabel *> Instances;
@@ -348,10 +339,11 @@ class MCContext {
 
   MCDataFragment *allocInitialFragment(MCSection &Sec);
 
-  MCSymbol *createSymbolImpl(const StringMapEntry<bool> *Name,
-                             bool IsTemporary);
-  MCSymbol *createSymbol(StringRef Name, bool AlwaysAddSuffix,
-                         bool IsTemporary);
+  MCSymbolTableEntry &getSymbolTableEntry(StringRef Name);
+
+  MCSymbol *createSymbolImpl(const MCSymbolTableEntry *Name, bool IsTemporary);
+  MCSymbol *createRenamableSymbol(const Twine &Name, bool AlwaysAddSuffix,
+                                  bool IsTemporary);
 
   MCSymbol *getOrCreateDirectionalLocalSymbol(unsigned LocalLabelVal,
                                               unsigned Instance);
@@ -362,7 +354,7 @@ class MCContext {
                                      unsigned UniqueID,
                                      const MCSymbolELF *LinkedToSym);
 
-  MCSymbolXCOFF *createXCOFFSymbolImpl(const StringMapEntry<bool> *Name,
+  MCSymbolXCOFF *createXCOFFSymbolImpl(const MCSymbolTableEntry *Name,
                                        bool IsTemporary);
 
   /// Map of currently defined macros.

llvm/include/llvm/MC/MCSymbol.h

@@ -17,6 +17,7 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/MC/MCExpr.h"
 #include "llvm/MC/MCFragment.h"
+#include "llvm/MC/MCSymbolTableEntry.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/MathExtras.h"
 #include <cassert>
@@ -156,11 +157,11 @@ class MCSymbol {
   /// system, the name is a pointer so isn't going to satisfy the 8 byte
   /// alignment of uint64_t.  Account for that here.
   using NameEntryStorageTy = union {
-    const StringMapEntry<bool> *NameEntry;
+    const MCSymbolTableEntry *NameEntry;
     uint64_t AlignmentPadding;
   };
 
-  MCSymbol(SymbolKind Kind, const StringMapEntry<bool> *Name, bool isTemporary)
+  MCSymbol(SymbolKind Kind, const MCSymbolTableEntry *Name, bool isTemporary)
       : IsTemporary(isTemporary), IsRedefinable(false), IsUsed(false),
         IsRegistered(false), IsExternal(false), IsPrivateExtern(false),
         IsWeakExternal(false), Kind(Kind), IsUsedInReloc(false),
@@ -173,8 +174,7 @@ class MCSymbol {
 
   // Provide custom new/delete as we will only allocate space for a name
   // if we need one.
-  void *operator new(size_t s, const StringMapEntry<bool> *Name,
-                     MCContext &Ctx);
+  void *operator new(size_t s, const MCSymbolTableEntry *Name, MCContext &Ctx);
 
 private:
   void operator delete(void *);
@@ -188,12 +188,12 @@ class MCSymbol {
   }
 
   /// Get a reference to the name field.  Requires that we have a name
-  const StringMapEntry<bool> *&getNameEntryPtr() {
+  const MCSymbolTableEntry *&getNameEntryPtr() {
     assert(HasName && "Name is required");
     NameEntryStorageTy *Name = reinterpret_cast<NameEntryStorageTy *>(this);
     return (*(Name - 1)).NameEntry;
   }
-  const StringMapEntry<bool> *&getNameEntryPtr() const {
+  const MCSymbolTableEntry *&getNameEntryPtr() const {
     return const_cast<MCSymbol*>(this)->getNameEntryPtr();
   }
 

llvm/include/llvm/MC/MCSymbolCOFF.h

@@ -11,6 +11,7 @@
 
 #include "llvm/BinaryFormat/COFF.h"
 #include "llvm/MC/MCSymbol.h"
+#include "llvm/MC/MCSymbolTableEntry.h"
 #include <cstdint>
 
 namespace llvm {
@@ -29,7 +30,7 @@ class MCSymbolCOFF : public MCSymbol {
   };
 
 public:
-  MCSymbolCOFF(const StringMapEntry<bool> *Name, bool isTemporary)
+  MCSymbolCOFF(const MCSymbolTableEntry *Name, bool isTemporary)
       : MCSymbol(SymbolKindCOFF, Name, isTemporary) {}
 
   uint16_t getType() const {

llvm/include/llvm/MC/MCSymbolELF.h

@@ -9,6 +9,7 @@
 #define LLVM_MC_MCSYMBOLELF_H
 
 #include "llvm/MC/MCSymbol.h"
+#include "llvm/MC/MCSymbolTableEntry.h"
 
 namespace llvm {
 class MCSymbolELF : public MCSymbol {
@@ -17,7 +18,7 @@ class MCSymbolELF : public MCSymbol {
   const MCExpr *SymbolSize = nullptr;
 
 public:
-  MCSymbolELF(const StringMapEntry<bool> *Name, bool isTemporary)
+  MCSymbolELF(const MCSymbolTableEntry *Name, bool isTemporary)
       : MCSymbol(SymbolKindELF, Name, isTemporary) {}
   void setSize(const MCExpr *SS) { SymbolSize = SS; }
 

llvm/include/llvm/MC/MCSymbolGOFF.h

@@ -14,12 +14,13 @@
 #define LLVM_MC_MCSYMBOLGOFF_H
 
 #include "llvm/MC/MCSymbol.h"
+#include "llvm/MC/MCSymbolTableEntry.h"
 
 namespace llvm {
 
 class MCSymbolGOFF : public MCSymbol {
 public:
-  MCSymbolGOFF(const StringMapEntry<bool> *Name, bool IsTemporary)
+  MCSymbolGOFF(const MCSymbolTableEntry *Name, bool IsTemporary)
       : MCSymbol(SymbolKindGOFF, Name, IsTemporary) {}
   static bool classof(const MCSymbol *S) { return S->isGOFF(); }
 };

llvm/include/llvm/MC/MCSymbolMachO.h

@@ -10,6 +10,7 @@
 
 #include "llvm/ADT/Twine.h"
 #include "llvm/MC/MCSymbol.h"
+#include "llvm/MC/MCSymbolTableEntry.h"
 
 namespace llvm {
 class MCSymbolMachO : public MCSymbol {
@@ -42,7 +43,7 @@ class MCSymbolMachO : public MCSymbol {
   };
 
 public:
-  MCSymbolMachO(const StringMapEntry<bool> *Name, bool isTemporary)
+  MCSymbolMachO(const MCSymbolTableEntry *Name, bool isTemporary)
       : MCSymbol(SymbolKindMachO, Name, isTemporary) {}
 
   // Reference type methods.

llvm/include/llvm/MC/MCSymbolTableEntry.h

@@ -0,0 +1,45 @@
+//===-- llvm/MC/MCSymbolTableEntry.h - Symbol table entry -------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_MC_MCSYMBOLTABLEENTRY_H
+#define LLVM_MC_MCSYMBOLTABLEENTRY_H
+
+#include "llvm/ADT/StringMapEntry.h"
+
+namespace llvm {
+
+class MCSymbol;
+
+/// The value for an entry in the symbol table of an MCContext.
+///
+/// This is in a separate file, because MCSymbol uses MCSymbolTableEntry (see
+/// below) to reuse the name that is stored in the symbol table.
+struct MCSymbolTableValue {
+  /// The symbol associated with the name, if any.
+  MCSymbol *Symbol = nullptr;
+
+  /// The next ID to dole out to an unnamed assembler temporary symbol with
+  /// the prefix (symbol table key).
+  unsigned NextUniqueID = 0;
+
+  /// Whether the name associated with this value is used for a symbol. This is
+  /// not necessarily true: sometimes, we use a symbol table value without an
+  /// associated symbol for accessing NextUniqueID when a suffix is added to a
+  /// name. However, Used might be true even if Symbol is nullptr: temporary
+  /// named symbols are not added to the symbol table.
+  bool Used = false;
+};
+
+/// MCContext stores MCSymbolTableValue in a string map (see MCSymbol::operator
+/// new). To avoid redundant storage of the name, MCSymbol stores a pointer (8
+/// bytes -- half the size of a StringRef) to the entry to access it.
+using MCSymbolTableEntry = StringMapEntry<MCSymbolTableValue>;
+
+} // end namespace llvm
+
+#endif

llvm/include/llvm/MC/MCSymbolWasm.h

@@ -10,6 +10,7 @@
 
 #include "llvm/BinaryFormat/Wasm.h"
 #include "llvm/MC/MCSymbol.h"
+#include "llvm/MC/MCSymbolTableEntry.h"
 
 namespace llvm {
 
@@ -33,7 +34,7 @@ class MCSymbolWasm : public MCSymbol {
   const MCExpr *SymbolSize = nullptr;
 
 public:
-  MCSymbolWasm(const StringMapEntry<bool> *Name, bool isTemporary)
+  MCSymbolWasm(const MCSymbolTableEntry *Name, bool isTemporary)
       : MCSymbol(SymbolKindWasm, Name, isTemporary) {}
   static bool classof(const MCSymbol *S) { return S->isWasm(); }
 

llvm/include/llvm/MC/MCSymbolXCOFF.h

@@ -11,6 +11,7 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/BinaryFormat/XCOFF.h"
 #include "llvm/MC/MCSymbol.h"
+#include "llvm/MC/MCSymbolTableEntry.h"
 
 namespace llvm {
 
@@ -21,7 +22,7 @@ class MCSymbolXCOFF : public MCSymbol {
   enum XCOFFSymbolFlags : uint16_t { SF_EHInfo = 0x0001 };
 
 public:
-  MCSymbolXCOFF(const StringMapEntry<bool> *Name, bool isTemporary)
+  MCSymbolXCOFF(const MCSymbolTableEntry *Name, bool isTemporary)
       : MCSymbol(SymbolKindXCOFF, Name, isTemporary) {}
 
   static bool classof(const MCSymbol *S) { return S->isXCOFF(); }