On Thu, 31 Aug 2017, Richard Biener wrote:

> 
> As suspected during review the DECL_ABSTRACT_P handling in
> gen_formal_parameter_die is no longer necessary so the following
> patch removes it.
> 
> [LTO] bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
> The gdb testsuite shows no regression.
> 
> Will apply after testing finished.

Ok, so it doesn't work - it regresses for example 
gcc.dg/debug/dwarf2/inline1.c because we end up annotating the
abstract DIE with a location.  This is because in gen_subprogram_die
(with decl set as abstract-self and thus generating a concrete
instance subroutine die) we call

          else if (parm && !POINTER_BOUNDS_P (parm))
            {
              dw_die_ref parm_die = gen_decl_die (parm, NULL, NULL, 
subr_die);

thus unconditionally pass NULL as origin where gen_formal_parameter_die
just has

  /* If we have a previously generated DIE, use it, unless this is an
     concrete instance (origin != NULL), in which case we need a new
     DIE with a corresponding DW_AT_abstract_origin.  */
  bool reusing_die;
  if (parm_die && origin == NULL)
    reusing_die = true;
  else
    {
      parm_die = new_die (DW_TAG_formal_parameter, context_die, node);
      reusing_die = false;
    }

but obviously that logic is flawed with us passing origin as NULL...

What saved us here is the check whether the existing param_die has
the correct parent, if not we didn't re-use it.  OTOH for die_parent
== NULL we have the extra check that would have been dead code.

Any hint as to whether we should pass in anything as origin or
whether we should keep the existing die_parent logic somehow?
(do we ever get a NULL context_die passed?)

Anyway, retracting the patch for now -- at least the comment above
doesn't match reality (we're getting origin == NULL for the
concrete instance).

Thanks,
Richard.


> Richard.
> 
> 2017-08-31  Richard Biener  <rguent...@suse.de>
> 
>       * dwarf2out.c (gen_formal_parameter_die): Remove no longer
>       needed DECL_ABSTRACT_P handling.
> 
> Index: gcc/dwarf2out.c
> ===================================================================
> --- gcc/dwarf2out.c   (revision 251559)
> +++ gcc/dwarf2out.c   (working copy)
> @@ -21288,28 +21288,9 @@ gen_formal_parameter_die (tree node, tre
>    tree ultimate_origin;
>    dw_die_ref parm_die = NULL;
>    
> -  if (TREE_CODE_CLASS (TREE_CODE (node_or_origin)) == tcc_declaration)
> +  if (DECL_P (node_or_origin))
>      {
>        parm_die = lookup_decl_die (node);
> -
> -      /* If the contexts differ, we may not be talking about the same
> -      thing.
> -      ???  When in LTO the DIE parent is the "abstract" copy and the
> -      context_die is the specification "copy".  But this whole block
> -      should eventually be no longer needed.  */
> -      if (parm_die && parm_die->die_parent != context_die && !in_lto_p)
> -     {
> -       if (!DECL_ABSTRACT_P (node))
> -         {
> -           /* This can happen when creating an inlined instance, in
> -              which case we need to create a new DIE that will get
> -              annotated with DW_AT_abstract_origin.  */
> -           parm_die = NULL;
> -         }
> -       else
> -         gcc_unreachable ();
> -     }
> -
>        if (parm_die && parm_die->die_parent == NULL)
>       {
>         /* Check that parm_die already has the right attributes that
> 

-- 
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 
21284 (AG Nuernberg)

Reply via email to