[DebugInfo] Allow <Symbol, Offset> pairs in AddressPool [NFC]

[dstenb]

Summary

This is a preparatory commit for D68465. That commit introduces location
list range trimming for values that are call-clobbered, in order to
improve compatibility with GDB. If a variable is described by a mix of
values that are call-clobbered and those that are not, then we need to
be able to start a location list entry inside the call instruction for
all values that are not call-clobbered. This commit adds support for
inserting such expressions in the address pool.

My initial thought was to allow MCExprs in the address pool, but I gave
up on that idea when I did not find a good way to unique them, and went
with simple <Symbol, Offset> pairs instead, as I did not foresee the
need for more advanced expressions.

D68465 only adds tests for cases where the offset is negative. I'm not
sure how to test cases where the offset is positive. Should that be
covered by an assert instead?

[aprantl]

llvm/lib/CodeGen/AsmPrinter/AddressPool.cpp

30

std::make_pair(a, b) -> {a, b}

85

This looks up the entry twice. Does this work?

auto &Entry = Entries[I.second.Number];
SymbolExpr OldSymbolExpr = Entry;
Entry = MCBinaryExpr::create(Op, OldSymbolExpr, OffsetExpr, Asm.OutContext);

llvm/lib/CodeGen/AsmPrinter/AddressPool.h

21

I usually recommend defining a struct instead, so the members can have descriptive names for better readability. Since you still want the DenseMap to work, a trick I used elsewhere is to inherit from std::pair and provide accessors. Maybe that's excessive here.

[dstenb]

Address review comments.

llvm/lib/CodeGen/AsmPrinter/AddressPool.cpp

85

Yes. Thanks.

llvm/lib/CodeGen/AsmPrinter/AddressPool.h

21

