diff mbox series

PR driver/91130 Use CL_DRIVER when handling of COLLECT_GCC_OPTIONS in lto-wrapper.c

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

Commit Message

Richard Earnshaw (lists) Aug. 7, 2019, 12:42 p.m. UTC 
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...

Comments

Jakub Jelinek Aug. 7, 2019, 1:51 p.m. UTC | #1 
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
Richard Earnshaw (lists) Aug. 7, 2019, 4:08 p.m. UTC | #2 
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.
Jakub Jelinek Aug. 7, 2019, 4:15 p.m. UTC | #3 
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
Richard Earnshaw (lists) Aug. 7, 2019, 4:18 p.m. UTC | #4 
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.
Richard Biener Aug. 12, 2019, 10:58 a.m. UTC | #5 
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);
Richard Biener Aug. 12, 2019, 11:08 a.m. UTC | #6 
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);
>  
>
diff mbox series

Patch

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);