[PATCH] D26567: Improve DWARF parsing speed by improving DWARFAbbreviationDeclaration

[Greg Clayton via llvm-commits]

clayborg added inline comments.


================
Comment at: include/llvm/DebugInfo/DWARF/DWARFDebugInfoEntry.h:50
   /// doesn't change OffsetPtr.
-  bool extractFast(const DWARFUnit *U, uint32_t *OffsetPtr);
+  bool extractFast(const DWARFUnit &U, uint32_t *OffsetPtr);
+  /// High performance extraction should use this call.
----------------
aprantl wrote:
> This is pointer to an offset in the section? Would a `uint32_t &` be more readable here? (Not sure since it is then less clear that this is an inout argument at the call site.)
I can change this, but I should probably do this in a separate patch as there are many many functions that take a uint32_t pointer to an offset that gets updated. So lets not fix this now and make it inconsistent with 20 other function calls. Is that ok?


================
Comment at: include/llvm/DebugInfo/DWARF/DWARFDebugInfoEntry.h:51
+  bool extractFast(const DWARFUnit &U, uint32_t *OffsetPtr);
+  /// High performance extraction should use this call.
+  bool extractFast(const DWARFUnit &U, uint32_t *OffsetPtr,
----------------
aprantl wrote:
> Why are there two methods called `extractFast`, is that a transitional thing that will go away in a subsequent patch?
> 
> That's a general problem I have even with the original interface: Just from looking at the documentation here  it isn't clear to me what this function does and what the alternative to extractFast would be.
extractFast is used elsewhere in places I didn't write. I would like to get rid of this, and yes I will do that in another patch, but not this one.


https://reviews.llvm.org/D26567