[PATCH] Do not split variables definitions into 2 DIEs.

[David Blaikie]

On Tue, Sep 23, 2014 at 10:06 AM, Frederic Riss <friss at apple.com> wrote:

> ================
> Comment at: test/DebugInfo/X86/gnu-public-names.ll:56
> @@ -56,2 +55,3 @@
> +; CHECK: [[STATIC_MEM_VAR:0x[0-9a-f]+]]: DW_TAG_member
>  ; CHECK-NEXT: DW_AT_name {{.*}} "static_member_variable"
>
> ----------------
> dblaikie wrote:
> > This actually seems like it /might/ be a reasonable place for a separate
> declaration and definition, since the user wrote an out of line definition.
> >
> > What location do we attribute the definition to? It might be worth
> checking the decl_line attribute in this test. Did we previously attribute
> the declaration and definition to their respective lines, or just duplicate
> one/the other of them?
> >
> > How does this compare to other nested definitions (I suppose for
> functions in namespaces we don't emit both distinguishing the declaration
> and definition in cases like: "namespace x { void func(); } void x::func()
> {}" so there's arguably no reason to similarly for variables)
> I would agree totally if we somehow emitted information corresponding to
> the definition location in the debug info. But the only line information
> that we provide is the one of the declaration in all cases (the attribute
> is called decl_line, so maybe that's really how it should be). If the
> definition DIE has no overlapping attributes with the declaration, I don't
> see a reason to keep both.
> There is one thing that is special to class members though. If we only
> emit a definition (what the patch actually does), it will be inside the
> type. And this is be an issue if we want to emit type units, isn't it?


It could be, but I think it'll actually be fine. The trick we pull is that
members that aren't definitively part of types (implicit special members,
nested types, member template specializations, and, I believe, static
member definitions) aren't actually added to the types member list -
they're just given the type as their context reference. This means when we
emit the type for a type unit, and we just walk the member list, we don't
get any of these things that can vary per translation unit, we just get the
pristine type.

So I /believe/ that putting the definition of a static variable inside the
type is not actually a problem (and furthermore, I believe we could even do
this for member function definitions under some circumstances*)

If this makes the code any simpler - I'd be happy for it to go in, but
please provide a small test case showing that this does the right thing for
type units (& for non-type units, of course). I believe it will.

* This would work fine in type units, but in the absence of type units we
could end up getting both a declaration and a definition of the member. So
then we might end up needing to either emit /just/ the definition from the
frontend and have the backend know how to split that into a declaration and
definition (so it could put the declaration in the type unit and the
definition in the compile_unit itself) or have the frontend be type
unit-aware... hmm, no, we could just make the backend smarter and have it
only out-of-line the definition when not in a type unit.


> Maybe this whole split-if-not-file/function-scope heuristic is to only
> ever emit declarations into classes.
> For functions in namespaces, we only emit one definition DIE, but for
> class methods we split declaration and definition. So maybe we should
> really keep the class type exempt of definitions.
>
> http://reviews.llvm.org/D5457
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140925/314a733f/attachment.html>