| Message ID | 20110405141939.GB17079@tyan-ft48-01.lab.bos.redhat.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Apr 05, 2011 at 04:19:39PM +0200, Jakub Jelinek wrote: > + for (i = 0; i < VEC_length (deferred_locations, deferred_locations_list); i++) > + { > + add_location_or_const_value_attribute ( > + VEC_index (deferred_locations, deferred_locations_list, i)->die, > + VEC_index (deferred_locations, deferred_locations_list, i)->variable, > + false, > + DW_AT_location); > + } Tiny, non-binding suggestion: use FOR_EACH_VEC_ELT here? -Nathan
On Tue, Apr 05, 2011 at 02:06:14PM -0700, Nathan Froyd wrote: > On Tue, Apr 05, 2011 at 04:19:39PM +0200, Jakub Jelinek wrote: > > + for (i = 0; i < VEC_length (deferred_locations, deferred_locations_list); i++) > > + { > > + add_location_or_const_value_attribute ( > > + VEC_index (deferred_locations, deferred_locations_list, i)->die, > > + VEC_index (deferred_locations, deferred_locations_list, i)->variable, > > + false, > > + DW_AT_location); > > + } > > Tiny, non-binding suggestion: use FOR_EACH_VEC_ELT here? Feel free to do that afterwards, while diff decided to include that part of code in the patch, it wasn't actually changing at all, what changed was that the code afterwards was moved into a separate function and the length of the code being moved probably was bigger than length from dwarf2out_finish start to that point. Jakub
On 04/05/2011 10:19 AM, Jakub Jelinek wrote: > i686-linux LTO bootstrap currently fails, because in one partition > we emit .Ldebug_info0 label twice. The problem is that > resolve_addr for call_site support attempts to force_decl_die external > function decls, and at least with LTO that in turn can attempt > to create new type DIEs, in this case an enumeration with context_die > being NULL. Unfortunately the code to add proper parents to limbo nodes > is done right before resolve_addr (and should be done there, so that > resolve_addr reaches all the needed DIEs). > > +/* 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. */ Sounds like this comment needs to be updated if there can be types on the list as well. Jason
On Fri, Apr 08, 2011 at 01:58:14PM -0400, Jason Merrill wrote: > On 04/05/2011 10:19 AM, Jakub Jelinek wrote: > >i686-linux LTO bootstrap currently fails, because in one partition > >we emit .Ldebug_info0 label twice. The problem is that > >resolve_addr for call_site support attempts to force_decl_die external > >function decls, and at least with LTO that in turn can attempt > >to create new type DIEs, in this case an enumeration with context_die > >being NULL. Unfortunately the code to add proper parents to limbo nodes > >is done right before resolve_addr (and should be done there, so that > >resolve_addr reaches all the needed DIEs). > > > >+/* 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. */ > > Sounds like this comment needs to be updated if there can be types > on the list as well. On a closer look, this seems to be because LTO messes up types terribly, struct cpp_options's lang field doesn't have enum c_lang type, but enum prec whose TYPE_CONTEXT is c_parser_binary_expression function from c_parser.c. So when trying to create DIE for cpp_options and stuff in it we end up with the surprising limbo die. Therefore, I'm withdrawing my patch and will look into this mess on Monday. Jakub
On Fri, 8 Apr 2011, Jakub Jelinek wrote: > On Fri, Apr 08, 2011 at 01:58:14PM -0400, Jason Merrill wrote: > > On 04/05/2011 10:19 AM, Jakub Jelinek wrote: > > >i686-linux LTO bootstrap currently fails, because in one partition > > >we emit .Ldebug_info0 label twice. The problem is that > > >resolve_addr for call_site support attempts to force_decl_die external > > >function decls, and at least with LTO that in turn can attempt > > >to create new type DIEs, in this case an enumeration with context_die > > >being NULL. Unfortunately the code to add proper parents to limbo nodes > > >is done right before resolve_addr (and should be done there, so that > > >resolve_addr reaches all the needed DIEs). > > > > > >+/* 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. */ > > > > Sounds like this comment needs to be updated if there can be types > > on the list as well. > > On a closer look, this seems to be because LTO messes up types terribly, > struct cpp_options's lang field doesn't have enum c_lang type, but > enum prec whose TYPE_CONTEXT is c_parser_binary_expression > function from c_parser.c. So when trying to create DIE for cpp_options > and stuff in it we end up with the surprising limbo die. > Therefore, I'm withdrawing my patch and will look into this mess on Monday. We are definitely unifying enum types too eagerly. It's on my TODO to fix that, but it had low priority sofar. Richard.
Hi, On Fri, 8 Apr 2011, Richard Guenther wrote: > > > Sounds like this comment needs to be updated if there can be types > > > on the list as well. > > > > On a closer look, this seems to be because LTO messes up types > > terribly, struct cpp_options's lang field doesn't have enum c_lang > > type, but enum prec whose TYPE_CONTEXT is c_parser_binary_expression > > function from c_parser.c. So when trying to create DIE for > > cpp_options and stuff in it we end up with the surprising limbo die. > > Therefore, I'm withdrawing my patch and will look into this mess on > > Monday. > > We are definitely unifying enum types too eagerly. It's on my TODO to > fix that, but it had low priority sofar. It's too eager "only" for debug info, and that is in a suboptimal state for LTO anyway. early-debug-info will fix all of our problems. ahem :-) Ciao, Michael.
--- gcc/dwarf2out.c.jj 2011-04-05 12:34:11.000000000 +0200 +++ gcc/dwarf2out.c 2011-04-05 13:34:04.628420737 +0200 @@ -23495,47 +23495,17 @@ optimize_location_lists (dw_die_ref die) htab_delete (htab); } -/* Output stuff that dwarf requires at the end of every file, - and generate the DWARF-2 debugging info. */ +/* 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 -dwarf2out_finish (const char *filename) +add_parents_for_limbo_dies (void) { limbo_die_node *node, *next_node; - comdat_type_node *ctnode; - htab_t comdat_type_table; - unsigned int i; - - 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. */ - add_name_attribute (comp_unit_die (), remap_debug_filename (filename)); - if (!IS_ABSOLUTE_PATH (filename)) - add_comp_dir_attribute (comp_unit_die ()); - else if (get_AT (comp_unit_die (), DW_AT_comp_dir) == NULL) - { - bool p = false; - htab_traverse (file_table, file_table_relative_p, &p); - if (p) - add_comp_dir_attribute (comp_unit_die ()); - } - - for (i = 0; i < VEC_length (deferred_locations, deferred_locations_list); i++) - { - add_location_or_const_value_attribute ( - VEC_index (deferred_locations, deferred_locations_list, i)->die, - VEC_index (deferred_locations, deferred_locations_list, i)->variable, - false, - DW_AT_location); - } - - /* 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; @@ -23587,9 +23557,52 @@ dwarf2out_finish (const char *filename) } 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) +{ + limbo_die_node *node; + comdat_type_node *ctnode; + htab_t comdat_type_table; + unsigned int i; + + 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. */ + add_name_attribute (comp_unit_die (), remap_debug_filename (filename)); + if (!IS_ABSOLUTE_PATH (filename)) + add_comp_dir_attribute (comp_unit_die ()); + else if (get_AT (comp_unit_die (), DW_AT_comp_dir) == NULL) + { + bool p = false; + htab_traverse (file_table, file_table_relative_p, &p); + if (p) + add_comp_dir_attribute (comp_unit_die ()); + } + + for (i = 0; i < VEC_length (deferred_locations, deferred_locations_list); i++) + { + add_location_or_const_value_attribute ( + VEC_index (deferred_locations, deferred_locations_list, i)->die, + VEC_index (deferred_locations, deferred_locations_list, i)->variable, + false, + DW_AT_location); + } + + add_parents_for_limbo_dies (); resolve_addr (comp_unit_die ()); + /* resolve_addr might have created new DIEs. */ + if (limbo_die_list) + add_parents_for_limbo_dies (); + for (node = deferred_asm_name; node; node = node->next) { tree decl = node->created_for;