[patch 10/10] debug-early merge: compiler proper

[Jason Merrill]

On 05/20/2015 11:50 AM, Aldy Hernandez wrote:
> +     determine anscestry later.  */

ancestry

> +static bool early_dwarf_dumping;

Sorry for the late bikeshedding, but "dumping" suddently strikes me as 
odd, since there is no output as with other dumping in the compiler. 
Can we change that to "generation" or "building"?

> +	      /* Reuse DIE even with a differing context.
> +
> +		 This happens when called through
> +		 dwarf2out_abstract_function for formal parameter
> +		 packs.  */
> +	      gcc_assert (parm_die->die_parent->die_tag
> +			  == DW_TAG_GNU_formal_parameter_pack);

Does this mean we're generating a new DW_TAG_GNU_formal_parameter_pack 
in late debug even though we already generated one in early debug?  If 
so, why?

> -  /* It is possible to have both DECL_ABSTRACT_P and DECLARATION be true if we
> -     started to generate the abstract instance of an inline, decided to output
> -     its containing class, and proceeded to emit the declaration of the inline
> -     from the member list for the class.  If so, DECLARATION takes priority;
> -     we'll get back to the abstract instance when done with the class.  */
> -
> -  /* The class-scope declaration DIE must be the primary DIE.  */
> -  if (origin && declaration && class_or_namespace_scope_p (context_die))
> -    {
> -      origin = NULL;
> -      gcc_assert (!old_die);
> -    }

Can't this happen anymore?

> +      if ((is_cu_die (old_die->die_parent)
> +	   /* FIXME: Jason doesn't like this condition, but it fixes
> +	      the inconsistency/ICE with the following Fortran test:
> +
> +		 module some_m
> +		 contains
> +		    logical function funky (FLAG)
> +		      funky = .true.
> +		   end function
> +		 end module
> +
> +	      Another alternative is !is_cu_die (context_die).
> +	   */
> +	   || old_die->die_parent->die_tag == DW_TAG_module

I like it now.  :)
You can leave the rest of the comment.

> +  /* For non DECL_EXTERNALs, if range information is available, fill
> +     the DIE with it.  */
>    else if (!DECL_EXTERNAL (decl))
>      {
>        HOST_WIDE_INT cfa_fb_offset;
> +
>        struct function *fun = DECL_STRUCT_FUNCTION (decl);
>
> -      if (!old_die || !get_AT (old_die, DW_AT_inline))
> -	equate_decl_number_to_die (decl, subr_die);
> +      /* If we have no fun->fde, we have no range information.
> +	 Skip over and fill in range information in the second
> +	 dwarf pass.  */
> +      if (!fun->fde)
> +	goto no_fde_continue;

How about controlling this block with !early_dwarf so you don't need to 
deal with missing FDE?

>  	  if (generic_decl_parm
>  	      && lang_hooks.function_parameter_pack_p (generic_decl_parm))
> -	    gen_formal_parameter_pack_die (generic_decl_parm,
> -					   parm, subr_die,
> -					   &parm);
> +	    {
> +	      if (early_dwarf_dumping)
> +		gen_formal_parameter_pack_die (generic_decl_parm,
> +					       parm, subr_die,
> +					       &parm);
> +	      else if (parm)
> +		parm = DECL_CHAIN (parm);
> +	    }

Let's try only setting generic_decl when early_dwarf.

> +  /* Unless we have an existing non-declaration DIE, equate the new
> +     DIE.  */
> +  if (!old_die || is_declaration_die (old_die))
> +    equate_decl_number_to_die (decl, subr_die);
...
> +  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 && early_dwarf_dumping)))
>      equate_decl_number_to_die (decl, var_die);

Why are the conditions so different?  Can we use the function condition 
for variables, too?

> +	  /* Do nothing.  This must have been early dumped and it
> +	     won't even need location information since it's a
> +	     DW_AT_inline function.  */
> +	  for (dw_die_ref c = context_die; c; c = c->die_parent)
> +	    if (c->die_tag == DW_TAG_inlined_subroutine
> +		|| c->die_tag == DW_TAG_subprogram)
> +	      {
> +		gcc_assert (get_AT (c, DW_AT_inline));
> +		break;
> +	      }

Maybe wrap this in #ifdef ENABLE_CHECKING.

> +	  /* Do the new DIE dance.  */
> +	  stmt_die = new_die (DW_TAG_lexical_block, context_die, stmt);
> +	  BLOCK_DIE (stmt) = stmt_die;
> +	}
> +    }
> +  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);
> +    }
> +  else
> +    {
> +      if (!stmt_die)
> +	{
> +	  /* This is the first time we are creating something for this
> +	     block.  */
> +	  stmt_die = new_die (DW_TAG_lexical_block, context_die, stmt);
> +	  BLOCK_DIE (stmt) = stmt_die;
> +	}

Surely we don't need to repeat the new_die call three times; the first 
and last are both controlled by !stmt_die.  And don't we want to set 
BLOCK_DIE for the inlined case as well, so that we can find the DIE 
again in late debug?

> +  /* Fill in the size of variable-length fields in late dwarf.  */
> +  if (TREE_ASM_WRITTEN (type)
> +      && !early_dwarf_dumping)
> +    {
> +      tree member;
> +      for (member = TYPE_FIELDS (type); member; member = DECL_CHAIN (member))
> +	fill_variable_array_bounds (TREE_TYPE (member));
> +      return;
> +    }

Why is this happening in late dwarf?  I'm concerned that front-end 
information that is necessary to do this might be lost by that point.

> +      /* Variable-length types may be incomplete even if
> +	 TREE_ASM_WRITTEN.  For such types, fall through to
> +	 gen_array_type_die() and possibly fill in
> +	 DW_AT_{upper,lower}_bound attributes.  */
> +      if ((TREE_CODE (type) != ARRAY_TYPE
> +	   && TREE_CODE (type) != RECORD_TYPE
> +	   && TREE_CODE (type) != UNION_TYPE
> +	   && TREE_CODE (type) != QUAL_UNION_TYPE)
> +	  || (TYPE_SIZE (type)
> +	      && TREE_CODE (TYPE_SIZE (type)) == INTEGER_CST))

Similarly, why check for INTEGER_CST here?

> +      bool t = early_dwarf_dumping;
> +      early_dwarf_dumping = true;
> +      dwarf2out_decl (decl);
> +      early_dwarf_dumping = t;

Let's use a RAII (resource acquisition is initialization) pattern for 
this and dwarf2out_imported_module_or_decl and dwarf2out_early_global_decl:

struct set_early_dwarf {
  bool saved;
  set_early_dwarf(): saved(early_dwarf) { early_dwarf = true; }
  ~set_early_dwarf() { early_dwarf = saved; }
};

set_early_dwarf s;
dwarf2out_decl (decl);

> +      /* When generating LTO bytecode we can not generate new assembler
> +         names at this point and all important decls got theirs via
> +	 free-lang-data.  */
> +      if (((!flag_generate_lto && !flag_generate_offload)
> +	   || DECL_ASSEMBLER_NAME_SET_P (decl))
> +	  && DECL_ASSEMBLER_NAME (decl) != DECL_NAME (decl)

Doesn't early_finish happen before free_lang_data, so we should be fine?

Jason