[PATCH] D155724: Change DW_LLE_baseaddr to DW_LLE_baseaddrx in .debug_loclist section

[Adrian Prantl via Phabricator via llvm-commits]

aprantl added inline comments.


================
Comment at: llvm/include/llvm/DWARFLinker/DWARFLinker.h:638
+    DenseMap<uint64_t, uint64_t> AddrIndexMap;
+    SmallVector<uint64_t> Addrs;
+
----------------
This hasn't been introduced by this patch, but I'm curious why we need to store the addresses twice? Would   `AddrIndexMap.insert(std::make_pair(Addr, AddrIndexMap.size()))` work just as well?


================
Comment at: llvm/lib/DWARFLinker/DWARFLinker.cpp:2002
     TheDwarfEmitter->emitDwarfDebugLocListFragment(
-        Unit, LinkedLocationExpressions, CurLocAttr);
+        Unit, LinkedLocationExpressions, CurLocAttr, Index);
   }
----------------
should we `assert(BaseAddress)` here to make sure Index was initialized?


================
Comment at: llvm/lib/DWARFLinker/DWARFStreamer.cpp:547
     const DWARFLocationExpressionsVector &LinkedLocationExpression,
-    PatchLocation Patch) {
+    PatchLocation Patch, uint64_t Index) {
   if (Unit.getOrigUnit().getVersion() < 5) {
----------------
Should this be called BaseAddressIndex?


================
Comment at: llvm/lib/DWARFLinker/DWARFStreamer.cpp:665
     const DWARFLocationExpressionsVector &LinkedLocationExpression,
-    PatchLocation Patch) {
+    PatchLocation Patch, uint64_t Index) {
   Patch.set(LocListsSectionSize);
----------------
ditto


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

https://reviews.llvm.org/D155724