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)