[PATCH] D153080: Emit DW_LLE_base_address + DW_LLE_offset_pairs instead of DW_LLE_start_length in debug_loclists section

[Shubham Sandeep Rastogi via Phabricator via llvm-commits]

rastogishubham added inline comments.


================
Comment at: llvm/include/llvm/DebugInfo/DWARF/DWARFLocationExpression.h:47
+struct DWARFLocationExpressionsFragment {
+  DWARFLocationExpressionsVector LinkedLocationExpression;
+  uint64_t BaseAddress = 0;
----------------
avl wrote:
> probably not name it "LinkedLocationExpression" as linked is dsymutil term. DebugInfo/DWARF does not name addresses linked or relocated.  maybe LinkedLocationExpression -> LocationExpressions or LinkedLocationExpression -> LocationExpressionsVector.
We don't need this anymore, because your other comment about moving the calculations to DWARFStreamer makes sense


================
Comment at: llvm/lib/DWARFLinker/DWARFStreamer.cpp:614
     const CompileUnit &Unit,
-    const DWARFLocationExpressionsVector &LinkedLocationExpression,
+    const DWARFLocationExpressionsFragment &LinkedLocationExpressionsFrag,
     PatchLocation Patch) {
----------------
avl wrote:
> may be we can do all calculations inside DwarfStreamer::emitDwarfDebugLocListsTableFragment ? i.e. do not create new structure DWARFLocationExpressionsFragment, do not change DWARFLinker::generateUnitLocations, calculate and apply BaseAddress inside DwarfStreamer::emitDwarfDebugLocListsTableFragment ? Such approach allows to change output format not changing high level structures and makes overall patch smaller.
Yes, this is a fantastic observation, and I don't know why I didn't think of it!


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

https://reviews.llvm.org/D153080