[Lldb-commits] [PATCH] D122988: [BOLT][DWARF] Add version 5 split dwarf support

Thu Apr 28 16:43:40 PDT 2022

ayermolo added inline comments.

================
Comment at: bolt/include/bolt/Core/DebugData.h:375-377
+    if (Optional<uint64_t> DWOId = Unit.getDWOId())
+      return *DWOId;
+    return Unit.getOffset();
----------------
dblaikie wrote:
> ayermolo wrote:
> > ayermolo wrote:
> > > dblaikie wrote:
> > > > That seems like a somewhat problematic API - returning two very different kinds of data (the DWO_ID or the unit offset) seems quite easy to misuse this?
> > > The idea I had behind this APIS is for it to return unique ID representing the CU. As it applies to .debug_addr. For monolithic case is its offset with .debug_info for Split Dwarf case is its DWO ID. At least in my head viewed in that context the return data is the same. It's just something that uniquely identifies this CU, and logic is encapsulated in it.
> > > 
> > @dblaikie What do you think?
> Could you use the offset consistently? The debug_addr section is always in the executable - the offset of the skeleton CU would be unique?
In theory yes, if I always use offset of the CU/Skeleton CU in the main binary. For that I still need to do a check of CU that is passed in is DWO CU, get it skeleton CU, then get it's offset. At the end what we get is offset all the time.  Is it really better? Sure we always getting an offset, but intent of this API is not really get an offset but the unique ID of the CU. Are you concern that offset and DWO ID will collide?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122988