[llvm] [MC][ELF] Eliminate some hash maps from ELFObjectWriter (PR #97421)

Alexis Engelke via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 2 07:21:06 PDT 2024


https://github.com/aengelke created https://github.com/llvm/llvm-project/pull/97421

Remove some maps. Unfortunately, it didn't result in a measurable performance improvements, but at least some parts of this are a nice to have regardless.

- Replace SectionIndexMap with layout order: The section layout order is only used in MachO, so we can repurpose the field as section table index.
- Store section offsets in MCSectionELF: No need for a map, and especially not a std::map. Direct access to the underlying (and easily modifyable) data structure is always faster.
- Store signature group index in MCSymbolELF: This removes the need for a reverse lookup map.
- Improve storage of groups: There's no point in having a DenseMap, the number of sections and groups are reasonably small to use vectors.

---

I'm unsure about the third part, it increases the size of MCSymbolELF and therefore max-rss. On the other hand, MCSymbol currently has 28 padding bits and an extra four padding bytes (if I counted correctly, the bit fields sum up to 36), but this seems unintended. @MaskRay what do you think? Use padding bytes in MCSymbol or drop the third commit and fix MCSymbol bit fields later?

>From 11e95ce9dccbc19e9c0d22ef0a3ff587961a8692 Mon Sep 17 00:00:00 2001
From: Alexis Engelke <engelke at in.tum.de>
Date: Wed, 19 Jun 2024 16:09:14 +0200
Subject: [PATCH 1/4] [MC][ELF] Replace SectionIndexMap with layout order

The section layout order is only used in MachO, so we can repurpose the
field as section table index.
---
 llvm/lib/MC/ELFObjectWriter.cpp | 41 +++++++++++----------------------
 1 file changed, 13 insertions(+), 28 deletions(-)

diff --git a/llvm/lib/MC/ELFObjectWriter.cpp b/llvm/lib/MC/ELFObjectWriter.cpp
index bcc6dfeeeccd6..12d746b99f03a 100644
--- a/llvm/lib/MC/ELFObjectWriter.cpp
+++ b/llvm/lib/MC/ELFObjectWriter.cpp
@@ -66,8 +66,6 @@ using namespace llvm;
 
 namespace {
 
-using SectionIndexMapTy = DenseMap<const MCSectionELF *, uint32_t>;
-
 class ELFObjectWriter;
 struct ELFWriter;
 
@@ -181,10 +179,8 @@ struct ELFWriter {
   /// Compute the symbol table data
   ///
   /// \param Asm - The assembler.
-  /// \param SectionIndexMap - Maps a section to its index.
   /// \param RevGroupMap - Maps a signature symbol to the group section.
   void computeSymbolTable(MCAssembler &Asm,
-                          const SectionIndexMapTy &SectionIndexMap,
                           const RevGroupMapTy &RevGroupMap,
                           SectionOffsetsTy &SectionOffsets);
 
@@ -194,7 +190,6 @@ struct ELFWriter {
                                         const MCSectionELF &Sec);
 
   void writeSectionHeader(const MCAssembler &Asm,
-                          const SectionIndexMapTy &SectionIndexMap,
                           const SectionOffsetsTy &SectionOffsets);
 
   void writeSectionData(const MCAssembler &Asm, MCSection &Sec);
@@ -207,8 +202,7 @@ struct ELFWriter {
   void writeRelocations(const MCAssembler &Asm, const MCSectionELF &Sec);
 
   uint64_t writeObject(MCAssembler &Asm);
-  void writeSection(const SectionIndexMapTy &SectionIndexMap,
-                    uint32_t GroupSymbolIndex, uint64_t Offset, uint64_t Size,
+  void writeSection(uint32_t GroupSymbolIndex, uint64_t Offset, uint64_t Size,
                     const MCSectionELF &Section);
 };
 
@@ -612,7 +606,6 @@ bool ELFWriter::isInSymtab(const MCAssembler &Asm, const MCSymbolELF &Symbol,
 }
 
 void ELFWriter::computeSymbolTable(MCAssembler &Asm,
-                                   const SectionIndexMapTy &SectionIndexMap,
                                    const RevGroupMapTy &RevGroupMap,
                                    SectionOffsetsTy &SectionOffsets) {
   MCContext &Ctx = Asm.getContext();
@@ -697,7 +690,7 @@ void ELFWriter::computeSymbolTable(MCAssembler &Asm,
 
       if (Mode == NonDwoOnly && isDwoSection(Section))
         continue;
-      MSD.SectionIndex = SectionIndexMap.lookup(&Section);
+      MSD.SectionIndex = Section.getLayoutOrder();
       assert(MSD.SectionIndex && "Invalid section index!");
       if (MSD.SectionIndex >= ELF::SHN_LORESERVE)
         HasLargeSectionIndex = true;
@@ -1030,8 +1023,7 @@ void ELFWriter::writeRelocations(const MCAssembler &Asm,
   }
 }
 
-void ELFWriter::writeSection(const SectionIndexMapTy &SectionIndexMap,
-                             uint32_t GroupSymbolIndex, uint64_t Offset,
+void ELFWriter::writeSection(uint32_t GroupSymbolIndex, uint64_t Offset,
                              uint64_t Size, const MCSectionELF &Section) {
   uint64_t sh_link = 0;
   uint64_t sh_info = 0;
@@ -1050,7 +1042,7 @@ void ELFWriter::writeSection(const SectionIndexMapTy &SectionIndexMap,
     sh_link = SymbolTableIndex;
     assert(sh_link && ".symtab not found");
     const MCSection *InfoSection = Section.getLinkedToSection();
-    sh_info = SectionIndexMap.lookup(cast<MCSectionELF>(InfoSection));
+    sh_info = InfoSection->getLayoutOrder();
     break;
   }
 
@@ -1075,10 +1067,8 @@ void ELFWriter::writeSection(const SectionIndexMapTy &SectionIndexMap,
     // If the value in the associated metadata is not a definition, Sym will be
     // undefined. Represent this with sh_link=0.
     const MCSymbol *Sym = Section.getLinkedToSymbol();
-    if (Sym && Sym->isInSection()) {
-      const MCSectionELF *Sec = cast<MCSectionELF>(&Sym->getSection());
-      sh_link = SectionIndexMap.lookup(Sec);
-    }
+    if (Sym && Sym->isInSection())
+      sh_link = Sym->getSection().getLayoutOrder();
   }
 
   WriteSecHdrEntry(StrTabBuilder.getOffset(Section.getName()),
@@ -1088,7 +1078,6 @@ void ELFWriter::writeSection(const SectionIndexMapTy &SectionIndexMap,
 }
 
 void ELFWriter::writeSectionHeader(const MCAssembler &Asm,
-                                   const SectionIndexMapTy &SectionIndexMap,
                                    const SectionOffsetsTy &SectionOffsets) {
   const unsigned NumSections = SectionTable.size();
 
@@ -1113,8 +1102,7 @@ void ELFWriter::writeSectionHeader(const MCAssembler &Asm,
     else
       Size = Offsets.second - Offsets.first;
 
-    writeSection(SectionIndexMap, GroupSymbolIndex, Offsets.first, Size,
-                 *Section);
+    writeSection(GroupSymbolIndex, Offsets.first, Size, *Section);
   }
 }
 
@@ -1127,7 +1115,6 @@ uint64_t ELFWriter::writeObject(MCAssembler &Asm) {
   StringTableIndex = addToSectionTable(StrtabSection);
 
   RevGroupMapTy RevGroupMap;
-  SectionIndexMapTy SectionIndexMap;
 
   DenseMap<const MCSymbol *, SmallVector<const MCSectionELF *, 0>> GroupMembers;
 
@@ -1172,9 +1159,9 @@ uint64_t ELFWriter::writeObject(MCAssembler &Asm) {
         Members.push_back(RelSection);
     }
 
-    SectionIndexMap[&Section] = addToSectionTable(&Section);
+    Section.setLayoutOrder(addToSectionTable(&Section));
     if (RelSection) {
-      SectionIndexMap[RelSection] = addToSectionTable(RelSection);
+      RelSection->setLayoutOrder(addToSectionTable(RelSection));
       Relocations.push_back(RelSection);
     }
 
@@ -1188,10 +1175,8 @@ uint64_t ELFWriter::writeObject(MCAssembler &Asm) {
     const MCSymbol *SignatureSymbol = Group->getGroup();
     assert(SignatureSymbol);
     write(uint32_t(Group->isComdat() ? unsigned(ELF::GRP_COMDAT) : 0));
-    for (const MCSectionELF *Member : GroupMembers[SignatureSymbol]) {
-      uint32_t SecIndex = SectionIndexMap.lookup(Member);
-      write(SecIndex);
-    }
+    for (const MCSectionELF *Member : GroupMembers[SignatureSymbol])
+      write(Member->getLayoutOrder());
 
     uint64_t SecEnd = W.OS.tell();
     SectionOffsets[Group] = std::make_pair(SecStart, SecEnd);
@@ -1210,7 +1195,7 @@ uint64_t ELFWriter::writeObject(MCAssembler &Asm) {
     }
 
     // Compute symbol table information.
-    computeSymbolTable(Asm, SectionIndexMap, RevGroupMap, SectionOffsets);
+    computeSymbolTable(Asm, RevGroupMap, SectionOffsets);
 
     for (MCSectionELF *RelSection : Relocations) {
       // Remember the offset into the file for this section.
@@ -1240,7 +1225,7 @@ uint64_t ELFWriter::writeObject(MCAssembler &Asm) {
   const uint64_t SectionHeaderOffset = align(is64Bit() ? Align(8) : Align(4));
 
   // ... then the section header table ...
-  writeSectionHeader(Asm, SectionIndexMap, SectionOffsets);
+  writeSectionHeader(Asm, SectionOffsets);
 
   uint16_t NumSections = support::endian::byte_swap<uint16_t>(
       (SectionTable.size() + 1 >= ELF::SHN_LORESERVE) ? (uint16_t)ELF::SHN_UNDEF

>From a9f1734d2bd905083a4497fed2197c0b359a08a0 Mon Sep 17 00:00:00 2001
From: Alexis Engelke <engelke at in.tum.de>
Date: Wed, 19 Jun 2024 16:26:33 +0200
Subject: [PATCH 2/4] [MC][ELF] Store section offsets in MCSectionELF

No need for a map, and especially not a std::map. Direct access to the
underlying (and easily modifyable) data structure is always faster.
---
 llvm/include/llvm/MC/MCSectionELF.h | 12 ++++++++
 llvm/lib/MC/ELFObjectWriter.cpp     | 47 +++++++++++------------------
 2 files changed, 30 insertions(+), 29 deletions(-)

diff --git a/llvm/include/llvm/MC/MCSectionELF.h b/llvm/include/llvm/MC/MCSectionELF.h
index 3d45d3da10ca1..d43ffbd885c96 100644
--- a/llvm/include/llvm/MC/MCSectionELF.h
+++ b/llvm/include/llvm/MC/MCSectionELF.h
@@ -46,6 +46,10 @@ class MCSectionELF final : public MCSection {
   /// section header index of the section where LinkedToSym is defined.
   const MCSymbol *LinkedToSym;
 
+  /// Start/end offset in file, used by ELFWriter.
+  uint64_t StartOffset;
+  uint64_t EndOffset;
+
 private:
   friend class MCContext;
 
@@ -92,6 +96,14 @@ class MCSectionELF final : public MCSection {
   }
   const MCSymbol *getLinkedToSymbol() const { return LinkedToSym; }
 
+  void setOffsets(uint64_t Start, uint64_t End) {
+    StartOffset = Start;
+    EndOffset = End;
+  }
+  std::pair<uint64_t, uint64_t> getOffsets() const {
+    return std::make_pair(StartOffset, EndOffset);
+  }
+
   static bool classof(const MCSection *S) {
     return S->getVariant() == SV_ELF;
   }
diff --git a/llvm/lib/MC/ELFObjectWriter.cpp b/llvm/lib/MC/ELFObjectWriter.cpp
index 12d746b99f03a..77a56878f6d81 100644
--- a/llvm/lib/MC/ELFObjectWriter.cpp
+++ b/llvm/lib/MC/ELFObjectWriter.cpp
@@ -134,8 +134,8 @@ struct ELFWriter {
   unsigned SymbolTableIndex = ~0u;
 
   // Sections in the order they are to be output in the section table.
-  std::vector<const MCSectionELF *> SectionTable;
-  unsigned addToSectionTable(const MCSectionELF *Sec);
+  std::vector<MCSectionELF *> SectionTable;
+  unsigned addToSectionTable(MCSectionELF *Sec);
 
   // TargetObjectWriter wrappers.
   bool is64Bit() const;
@@ -169,10 +169,6 @@ struct ELFWriter {
   void writeSymbol(const MCAssembler &Asm, SymbolTableWriter &Writer,
                    uint32_t StringIndex, ELFSymbolData &MSD);
 
-  // Start and end offset of each section
-  using SectionOffsetsTy =
-      std::map<const MCSectionELF *, std::pair<uint64_t, uint64_t>>;
-
   // Map from a signature symbol to the group section index
   using RevGroupMapTy = DenseMap<const MCSymbol *, unsigned>;
 
@@ -181,16 +177,14 @@ struct ELFWriter {
   /// \param Asm - The assembler.
   /// \param RevGroupMap - Maps a signature symbol to the group section.
   void computeSymbolTable(MCAssembler &Asm,
-                          const RevGroupMapTy &RevGroupMap,
-                          SectionOffsetsTy &SectionOffsets);
+                          const RevGroupMapTy &RevGroupMap);
 
   void writeAddrsigSection();
 
   MCSectionELF *createRelocationSection(MCContext &Ctx,
                                         const MCSectionELF &Sec);
 
-  void writeSectionHeader(const MCAssembler &Asm,
-                          const SectionOffsetsTy &SectionOffsets);
+  void writeSectionHeader(const MCAssembler &Asm);
 
   void writeSectionData(const MCAssembler &Asm, MCSection &Sec);
 
@@ -324,7 +318,7 @@ uint64_t ELFWriter::align(Align Alignment) {
   return NewOffset;
 }
 
-unsigned ELFWriter::addToSectionTable(const MCSectionELF *Sec) {
+unsigned ELFWriter::addToSectionTable(MCSectionELF *Sec) {
   SectionTable.push_back(Sec);
   StrTabBuilder.add(Sec->getName());
   return SectionTable.size();
@@ -606,8 +600,7 @@ bool ELFWriter::isInSymtab(const MCAssembler &Asm, const MCSymbolELF &Symbol,
 }
 
 void ELFWriter::computeSymbolTable(MCAssembler &Asm,
-                                   const RevGroupMapTy &RevGroupMap,
-                                   SectionOffsetsTy &SectionOffsets) {
+                                   const RevGroupMapTy &RevGroupMap) {
   MCContext &Ctx = Asm.getContext();
   SymbolTableWriter Writer(*this, is64Bit());
 
@@ -768,7 +761,7 @@ void ELFWriter::computeSymbolTable(MCAssembler &Asm,
   }
 
   uint64_t SecEnd = W.OS.tell();
-  SectionOffsets[SymtabSection] = std::make_pair(SecStart, SecEnd);
+  SymtabSection->setOffsets(SecStart, SecEnd);
 
   ArrayRef<uint32_t> ShndxIndexes = Writer.getShndxIndexes();
   if (ShndxIndexes.empty()) {
@@ -778,12 +771,11 @@ void ELFWriter::computeSymbolTable(MCAssembler &Asm,
   assert(SymtabShndxSectionIndex != 0);
 
   SecStart = W.OS.tell();
-  const MCSectionELF *SymtabShndxSection =
-      SectionTable[SymtabShndxSectionIndex - 1];
+  MCSectionELF *SymtabShndxSection = SectionTable[SymtabShndxSectionIndex - 1];
   for (uint32_t Index : ShndxIndexes)
     write(Index);
   SecEnd = W.OS.tell();
-  SectionOffsets[SymtabShndxSection] = std::make_pair(SecStart, SecEnd);
+  SymtabShndxSection->setOffsets(SecStart, SecEnd);
 }
 
 void ELFWriter::writeAddrsigSection() {
@@ -1077,8 +1069,7 @@ void ELFWriter::writeSection(uint32_t GroupSymbolIndex, uint64_t Offset,
                    Section.getEntrySize());
 }
 
-void ELFWriter::writeSectionHeader(const MCAssembler &Asm,
-                                   const SectionOffsetsTy &SectionOffsets) {
+void ELFWriter::writeSectionHeader(const MCAssembler &Asm) {
   const unsigned NumSections = SectionTable.size();
 
   // Null section first.
@@ -1094,8 +1085,7 @@ void ELFWriter::writeSectionHeader(const MCAssembler &Asm,
     else
       GroupSymbolIndex = Section->getGroup()->getIndex();
 
-    const std::pair<uint64_t, uint64_t> &Offsets =
-        SectionOffsets.find(Section)->second;
+    std::pair<uint64_t, uint64_t> Offsets = Section->getOffsets();
     uint64_t Size;
     if (Type == ELF::SHT_NOBITS)
       Size = Asm.getSectionAddressSize(*Section);
@@ -1122,7 +1112,6 @@ uint64_t ELFWriter::writeObject(MCAssembler &Asm) {
   writeHeader(Asm);
 
   // ... then the sections ...
-  SectionOffsetsTy SectionOffsets;
   std::vector<MCSectionELF *> Groups;
   std::vector<MCSectionELF *> Relocations;
   for (MCSection &Sec : Asm) {
@@ -1139,7 +1128,7 @@ uint64_t ELFWriter::writeObject(MCAssembler &Asm) {
     writeSectionData(Asm, Section);
 
     uint64_t SecEnd = W.OS.tell();
-    SectionOffsets[&Section] = std::make_pair(SecStart, SecEnd);
+    Section.setOffsets(SecStart, SecEnd);
 
     MCSectionELF *RelSection = createRelocationSection(Ctx, Section);
 
@@ -1179,7 +1168,7 @@ uint64_t ELFWriter::writeObject(MCAssembler &Asm) {
       write(Member->getLayoutOrder());
 
     uint64_t SecEnd = W.OS.tell();
-    SectionOffsets[Group] = std::make_pair(SecStart, SecEnd);
+    Group->setOffsets(SecStart, SecEnd);
   }
 
   if (Mode == DwoOnly) {
@@ -1195,7 +1184,7 @@ uint64_t ELFWriter::writeObject(MCAssembler &Asm) {
     }
 
     // Compute symbol table information.
-    computeSymbolTable(Asm, RevGroupMap, SectionOffsets);
+    computeSymbolTable(Asm, RevGroupMap);
 
     for (MCSectionELF *RelSection : Relocations) {
       // Remember the offset into the file for this section.
@@ -1205,27 +1194,27 @@ uint64_t ELFWriter::writeObject(MCAssembler &Asm) {
                        cast<MCSectionELF>(*RelSection->getLinkedToSection()));
 
       uint64_t SecEnd = W.OS.tell();
-      SectionOffsets[RelSection] = std::make_pair(SecStart, SecEnd);
+      RelSection->setOffsets(SecStart, SecEnd);
     }
 
     if (OWriter.EmitAddrsigSection) {
       uint64_t SecStart = W.OS.tell();
       writeAddrsigSection();
       uint64_t SecEnd = W.OS.tell();
-      SectionOffsets[AddrsigSection] = std::make_pair(SecStart, SecEnd);
+      AddrsigSection->setOffsets(SecStart, SecEnd);
     }
   }
 
   {
     uint64_t SecStart = W.OS.tell();
     StrTabBuilder.write(W.OS);
-    SectionOffsets[StrtabSection] = std::make_pair(SecStart, W.OS.tell());
+    StrtabSection->setOffsets(SecStart, W.OS.tell());
   }
 
   const uint64_t SectionHeaderOffset = align(is64Bit() ? Align(8) : Align(4));
 
   // ... then the section header table ...
-  writeSectionHeader(Asm, SectionOffsets);
+  writeSectionHeader(Asm);
 
   uint16_t NumSections = support::endian::byte_swap<uint16_t>(
       (SectionTable.size() + 1 >= ELF::SHN_LORESERVE) ? (uint16_t)ELF::SHN_UNDEF

>From ece182bea6a8ca0ac0c59e455bd2b5614a3a59b0 Mon Sep 17 00:00:00 2001
From: Alexis Engelke <engelke at in.tum.de>
Date: Wed, 19 Jun 2024 16:46:24 +0200
Subject: [PATCH 3/4] [MC][ELF] Store signature group idx in MCSymbolELF

This removes the need for a reverse lookup map.
---
 llvm/include/llvm/MC/MCSymbolELF.h |  6 ++++++
 llvm/lib/MC/ELFObjectWriter.cpp    | 19 ++++++-------------
 2 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/llvm/include/llvm/MC/MCSymbolELF.h b/llvm/include/llvm/MC/MCSymbolELF.h
index 13c2c6b13f8e1..aa1531a897121 100644
--- a/llvm/include/llvm/MC/MCSymbolELF.h
+++ b/llvm/include/llvm/MC/MCSymbolELF.h
@@ -17,6 +17,9 @@ class MCSymbolELF : public MCSymbol {
   /// symbol has no size this field will be NULL.
   const MCExpr *SymbolSize = nullptr;
 
+  /// Group section index for signature symbols. Set by ELFWriter.
+  mutable unsigned GroupIndex = 0;
+
 public:
   MCSymbolELF(const MCSymbolTableEntry *Name, bool isTemporary)
       : MCSymbol(SymbolKindELF, Name, isTemporary) {}
@@ -44,6 +47,9 @@ class MCSymbolELF : public MCSymbol {
   void setIsSignature() const;
   bool isSignature() const;
 
+  void setGroupIndex(unsigned Index) const { GroupIndex = Index; }
+  unsigned getGroupIndex() const { return GroupIndex; }
+
   void setMemtag(bool Tagged);
   bool isMemtag() const;
 
diff --git a/llvm/lib/MC/ELFObjectWriter.cpp b/llvm/lib/MC/ELFObjectWriter.cpp
index 77a56878f6d81..64d0d43017c8e 100644
--- a/llvm/lib/MC/ELFObjectWriter.cpp
+++ b/llvm/lib/MC/ELFObjectWriter.cpp
@@ -169,15 +169,10 @@ struct ELFWriter {
   void writeSymbol(const MCAssembler &Asm, SymbolTableWriter &Writer,
                    uint32_t StringIndex, ELFSymbolData &MSD);
 
-  // Map from a signature symbol to the group section index
-  using RevGroupMapTy = DenseMap<const MCSymbol *, unsigned>;
-
   /// Compute the symbol table data
   ///
   /// \param Asm - The assembler.
-  /// \param RevGroupMap - Maps a signature symbol to the group section.
-  void computeSymbolTable(MCAssembler &Asm,
-                          const RevGroupMapTy &RevGroupMap);
+  void computeSymbolTable(MCAssembler &Asm);
 
   void writeAddrsigSection();
 
@@ -599,8 +594,7 @@ bool ELFWriter::isInSymtab(const MCAssembler &Asm, const MCSymbolELF &Symbol,
   return true;
 }
 
-void ELFWriter::computeSymbolTable(MCAssembler &Asm,
-                                   const RevGroupMapTy &RevGroupMap) {
+void ELFWriter::computeSymbolTable(MCAssembler &Asm) {
   MCContext &Ctx = Asm.getContext();
   SymbolTableWriter Writer(*this, is64Bit());
 
@@ -658,7 +652,7 @@ void ELFWriter::computeSymbolTable(MCAssembler &Asm,
       }
     } else if (Symbol.isUndefined()) {
       if (isSignature && !Used) {
-        MSD.SectionIndex = RevGroupMap.lookup(&Symbol);
+        MSD.SectionIndex = Symbol.getGroupIndex();
         if (MSD.SectionIndex >= ELF::SHN_LORESERVE)
           HasLargeSectionIndex = true;
       } else {
@@ -1104,8 +1098,6 @@ uint64_t ELFWriter::writeObject(MCAssembler &Asm) {
       Ctx.getELFSection(".strtab", ELF::SHT_STRTAB, 0);
   StringTableIndex = addToSectionTable(StrtabSection);
 
-  RevGroupMapTy RevGroupMap;
-
   DenseMap<const MCSymbol *, SmallVector<const MCSectionELF *, 0>> GroupMembers;
 
   // Write out the ELF header ...
@@ -1133,11 +1125,12 @@ uint64_t ELFWriter::writeObject(MCAssembler &Asm) {
     MCSectionELF *RelSection = createRelocationSection(Ctx, Section);
 
     if (SignatureSymbol) {
-      unsigned &GroupIdx = RevGroupMap[SignatureSymbol];
+      unsigned GroupIdx = SignatureSymbol->getGroupIndex();
       if (!GroupIdx) {
         MCSectionELF *Group =
             Ctx.createELFGroupSection(SignatureSymbol, Section.isComdat());
         GroupIdx = addToSectionTable(Group);
+        SignatureSymbol->setGroupIndex(GroupIdx);
         Group->setAlignment(Align(4));
         Groups.push_back(Group);
       }
@@ -1184,7 +1177,7 @@ uint64_t ELFWriter::writeObject(MCAssembler &Asm) {
     }
 
     // Compute symbol table information.
-    computeSymbolTable(Asm, RevGroupMap);
+    computeSymbolTable(Asm);
 
     for (MCSectionELF *RelSection : Relocations) {
       // Remember the offset into the file for this section.

>From 7a844269e053e835a03cf27b4447ee12e4a0dca1 Mon Sep 17 00:00:00 2001
From: Alexis Engelke <engelke at in.tum.de>
Date: Wed, 19 Jun 2024 17:13:06 +0200
Subject: [PATCH 4/4] [MC][ELF] Improve storage of groups and relocations

There's no point in having a DenseMap, the number of sections and groups
are reasonably small to use vectors.
---
 llvm/lib/MC/ELFObjectWriter.cpp | 35 ++++++++++++++++++---------------
 1 file changed, 19 insertions(+), 16 deletions(-)

diff --git a/llvm/lib/MC/ELFObjectWriter.cpp b/llvm/lib/MC/ELFObjectWriter.cpp
index 64d0d43017c8e..de55bdc00301b 100644
--- a/llvm/lib/MC/ELFObjectWriter.cpp
+++ b/llvm/lib/MC/ELFObjectWriter.cpp
@@ -1098,14 +1098,14 @@ uint64_t ELFWriter::writeObject(MCAssembler &Asm) {
       Ctx.getELFSection(".strtab", ELF::SHT_STRTAB, 0);
   StringTableIndex = addToSectionTable(StrtabSection);
 
-  DenseMap<const MCSymbol *, SmallVector<const MCSectionELF *, 0>> GroupMembers;
-
   // Write out the ELF header ...
   writeHeader(Asm);
 
   // ... then the sections ...
-  std::vector<MCSectionELF *> Groups;
-  std::vector<MCSectionELF *> Relocations;
+  SmallVector<std::pair<MCSectionELF *, SmallVector<unsigned>>, 0> Groups;
+  // Map from group section index to group
+  SmallVector<unsigned, 0> GroupMap;
+  SmallVector<MCSectionELF *> Relocations;
   for (MCSection &Sec : Asm) {
     MCSectionELF &Section = static_cast<MCSectionELF &>(Sec);
     if (Mode == NonDwoOnly && isDwoSection(Section))
@@ -1124,21 +1124,20 @@ uint64_t ELFWriter::writeObject(MCAssembler &Asm) {
 
     MCSectionELF *RelSection = createRelocationSection(Ctx, Section);
 
+    unsigned GroupIdx = 0;
     if (SignatureSymbol) {
-      unsigned GroupIdx = SignatureSymbol->getGroupIndex();
+      GroupIdx = SignatureSymbol->getGroupIndex();
       if (!GroupIdx) {
         MCSectionELF *Group =
             Ctx.createELFGroupSection(SignatureSymbol, Section.isComdat());
         GroupIdx = addToSectionTable(Group);
         SignatureSymbol->setGroupIndex(GroupIdx);
         Group->setAlignment(Align(4));
-        Groups.push_back(Group);
+
+        GroupMap.resize(GroupIdx + 1);
+        GroupMap[GroupIdx] = Groups.size();
+        Groups.emplace_back(Group, SmallVector<unsigned>{});
       }
-      SmallVector<const MCSectionELF *, 0> &Members =
-          GroupMembers[SignatureSymbol];
-      Members.push_back(&Section);
-      if (RelSection)
-        Members.push_back(RelSection);
     }
 
     Section.setLayoutOrder(addToSectionTable(&Section));
@@ -1147,18 +1146,22 @@ uint64_t ELFWriter::writeObject(MCAssembler &Asm) {
       Relocations.push_back(RelSection);
     }
 
+    if (GroupIdx) {
+      auto &Members = Groups[GroupMap[GroupIdx]];
+      Members.second.push_back(Section.getLayoutOrder());
+      if (RelSection)
+        Members.second.push_back(RelSection->getLayoutOrder());
+    }
+
     OWriter.TargetObjectWriter->addTargetSectionFlags(Ctx, Section);
   }
 
-  for (MCSectionELF *Group : Groups) {
+  for (auto &[Group, Members] : Groups) {
     // Remember the offset into the file for this section.
     const uint64_t SecStart = align(Group->getAlign());
 
-    const MCSymbol *SignatureSymbol = Group->getGroup();
-    assert(SignatureSymbol);
     write(uint32_t(Group->isComdat() ? unsigned(ELF::GRP_COMDAT) : 0));
-    for (const MCSectionELF *Member : GroupMembers[SignatureSymbol])
-      write(Member->getLayoutOrder());
+    W.write<unsigned>(Members);
 
     uint64_t SecEnd = W.OS.tell();
     Group->setOffsets(SecStart, SecEnd);



More information about the llvm-commits mailing list