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

[Adrian Prantl via Phabricator via llvm-commits]

aprantl added inline comments.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/AddressPool.cpp:22
+  auto IterBool = Pool.insert(std::make_pair(
+      std::make_pair(Sym, 0), AddressPoolEntry(Pool.size(), TLS)));
+  return IterBool.first->second.Number;
----------------
std::make_pair(a, b) -> {a, b}


================
Comment at: llvm/lib/CodeGen/AsmPrinter/AddressPool.cpp:85
+        MCConstantExpr::create(std::abs(Offset), Asm.OutContext);
+    const auto SymbolExpr = Entries[I.second.Number];
+    Entries[I.second.Number] =
----------------
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);
```


================
Comment at: llvm/lib/CodeGen/AsmPrinter/AddressPool.h:21
+/// Pair of a symbol and an offset in number of bytes.
+typedef std::pair<const MCSymbol *, int> SymbolWithOffset;
+
----------------
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.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68466/new/

https://reviews.llvm.org/D68466