| Message ID | 3166871c-42bb-f88e-7d28-c8dc41fcbdc1@arm.com |
|---|---|
| State | New |
| Headers | show |
| Series | PR driver/91130 Use CL_DRIVER when handling of COLLECT_GCC_OPTIONS in lto-wrapper.c | expand |
On Wed, Aug 07, 2019 at 01:42:34PM +0100, Richard Earnshaw (lists) wrote: > Some options are handled differently by the main driver (gcc, g++, etc) from > the back-end compiler programs (cc1, cc1plus, etc) in that in the driver > they do not take an additional argument, while in the compiler programs they > do. The processing option option CL_DRIVER controls this alternative > interpretation of the options. > > The environment variable COLLECT_GCC_OPTIONS is the list of options to add > to a compile if the compiler re-invokes itself at some point. As such, the > options are driver options, so CL_DRIVER should be used when processing this > list. Currently lto-wrapper is doing this incorrectly. > > PR driver/91130 > * lto-wrapper.c (find_and_merge_options): Use CL_DRIVER when > processing COLLECT_GCC_OPTIONS. > (run_gcc): Likewise. > > Bootstrapped on aarch64-linux > > OK? > > NB, this is essentially the same as Richi's patch in the PR. I'll let you > decide which to take... I think I'd prefer the patch Richi pasted with the --- gcc/lto-wrapper.c 2019-08-07 14:36:30.781562354 +0200 +++ gcc/lto-wrapper.c 2019-08-07 15:48:55.140279384 +0200 @@ -128,7 +128,7 @@ maybe_unlink (const char *file) #define DUMPBASE_SUFFIX ".ltrans18446744073709551615" /* Create decoded options from the COLLECT_GCC and COLLECT_GCC_OPTIONS - environment according to LANG_MASK. */ + environment. */ static void get_options_from_collect_gcc_options (const char *collect_gcc, incremental change, because it really doesn't make sense to pass to get_options_from_collect_gcc_options the same constant value in both invocations (well, it didn't make sense before either). Though, I'm fine if you commit your patch now as a fix and Richi's patch with the above incremental change is applied on top of it incrementally as a cleanup. Jakub
On 07/08/2019 14:51, Jakub Jelinek wrote: > On Wed, Aug 07, 2019 at 01:42:34PM +0100, Richard Earnshaw (lists) wrote: >> Some options are handled differently by the main driver (gcc, g++, etc) from >> the back-end compiler programs (cc1, cc1plus, etc) in that in the driver >> they do not take an additional argument, while in the compiler programs they >> do. The processing option option CL_DRIVER controls this alternative >> interpretation of the options. >> >> The environment variable COLLECT_GCC_OPTIONS is the list of options to add >> to a compile if the compiler re-invokes itself at some point. As such, the >> options are driver options, so CL_DRIVER should be used when processing this >> list. Currently lto-wrapper is doing this incorrectly. >> >> PR driver/91130 >> * lto-wrapper.c (find_and_merge_options): Use CL_DRIVER when >> processing COLLECT_GCC_OPTIONS. >> (run_gcc): Likewise. >> >> Bootstrapped on aarch64-linux >> >> OK? >> >> NB, this is essentially the same as Richi's patch in the PR. I'll let you >> decide which to take... > > I think I'd prefer the patch Richi pasted with the > > --- gcc/lto-wrapper.c 2019-08-07 14:36:30.781562354 +0200 > +++ gcc/lto-wrapper.c 2019-08-07 15:48:55.140279384 +0200 > @@ -128,7 +128,7 @@ maybe_unlink (const char *file) > #define DUMPBASE_SUFFIX ".ltrans18446744073709551615" > > /* Create decoded options from the COLLECT_GCC and COLLECT_GCC_OPTIONS > - environment according to LANG_MASK. */ > + environment. */ > > static void > get_options_from_collect_gcc_options (const char *collect_gcc, > > incremental change, because it really doesn't make sense to pass to > get_options_from_collect_gcc_options the same constant value in both > invocations (well, it didn't make sense before either). > > Though, I'm fine if you commit your patch now as a fix and Richi's patch > with the above incremental change is applied on top of it incrementally > as a cleanup. > > Jakub > Ok, I'll do that. Do you want it on the gcc-9 branch as well? I'm running a bootstrap of it right now. R.
On Wed, Aug 07, 2019 at 05:08:28PM +0100, Richard Earnshaw (lists) wrote: > > Though, I'm fine if you commit your patch now as a fix and Richi's patch > > with the above incremental change is applied on top of it incrementally > > as a cleanup. > > > > Jakub > > > > Ok, I'll do that. Thanks. > Do you want it on the gcc-9 branch as well? I'm running > a bootstrap of it right now. Yes, but can you defer for 9.2.1, i.e. Tuesday+ next week? Jakub
On 07/08/2019 17:15, Jakub Jelinek wrote: > On Wed, Aug 07, 2019 at 05:08:28PM +0100, Richard Earnshaw (lists) wrote: >>> Though, I'm fine if you commit your patch now as a fix and Richi's patch >>> with the above incremental change is applied on top of it incrementally >>> as a cleanup. >>> >>> Jakub >>> >> >> Ok, I'll do that. > > Thanks. > >> Do you want it on the gcc-9 branch as well? I'm running >> a bootstrap of it right now. > > Yes, but can you defer for 9.2.1, i.e. Tuesday+ next week? > > Jakub > I'm OoO next week, but can do it when I get back. R.
On Wed, 7 Aug 2019, Richard Earnshaw (lists) wrote: > On 07/08/2019 17:15, Jakub Jelinek wrote: > > On Wed, Aug 07, 2019 at 05:08:28PM +0100, Richard Earnshaw (lists) wrote: > > > > Though, I'm fine if you commit your patch now as a fix and Richi's patch > > > > with the above incremental change is applied on top of it incrementally > > > > as a cleanup. > > > > > > > > Jakub > > > > > > > > > > Ok, I'll do that. > > > > Thanks. > > > > > Do you want it on the gcc-9 branch as well? I'm running > > > a bootstrap of it right now. > > > > Yes, but can you defer for 9.2.1, i.e. Tuesday+ next week? > > > > Jakub > > > > I'm OoO next week, but can do it when I get back. I installed the following followup after testing on x86_64-unknown-linux-gnu. Richard. 2019-08-12 Richard Biener <rguenther@suse.de> PR driver/91130 * lto-wrapper.c (get_options_from_collect_gcc_options): Remove lang_mask option, always use CL_DRIVER. (get_options_from_collect_gcc_options): Adjust. (find_and_merge_options): Likewise. (run_gcc): Likewise. Index: gcc/lto-wrapper.c =================================================================== --- gcc/lto-wrapper.c (revision 274235) +++ gcc/lto-wrapper.c (working copy) @@ -128,12 +128,11 @@ maybe_unlink (const char *file) #define DUMPBASE_SUFFIX ".ltrans18446744073709551615" /* Create decoded options from the COLLECT_GCC and COLLECT_GCC_OPTIONS - environment according to LANG_MASK. */ + environment. */ static void get_options_from_collect_gcc_options (const char *collect_gcc, const char *collect_gcc_options, - unsigned int lang_mask, struct cl_decoded_option **decoded_options, unsigned int *decoded_options_count) { @@ -176,8 +175,7 @@ get_options_from_collect_gcc_options (co argc = obstack_object_size (&argv_obstack) / sizeof (void *) - 1; argv = XOBFINISH (&argv_obstack, const char **); - decode_cmdline_options_to_array (argc, (const char **)argv, - lang_mask, + decode_cmdline_options_to_array (argc, (const char **)argv, CL_DRIVER, decoded_options, decoded_options_count); obstack_free (&argv_obstack, NULL); } @@ -1009,8 +1007,7 @@ find_and_merge_options (int fd, off_t fi { struct cl_decoded_option *f2decoded_options; unsigned int f2decoded_options_count; - get_options_from_collect_gcc_options (collect_gcc, - fopts, CL_DRIVER, + get_options_from_collect_gcc_options (collect_gcc, fopts, &f2decoded_options, &f2decoded_options_count); if (!fdecoded_options) @@ -1282,7 +1279,6 @@ run_gcc (unsigned argc, char *argv[]) fatal_error (input_location, "environment variable %<COLLECT_GCC_OPTIONS%> must be set"); get_options_from_collect_gcc_options (collect_gcc, collect_gcc_options, - CL_DRIVER, &decoded_options, &decoded_options_count);
On Mon, 12 Aug 2019, Richard Biener wrote: > On Wed, 7 Aug 2019, Richard Earnshaw (lists) wrote: > > > On 07/08/2019 17:15, Jakub Jelinek wrote: > > > On Wed, Aug 07, 2019 at 05:08:28PM +0100, Richard Earnshaw (lists) wrote: > > > > > Though, I'm fine if you commit your patch now as a fix and Richi's patch > > > > > with the above incremental change is applied on top of it incrementally > > > > > as a cleanup. > > > > > > > > > > Jakub > > > > > > > > > > > > > Ok, I'll do that. > > > > > > Thanks. > > > > > > > Do you want it on the gcc-9 branch as well? I'm running > > > > a bootstrap of it right now. > > > > > > Yes, but can you defer for 9.2.1, i.e. Tuesday+ next week? > > > > > > Jakub > > > > > > > I'm OoO next week, but can do it when I get back. > > I installed the following followup after testing on > x86_64-unknown-linux-gnu. And dealing with the backporting now. Richard. > Richard. > > 2019-08-12 Richard Biener <rguenther@suse.de> > > PR driver/91130 > * lto-wrapper.c (get_options_from_collect_gcc_options): Remove > lang_mask option, always use CL_DRIVER. > (get_options_from_collect_gcc_options): Adjust. > (find_and_merge_options): Likewise. > (run_gcc): Likewise. > > Index: gcc/lto-wrapper.c > =================================================================== > --- gcc/lto-wrapper.c (revision 274235) > +++ gcc/lto-wrapper.c (working copy) > @@ -128,12 +128,11 @@ maybe_unlink (const char *file) > #define DUMPBASE_SUFFIX ".ltrans18446744073709551615" > > /* Create decoded options from the COLLECT_GCC and COLLECT_GCC_OPTIONS > - environment according to LANG_MASK. */ > + environment. */ > > static void > get_options_from_collect_gcc_options (const char *collect_gcc, > const char *collect_gcc_options, > - unsigned int lang_mask, > struct cl_decoded_option **decoded_options, > unsigned int *decoded_options_count) > { > @@ -176,8 +175,7 @@ get_options_from_collect_gcc_options (co > argc = obstack_object_size (&argv_obstack) / sizeof (void *) - 1; > argv = XOBFINISH (&argv_obstack, const char **); > > - decode_cmdline_options_to_array (argc, (const char **)argv, > - lang_mask, > + decode_cmdline_options_to_array (argc, (const char **)argv, CL_DRIVER, > decoded_options, decoded_options_count); > obstack_free (&argv_obstack, NULL); > } > @@ -1009,8 +1007,7 @@ find_and_merge_options (int fd, off_t fi > { > struct cl_decoded_option *f2decoded_options; > unsigned int f2decoded_options_count; > - get_options_from_collect_gcc_options (collect_gcc, > - fopts, CL_DRIVER, > + get_options_from_collect_gcc_options (collect_gcc, fopts, > &f2decoded_options, > &f2decoded_options_count); > if (!fdecoded_options) > @@ -1282,7 +1279,6 @@ run_gcc (unsigned argc, char *argv[]) > fatal_error (input_location, > "environment variable %<COLLECT_GCC_OPTIONS%> must be set"); > get_options_from_collect_gcc_options (collect_gcc, collect_gcc_options, > - CL_DRIVER, > &decoded_options, > &decoded_options_count); > > -- Richard Biener <rguenther@suse.de> SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany; GF: Felix Imendörffer, Mary Higgins, Sri Rasiah; HRB 21284 (AG Nürnberg)
diff --git a/gcc/lto-wrapper.c b/gcc/lto-wrapper.c index 3414ade..f93ff50 100644 --- a/gcc/lto-wrapper.c +++ b/gcc/lto-wrapper.c @@ -1010,7 +1010,7 @@ find_and_merge_options (int fd, off_t file_offset, const char *prefix, struct cl_decoded_option *f2decoded_options; unsigned int f2decoded_options_count; get_options_from_collect_gcc_options (collect_gcc, - fopts, CL_LANG_ALL, + fopts, CL_DRIVER, &f2decoded_options, &f2decoded_options_count); if (!fdecoded_options) @@ -1283,7 +1283,7 @@ run_gcc (unsigned argc, char *argv[]) fatal_error (input_location, "environment variable %<COLLECT_GCC_OPTIONS%> must be set"); get_options_from_collect_gcc_options (collect_gcc, collect_gcc_options, - CL_LANG_ALL, + CL_DRIVER, &decoded_options, &decoded_options_count);