[debug-early] emit locals early patchset

[Jason Merrill]

On 10/27/2014 08:00 PM, Aldy Hernandez wrote:
> 2. Changes to gen_variable_die() to handle multiple passes (early/late
> dwarf generation).
>
> A lot of this is complicated by the fact that old_die's are cached and
> keyed by `tree', but an abstract instance and an inline instance share
> trees, while dwarf2out_abstract_function() sets DECL_ABSTRACT_P behind
> the scenes.
>
> The current support (and my changes) maintain this shared and delicate
> design.  I wonder whether we could simplify a lot of this code by
> unsharing these trees, but this may be beyond the scope of this work.

Copying all the trees in a function just for debug generation?  No, that 
sounds undesirable.

> 3. I've removed deferred_locations.  With multiple dwarf passes we can
> do without them.

Yay!

> Kind words greatly appreciated.  Basically I'm looking for feedback and positive reinforcement that this is all eventually useful

This all looks very good, just a few nitpicks:

> -     instance tree [that has DW_AT_inline] should not contain any
> +     instance tree [has DW_AT_inline] should not contain any

This doesn't seem like an improvement.

> +      /* Find and reuse a previously generated DW_TAG_subrange_type if
> +	 available.  */

Let's expand this comment a bit to clarify how this works for 
multi-dimensional arrays.

> -     abstract instance (origin != NULL), in which case we need a new
> +     inline instance (origin != NULL), in which case we need a new DIE

I think "concrete instance" is what you want here.

> +	      /* Even if we have locations, we need to recurse through
> +		 the locals to make sure they also have locations.  */

Why?  What is adding a location to the function without doing the same 
for the locals?

> +      current_function_has_inlines = 0;
> +
> +      /* The first time through decls_for_scope we will generate the
> +	 DIEs for the locals.  The second time, we fill in the
> +	 location info.  */
> +      decls_for_scope (outer_scope, subr_die, 0);
> +
>        /* Emit a DW_TAG_variable DIE for a named return value.  */
>        if (DECL_NAME (DECL_RESULT (decl)))
>  	gen_decl_die (DECL_RESULT (decl), NULL, subr_die);
>
> -      current_function_has_inlines = 0;
> -      decls_for_scope (outer_scope, subr_die, 0);

Why does this need to be reordered?

> +  /* If the compiler emitted a definition for the DECL declaration
> +     and we already emitted a DIE for it, don't emit a second
> +     DIE for it again. Allow re-declarations of DECLs that are
> +     inside functions, though.  */
> +  else if (old_die && !declaration && !local_scope_p (context_die))
> +    return;

What DECLs in functions need re-declaration?

> -  if (decl && (DECL_ABSTRACT_P (decl) || declaration || old_die == NULL))
> +  if (decl && (DECL_ABSTRACT_P (decl) || declaration || old_die == NULL
> +	       /* If we make it to a specialization, we have already
> +		  handled the declaration by virtue of early dwarf.
> +		  If so, make a new assocation if available, so late
> +		  dwarf can find it.  */
> +	       || (specialization_p && old_die && old_die->dumped_early)))
>      equate_decl_number_to_die (decl, var_die);

Instead of old_die->dumped_early, I think it would make more sense to 
check early_dwarf_dumping; the reason we need to call 
equate_decl_number_to_die is because we're early-dumping the definition 
and we will need to find it again later.

> +  else if (BLOCK_ABSTRACT_ORIGIN (stmt))
>      {
> +      /* If this is an inlined instance, create a new lexical die for
> +	 anything below to attach DW_AT_abstract_origin to.  */
> +      stmt_die = new_die (DW_TAG_lexical_block, context_die, stmt);
> +    }

What if we early dumped this block?

> +      /* Variabled-lengthed types may be incomplete even if
> +	 TREE_ASM_WRITTEN.

"variable-length", I think.

Jason