Re: [PING][PATCH][2/2] Early LTO debug -- main part

[Jason Merrill <jason@redhat.com>]

On Thu, Nov 24, 2016 at 8:50 AM, Richard Biener <rguenther@suse.de> wrote:
>> > +      /* ???  We can't annotate types late, but for LTO we may not
>> > +    generate a location early either (gfortran.dg/save_5.f90).
>> > +    The proper way is to handle it like VLAs though it is told
>> > +    that DW_AT_string_length does not support this.  */
>>
>> I think go ahead and handle it like VLAs, this is an obvious generalization
>> and should go into the spec soon enough.  This can happen later.
>
> Ok, note that VLAs are now handled by re-emitting types late to avoid
> some guality regressions.  With inlining VLA types also get copied
> so we'd need to re-design how we handle them a bit.
> I'll see what the DWARF people come up with.

Does the discussion in
https://sourceware.org/bugzilla/show_bug.cgi?id=20426 cover the issue?

>> > +     /* ???  This all (and above) should probably be simply
>> > +        a ! early_dwarf check somehow.  */
>> > +      && ((DECL_ARTIFICIAL (decl) || in_lto_p)
>> >            || (get_AT_file (old_die, DW_AT_decl_file) == file_index
>> >                && (get_AT_unsigned (old_die, DW_AT_decl_line)
>> >                    == (unsigned) s.line))))
>>
>> Why doesn't the existing source position check handle the LTO case? Also the
>> extra parens aren't necessary.
>
> Because in LTRANS we do not see those attributes anymore but they are
> present in the early created DIEs.

Ah, OK.  But the comment seems wrong, since we go through here in
early dwarf for local class methods.

>> > +init_sections_and_labels (bool early_lto_debug)
>>
>> You're changing this function to do the same thing in four slightly different
>> ways rather than two.  I'd rather control each piece as appropriate; we ought
>> to make SECTION_DEBUG or SECTION_DEBUG|SECTION_EXCLUDE a local variable, and
>> select between *_SECTION and the DWO variant at each statement rather than in
>> different blocks.
>
> Note that the section names change between LTO, LTO_DWO, DWO and
> regular section names.  It's basically modeled after what we have now
> which switches between regular and DWO section names.  We might
> be able to refactor this with a new array
>
> enum section_kind { NORMAL, DWO, LTO, LTO_DWO };
> char **section_names[section_kind][] = { { DEBUG_INFO_SECTION, ... },
> { DEBUG_DWO_INFO_SECTION, ... },
> { DEBUG_LTO_INFO_SECTION, ... },
> { DEBUG_LTO_DWO_INFO_SECTION, .. } };
>
> would you prefer that?

That sounds better, thanks.

Jason