Page MenuHomePhabricator

WIP: Dont vend lldb_private::CompileUnits for DWARFTypeUnits

Authored by labath on May 24 2019, 7:50 AM.



This patch isn't meant to be committed, but it is here to demostrate
what it would take to implement the idea proposed by Greg in

The background here is that a type unit build will have substantially
more "units" than a regular one (e.g., a type unit build of lldb will
have about 1k compile units, but almost 400k type units). Vending them
all as lldb_private::CompileUnits is a bit wasteful. It can be made less
wasteful by making sure the CompileUnits share line tables and stuff
(just like DWARF units do), but it's not clear to me whether this is
really useful/needed. For instance, lldb CompileUnits expect to have a
"name" (a file path), but type units don't have that as they're supposed
to be independent/shared between real compile units.

The majority of changes here just deal with the remapping of dwarf vs.
lldb unit indexes, as now we can now no longer use a 1:1 mapping.
However, there are a couple of things that are worth calling out

  • DWARFASTParserClang goes through the lldb CompileUnits in order to fetch file names. Right now I've just deleted that code. In a proper solution, we'd need to find a way to get this information strictly through DWARF structures.
  • The main visible side effect I can see from this is that we will stop filling out the SymbolContextScope field of the Type objects that we hand about. More precisely, we can still fill out the "module" part, but not the "compile unit". I'm not sure what are the implications of that, but maybe that is even more correct as the type unit does not really belong to any single compilation. This patch breaks just three tests, and all of them seem related to me stopping to parse the file names, and not this...

So, does anyone see why (a cleaned up version of) this would be a bad

Event Timeline

labath created this revision.May 24 2019, 7:50 AM
clayborg added inline comments.May 24 2019, 10:51 AM

We should still be able to fill this in. We just need to unique all of the DWARF line tables internally and then access the line table using the DWARF compile/type unit, and not the lldb_private::CompileUnit


Cache this in local. Kind of expensive to get twice from the DIE since it will have to grab the DW_AT_language attribute from the DIE and not just use a cached version? Unless this just grabs it from the DWARFUnit underneath? And actually a DIE's language could differ from the CU/TU language. We might have a C type in C++, so it is theoretically possible to have a die with a DW_AT_language attribute? If "die.GetLanguage()" is just using the language of the DWARFUnit, then this is fine, otherwise we should explicitly use the DWARFUnit language.


Again, we can still fill this in correctly, just don't use the lldb_private::CompileUnit, grab the uniqued line table from DWARFContext or SymbolFileDWARF. We need to cache line tables anyway because many type units re-use existing compile unit line tables and we don't want to re-parse line table for each TU.


Again, do we want CU/TU language or DIE language?


fill in using uniqued DWARF line tables


fill in using uniqued DWARF line tables


fill in using uniqued DWARF line tables


fill in using uniqued DWARF line tables


fill in using uniqued DWARF line tables


fill in using uniqued DWARF line tables


What about DW_UT_split_type?


BuildCUTranslationTable? Not sure we need the "Lldb" in the name?

Thanks for looking at this. I'm not going to respond to the individual comments here, as this patch is not is not intended to be submitted like this. I just posted it here to get some high level feedback about the overall direction. Instead, I'm going to split off smaller patches from here, and we can discuss the details there.