[PATCH] Move DwarfCompileUnit::getOrCreateGlobalVariableDIE to DwarfUnit.

[David Blaikie]

On Fri, Oct 24, 2014 at 10:27 AM, Frédéric Riss <friss at apple.com> wrote:

>
> > On Oct 24, 2014, at 10:13 AM, David Blaikie <dblaikie at gmail.com> wrote:
> >
> > Looks like the right answer for your end goal is instead to sink
> constructImportedEntityDIE into DwarfCompileUnit - it's only needed for
> compile units, not type units.
> >
> > (at some point, we should supported imported entities at class scope -
> but those are a special case, they can't refer to global variables, only to
> functions declared in base classes - so we still wouldn't need the ability
> to emit global variable definitions or declarations in a type unit)
>
> This is what I was wondering, I thought you’d want to reuse this function
> for type units. I’ll resubmit the move of constructImportedEntittyDIE.
>

Yep - I think I'd previously thought C++ allowed all sorts of "using" at
class scope, but when I checked it only allows using base class functions
(& maybe base class variables?). The way we'll probably implement this
would be a bit different from the current imported entity support - it
doesn't need to go in a global list, we can put these class scope imported
entities in the class's own member list, and the code won't need to be
generic enough to manifest global variable declarations, because they're
not applicable in that context.

We'll see how it all shakes out some day. I doubt it's much of a priority -
I don't think GDB (& probably not LLDB) worry too much about the visibility
issues of shadowed member functions, etc.


>
> > Sorry for the rather terse commit messages on some of my recent
> refactorings. I'd mentioned in some detail the direction in some of the
> commit messages, but it seemed verbose to repeat it on every step. The
> basic idea is that we need to emit -gmlt-like data into the skeleton type
> unit to support in-process symbolication (including inlining) in tools like
> ASan even when using fission/-gsplit-dwarf. This means I need much of the
> subprogram emission functionality to be reusable for both the full and
> skeleton type unit - currently all that state exists in DwarfDebug, so it
> can't be reused to do both (eg: the AbstractSPDies map - from MDNode* to
> DIE*, well if we produce abstract subprograms in both the full and skeleton
> unit, tehre will be two such DIEs for every MDNode, so the map can't live
> here) - by sinking these variables down into DwarfFile, there will be one
> for each file and we'll be able to produce subprograms and inlining in both
> the full and skeleton DIEs.
> >
> > Unfortunately, variables are rather inseparable from function emission,
> so this is sinking too, even though -gmlt doesn't need/have any variables.
> It seems like better layering anyway.
> >
> > Sinking operations into DwarfCompileUnit (in theory I could've just sunk
> all the functions and their variables only as far as DwarfFile) just seems
> better for layering - they're compile unit-y kind of operations. It also
> makes for smaller files - DwarfDebug is pretty big, so it's nice to be able
> to pull out a bunch of its logic into the one place that should really be
> handling that.
> >
> > Hence the suggestion to pull constructImportedEntityDIE down into
> DwarfCompileUnit - we would never emit a global variable into a type unit
> (type units can't reference addresses, so it wouldn't make sense to do
> that) so it's nice to have that enforced/supported by the API. Indeed when
> I made this layering change I found and fixed at least one bug, if I recall
> correctly... though it is a bit hazy now.
>
> Thanks for the explanation. I had seen the references to the skeleton
> units for fission, but having this detailed summary is nice.
>
> Fred
>
> > http://reviews.llvm.org/D5975
> >
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20141024/e87db722/attachment.html>