[PATCH] D82173: [DWARFYAML][debug_info] Use 'AbbrCode' to index the abbreviation.

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/test/tools/yaml2obj/ELF/DWARF/debug-info.yaml:597
+      Entries:
+        ## Test that yaml2obj ignores entries whose abbreviation code is 0.
+        - AbbrCode: 0
----------------
Probably "ignores the contents of entries with abbrev code 0". (I wouldn't use the full term "abbreviation" as I think it's not particularly common usage in this context).

I'd also move this case later in the sequence (possibly the last one), since we should allow a non-zero value to be the "First" abbrev.


================
Comment at: llvm/test/tools/yaml2obj/ELF/DWARF/debug-info.yaml:601
+            - Value: 0x1234
+        ## Test that yaml2obj emits values using the abbreviation whose index is 2
+        ## according to the 'AbbrCode'.
----------------
using the ... -> when the abbrev code is specified.

I think the value 2 is unimportant here.


================
Comment at: llvm/test/tools/yaml2obj/ELF/DWARF/debug-info.yaml:607
+            - Value: 0x87654321
+        ## Test that yaml2obj emits values using the abbreviation whose index is 1
+        ## according to the 'AbbrCode'.
----------------
using the ... -> when the abbrev code is specified to be lower than the first abbrev.


================
Comment at: llvm/test/tools/yaml2obj/ELF/DWARF/debug-info.yaml:617-625
+      AbbrOffset: 0x1234
+      AddrSize:   4
+      Entries:
+        ## Test that yaml2obj uses the same abbreviation table to generate values
+        ## across different compilation units.
+        - AbbrCode: 2
+          Values:
----------------
There's a bit of tension here between the AbbrOffset and the behaviour. Since the AbbrOffset should point to the start of the table containing the corresponding abbrevs, it's a little unclear what the behaviour should be if the offset is nonsensical. It also prevents us having multiple abbrev tables as it stands.

I think it's okay to add this test case for now, but I might suggest a change for the future, to split the abbrev section into distinct tables which can be referenced by some ID from the .debug_info. Different .debug_info tables can refer to the same or different abbrev tables that way. The offset would be auto-calculated to match, but could be overridden to a different value (the ID could still be used to actually identify how to write the values).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82173