On Wed, 19 Aug 2015, Richard Biener wrote:
> On Tue, 18 Aug 2015, Aldy Hernandez wrote:
>
> > On 08/18/2015 07:20 AM, Richard Biener wrote:
> > >
> > > This starts a series of patches (still in development) to refactor
> > > dwarf2out.c to better cope with early debug (and LTO debug).
> >
> > Awesome! Thanks.
> >
> > > Aldyh, what other testing did you usually do for changes? Run
> > > the gdb testsuite against the new compiler? Anything else?
> >
> > gdb testsuite, and make sure you test GCC with
> > --enable-languages=all,go,ada,
> > though the latter is mostly useful while you iron out bugs initially. I
> > found
> > that ultimately, the best test was C++.
>
> I see.
>
> > Pre merge I also bootstrapped the compiler and compared .debug* section
> > sizes
> > in object files to make sure things were within reason.
> >
> > > +
> > > +static void
> > > +vmsdbgout_early_finish (const char *filename ATTRIBUTE_UNUSED)
> > > +{
> > > + if (write_symbols == VMS_AND_DWARF2_DEBUG)
> > > + (*dwarf2_debug_hooks.early_finish) (filename);
> > > +}
> >
> > You can get rid of ATTRIBUTE_UNUSED now.
>
> Done. I've also refrained from moving
>
> gen_scheduled_generic_parms_dies ();
> gen_remaining_tmpl_value_param_die_attribute ();
>
> for now as that causes regressions I have to investigate.
>
> The patch below has passed bootstrap & regtest on x86_64-unknown-linux-gnu
> as well as gdb testing. Twice unpatched, twice patched - results seem
> to be somewhat unstable!? I even refrained from using any -j with
> make check-gdb... maybe it's just contrib/test_summary not coping well
> with gdb? any hints? Difference between unpatched run 1 & 2 is
> for example
>
> --- results.unpatched 2015-08-19 15:08:36.152899926 +0200
> +++ results.unpatched2 2015-08-19 15:29:46.902060797 +0200
> @@ -209,7 +209,6 @@
> WARNING: remote_expect statement without a default case?!
> WARNING: remote_expect statement without a default case?!
> WARNING: remote_expect statement without a default case?!
> -FAIL: gdb.base/varargs.exp: print find_max_float_real(4, fc1, fc2, fc3,
> fc4)
> FAIL: gdb.cp/inherit.exp: print g_vD
> FAIL: gdb.cp/inherit.exp: print g_vE
> FAIL: gdb.cp/no-dmgl-verbose.exp: setting breakpoint at 'f(std::string)'
> @@ -238,6 +237,7 @@
> UNRESOLVED: gdb.fortran/types.exp: set print sevenbit-strings
> FAIL: gdb.fortran/whatis_type.exp: run to MAIN__
> WARNING: remote_expect statement without a default case?!
> +FAIL: gdb.gdb/complaints.exp: print symfile_complaints->root->fmt
> WARNING: remote_expect statement without a default case?!
> WARNING: remote_expect statement without a default case?!
> WARNING: remote_expect statement without a default case?!
> @@ -362,12 +362,12 @@
> === gdb Summary ===
>
> -# of expected passes 30881
> +# of expected passes 30884
> # of unexpected failures 284
> # of unexpected successes 2
> -# of expected failures 85
> +# of expected failures 83
> # of unknown successes 2
> -# of known failures 60
> +# of known failures 59
> # of unresolved testcases 6
> # of untested testcases 32
> # of unsupported tests 165
>
> the sames changes randomly appear/disappear in the patched case.
> Otherwise patched/unpatched agree.
>
> Ok?
Jason, are you willing to review these refactoring patches or can
I invoke my middle-end maintainer powers to remove some of this noise
from the LTO parts?
Thanks,
Richard.
> Thanks,
> Richard.
>
> 2015-08-18 Richard Biener <rguent...@suse.de>
>
> * debug.h (gcc_debug_hooks::early_finish): Add filename argument.
> * dbxout.c (dbx_debug_hooks): Adjust.
> * debug.c (do_nothing_hooks): Likewise.
> * sdbout.c (sdb_debug_hooks): Likewise.
> * vmsdbgout.c (vmsdbgout_early_finish): New function dispatching
> to dwarf2out variant if needed.
> (vmsdbg_debug_hooks): Adjust.
> * dwarf2out.c (dwarf2_line_hooks): Adjust.
> (flush_limbo_die_list): New function.
> (dwarf2out_finish): Call flush_limbo_die_list instead of
> dwarf2out_early_finish. Assert there are no deferred asm-names.
> Move early stuff ...
> (dwarf2out_early_finish): ... here.
> * cgraphunit.c (symbol_table::finalize_compilation_unit):
> Call early_finish with main_input_filename argument.
>
>
> Index: gcc/cgraphunit.c
> ===================================================================
> --- gcc/cgraphunit.c (revision 226966)
> +++ gcc/cgraphunit.c (working copy)
> @@ -2490,7 +2490,7 @@ symbol_table::finalize_compilation_unit
>
> /* Clean up anything that needs cleaning up after initial debug
> generation. */
> - (*debug_hooks->early_finish) ();
> + (*debug_hooks->early_finish) (main_input_filename);
>
> /* Finally drive the pass manager. */
> compile ();
> Index: gcc/dbxout.c
> ===================================================================
> --- gcc/dbxout.c (revision 226966)
> +++ gcc/dbxout.c (working copy)
> @@ -354,7 +354,7 @@ const struct gcc_debug_hooks dbx_debug_h
> {
> dbxout_init,
> dbxout_finish,
> - debug_nothing_void,
> + debug_nothing_charstar,
> debug_nothing_void,
> debug_nothing_int_charstar,
> debug_nothing_int_charstar,
> Index: gcc/debug.c
> ===================================================================
> --- gcc/debug.c (revision 226966)
> +++ gcc/debug.c (working copy)
> @@ -28,7 +28,7 @@ const struct gcc_debug_hooks do_nothing_
> {
> debug_nothing_charstar,
> debug_nothing_charstar,
> - debug_nothing_void, /* early_finish */
> + debug_nothing_charstar, /* early_finish */
> debug_nothing_void,
> debug_nothing_int_charstar,
> debug_nothing_int_charstar,
> Index: gcc/debug.h
> ===================================================================
> --- gcc/debug.h (revision 226966)
> +++ gcc/debug.h (working copy)
> @@ -31,7 +31,7 @@ struct gcc_debug_hooks
> void (* finish) (const char *main_filename);
>
> /* Run cleanups necessary after early debug generation. */
> - void (* early_finish) (void);
> + void (* early_finish) (const char *main_filename);
>
> /* Called from cgraph_optimize before starting to assemble
> functions/variables/toplevel asms. */
> Index: gcc/dwarf2out.c
> ===================================================================
> --- gcc/dwarf2out.c (revision 226966)
> +++ gcc/dwarf2out.c (working copy)
> @@ -2420,7 +2420,7 @@ build_cfa_aligned_loc (dw_cfa_location *
>
> static void dwarf2out_init (const char *);
> static void dwarf2out_finish (const char *);
> -static void dwarf2out_early_finish (void);
> +static void dwarf2out_early_finish (const char *);
> static void dwarf2out_assembly_start (void);
> static void dwarf2out_define (unsigned int, const char *);
> static void dwarf2out_undef (unsigned int, const char *);
> @@ -2494,7 +2494,7 @@ const struct gcc_debug_hooks dwarf2_line
> {
> dwarf2out_init,
> debug_nothing_charstar,
> - debug_nothing_void,
> + debug_nothing_charstar,
> debug_nothing_void,
> debug_nothing_int_charstar,
> debug_nothing_int_charstar,
> @@ -25128,47 +25128,81 @@ optimize_location_lists (dw_die_ref die)
> optimize_location_lists_1 (die, &htab);
> }
>
> +/* Traverse the limbo die list, and add parent/child links. The only
> + dies without parents that should be here are concrete instances of
> + inline functions, and the comp_unit_die. We can ignore the comp_unit_die.
> + For concrete instances, we can get the parent die from the abstract
> + instance. */
> +
> +static void
> +flush_limbo_die_list (void)
> +{
> + limbo_die_node *node, *next_node;
> +
> + for (node = limbo_die_list; node; node = next_node)
> + {
> + dw_die_ref die = node->die;
> + next_node = node->next;
> +
> + if (die->die_parent == NULL)
> + {
> + dw_die_ref origin = get_AT_ref (die, DW_AT_abstract_origin);
> +
> + if (origin && origin->die_parent)
> + add_child_die (origin->die_parent, die);
> + else if (is_cu_die (die))
> + ;
> + else if (seen_error ())
> + /* It's OK to be confused by errors in the input. */
> + add_child_die (comp_unit_die (), die);
> + else
> + {
> + /* In certain situations, the lexical block containing a
> + nested function can be optimized away, which results
> + in the nested function die being orphaned. Likewise
> + with the return type of that nested function. Force
> + this to be a child of the containing function.
> +
> + It may happen that even the containing function got fully
> + inlined and optimized out. In that case we are lost and
> + assign the empty child. This should not be big issue as
> + the function is likely unreachable too. */
> + gcc_assert (node->created_for);
> +
> + if (DECL_P (node->created_for))
> + origin = get_context_die (DECL_CONTEXT (node->created_for));
> + else if (TYPE_P (node->created_for))
> + origin = scope_die_for (node->created_for, comp_unit_die ());
> + else
> + origin = comp_unit_die ();
> +
> + add_child_die (origin, die);
> + }
> + }
> + }
> +
> + limbo_die_list = NULL;
> +}
> +
> /* Output stuff that dwarf requires at the end of every file,
> and generate the DWARF-2 debugging info. */
>
> static void
> -dwarf2out_finish (const char *filename)
> +dwarf2out_finish (const char *)
> {
> comdat_type_node *ctnode;
> dw_die_ref main_comp_unit_die;
>
> /* Flush out any latecomers to the limbo party. */
> - dwarf2out_early_finish ();
> + flush_limbo_die_list ();
>
> - /* PCH might result in DW_AT_producer string being restored from the
> - header compilation, so always fill it with empty string initially
> - and overwrite only here. */
> - dw_attr_ref producer = get_AT (comp_unit_die (), DW_AT_producer);
> - producer_string = gen_producer_string ();
> - producer->dw_attr_val.v.val_str->refcount--;
> - producer->dw_attr_val.v.val_str = find_AT_string (producer_string);
> + /* We shouldn't have any symbols with delayed asm names for
> + DIEs generated after early finish. */
> + gcc_assert (deferred_asm_name == NULL);
>
> gen_scheduled_generic_parms_dies ();
> gen_remaining_tmpl_value_param_die_attribute ();
>
> - /* Add the name for the main input file now. We delayed this from
> - dwarf2out_init to avoid complications with PCH.
> - For LTO produced units use a fixed artificial name to avoid
> - leaking tempfile names into the dwarf. */
> - if (!in_lto_p)
> - add_name_attribute (comp_unit_die (), remap_debug_filename (filename));
> - else
> - add_name_attribute (comp_unit_die (), "<artificial>");
> - if (!IS_ABSOLUTE_PATH (filename) || targetm.force_at_comp_dir)
> - add_comp_dir_attribute (comp_unit_die ());
> - else if (get_AT (comp_unit_die (), DW_AT_comp_dir) == NULL)
> - {
> - bool p = false;
> - file_table->traverse<bool *, file_table_relative_p> (&p);
> - if (p)
> - add_comp_dir_attribute (comp_unit_die ());
> - }
> -
> #if ENABLE_ASSERT_CHECKING
> {
> dw_die_ref die = comp_unit_die (), c;
> @@ -25482,9 +25516,9 @@ dwarf2out_finish (const char *filename)
> has run. */
>
> static void
> -dwarf2out_early_finish (void)
> +dwarf2out_early_finish (const char *filename)
> {
> - limbo_die_node *node, *next_node;
> + limbo_die_node *node;
>
> /* Add DW_AT_linkage_name for all deferred DIEs. */
> for (node = deferred_asm_name; node; node = node->next)
> @@ -25502,57 +25536,35 @@ dwarf2out_early_finish (void)
> }
> deferred_asm_name = NULL;
>
> - /* Traverse the limbo die list, and add parent/child links. The only
> - dies without parents that should be here are concrete instances of
> - inline functions, and the comp_unit_die. We can ignore the
> comp_unit_die.
> - For concrete instances, we can get the parent die from the abstract
> - instance.
> -
> - The point here is to flush out the limbo list so that it is empty
> + /* The point here is to flush out the limbo list so that it is empty
> and we don't need to stream it for LTO. */
> - for (node = limbo_die_list; node; node = next_node)
> - {
> - dw_die_ref die = node->die;
> - next_node = node->next;
> -
> - if (die->die_parent == NULL)
> - {
> - dw_die_ref origin = get_AT_ref (die, DW_AT_abstract_origin);
> -
> - if (origin && origin->die_parent)
> - add_child_die (origin->die_parent, die);
> - else if (is_cu_die (die))
> - ;
> - else if (seen_error ())
> - /* It's OK to be confused by errors in the input. */
> - add_child_die (comp_unit_die (), die);
> - else
> - {
> - /* In certain situations, the lexical block containing a
> - nested function can be optimized away, which results
> - in the nested function die being orphaned. Likewise
> - with the return type of that nested function. Force
> - this to be a child of the containing function.
> + flush_limbo_die_list ();
>
> - It may happen that even the containing function got fully
> - inlined and optimized out. In that case we are lost and
> - assign the empty child. This should not be big issue as
> - the function is likely unreachable too. */
> - gcc_assert (node->created_for);
> -
> - if (DECL_P (node->created_for))
> - origin = get_context_die (DECL_CONTEXT (node->created_for));
> - else if (TYPE_P (node->created_for))
> - origin = scope_die_for (node->created_for, comp_unit_die ());
> - else
> - origin = comp_unit_die ();
> + /* PCH might result in DW_AT_producer string being restored from the
> + header compilation, so always fill it with empty string initially
> + and overwrite only here. */
> + dw_attr_ref producer = get_AT (comp_unit_die (), DW_AT_producer);
> + producer_string = gen_producer_string ();
> + producer->dw_attr_val.v.val_str->refcount--;
> + producer->dw_attr_val.v.val_str = find_AT_string (producer_string);
>
> - add_child_die (origin, die);
> - }
> - }
> + /* Add the name for the main input file now. We delayed this from
> + dwarf2out_init to avoid complications with PCH.
> + For LTO produced units use a fixed artificial name to avoid
> + leaking tempfile names into the dwarf. */
> + if (!in_lto_p)
> + add_name_attribute (comp_unit_die (), remap_debug_filename (filename));
> + else
> + add_name_attribute (comp_unit_die (), "<artificial>");
> + if (!IS_ABSOLUTE_PATH (filename) || targetm.force_at_comp_dir)
> + add_comp_dir_attribute (comp_unit_die ());
> + else if (get_AT (comp_unit_die (), DW_AT_comp_dir) == NULL)
> + {
> + bool p = false;
> + file_table->traverse<bool *, file_table_relative_p> (&p);
> + if (p)
> + add_comp_dir_attribute (comp_unit_die ());
> }
> -
> - limbo_die_list = NULL;
> }
>
> /* Reset all state within dwarf2out.c so that we can rerun the compiler
> Index: gcc/sdbout.c
> ===================================================================
> --- gcc/sdbout.c (revision 226966)
> +++ gcc/sdbout.c (working copy)
> @@ -278,7 +278,7 @@ const struct gcc_debug_hooks sdb_debug_h
> {
> sdbout_init, /* init */
> sdbout_finish, /* finish */
> - debug_nothing_void, /* early_finish */
> + debug_nothing_charstar, /* early_finish */
> debug_nothing_void, /* assembly_start */
> debug_nothing_int_charstar, /* define */
> debug_nothing_int_charstar, /* undef */
> Index: gcc/vmsdbgout.c
> ===================================================================
> --- gcc/vmsdbgout.c (revision 226966)
> +++ gcc/vmsdbgout.c (working copy)
> @@ -147,6 +147,7 @@ static int write_srccorrs (int);
>
> static void vmsdbgout_init (const char *);
> static void vmsdbgout_finish (const char *);
> +static void vmsdbgout_early_finish (const char *);
> static void vmsdbgout_assembly_start (void);
> static void vmsdbgout_define (unsigned int, const char *);
> static void vmsdbgout_undef (unsigned int, const char *);
> @@ -174,7 +175,7 @@ static void vmsdbgout_abstract_function
> const struct gcc_debug_hooks vmsdbg_debug_hooks
> = {vmsdbgout_init,
> vmsdbgout_finish,
> - debug_nothing_void,
> + vmsdbgout_early_finish,
> vmsdbgout_assembly_start,
> vmsdbgout_define,
> vmsdbgout_undef,
> @@ -1552,7 +1553,17 @@ vmsdbgout_abstract_function (tree decl)
> VMS Debug debugging info. */
>
> static void
> -vmsdbgout_finish (const char *filename ATTRIBUTE_UNUSED)
> +vmsdbgout_early_finish (const char *filename)
> +{
> + if (write_symbols == VMS_AND_DWARF2_DEBUG)
> + (*dwarf2_debug_hooks.early_finish) (filename);
> +}
> +
> +/* Output stuff that Debug requires at the end of every file and generate the
> + VMS Debug debugging info. */
> +
> +static void
> +vmsdbgout_finish (const char *filename)
> {
> unsigned int i, ifunc;
> int totsize;
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
