[PATCH] D47158: [DWARFv5] Put DWO ID in its place

[Paul Robinson via Phabricator via llvm-commits]

probinson added inline comments.


================
Comment at: llvm/include/llvm/DebugInfo/DWARF/DWARFUnit.h:64
   uint32_t TypeOffset = 0;
 
   // Unit type as parsed, or derived from the section kind.
----------------
aprantl wrote:
> This is probably a silly micro-optimization, but for better packing we could move the DWOId here (assuming that it is 9 bytes large on a 64-bit architecture). Alternatively: is `0` a valid DWOId?
LLVM generates DWOId using MD5, so in principle yes 0 is a valid ID.  I can move it; didn't think about it occupying +1 byte.


================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.h:86
 
+  // DWO ID for correlating skeleton and split units.
+  uint64_t DWOId = 0;
----------------
aprantl wrote:
> ///
This header is inconsistent (might be nice to tidy that up someday) but sure.


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFContext.cpp:572
+      else
+        continue; // No DWO ID?
+    }
----------------
aprantl wrote:
> ```
> // No DWO ID?
> continue;
> ```
Would it then be multiline and require braces?  :-)
Sure.


Repository:
  rL LLVM

https://reviews.llvm.org/D47158