Message ID | 20170708110331.GH71260@kam.mff.cuni.cz |
---|---|
State | New |
Headers | show |
On Sat, Jul 8, 2017 at 1:03 PM, Jan Hubicka <hubicka@ucw.cz> wrote: > Hi, > PR lto/80838 is about lto+profiledbootstrapped compiler being slower than > profiledboostrapped compiler. > > This is caused by a fact that some of object files linked into cc1plus binary > are built with -fPIC and lto-wrapper then decides to make whole binary PIC that > is very slow. While we probably ought to avoid linking PIC code into static > binary but I saw similar issue with firefox and other programs. > > I do not think we want to support mixed PIC/non-PIC symbols internally, because > it would need quite some work and I do not see any reasonable use cases. > > This patch makes merging more realistic/agressive. Linking -fPIC and non-PIC > code together results in non-PIC binary and thus the corresponding flags are > dropped when mismatches occurs. > > It would be nice to warn about it, but I do not know how to make warning > meaningful on targets that are PIC by default. > > Bootstrapped/regtested x86_64-linux, plan to commit it tomorrow after > lto-boottrapping if there are no complains. Hum. I wonder if/why we can't ask the linker about the output binary kind? Richard. > Honza > > PR lto/80838 > * lto-wrapper.c (remove_option): New function. > (merge_and_complain): Merge PIC/PIE options more realistically. > Index: lto-wrapper.c > =================================================================== > --- lto-wrapper.c (revision 250054) > +++ lto-wrapper.c (working copy) > @@ -192,6 +192,20 @@ append_option (struct cl_decoded_option > sizeof (struct cl_decoded_option)); > } > > +/* Remove option number INDEX from DECODED_OPTIONS, update > + DECODED_OPTIONS_COUNT. */ > + > +static void > +remove_option (struct cl_decoded_option **decoded_options, > + int index, unsigned int *decoded_options_count) > +{ > + --*decoded_options_count; > + memmove (&(*decoded_options)[index + 1], > + &(*decoded_options)[index], > + sizeof (struct cl_decoded_option) > + * (*decoded_options_count - index)); > +} > + > /* Try to merge and complain about options FDECODED_OPTIONS when applied > ontop of DECODED_OPTIONS. */ > > @@ -202,6 +216,8 @@ merge_and_complain (struct cl_decoded_op > unsigned int fdecoded_options_count) > { > unsigned int i, j; > + struct cl_decoded_option *pic_option = NULL; > + struct cl_decoded_option *pie_option = NULL; > > /* ??? Merge options from files. Most cases can be > handled by either unioning or intersecting > @@ -238,10 +254,6 @@ merge_and_complain (struct cl_decoded_op > case OPT_fdiagnostics_show_option: > case OPT_fdiagnostics_show_location_: > case OPT_fshow_column: > - case OPT_fPIC: > - case OPT_fpic: > - case OPT_fPIE: > - case OPT_fpie: > case OPT_fcommon: > case OPT_fgnu_tm: > /* Do what the old LTO code did - collect exactly one option > @@ -255,6 +267,16 @@ merge_and_complain (struct cl_decoded_op > append_option (decoded_options, decoded_options_count, foption); > break; > > + /* Figure out what PIC/PIE level wins and merge the results. */ > + case OPT_fPIC: > + case OPT_fpic: > + pic_option = foption; > + break; > + case OPT_fPIE: > + case OPT_fpie: > + pie_option = foption; > + break; > + > case OPT_fopenmp: > case OPT_fopenacc: > case OPT_fcilkplus: > @@ -286,18 +308,6 @@ merge_and_complain (struct cl_decoded_op > foption->orig_option_with_args_text); > break; > > - case OPT_foffload_abi_: > - for (j = 0; j < *decoded_options_count; ++j) > - if ((*decoded_options)[j].opt_index == foption->opt_index) > - break; > - if (j == *decoded_options_count) > - append_option (decoded_options, decoded_options_count, foption); > - else if (foption->value != (*decoded_options)[j].value) > - fatal_error (input_location, > - "Option %s not used consistently in all LTO input" > - " files", foption->orig_option_with_args_text); > - break; > - > case OPT_O: > case OPT_Ofast: > case OPT_Og: > @@ -368,12 +378,70 @@ merge_and_complain (struct cl_decoded_op > (*decoded_options)[j].value = 1; > } > break; > + > + > + case OPT_foffload_abi_: > + for (j = 0; j < *decoded_options_count; ++j) > + if ((*decoded_options)[j].opt_index == foption->opt_index) > + break; > + if (j == *decoded_options_count) > + append_option (decoded_options, decoded_options_count, foption); > + else if (foption->value != (*decoded_options)[j].value) > + fatal_error (input_location, > + "Option %s not used consistently in all LTO input" > + " files", foption->orig_option_with_args_text); > + break; > + > > case OPT_foffload_: > append_option (decoded_options, decoded_options_count, foption); > break; > } > } > + > + /* Merge PIC options: > + -fPIC + -fpic = -fpic > + -fPIC + -fno-pic = -fno-pic > + -fpic/-fPIC + nothin = nothing. > + It is a common mistake to mix few -fPIC compiled objects into otherwise > + non-PIC code. We do not want to build everything with PIC then. > + > + It would be good to warn on mismatches, but it is bit hard to do as > + we do not know what nothing translates to. */ > + > + for (unsigned int j = 0; j < *decoded_options_count;) > + if ((*decoded_options)[j].opt_index == OPT_fPIC > + || (*decoded_options)[j].opt_index == OPT_fpic) > + { > + if (!pic_option > + || (pic_option->value > 0) != ((*decoded_options)[j].value > 0)) > + remove_option (decoded_options, j, decoded_options_count); > + else if (pic_option->opt_index == OPT_fPIC > + && (*decoded_options)[j].opt_index == OPT_fpic) > + { > + (*decoded_options)[j] = *pic_option; > + j++; > + } > + else > + j++; > + } > + else if ((*decoded_options)[j].opt_index == OPT_fPIE > + || (*decoded_options)[j].opt_index == OPT_fpie) > + { > + if (!pie_option > + || pie_option->value != (*decoded_options)[j].value) > + remove_option (decoded_options, j, decoded_options_count); > + else if (pie_option->opt_index == OPT_fPIE > + && (*decoded_options)[j].opt_index == OPT_fpie) > + { > + (*decoded_options)[j] = *pie_option; > + j++; > + } > + else > + j++; > + } > + else > + j++; > } > > /* Auxiliary function that frees elements of PTR and PTR itself.
Index: lto-wrapper.c =================================================================== --- lto-wrapper.c (revision 250054) +++ lto-wrapper.c (working copy) @@ -192,6 +192,20 @@ append_option (struct cl_decoded_option sizeof (struct cl_decoded_option)); } +/* Remove option number INDEX from DECODED_OPTIONS, update + DECODED_OPTIONS_COUNT. */ + +static void +remove_option (struct cl_decoded_option **decoded_options, + int index, unsigned int *decoded_options_count) +{ + --*decoded_options_count; + memmove (&(*decoded_options)[index + 1], + &(*decoded_options)[index], + sizeof (struct cl_decoded_option) + * (*decoded_options_count - index)); +} + /* Try to merge and complain about options FDECODED_OPTIONS when applied ontop of DECODED_OPTIONS. */ @@ -202,6 +216,8 @@ merge_and_complain (struct cl_decoded_op unsigned int fdecoded_options_count) { unsigned int i, j; + struct cl_decoded_option *pic_option = NULL; + struct cl_decoded_option *pie_option = NULL; /* ??? Merge options from files. Most cases can be handled by either unioning or intersecting @@ -238,10 +254,6 @@ merge_and_complain (struct cl_decoded_op case OPT_fdiagnostics_show_option: case OPT_fdiagnostics_show_location_: case OPT_fshow_column: - case OPT_fPIC: - case OPT_fpic: - case OPT_fPIE: - case OPT_fpie: case OPT_fcommon: case OPT_fgnu_tm: /* Do what the old LTO code did - collect exactly one option @@ -255,6 +267,16 @@ merge_and_complain (struct cl_decoded_op append_option (decoded_options, decoded_options_count, foption); break; + /* Figure out what PIC/PIE level wins and merge the results. */ + case OPT_fPIC: + case OPT_fpic: + pic_option = foption; + break; + case OPT_fPIE: + case OPT_fpie: + pie_option = foption; + break; + case OPT_fopenmp: case OPT_fopenacc: case OPT_fcilkplus: @@ -286,18 +308,6 @@ merge_and_complain (struct cl_decoded_op foption->orig_option_with_args_text); break; - case OPT_foffload_abi_: - for (j = 0; j < *decoded_options_count; ++j) - if ((*decoded_options)[j].opt_index == foption->opt_index) - break; - if (j == *decoded_options_count) - append_option (decoded_options, decoded_options_count, foption); - else if (foption->value != (*decoded_options)[j].value) - fatal_error (input_location, - "Option %s not used consistently in all LTO input" - " files", foption->orig_option_with_args_text); - break; - case OPT_O: case OPT_Ofast: case OPT_Og: @@ -368,12 +378,70 @@ merge_and_complain (struct cl_decoded_op (*decoded_options)[j].value = 1; } break; + + + case OPT_foffload_abi_: + for (j = 0; j < *decoded_options_count; ++j) + if ((*decoded_options)[j].opt_index == foption->opt_index) + break; + if (j == *decoded_options_count) + append_option (decoded_options, decoded_options_count, foption); + else if (foption->value != (*decoded_options)[j].value) + fatal_error (input_location, + "Option %s not used consistently in all LTO input" + " files", foption->orig_option_with_args_text); + break; + case OPT_foffload_: append_option (decoded_options, decoded_options_count, foption); break; } } + + /* Merge PIC options: + -fPIC + -fpic = -fpic + -fPIC + -fno-pic = -fno-pic + -fpic/-fPIC + nothin = nothing. + It is a common mistake to mix few -fPIC compiled objects into otherwise + non-PIC code. We do not want to build everything with PIC then. + + It would be good to warn on mismatches, but it is bit hard to do as + we do not know what nothing translates to. */ + + for (unsigned int j = 0; j < *decoded_options_count;) + if ((*decoded_options)[j].opt_index == OPT_fPIC + || (*decoded_options)[j].opt_index == OPT_fpic) + { + if (!pic_option + || (pic_option->value > 0) != ((*decoded_options)[j].value > 0)) + remove_option (decoded_options, j, decoded_options_count); + else if (pic_option->opt_index == OPT_fPIC + && (*decoded_options)[j].opt_index == OPT_fpic) + { + (*decoded_options)[j] = *pic_option; + j++; + } + else + j++; + } + else if ((*decoded_options)[j].opt_index == OPT_fPIE + || (*decoded_options)[j].opt_index == OPT_fpie) + { + if (!pie_option + || pie_option->value != (*decoded_options)[j].value) + remove_option (decoded_options, j, decoded_options_count); + else if (pie_option->opt_index == OPT_fPIE + && (*decoded_options)[j].opt_index == OPT_fpie) + { + (*decoded_options)[j] = *pie_option; + j++; + } + else + j++; + } + else + j++; } /* Auxiliary function that frees elements of PTR and PTR itself.