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

[Jason Merrill <jason at redhat dot com>]

On 11/11/2016 03:06 AM, Richard Biener wrote:
> +    /* ???  In some cases the C++ FE (at least) fails to
> +       set DECL_CONTEXT properly.  Simply globalize stuff
> +       in this case.  For example
> +       __dso_handle created via iostream line 74 col 25.  */

The comment for DECL_CONTEXT says that a VAR_DECL can have 'NULL_TREE or 
a TRANSLATION_UNIT_DECL if the given decl has "file scope"'

So this doesn't seem like a FE bug.

> +		  /* ???  We cannot unconditionally output die_offset if
> +		     non-zero - at least -feliminate-dwarf2-dups will
> +		     create references to those DIEs via symbols.  And we
> +		     do not clear its DIE offset after outputting it
> +		     (and the label refers to the actual DIEs, not the
> +		     DWARF CU unit header which is when using label + offset
> +		     would be the correct thing to do).

I'd be happy to remove or disable -feliminate-dwarf2-dups at this point, 
since it's already useless for C++ without reimplementation.

> +      /* "Unwrap" the decls DIE which we put in the imported unit context.
> +          ???  If we finish dwarf2out_function_decl refactoring we can
> +	  do this in a better way from the start and only lazily emit
> +	  the early DIE references.  */

Can you elaborate more on the refactoring?  dwarf2out_function_decl is 
already very small, I'm guessing you mean gen_subprogram_die?

> +      /* ???  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.

> +	  /* ???  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.

>        /* If we're emitting an out-of-line copy of an inline function,
>  	 emit info for the abstract instance and set up to refer to it.  */
> +      /* ???  We have output an abstract instance early already and
> +         could just re-use that.  This is how LTO treats all functions
> +	 for example.  */

Isn't this what you do now?

> +      /* Avoid generating stray type DIEs during late dwarf dumping.
> +         All types have been dumped early.  */
> +      if (! (decl ? lookup_decl_die (decl) : NULL)

Why do you still want to gen_type_die if decl_or_origin is origin?

> +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.

> +      /* Remove DW_AT_macro from the early output.  */
> +      if (have_macinfo)
> +	remove_AT (comp_unit_die (),
> +		   dwarf_strict ? DW_AT_macro_info : DW_AT_GNU_macros);

This will need adjustment for Jakub's DWARF 5 work.  Please make the 
choice of AT value a macro.

> +  /* ???  Mostly duplicated from dwarf2out_finish.  */

:(

Jason