On Sat, Jul 8, 2017 at 1:03 PM, Jan Hubicka <hubi...@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.
