[llvm] [LLVM][DWARF] Change .debug_names abbrev to be an index (PR #81200)

[David Blaikie via llvm-commits]

================
@@ -223,6 +225,15 @@ class Dwarf5AccelTableWriter : public AccelTableWriter {
   bool IsSplitDwarf = false;
   /// Stores the DIE offsets which are indexed by this table.
   DenseSet<OffsetAndUnitID> IndexedOffsets;
+  /// Mapping between AbbrevTag and Index.
+  DenseMap<uint32_t, uint32_t> AbbrevTagToIndexMap;
----------------
dwblaikie wrote:

It's awkward/brittle to maintain two different maps - I think the Abbreviations map should be the only one.

It should be keyed on the AbbrevDescriptor (it shouldn't need bitcasts to/from - define a hash and equality function for AbbrevDescriptor and use that type itself as the key). Actually, perhaps it should be a DenseSet instead? With the attributes as part of the key/hash - we are currentyl duplicately encoding the attributes in the AbbrevDescirptor bitfields, and then in the SmallVector of AttributeEncodings. That redundancy seems unfortunate/unnecessary. The index can be populated when it's inserted into the abbrev table.

Ultimately, I think it'd be suitable for this code to be shared with the DIE abbreviation handling, which looks like this: https://github.com/llvm/llvm-project/blob/b081e9d4cafe2563c513ed7b5ae3ced6d177b657/llvm/lib/CodeGen/AsmPrinter/DIE.cpp#L140 (the DIE abbrev stuff could maybe be cleaned up a bit - it's gone pretty much untouched since the early days (which is great - means it's been a robust implementation/not a major source of issues - but equally, could use some love if we're looking at it anyway now))


So some path that goes towards that seems good to me.



https://github.com/llvm/llvm-project/pull/81200