[PATCH] D69672: DWARFDebugLoclists: Move to a incremental parsing model

[Pavel Labath via Phabricator via llvm-commits]

labath marked 3 inline comments as done.
labath added a comment.

It looks like there will be some conflicts between this patch and D69462 <https://reviews.llvm.org/D69462>, but it does not seem they should be hard to resolve, as that patch mostly deals with the generation side of things.



================
Comment at: llvm/include/llvm/DebugInfo/DWARF/DWARFDebugLoc.h:82
+
   struct Entry {
     uint8_t Kind;
----------------
Something which isn't obvious from this patch is that our idea was to make the `Entry` class be shared between the two location list implementations. The v4 entries will get "upgraded" to v5 during parsing. This will make it possible to share the code translating the location list entries into actual address ranges. Dumping will be handled specially, in order to make the v4 dump come out as it was originally in the file.


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFDebugLoc.cpp:210-219
+  if (DumpOpts.Verbose) {
+    uint64_t OffsetCopy = *Offset;
+    consumeError(
+        visitLocationList(Data, &OffsetCopy, Version, [&](const Entry &E) {
+          MaxEncodingStringLength =
+              std::max(MaxEncodingStringLength,
+                       dwarf::LocListEncodingString(E.Kind).size());
----------------
Parsing twice just to get the maximum name length is somewhat wasteful. A different option would be to get the maximum of *all* entry types, not just of those that happen to be used in this location lists.


================
Comment at: llvm/test/DebugInfo/X86/fission-ranges.ll:35-38
+; CHECK: DW_AT_location [DW_FORM_sec_offset]   ([[A:0x[0-9a-z]*]]:
+; CHECK: DW_AT_location [DW_FORM_sec_offset]   ([[E:0x[0-9a-z]*]]:
+; CHECK: DW_AT_location [DW_FORM_sec_offset]   ([[B:0x[0-9a-z]*]]:
+; CHECK: DW_AT_location [DW_FORM_sec_offset]   ([[D:0x[0-9a-z]*]]:
----------------
This is +/- the only functional change in this patch. It is unfortunate because the range list code does not print the extra colon. It wouldn't be *too* hard to avoid that. I'd just need to move the offset printing code out of the "dumpLocationList" function and into the individual (about 3 of them) callers. However, that did not seem worth it, particularly as I am hoping that this can be rectified once we have a more high-level method of printing "inline" location lists.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69672