[Lldb-commits] [PATCH] D73206: Pass `CompileUnit *` along `DWARFDIE` for DWZ

Wed Feb 5 09:36:49 PST 2020

labath added a comment.

Thanks for exploring this.

>From a high-level, this approach looks like it could be viable. However, you have taken this a lot further than I would have expected. What I was expecting to see would be that a bunch of the DWARF internal functions would get an extra argument (be it a CompileUnit, DWARFUnit, or whatever), but I did not expect that this would also appear on the "generic" interfaces. My idea was that the external interface would keep getting just a single user_id_t, but then during the process of decoding that integer into a DWARFDie, we would also produce the additional CompileUnit object.

Now, that doesn't mean that adding a CompileUnit argument to these functions too is a bad idea. Quite the opposite -- _if it actually works_, it would be great because it would give us a lot more freedom in encoding the information into the user_id_t value. However, I have some doubts about that. For example, right now we don't create any CompileUnit objects for types  in DWARF type units, and I'm not sure how much we want to change that because of the sheer number of type units.  That means we'd still have to have a way to encode a precise type location in a user_id_t without a CompileUnit, though maybe we would be ablle to assume that a CompileUnit is always available in the DWZ case (?).

This duality does make me think that we'd be better off in not adding the CompileUnit arguments and encoding everything into a user_id_t. However, I am not sure if we really have a choice here -- it may be that this information is so complex that it will not be possible to reasonably encode it into 64 bits.

PS. It would be easier to evaluate this if this was based on master instead of some patch which is taking the source in the direction we don't want to go it.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73206