| Message ID | 20180508153137.GF16916@kam.mff.cuni.cz |
|---|---|
| State | New |
| Headers | show |
| Series | Incremental LTO linking part 6: dwarf2out support | expand |
On 05/08/2018 09:31 AM, Jan Hubicka wrote: > Hi, > this patch tells dwarf2out that it can have early debug not only in WPA mode > but also when incrementally linking. This prevents ICE on almost every testcase > compiled with -g. > > Bootstrapped/regtested x86_64-linux with rest of incremental linking patchet. > Makes sense? > > Honza > > * dwarf2out.c (dwarf2out_die_ref_for_decl, > darf2out_register_external_decl): Support incremental link. OK. I think the full series is ACK'd now. Can you confirm? Thanks, Jeff
> On 05/08/2018 09:31 AM, Jan Hubicka wrote: > > Hi, > > this patch tells dwarf2out that it can have early debug not only in WPA mode > > but also when incrementally linking. This prevents ICE on almost every testcase > > compiled with -g. > > > > Bootstrapped/regtested x86_64-linux with rest of incremental linking patchet. > > Makes sense? > > > > Honza > > > > * dwarf2out.c (dwarf2out_die_ref_for_decl, > > darf2out_register_external_decl): Support incremental link. > OK. I think the full series is ACK'd now. Can you confirm? I hope so. I am not 100% sure if I need ACK from liberty maintainer for simple-object change. But since Richard is happy with it and he is the author of the code and gcc is the only user of this logic, I hope it is OK. As discussed with Richard on IRC, I still plan to update the driver to accept -fno-lto to imply codegen rather than requiring user to pass -flinker-output=nolto-rel which is somewhat odd, so I will send updated patch for that part. Honza > > Thanks, > Jeff
On 05/23/2018 09:54 AM, Jan Hubicka wrote: >> On 05/08/2018 09:31 AM, Jan Hubicka wrote: >>> Hi, >>> this patch tells dwarf2out that it can have early debug not only in WPA mode >>> but also when incrementally linking. This prevents ICE on almost every testcase >>> compiled with -g. >>> >>> Bootstrapped/regtested x86_64-linux with rest of incremental linking patchet. >>> Makes sense? >>> >>> Honza >>> >>> * dwarf2out.c (dwarf2out_die_ref_for_decl, >>> darf2out_register_external_decl): Support incremental link. >> OK. I think the full series is ACK'd now. Can you confirm? > > I hope so. I am not 100% sure if I need ACK from liberty maintainer for > simple-object change. But since Richard is happy with it and he is the author > of the code and gcc is the only user of this logic, I hope it is OK. I think Richi's ACK for the libiberty bits is sufficient. > > As discussed with Richard on IRC, I still plan to update the driver to accept > -fno-lto to imply codegen rather than requiring user to pass -flinker-output=nolto-rel > which is somewhat odd, so I will send updated patch for that part. Understood. jeff
> On 05/23/2018 09:54 AM, Jan Hubicka wrote: > >> On 05/08/2018 09:31 AM, Jan Hubicka wrote: > >>> Hi, > >>> this patch tells dwarf2out that it can have early debug not only in WPA mode > >>> but also when incrementally linking. This prevents ICE on almost every testcase > >>> compiled with -g. > >>> > >>> Bootstrapped/regtested x86_64-linux with rest of incremental linking patchet. > >>> Makes sense? > >>> > >>> Honza > >>> > >>> * dwarf2out.c (dwarf2out_die_ref_for_decl, > >>> darf2out_register_external_decl): Support incremental link. > >> OK. I think the full series is ACK'd now. Can you confirm? > > > > I hope so. I am not 100% sure if I need ACK from liberty maintainer for > > simple-object change. But since Richard is happy with it and he is the author > > of the code and gcc is the only user of this logic, I hope it is OK. > I think Richi's ACK for the libiberty bits is sufficient. > > > > > > As discussed with Richard on IRC, I still plan to update the driver to accept > > -fno-lto to imply codegen rather than requiring user to pass -flinker-output=nolto-rel > > which is somewhat odd, so I will send updated patch for that part. > Understood. Actually I looked into it today and reminded why I did not do that at first place. -fno-lto is already supported and it disables the lto path - so the result is incremental link of all IL objects without merging the common sections (via linker) and is used by Andi's linux kernel patchset. I guess we want to keep this behaviour and thus I decided to go with the -flinker-output=nolto-rel. We can improve on it incrementally if we find better semantics. Honza > > jeff
Index: dwarf2out.c =================================================================== --- dwarf2out.c (revision 260042) +++ dwarf2out.c (working copy) @@ -5822,7 +5822,7 @@ { dw_die_ref die; - if (flag_wpa && !decl_die_table) + if ((flag_wpa || flag_incremental_link == 2) && !decl_die_table) return false; if (TREE_CODE (decl) == BLOCK) @@ -5832,10 +5832,10 @@ if (!die) return false; - /* During WPA stage we currently use DIEs to store the - decl <-> label + offset map. That's quite inefficient but it - works for now. */ - if (flag_wpa) + /* During WPA stage and incremental linking we currently use DIEs + to store the decl <-> label + offset map. That's quite inefficient + but it works for now. */ + if (flag_wpa || flag_incremental_link == 2) { dw_die_ref ref = get_AT_ref (die, DW_AT_abstract_origin); if (!ref) @@ -5886,7 +5886,7 @@ if (debug_info_level == DINFO_LEVEL_NONE) return; - if (flag_wpa && !decl_die_table) + if ((flag_wpa || flag_incremental_link == 2) && !decl_die_table) decl_die_table = hash_table<decl_die_hasher>::create_ggc (1000); dw_die_ref die @@ -5921,7 +5921,8 @@ parent = BLOCK_DIE (ctx); else if (TREE_CODE (ctx) == TRANSLATION_UNIT_DECL /* Keep the 1:1 association during WPA. */ - && !flag_wpa) + && !flag_wpa + && flag_incremental_link != 2) /* Otherwise all late annotations go to the main CU which imports the original CUs. */ parent = comp_unit_die (); @@ -5942,7 +5943,7 @@ switch (TREE_CODE (decl)) { case TRANSLATION_UNIT_DECL: - if (! flag_wpa) + if (! flag_wpa && flag_incremental_link != 2) { die = comp_unit_die (); dw_die_ref import = new_die (DW_TAG_imported_unit, die, NULL_TREE);