[PATCH] D154638: Emit a .debug_addr section with dsymutil

[Alexey Lapshin via Phabricator via llvm-commits]

avl added inline comments.


================
Comment at: llvm/include/llvm/DWARFLinker/DWARFLinker.h:656
+
+      dwarf::Form getAddrxFormForIndex(uint64_t Index) {
+        if (Index < UINT8_MAX)
----------------
avl wrote:
> Doing things that way we create many unnecessary abbreviations. f.e. for index 255 there would be used this abbreviation:
> 
> ```
> DW_TAG_subprogram 
>   DW_AT_low_pc [DW_FORM_addrx1]
> 
> ```
> For index 256 there would be created new abbreviation:
> 
> ```
> DW_TAG_subprogram 
>   DW_AT_low_pc [DW_FORM_addrx2]
> 
> ```
> 
> I cann't tell whether that solution takes less space. It is necessary to measure impact of many abbreviations to the DWARF size. From one side DIE records having DW_FORM_addrx1 would take less bytes. From another side many abbreviation codes will be increased and then DIEs would take more space.
> 
> The solution from the previous version of this patch looks simpler and is OK. It is safe to just copy source form. DWARFLinker does not add new addresses for compile unit, so original form should be enough to keep address index. Just copy original form and check by assertion that everything is correct.
Or, as another alternative, we can always replace sized forms with DW_FORM_addrx. DW_FORM_addrx would always be enough. In many cases it would be shorter then DW_FORM_addrx3 or DW_FORM_addrx4. This is simple solution which does not increase number of abbreviations.


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

https://reviews.llvm.org/D154638