Thanks for the suggestion! I have probably overlooked something, but when I inherited from std::pair I still had to implement DenseMapInfo for the inherited class, so all-in-all the code was about as large as implementing a normal struct, so I went with the latter (using the std::pair's DenseMapInfo::getHashValue() implementation though).

[aprantl]

llvm/lib/CodeGen/AsmPrinter/AddressPool.h

21

The idea is to use a DenseMap<std::pair<>> for the automatic DenseMapInfo and cast to the struct as needed.

[dstenb]

Abandoning this helper patch as D68465 was abandoned.

llvm/lib/CodeGen/AsmPrinter/AddressPool.h

 #include "llvm/ADT/DenseMap.h"
 
 namespace llvm {
 
 class AsmPrinter;
 class MCSection;
 class MCSymbol;
 
+/// Pair of a symbol and an offset in number of bytes.
+struct SymbolWithOffset {
+  const MCSymbol *Symbol;
+  int Offset;
+
+  bool operator==(const SymbolWithOffset &O) const {
+    return Symbol == O.Symbol && Offset == O.Offset;
+  }
+};
+
+template <> struct DenseMapInfo<SymbolWithOffset> {
+  using SymbolInfo = DenseMapInfo<const MCSymbol *>;
+  using OffsetInfo = DenseMapInfo<int>;
+  using PairInfo = DenseMapInfo<std::pair<const MCSymbol *, int>>;
+
+  static SymbolWithOffset getEmptyKey() {
+    return {SymbolInfo::getEmptyKey(), OffsetInfo::getEmptyKey()};
+  }
+
+  static SymbolWithOffset getTombstoneKey() {
+    return {SymbolInfo::getTombstoneKey(), OffsetInfo::getTombstoneKey()};
+  }
+
+  static unsigned getHashValue(const SymbolWithOffset &Val) {
+    return PairInfo::getHashValue({Val.Symbol, Val.Offset});
+  }
+
+  static bool isEqual(const SymbolWithOffset &LHS,
+                      const SymbolWithOffset &RHS) {
+    return LHS == RHS;
+  }
+};
+
 // Collection of addresses for this unit and assorted labels.
 // A Symbol->unsigned mapping of addresses used by indirect
 // references.
 class AddressPool {
   struct AddressPoolEntry {
     unsigned Number;
     bool TLS;
 
     AddressPoolEntry(unsigned Number, bool TLS) : Number(Number), TLS(TLS) {}
   };
-  DenseMap<const MCSymbol *, AddressPoolEntry> Pool;
+  DenseMap<SymbolWithOffset, AddressPoolEntry> Pool;
 
   /// Record whether the AddressPool has been queried for an address index since
   /// the last "resetUsedFlag" call. Used to implement type unit fallback - a
   /// type that references addresses cannot be placed in a type unit when using
   /// fission.
   bool HasBeenUsed = false;
 
 public:
   AddressPool() = default;
 
   /// Returns the index into the address pool with the given
   /// label/symbol.
   unsigned getIndex(const MCSymbol *Sym, bool TLS = false);
 
+  /// Returns the index into the address pool with the given
+  /// label/symbol and offset.
+  unsigned getIndex(SymbolWithOffset SymWithOffs, bool TLS = false);
+
   void emit(AsmPrinter &Asm, MCSection *AddrSection);
 
   bool isEmpty() { return Pool.empty(); }
 
   bool hasBeenUsed() const { return HasBeenUsed; }
 
   void resetUsedFlag() { HasBeenUsed = false; }
 

llvm/lib/CodeGen/AsmPrinter/AddressPool.cpp

 #include "llvm/MC/MCStreamer.h"
 #include "llvm/Target/TargetLoweringObjectFile.h"
 #include <utility>
 
 using namespace llvm;
 
 unsigned AddressPool::getIndex(const MCSymbol *Sym, bool TLS) {
   HasBeenUsed = true;
+  auto IterBool = Pool.insert(
+      {SymbolWithOffset{Sym, 0}, AddressPoolEntry(Pool.size(), TLS)});
+  return IterBool.first->second.Number;
+}
+
+unsigned AddressPool::getIndex(SymbolWithOffset SymWithOffs, bool TLS) {
+  HasBeenUsed = true;
   auto IterBool =
-      Pool.insert(std::make_pair(Sym, AddressPoolEntry(Pool.size(), TLS)));
+      Pool.insert({SymWithOffs, AddressPoolEntry(Pool.size(), TLS)});
   return IterBool.first->second.Number;
 }
 
 MCSymbol *AddressPool::emitHeader(AsmPrinter &Asm, MCSection *Section) {
   static const uint8_t AddrSize = Asm.getDataLayout().getPointerSize();
   StringRef Prefix = "debug_addr_";
   MCSymbol *BeginLabel = Asm.createTempSymbol(Prefix + "start");
   MCSymbol *EndLabel = Asm.createTempSymbol(Prefix + "end");
 
void AddressPool::emit(AsmPrinter &Asm, MCSection *AddrSection) {
 
   // Define the symbol that marks the start of the contribution.
   // It is referenced via DW_AT_addr_base.
   Asm.OutStreamer->EmitLabel(AddressTableBaseSym);
 
   // Order the address pool entries by ID
   SmallVector<const MCExpr *, 64> Entries(Pool.size());
 
-  for (const auto &I : Pool)
-    Entries[I.second.Number] =
+  for (const auto &I : Pool) {
+    auto &Entry = Entries[I.second.Number] =
         I.second.TLS
-            ? Asm.getObjFileLowering().getDebugThreadLocalSymbol(I.first)
-            : MCSymbolRefExpr::create(I.first, Asm.OutContext);
+            ? Asm.getObjFileLowering().getDebugThreadLocalSymbol(I.first.Symbol)
+            : MCSymbolRefExpr::create(I.first.Symbol, Asm.OutContext);
+    auto Offset = I.first.Offset;
+    if (!Offset)
+      continue;
+    // Create an expression for the addition or subtraction of the offset.
+    auto Op = Offset < 0 ? MCBinaryExpr::Sub : MCBinaryExpr::Add;
+    const auto OffsetExpr =
+        MCConstantExpr::create(std::abs(Offset), Asm.OutContext);
+    const MCExpr *SymbolExpr = Entry;
+    Entry = MCBinaryExpr::create(Op, SymbolExpr, OffsetExpr, Asm.OutContext);
+  }
 
   for (const MCExpr *Entry : Entries)
     Asm.OutStreamer->EmitValue(Entry, Asm.getDataLayout().getPointerSize());
 
   if (EndLabel)
     Asm.OutStreamer->EmitLabel(EndLabel);
 }