| Message ID | 54B87E5B.1090502@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 01/15/2015 09:58 PM, Aldy Hernandez wrote: > The attached patch generates early DIEs for the C++ > clones in the C++ parser. This strikes me as an unnecessary abstraction violation. > + /* Emit debug information for clones. */ > + symtab_node *node; > + FOR_EACH_DEFINED_SYMBOL (node) > + if (DECL_ABSTRACT_ORIGIN (node->decl)) > + debug_hooks->early_global_decl (DECL_ABSTRACT_ORIGIN (node->decl)); And I note that you aren't even doing anything with the clone here, which suggests even more strongly that this is not what we want to do. > Now back to limbdo_die_list... My approach is to flush the limbo list, > generically, after the front-ends have finished, by adding a new > "early_finish" debug hook. This gets rid of any permanence into LTO > time. Then I flush it out again, if the middle end (or LTO, etc) has > added any limbo DIEs. Can you remove the first flush and just do it in the second place? Jason
On Fri, Jan 16, 2015 at 4:11 AM, Jason Merrill <jason@redhat.com> wrote: > On 01/15/2015 09:58 PM, Aldy Hernandez wrote: >> >> The attached patch generates early DIEs for the C++ >> clones in the C++ parser. > > > This strikes me as an unnecessary abstraction violation. I'd hope that in the very distant future all early DIEs would be "created" by the frontends (that is, dwarf2out.c wouldn't walk into parents/siblings so much). >> + /* Emit debug information for clones. */ >> + symtab_node *node; >> + FOR_EACH_DEFINED_SYMBOL (node) >> + if (DECL_ABSTRACT_ORIGIN (node->decl)) >> + debug_hooks->early_global_decl (DECL_ABSTRACT_ORIGIN (node->decl)); > > > And I note that you aren't even doing anything with the clone here, which > suggests even more strongly that this is not what we want to do. There must be a better place in the frontend to call early_global_decl for those? Maybe the point where we create the clone? >> Now back to limbdo_die_list... My approach is to flush the limbo list, >> generically, after the front-ends have finished, by adding a new >> "early_finish" debug hook. This gets rid of any permanence into LTO >> time. Then I flush it out again, if the middle end (or LTO, etc) has >> added any limbo DIEs. > > > Can you remove the first flush and just do it in the second place? I hoped we wouldn't need the limbo list at all ... that is, parent DIEs are always present when we create children. I think that should work in principle if the frontends would create DIEs while parsing. Note that dwarf2out forces parent DIE creation in some cases but not in some others - it would be interesting to sort out which parent DIEs it thinks it cannot create when we create the DIE for a sibling. Maybe it's just poor ordering of early_global_decl calls? Richard. > Jason >
On 01/16/2015 05:55 AM, Richard Biener wrote: > I'd hope that in the very distant future all early DIEs would be "created" > by the frontends (that is, dwarf2out.c wouldn't walk into parents/siblings > so much). Are you thinking that the front end would immediately call a debug hook for every block, local variable and such, or just for higher level entities? > I hoped we wouldn't need the limbo list at all ... that is, parent DIEs > are always present when we create children. I think that should > work in principle if the frontends would create DIEs while parsing. So create the function DIE as soon as we see the declaration? That seems reasonable. Then that would be the point of early debug, not later at EOF. > Note that dwarf2out forces parent DIE creation in some cases > but not in some others - it would be interesting to sort out which > parent DIEs it thinks it cannot create when we create the DIE > for a sibling. Maybe it's just poor ordering of early_global_decl > calls? Agreed. Jason
On 01/15/2015 07:11 PM, Jason Merrill wrote: > On 01/15/2015 09:58 PM, Aldy Hernandez wrote: >> Now back to limbdo_die_list... My approach is to flush the limbo list, >> generically, after the front-ends have finished, by adding a new >> "early_finish" debug hook. This gets rid of any permanence into LTO >> time. Then I flush it out again, if the middle end (or LTO, etc) has >> added any limbo DIEs. > > Can you remove the first flush and just do it in the second place? If I only flush the limbo list in the second place, that's basically what mainline does, albeit abstracted into a function. I thought the whole point was to get rid of the limbo list, or at least keep it from being a structure that has to go through LTO streaming. Aldy
On 01/16/2015 08:31 AM, Jason Merrill wrote: > On 01/16/2015 05:55 AM, Richard Biener wrote: >> I'd hope that in the very distant future all early DIEs would be >> "created" >> by the frontends (that is, dwarf2out.c wouldn't walk into >> parents/siblings >> so much). > > Are you thinking that the front end would immediately call a debug hook > for every block, local variable and such, or just for higher level > entities? In the very distant future, as in, when Aldy is retired and living in a tropical island somewhere? :). > >> I hoped we wouldn't need the limbo list at all ... that is, parent DIEs >> are always present when we create children. I think that should >> work in principle if the frontends would create DIEs while parsing. > > So create the function DIE as soon as we see the declaration? That > seems reasonable. Then that would be the point of early debug, not > later at EOF. I'm certainly game to exploring this option, though I think we should be able to get this working at EOF. The reason why I didn't take this approach originally is because it seemed like a lot more hassle, especially since we have to do the same thing for all other front-ends. > >> Note that dwarf2out forces parent DIE creation in some cases >> but not in some others - it would be interesting to sort out which >> parent DIEs it thinks it cannot create when we create the DIE >> for a sibling. Maybe it's just poor ordering of early_global_decl >> calls? > > Agreed. Regrettably, I have to agree as well. I can investigate why some DIEs end up orphaned and report back. Thanks. Aldy
On January 16, 2015 5:31:49 PM CET, Jason Merrill <jason@redhat.com> wrote: >On 01/16/2015 05:55 AM, Richard Biener wrote: >> I'd hope that in the very distant future all early DIEs would be >"created" >> by the frontends (that is, dwarf2out.c wouldn't walk into >parents/siblings >> so much). > >Are you thinking that the front end would immediately call a debug hook > >for every block, local variable and such, or just for higher level >entities? For every block, local variable and such. Yes. The FE then also has the chance to append whatever FE specific attributes without langhooks in dwarf2out. >> I hoped we wouldn't need the limbo list at all ... that is, parent >DIEs >> are always present when we create children. I think that should >> work in principle if the frontends would create DIEs while parsing. > >So create the function DIE as soon as we see the declaration? That >seems reasonable. Then that would be the point of early debug, not >later at EOF. Yes. Richard. >> Note that dwarf2out forces parent DIE creation in some cases >> but not in some others - it would be interesting to sort out which >> parent DIEs it thinks it cannot create when we create the DIE >> for a sibling. Maybe it's just poor ordering of early_global_decl >> calls? > >Agreed. > >Jason
On 01/16/2015 12:50 PM, Aldy Hernandez wrote: >> Can you remove the first flush and just do it in the second place? > > If I only flush the limbo list in the second place, that's basically > what mainline does, albeit abstracted into a function. I thought the > whole point was to get rid of the limbo list, or at least keep it from > being a structure that has to go through LTO streaming. It would expect it to be before free_lang_data and LTO streaming. Jason
On 01/16/2015 02:55 AM, Richard Biener wrote: > On Fri, Jan 16, 2015 at 4:11 AM, Jason Merrill <jason@redhat.com> wrote: >> On 01/15/2015 09:58 PM, Aldy Hernandez wrote: > I hoped we wouldn't need the limbo list at all ... that is, parent DIEs > are always present when we create children. I think that should > work in principle if the frontends would create DIEs while parsing. By the way, don't think I ignored this suggestion. I played around with this approach for the C++ front-end, and gave up after having to keep track of a few too many things to get the parenthood right. It's totally doable; it was just easier to flush out the limbo DIE list as with my current patch. Ok, call me lazy :). > Note that dwarf2out forces parent DIE creation in some cases > but not in some others - it would be interesting to sort out which > parent DIEs it thinks it cannot create when we create the DIE > for a sibling. Maybe it's just poor ordering of early_global_decl > calls? In my current patch, I added an ICE to make sure we're not creating limbo destined DIEs past early dwarf dumping. The only exception to this (temporarily) is that the ltrans stage is adding things late, but that's only because we're not streaming dwarf as the ultimate plan is. I've documented all this. Aldy
diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c index 691688b..b9a407a 100644 --- a/gcc/cp/decl2.c +++ b/gcc/cp/decl2.c @@ -4737,6 +4737,13 @@ c_parse_final_cleanups (void) generate initial debug information. */ timevar_stop (TV_PHASE_PARSING); timevar_start (TV_PHASE_DBGINFO); + + /* Emit debug information for clones. */ + symtab_node *node; + FOR_EACH_DEFINED_SYMBOL (node) + if (DECL_ABSTRACT_ORIGIN (node->decl)) + debug_hooks->early_global_decl (DECL_ABSTRACT_ORIGIN (node->decl)); + walk_namespaces (emit_debug_for_namespace, 0); if (vec_safe_length (pending_statics) != 0) { diff --git a/gcc/dbxout.c b/gcc/dbxout.c index 430a2eb..202ef8a 100644 --- a/gcc/dbxout.c +++ b/gcc/dbxout.c @@ -359,6 +359,7 @@ const struct gcc_debug_hooks dbx_debug_hooks = dbxout_init, dbxout_finish, debug_nothing_void, + debug_nothing_void, debug_nothing_int_charstar, debug_nothing_int_charstar, dbxout_start_source_file, @@ -400,6 +401,7 @@ const struct gcc_debug_hooks xcoff_debug_hooks = dbxout_init, dbxout_finish, debug_nothing_void, + debug_nothing_void, debug_nothing_int_charstar, debug_nothing_int_charstar, dbxout_start_source_file, diff --git a/gcc/debug.c b/gcc/debug.c index 449d3a1..d0e00c0 100644 --- a/gcc/debug.c +++ b/gcc/debug.c @@ -27,6 +27,7 @@ const struct gcc_debug_hooks do_nothing_debug_hooks = { debug_nothing_charstar, debug_nothing_charstar, + debug_nothing_void, /* early_finish */ debug_nothing_void, debug_nothing_int_charstar, debug_nothing_int_charstar, diff --git a/gcc/debug.h b/gcc/debug.h index f9485bc..a8d3f23 100644 --- a/gcc/debug.h +++ b/gcc/debug.h @@ -30,6 +30,9 @@ struct gcc_debug_hooks /* Output debug symbols. */ void (* finish) (const char *main_filename); + /* Run cleanups necessary after early debug generation. */ + void (* early_finish) (void); + /* Called from cgraph_optimize before starting to assemble functions/variables/toplevel asms. */ void (* assembly_start) (void); diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c index e3ccda2..80d43af 100644 --- a/gcc/dwarf2out.c +++ b/gcc/dwarf2out.c @@ -2424,6 +2424,7 @@ build_cfa_aligned_loc (dw_cfa_location *cfa, static void dwarf2out_init (const char *); static void dwarf2out_finish (const char *); +static void dwarf2out_early_finish (void); static void dwarf2out_assembly_start (void); static void dwarf2out_define (unsigned int, const char *); static void dwarf2out_undef (unsigned int, const char *); @@ -2451,6 +2452,7 @@ const struct gcc_debug_hooks dwarf2_debug_hooks = { dwarf2out_init, dwarf2out_finish, + dwarf2out_early_finish, dwarf2out_assembly_start, dwarf2out_define, dwarf2out_undef, @@ -24739,10 +24741,27 @@ optimize_location_lists (dw_die_ref die) static void dwarf2out_finish (const char *filename) { - limbo_die_node *node, *next_node; comdat_type_node *ctnode; dw_die_ref main_comp_unit_die; + /* Technically limbo_die_list should either be empty because + dwarf2out_early_finish took care of everything, or should _only_ + have inline instances that will be created late (which is OK, + because they'll be pointing to the early generated abstract). + + However, functions can be created after the parsers are done + (e.g. create_cilk_helper_decl), which will populate the + limbo_die_list again. Eventually all late function creation + should be banned, or these new functions should be created with a + proper context_die, thus avoiding limbo. For now, it doesn't + hurt, since late creation should have all the location + information needed (and if it doesn't, we don't care-- the code + is undebuggable). + + If there was anything created late, flush it out. */ + if (limbo_die_list) + dwarf2out_early_finish (); + /* 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. */ @@ -24767,55 +24786,6 @@ dwarf2out_finish (const char *filename) add_comp_dir_attribute (comp_unit_die ()); } - /* 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. */ - 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; - #if ENABLE_ASSERT_CHECKING { dw_die_ref die = comp_unit_die (), c; @@ -24860,9 +24830,10 @@ dwarf2out_finish (const char *filename) if (flag_eliminate_dwarf2_dups) break_out_includes (comp_unit_die ()); - /* Traverse the DIE's and add add sibling attributes to those DIE's + /* Traverse the DIE's and add sibling attributes to those DIE's that have children. */ add_sibling_attributes (comp_unit_die ()); + limbo_die_node *node; for (node = limbo_die_list; node; node = node->next) add_sibling_attributes (node->die); for (ctnode = comdat_type_list; ctnode != NULL; ctnode = ctnode->next) @@ -25124,6 +25095,66 @@ dwarf2out_finish (const char *filename) output_indirect_strings (); } +/* Perform any cleanups needed after the early debug generation pass + has run. */ + +static void +dwarf2out_early_finish (void) +{ + /* 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 + and we don't need to stream it for LTO. */ + 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; +} + /* Reset all state within dwarf2out.c so that we can rerun the compiler within the same process. For use by toplev::finalize. */ diff --git a/gcc/sdbout.c b/gcc/sdbout.c index d7b2d6b..43b8cf2 100644 --- a/gcc/sdbout.c +++ b/gcc/sdbout.c @@ -279,6 +279,7 @@ const struct gcc_debug_hooks sdb_debug_hooks = { sdbout_init, /* init */ sdbout_finish, /* finish */ + debug_nothing_void, /* early_finish */ debug_nothing_void, /* assembly_start */ debug_nothing_int_charstar, /* define */ debug_nothing_int_charstar, /* undef */ diff --git a/gcc/toplev.c b/gcc/toplev.c index 42a2cdc..12fa509 100644 --- a/gcc/toplev.c +++ b/gcc/toplev.c @@ -586,6 +586,10 @@ compile_file (void) if (flag_syntax_only || flag_wpa) return; + /* Clean up anything that needs cleaning up after initial debug + generation. */ + (*debug_hooks->early_finish) (); + ggc_protect_identifiers = false; /* Run the actual compilation process. */ diff --git a/gcc/vmsdbgout.c b/gcc/vmsdbgout.c index 5cb66bc..6da48eb 100644 --- a/gcc/vmsdbgout.c +++ b/gcc/vmsdbgout.c @@ -179,6 +179,7 @@ static void vmsdbgout_abstract_function (tree); const struct gcc_debug_hooks vmsdbg_debug_hooks = {vmsdbgout_init, vmsdbgout_finish, + debug_nothing_void, vmsdbgout_assembly_start, vmsdbgout_define, vmsdbgout_undef,
Hi Jason. Hi Richard. While adjusting the limbdo_die_list for early debug I noticed we weren't early generating C++ constructors/destructors. This got me on a detour of sorts. I noticed dwarf2out's gen_member_die() disallows generation of clones earlier, by design: /* Don't include clones in the member list. */ if (DECL_ABSTRACT_ORIGIN (member)) continue; I played around trying to disable this "feature", but my approach ran into various walls, and I decided instead to attack it from the front-end side. The attached patch generates early DIEs for the C++ clones in the C++ parser. I'd be (un)happy to revisit the dwarf2out approach if it's deemed more appropriate. Now back to limbdo_die_list... My approach is to flush the limbo list, generically, after the front-ends have finished, by adding a new "early_finish" debug hook. This gets rid of any permanence into LTO time. Then I flush it out again, if the middle end (or LTO, etc) has added any limbo DIEs. I wish we didn't need to flush the limbo list past the compilation proper, but some things like Cilk+ generate function trees way late in the game, so we end up generating limbo DIEs past where I'd like to. Please see the comment in the source code. All in all, this patch should get rid of any middle-end dependence on the early debug generated limbo DIEs. Are you happy with this? I'd like to get your input before committing to this approach. Thanks. Aldy commit cd20df2a48ce4f0d4e64626d61f8f61a810cf742 Author: Aldy Hernandez <aldyh@redhat.com> Date: Wed Jan 14 13:33:40 2015 -0800 * cp/decl2.c (c_parse_final_cleanups): Emit early debug information for clones. * dbxout.c (dbx_debug_hooks): Add early_finish field. (xcoff_debug_hooks): Same. * sdbout.c (sdb_debug_hooks): Same. * vmsdbgout.c (vmsdbg_debug_hooks): Same. * debug.c (do_nothing_debug_hooks): Same. * debug.h (struct gcc_debug_hooks): Same. * dwarf2out.c (dwarf2_debug_hooks): Same. (dwarf2out_finish): Move limbo list handling to... (dwarf2out_early_finish): ...here. * toplev.c (compile_file): Call debug_hooks->early_finish.