[PATCH] driver: Also prune joined switches with negation

[PATCH] driver: Also prune joined switches with negation

[H.J. Lu]

When -march=native is passed to host_detect_local_cpu to the backend,
it overrides all command lines after it.  That means

$ gcc -march=native -march=skylake-avx512

is the treated as

$ gcc -march=skylake-avx512 -march=native

Prune joined switches with negation to allow -march=skylake-avx512 to
override previous -march=native on command-line.

	PR driver/69471
	* opts-common.c (prune_options): Also prune joined switches
	with negation.
	* config/i386/i386.opt (march=): Add Negative(march=).
	(mtune=): Add Negative(mtune=).
---
 gcc/config/i386/i386.opt | 4 ++--
 gcc/opts-common.c        | 7 ++++---
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/gcc/config/i386/i386.opt b/gcc/config/i386/i386.opt
index 9b93241f790..b7998ee7363 100644
--- a/gcc/config/i386/i386.opt
+++ b/gcc/config/i386/i386.opt
@@ -253,7 +253,7 @@ EnumValue
 Enum(ix86_align_data) String(cacheline) Value(ix86_align_data_type_cacheline)
 
 march=
-Target RejectNegative Joined Var(ix86_arch_string)
+Target RejectNegative Negative(march=) Joined Var(ix86_arch_string)
 Generate code for given CPU.
 
 masm=
@@ -510,7 +510,7 @@ Target Report Mask(TLS_DIRECT_SEG_REFS)
 Use direct references against %gs when accessing tls data.
 
 mtune=
-Target RejectNegative Joined Var(ix86_tune_string)
+Target RejectNegative Negative(mtune=) Joined Var(ix86_tune_string)
 Schedule code for given CPU.
 
 mtune-ctrl=
diff --git a/gcc/opts-common.c b/gcc/opts-common.c
index ee8898b22ec..ba9eb144078 100644
--- a/gcc/opts-common.c
+++ b/gcc/opts-common.c
@@ -1014,8 +1014,8 @@ prune_options (struct cl_decoded_option **decoded_options,
 	  if (option->neg_index < 0)
 	    goto keep;
 
-	  /* Skip joined switches.  */
-	  if ((option->flags & CL_JOINED))
+	  /* Skip joined switches if there is no negation.  */
+	  if ((option->flags & CL_JOINED) && option->neg_index < 0)
 	    goto keep;
 
 	  for (j = i + 1; j < old_decoded_options_count; j++)
@@ -1027,7 +1027,8 @@ prune_options (struct cl_decoded_option **decoded_options,
 		continue;
 	      if (cl_options[next_opt_idx].neg_index < 0)
 		continue;
-	      if ((cl_options[next_opt_idx].flags & CL_JOINED))
+	      if ((cl_options[next_opt_idx].flags & CL_JOINED)
+		  && cl_options[next_opt_idx].neg_index < 0)
 		  continue;
 	      if (cancel_option (opt_idx, next_opt_idx, next_opt_idx))
 		break;
-- 
2.20.1

Re: [PATCH] driver: Also prune joined switches with negation

[H.J. Lu]

[-- Attachment #1: Type: text/plain, Size: 704 bytes --]

On Fri, Feb 8, 2019 at 3:02 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> When -march=native is passed to host_detect_local_cpu to the backend,
> it overrides all command lines after it.  That means
>
> $ gcc -march=native -march=skylake-avx512
>
> is the treated as
>
> $ gcc -march=skylake-avx512 -march=native
>
> Prune joined switches with negation to allow -march=skylake-avx512 to
> override previous -march=native on command-line.
>
>         PR driver/69471
>         * opts-common.c (prune_options): Also prune joined switches
>         with negation.
>         * config/i386/i386.opt (march=): Add Negative(march=).
>         (mtune=): Add Negative(mtune=).

Here is the updated patch.

-- 
H.J.

[-- Attachment #2: 0001-driver-Also-prune-joined-switches-with-negation.patch --]
[-- Type: text/x-patch, Size: 2425 bytes --]

From a5a453e48f8560955cfa2e2db3b8f4f22b9cf9b1 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Fri, 8 Feb 2019 14:52:57 -0800
Subject: [PATCH] driver: Also prune joined switches with negation

When -march=native is passed to host_detect_local_cpu to the backend,
it overrides all command lines after it.  That means

$ gcc -march=native -march=skylake-avx512

is the treated as

$ gcc -march=skylake-avx512 -march=native

Prune joined switches with negation to allow -march=skylake-avx512 to
override previous -march=native on command-line.

	PR driver/69471
	* opts-common.c (prune_options): Also prune joined switches
	with negation.
	* config/i386/i386.opt (march=): Add Negative(march=).
	(mtune=): Add Negative(mtune=).
---
 gcc/config/i386/i386.opt | 4 ++--
 gcc/opts-common.c        | 6 ------
 2 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/gcc/config/i386/i386.opt b/gcc/config/i386/i386.opt
index 9b93241f790..b7998ee7363 100644
--- a/gcc/config/i386/i386.opt
+++ b/gcc/config/i386/i386.opt
@@ -253,7 +253,7 @@ EnumValue
 Enum(ix86_align_data) String(cacheline) Value(ix86_align_data_type_cacheline)
 
 march=
-Target RejectNegative Joined Var(ix86_arch_string)
+Target RejectNegative Negative(march=) Joined Var(ix86_arch_string)
 Generate code for given CPU.
 
 masm=
@@ -510,7 +510,7 @@ Target Report Mask(TLS_DIRECT_SEG_REFS)
 Use direct references against %gs when accessing tls data.
 
 mtune=
-Target RejectNegative Joined Var(ix86_tune_string)
+Target RejectNegative Negative(mtune=) Joined Var(ix86_tune_string)
 Schedule code for given CPU.
 
 mtune-ctrl=
diff --git a/gcc/opts-common.c b/gcc/opts-common.c
index ee8898b22ec..7204bb5a8fa 100644
--- a/gcc/opts-common.c
+++ b/gcc/opts-common.c
@@ -1014,10 +1014,6 @@ prune_options (struct cl_decoded_option **decoded_options,
 	  if (option->neg_index < 0)
 	    goto keep;
 
-	  /* Skip joined switches.  */
-	  if ((option->flags & CL_JOINED))
-	    goto keep;
-
 	  for (j = i + 1; j < old_decoded_options_count; j++)
 	    {
 	      if (old_decoded_options[j].errors & ~CL_ERR_WRONG_LANG)
@@ -1027,8 +1023,6 @@ prune_options (struct cl_decoded_option **decoded_options,
 		continue;
 	      if (cl_options[next_opt_idx].neg_index < 0)
 		continue;
-	      if ((cl_options[next_opt_idx].flags & CL_JOINED))
-		  continue;
 	      if (cancel_option (opt_idx, next_opt_idx, next_opt_idx))
 		break;
 	    }
-- 
2.20.1

PING^1: [PATCH] driver: Also prune joined switches with negation

[H.J. Lu]

On Fri, Feb 8, 2019 at 3:09 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Fri, Feb 8, 2019 at 3:02 PM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > When -march=native is passed to host_detect_local_cpu to the backend,
> > it overrides all command lines after it.  That means
> >
> > $ gcc -march=native -march=skylake-avx512
> >
> > is the treated as
> >
> > $ gcc -march=skylake-avx512 -march=native
> >
> > Prune joined switches with negation to allow -march=skylake-avx512 to
> > override previous -march=native on command-line.
> >
> >         PR driver/69471
> >         * opts-common.c (prune_options): Also prune joined switches
> >         with negation.
> >         * config/i386/i386.opt (march=): Add Negative(march=).
> >         (mtune=): Add Negative(mtune=).
>
> Here is the updated patch.
>

PING:

https://gcc.gnu.org/ml/gcc-patches/2019-02/msg00492.html


-- 
H.J.

Re: PING^1: [PATCH] driver: Also prune joined switches with negation

[Joseph Myers]

On Tue, 12 Feb 2019, H.J. Lu wrote:

> > > Prune joined switches with negation to allow -march=skylake-avx512 to
> > > override previous -march=native on command-line.
> > >
> > >         PR driver/69471
> > >         * opts-common.c (prune_options): Also prune joined switches
> > >         with negation.
> > >         * config/i386/i386.opt (march=): Add Negative(march=).
> > >         (mtune=): Add Negative(mtune=).
> >
> > Here is the updated patch.
> >
> 
> PING:
> 
> https://gcc.gnu.org/ml/gcc-patches/2019-02/msg00492.html

I think this is changing architecture-independent code in a way that is 
not clearly safe based on the architecture-independent options design, in 
order to address an architecture-specific problem.  The exclusion of 
joined switches is presumably aimed at such switches that can be used 
multiple times with different arguments, with different semantics to just 
using them once (e.g. -I).  Is there any reason such an option should not 
have a negative form?

I think anything like this (would not be suitable for the current 
development stage and) would need a more detailed analysis of what 
existing options might be affected by the change and why it's OK for them, 
as well as updates to options.texi to discuss this issue with the 
semantics of Negative and kinds of options it cannot be used with.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: PING^1: [PATCH] driver: Also prune joined switches with negation

[Jakub Jelinek]

On Tue, Feb 12, 2019 at 11:21:04PM +0000, Joseph Myers wrote:
> I think this is changing architecture-independent code in a way that is 
> not clearly safe based on the architecture-independent options design, in 
> order to address an architecture-specific problem.  The exclusion of 

Actually, I think it is a problem common to many backends, in particular
those where *_host_detect_local_cpu emits for -m*=native sometimes more than
one option, so at least i386, s390, rs6000, maybe also those that emit just
one option because it likely ends up at a different spot on the command line
from where -m{arch,cpu,tune}=native was originally present (that would be
aarch64, alpha, arm, mips and sparc).  I guess the user expectations is that
-march=native -march=foobar will be handled as
-march=foobar, rather than -march=native -march=foobar -march=my_great_cpu -mfoo -mbar

	Jakub

Re: PING^1: [PATCH] driver: Also prune joined switches with negation

[Joseph Myers]

On Wed, 13 Feb 2019, Jakub Jelinek wrote:

> On Tue, Feb 12, 2019 at 11:21:04PM +0000, Joseph Myers wrote:
> > I think this is changing architecture-independent code in a way that is 
> > not clearly safe based on the architecture-independent options design, in 
> > order to address an architecture-specific problem.  The exclusion of 
> 
> Actually, I think it is a problem common to many backends, in particular
> those where *_host_detect_local_cpu emits for -m*=native sometimes more than
> one option, so at least i386, s390, rs6000, maybe also those that emit just
> one option because it likely ends up at a different spot on the command line
> from where -m{arch,cpu,tune}=native was originally present (that would be
> aarch64, alpha, arm, mips and sparc).  I guess the user expectations is that
> -march=native -march=foobar will be handled as
> -march=foobar, rather than -march=native -march=foobar -march=my_great_cpu -mfoo -mbar

It seems right in the march= case to handle that combination as 
-march=foobar - but it's less clear if that must always be the case for 
Joined options with negative versions (at least, the semantics would need 
defining more carefully in options.texi, with an analysis of existing 
affected options).

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: PING^1: [PATCH] driver: Also prune joined switches with negation

[H.J. Lu]

On Tue, Feb 12, 2019 at 4:43 PM Joseph Myers <joseph@codesourcery.com> wrote:
>
> On Wed, 13 Feb 2019, Jakub Jelinek wrote:
>
> > On Tue, Feb 12, 2019 at 11:21:04PM +0000, Joseph Myers wrote:
> > > I think this is changing architecture-independent code in a way that is
> > > not clearly safe based on the architecture-independent options design, in
> > > order to address an architecture-specific problem.  The exclusion of
> >
> > Actually, I think it is a problem common to many backends, in particular
> > those where *_host_detect_local_cpu emits for -m*=native sometimes more than
> > one option, so at least i386, s390, rs6000, maybe also those that emit just
> > one option because it likely ends up at a different spot on the command line
> > from where -m{arch,cpu,tune}=native was originally present (that would be
> > aarch64, alpha, arm, mips and sparc).  I guess the user expectations is that
> > -march=native -march=foobar will be handled as
> > -march=foobar, rather than -march=native -march=foobar -march=my_great_cpu -mfoo -mbar
>
> It seems right in the march= case to handle that combination as
> -march=foobar - but it's less clear if that must always be the case for
> Joined options with negative versions (at least, the semantics would need
> defining more carefully in options.texi, with an analysis of existing
> affected options).
>

Backend must have "Negative(march=) " to have negative Joined options.
It is not like it will happen automatically.  What is wrong to have a negative
Joined option when a backend is specifically asking for it?

-- 
H.J.

Re: PING^1: [PATCH] driver: Also prune joined switches with negation

[Jakub Jelinek]

On Wed, Feb 13, 2019 at 12:43:32AM +0000, Joseph Myers wrote:
> On Wed, 13 Feb 2019, Jakub Jelinek wrote:
> 
> > On Tue, Feb 12, 2019 at 11:21:04PM +0000, Joseph Myers wrote:
> > > I think this is changing architecture-independent code in a way that is 
> > > not clearly safe based on the architecture-independent options design, in 
> > > order to address an architecture-specific problem.  The exclusion of 
> > 
> > Actually, I think it is a problem common to many backends, in particular
> > those where *_host_detect_local_cpu emits for -m*=native sometimes more than
> > one option, so at least i386, s390, rs6000, maybe also those that emit just
> > one option because it likely ends up at a different spot on the command line
> > from where -m{arch,cpu,tune}=native was originally present (that would be
> > aarch64, alpha, arm, mips and sparc).  I guess the user expectations is that
> > -march=native -march=foobar will be handled as
> > -march=foobar, rather than -march=native -march=foobar -march=my_great_cpu -mfoo -mbar
> 
> It seems right in the march= case to handle that combination as 
> -march=foobar - but it's less clear if that must always be the case for 
> Joined options with negative versions (at least, the semantics would need 
> defining more carefully in options.texi, with an analysis of existing 
> affected options).

We have only a few Joined/JoinedOrMissing options with Negative:
find . -name \*.opt | xargs grep -B1 'Joined.*[[:blank:]]Negative\|[[:blank:]]Negative.*Joined\|^Negative.*Joined'
./config/s390/s390.opt-mstack-guard=
./config/s390/s390.opt:Target RejectNegative Negative(mno-stack-guard) Joined UInteger Var(s390_stack_guard) Save
./common.opt-gdwarf
./common.opt:Common Driver JoinedOrMissing Negative(gdwarf-)
./common.opt-gdwarf-
./common.opt:Common Driver Joined UInteger Var(dwarf_version) Init(4) Negative(gstabs)
./common.opt-gstabs
./common.opt:Common Driver JoinedOrMissing Negative(gstabs+)
./common.opt-gstabs+
./common.opt:Common Driver JoinedOrMissing Negative(gvms)
./common.opt-gvms
./common.opt:Common Driver JoinedOrMissing Negative(gxcoff)
./common.opt-gxcoff
./common.opt:Common Driver JoinedOrMissing Negative(gxcoff+)
./common.opt-gxcoff+
./common.opt:Common Driver JoinedOrMissing Negative(gdwarf)
./fortran/lang.opt-cpp=
./fortran/lang.opt:Fortran Joined Negative(nocpp) Undocumented NoDWARFRecord

The patch indeed does change behavior for say:
gcc -c test.s -gstabs2 -gdwarf-4 -gstabs3
gcc: error: debug format ‘dwarf-2’ conflicts with prior selection
gcc: error: debug format ‘stabs’ conflicts with prior selection
(previously the above errors, now accepted as -gstabs3)
but wouldn't that be an advantage here (use the latest option win)?

For s390 (which has it weird, as there is no Negative(mno-stack-size) on
very similar mstack-size= option), I believe it shouldn't change end result,
while the driver will not pass 3 options for
-mno-stack-guard -mstack-guard=64 -mno-stack-guard
but just the last one (similarly for other combinations), the option
handling in cc1 etc. will handle it the same anyway (last option wins)
it seems.

And finally Fortran -cpp= option is internally generated from -cpp which
should have normal Negative processing with -nocpp.

	Jakub

Re: PING^1: [PATCH] driver: Also prune joined switches with negation

[Jakub Jelinek]

On Wed, Feb 13, 2019 at 08:43:45AM +0100, Jakub Jelinek wrote:
> > It seems right in the march= case to handle that combination as 
> > -march=foobar - but it's less clear if that must always be the case for 
> > Joined options with negative versions (at least, the semantics would need 
> > defining more carefully in options.texi, with an analysis of existing 
> > affected options).
> 
> We have only a few Joined/JoinedOrMissing options with Negative:
> find . -name \*.opt | xargs grep -B1 'Joined.*[[:blank:]]Negative\|[[:blank:]]Negative.*Joined\|^Negative.*Joined'
> ./config/s390/s390.opt-mstack-guard=
> ./config/s390/s390.opt:Target RejectNegative Negative(mno-stack-guard) Joined UInteger Var(s390_stack_guard) Save
> ./common.opt-gdwarf
> ./common.opt:Common Driver JoinedOrMissing Negative(gdwarf-)
> ./common.opt-gdwarf-
> ./common.opt:Common Driver Joined UInteger Var(dwarf_version) Init(4) Negative(gstabs)
> ./common.opt-gstabs
> ./common.opt:Common Driver JoinedOrMissing Negative(gstabs+)
> ./common.opt-gstabs+
> ./common.opt:Common Driver JoinedOrMissing Negative(gvms)
> ./common.opt-gvms
> ./common.opt:Common Driver JoinedOrMissing Negative(gxcoff)
> ./common.opt-gxcoff
> ./common.opt:Common Driver JoinedOrMissing Negative(gxcoff+)
> ./common.opt-gxcoff+
> ./common.opt:Common Driver JoinedOrMissing Negative(gdwarf)
> ./fortran/lang.opt-cpp=
> ./fortran/lang.opt:Fortran Joined Negative(nocpp) Undocumented NoDWARFRecord
> 
> The patch indeed does change behavior for say:
> gcc -c test.s -gstabs2 -gdwarf-4 -gstabs3
> gcc: error: debug format ‘dwarf-2’ conflicts with prior selection
> gcc: error: debug format ‘stabs’ conflicts with prior selection
> (previously the above errors, now accepted as -gstabs3)
> but wouldn't that be an advantage here (use the latest option win)?
> 
> For s390 (which has it weird, as there is no Negative(mno-stack-size) on
> very similar mstack-size= option), I believe it shouldn't change end result,
> while the driver will not pass 3 options for
> -mno-stack-guard -mstack-guard=64 -mno-stack-guard
> but just the last one (similarly for other combinations), the option
> handling in cc1 etc. will handle it the same anyway (last option wins)
> it seems.
> 
> And finally Fortran -cpp= option is internally generated from -cpp which
> should have normal Negative processing with -nocpp.

On the other side, the reason this Skip Joined stuff has been added to
opts-common.c is PR28437 r115780, and the patch as posted indeed does break
-fno-builtin-free -fno-builtin-malloc -fno-builtin-calloc - only the last of
the options is passed in with the patch, all before.  If this didn't show up
during regtest, guess we want a testcase that will pass multiple
-fno-builtin- options and verify e.g. through bogus warnings that all of
them are in effect.
fbuiltin- is
C ObjC C++ ObjC++ Joined
and so neg_index is not -1, but the option index itself, but that is what
the patch wants to use for march= etc. too.
So, do we want a new *.opt flag that would enable this behavior, or key
that on cl_reject_negative?

	Jakub

Re: PING^1: [PATCH] driver: Also prune joined switches with negation

[H.J. Lu]

[-- Attachment #1: Type: text/plain, Size: 3352 bytes --]

On Wed, Feb 13, 2019 at 12:02 AM Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Wed, Feb 13, 2019 at 08:43:45AM +0100, Jakub Jelinek wrote:
> > > It seems right in the march= case to handle that combination as
> > > -march=foobar - but it's less clear if that must always be the case for
> > > Joined options with negative versions (at least, the semantics would need
> > > defining more carefully in options.texi, with an analysis of existing
> > > affected options).
> >
> > We have only a few Joined/JoinedOrMissing options with Negative:
> > find . -name \*.opt | xargs grep -B1 'Joined.*[[:blank:]]Negative\|[[:blank:]]Negative.*Joined\|^Negative.*Joined'
> > ./config/s390/s390.opt-mstack-guard=
> > ./config/s390/s390.opt:Target RejectNegative Negative(mno-stack-guard) Joined UInteger Var(s390_stack_guard) Save
> > ./common.opt-gdwarf
> > ./common.opt:Common Driver JoinedOrMissing Negative(gdwarf-)
> > ./common.opt-gdwarf-
> > ./common.opt:Common Driver Joined UInteger Var(dwarf_version) Init(4) Negative(gstabs)
> > ./common.opt-gstabs
> > ./common.opt:Common Driver JoinedOrMissing Negative(gstabs+)
> > ./common.opt-gstabs+
> > ./common.opt:Common Driver JoinedOrMissing Negative(gvms)
> > ./common.opt-gvms
> > ./common.opt:Common Driver JoinedOrMissing Negative(gxcoff)
> > ./common.opt-gxcoff
> > ./common.opt:Common Driver JoinedOrMissing Negative(gxcoff+)
> > ./common.opt-gxcoff+
> > ./common.opt:Common Driver JoinedOrMissing Negative(gdwarf)
> > ./fortran/lang.opt-cpp=
> > ./fortran/lang.opt:Fortran Joined Negative(nocpp) Undocumented NoDWARFRecord
> >
> > The patch indeed does change behavior for say:
> > gcc -c test.s -gstabs2 -gdwarf-4 -gstabs3
> > gcc: error: debug format ‘dwarf-2’ conflicts with prior selection
> > gcc: error: debug format ‘stabs’ conflicts with prior selection
> > (previously the above errors, now accepted as -gstabs3)
> > but wouldn't that be an advantage here (use the latest option win)?
> >
> > For s390 (which has it weird, as there is no Negative(mno-stack-size) on
> > very similar mstack-size= option), I believe it shouldn't change end result,
> > while the driver will not pass 3 options for
> > -mno-stack-guard -mstack-guard=64 -mno-stack-guard
> > but just the last one (similarly for other combinations), the option
> > handling in cc1 etc. will handle it the same anyway (last option wins)
> > it seems.
> >
> > And finally Fortran -cpp= option is internally generated from -cpp which
> > should have normal Negative processing with -nocpp.
>
> On the other side, the reason this Skip Joined stuff has been added to
> opts-common.c is PR28437 r115780, and the patch as posted indeed does break
> -fno-builtin-free -fno-builtin-malloc -fno-builtin-calloc - only the last of
> the options is passed in with the patch, all before.  If this didn't show up
> during regtest, guess we want a testcase that will pass multiple
> -fno-builtin- options and verify e.g. through bogus warnings that all of
> them are in effect.
> fbuiltin- is
> C ObjC C++ ObjC++ Joined
> and so neg_index is not -1, but the option index itself, but that is what
> the patch wants to use for march= etc. too.
> So, do we want a new *.opt flag that would enable this behavior, or key
> that on cl_reject_negative?
>

Like this?

-- 
H.J.

[-- Attachment #2: 0001-driver-Also-prune-joined-switches-with-negation.patch --]
[-- Type: text/x-patch, Size: 3825 bytes --]

From 6ba21bba3f2ce2fdf8f1d8b080d8b8b748025d89 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Fri, 8 Feb 2019 14:52:57 -0800
Subject: [PATCH] driver: Also prune joined switches with negation

When -march=native is passed to host_detect_local_cpu to the backend,
it overrides all command lines after it.  That means

$ gcc -march=native -march=skylake-avx512

is the treated as

$ gcc -march=skylake-avx512 -march=native

Prune joined switches with Negative and RejectNegative to allow
-march=skylake-avx512 to override previous -march=native on command-line.

gcc/

	PR driver/69471
	* opts-common.c (prune_options): Also prune joined switches
	with Negative and RejectNegative.
	* config/i386/i386.opt (march=): Add Negative(march=).
	(mtune=): Add Negative(mtune=).

gcc/testsuite/

	PR driver/69471
	* gcc.dg/pr69471-1.c: New test.
	* gcc.dg/pr69471-2.c: Likewise.
---
 gcc/config/i386/i386.opt         |  4 ++--
 gcc/opts-common.c                | 11 ++++++++---
 gcc/testsuite/gcc.dg/pr69471-1.c |  9 +++++++++
 gcc/testsuite/gcc.dg/pr69471-2.c |  8 ++++++++
 4 files changed, 27 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/pr69471-1.c
 create mode 100644 gcc/testsuite/gcc.dg/pr69471-2.c

diff --git a/gcc/config/i386/i386.opt b/gcc/config/i386/i386.opt
index 9b93241f790..b7998ee7363 100644
--- a/gcc/config/i386/i386.opt
+++ b/gcc/config/i386/i386.opt
@@ -253,7 +253,7 @@ EnumValue
 Enum(ix86_align_data) String(cacheline) Value(ix86_align_data_type_cacheline)
 
 march=
-Target RejectNegative Joined Var(ix86_arch_string)
+Target RejectNegative Negative(march=) Joined Var(ix86_arch_string)
 Generate code for given CPU.
 
 masm=
@@ -510,7 +510,7 @@ Target Report Mask(TLS_DIRECT_SEG_REFS)
 Use direct references against %gs when accessing tls data.
 
 mtune=
-Target RejectNegative Joined Var(ix86_tune_string)
+Target RejectNegative Negative(mtune=) Joined Var(ix86_tune_string)
 Schedule code for given CPU.
 
 mtune-ctrl=
diff --git a/gcc/opts-common.c b/gcc/opts-common.c
index ee8898b22ec..edbb3ac9b6d 100644
--- a/gcc/opts-common.c
+++ b/gcc/opts-common.c
@@ -1015,7 +1015,9 @@ prune_options (struct cl_decoded_option **decoded_options,
 	    goto keep;
 
 	  /* Skip joined switches.  */
-	  if ((option->flags & CL_JOINED))
+	  if ((option->flags & CL_JOINED)
+	      && (!option->cl_reject_negative
+		  || (unsigned int) option->neg_index != opt_idx))
 	    goto keep;
 
 	  for (j = i + 1; j < old_decoded_options_count; j++)
@@ -1027,8 +1029,11 @@ prune_options (struct cl_decoded_option **decoded_options,
 		continue;
 	      if (cl_options[next_opt_idx].neg_index < 0)
 		continue;
-	      if ((cl_options[next_opt_idx].flags & CL_JOINED))
-		  continue;
+	      if ((cl_options[next_opt_idx].flags & CL_JOINED)
+		  && (!cl_options[next_opt_idx].cl_reject_negative
+		      || ((unsigned int) cl_options[next_opt_idx].neg_index
+			  != next_opt_idx)))
+		continue;
 	      if (cancel_option (opt_idx, next_opt_idx, next_opt_idx))
 		break;
 	    }
diff --git a/gcc/testsuite/gcc.dg/pr69471-1.c b/gcc/testsuite/gcc.dg/pr69471-1.c
new file mode 100644
index 00000000000..3eac3b5bdbc
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr69471-1.c
@@ -0,0 +1,9 @@
+/* { dg-do compile } */
+/* { dg-options "-Wno-implicit-function-declaration -Wno-int-conversion -fno-builtin-free -fno-builtin-malloc" } */
+
+void *
+foo (void * p)
+{
+  free (p);
+  return malloc (p);
+}
diff --git a/gcc/testsuite/gcc.dg/pr69471-2.c b/gcc/testsuite/gcc.dg/pr69471-2.c
new file mode 100644
index 00000000000..d5799604b36
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr69471-2.c
@@ -0,0 +1,8 @@
+/* { dg-do compile } */
+/* { dg-options "-gstabs2 -gdwarf-4 -gstabs3" } */
+/* { dg-error "conflicts with prior selectio" "" { target *-*-* } 0 } */
+
+void
+foo (void)
+{
+}
-- 
2.20.1

Re: PING^1: [PATCH] driver: Also prune joined switches with negation

[Joseph Myers]

On Wed, 13 Feb 2019, H.J. Lu wrote:

> Like this?

This patch is missing any updates to options.texi to discuss the 
interation of Negative and RejectNegative with Joined.

> diff --git a/gcc/testsuite/gcc.dg/pr69471-1.c b/gcc/testsuite/gcc.dg/pr69471-1.c
> new file mode 100644
> index 00000000000..3eac3b5bdbc
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/pr69471-1.c
> @@ -0,0 +1,9 @@
> +/* { dg-do compile } */
> +/* { dg-options "-Wno-implicit-function-declaration -Wno-int-conversion -fno-builtin-free -fno-builtin-malloc" } */
> +
> +void *
> +foo (void * p)
> +{
> +  free (p);
> +  return malloc (p);

How does this test verify that both -fno-builtin-* options are in effect?  
That is, how does it fail if you remove either or both of those options?

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: PING^1: [PATCH] driver: Also prune joined switches with negation

[H.J. Lu]

On Wed, Feb 13, 2019 at 2:54 PM Joseph Myers <joseph@codesourcery.com> wrote:
>
> On Wed, 13 Feb 2019, H.J. Lu wrote:
>
> > Like this?
>
> This patch is missing any updates to options.texi to discuss the
> interation of Negative and RejectNegative with Joined.

I will add something.

> > diff --git a/gcc/testsuite/gcc.dg/pr69471-1.c b/gcc/testsuite/gcc.dg/pr69471-1.c
> > new file mode 100644
> > index 00000000000..3eac3b5bdbc
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.dg/pr69471-1.c
> > @@ -0,0 +1,9 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-Wno-implicit-function-declaration -Wno-int-conversion -fno-builtin-free -fno-builtin-malloc" } */
> > +
> > +void *
> > +foo (void * p)
> > +{
> > +  free (p);
> > +  return malloc (p);
>
> How does this test verify that both -fno-builtin-* options are in effect?
> That is, how does it fail if you remove either or both of those options?
>

Without -fno-builtin-free -fno-builtin-malloc

[hjl@gnu-cfl-1 gcc]$ cat
/export/gnu/import/git/gitlab/x86-gcc/gcc/testsuite/gcc.dg/pr69471-1.c
/* { dg-do compile } */
/* { dg-options "-Wno-implicit-function-declaration
-Wno-int-conversion -fno-builtin-free -fno-builtin-malloc" } */

void *
foo (void * p)
{
  free (p);
  return malloc (p);
}
[hjl@gnu-cfl-1 gcc]$ gcc -S -Wno-implicit-function-declaration
-Wno-int-conversion
/export/gnu/import/git/gitlab/x86-gcc/gcc/testsuite/gcc.dg/pr69471-1.c
/export/gnu/import/git/gitlab/x86-gcc/gcc/testsuite/gcc.dg/pr69471-1.c:
In function ‘foo’:
/export/gnu/import/git/gitlab/x86-gcc/gcc/testsuite/gcc.dg/pr69471-1.c:7:3:
warning: incompatible implicit declaration of built-in function ‘free’
   free (p);
   ^~~~
/export/gnu/import/git/gitlab/x86-gcc/gcc/testsuite/gcc.dg/pr69471-1.c:7:3:
note: include ‘<stdlib.h>’ or provide a declaration of ‘free’
/export/gnu/import/git/gitlab/x86-gcc/gcc/testsuite/gcc.dg/pr69471-1.c:1:1:
+#include <stdlib.h>
 /* { dg-do compile } */
/export/gnu/import/git/gitlab/x86-gcc/gcc/testsuite/gcc.dg/pr69471-1.c:7:3:
   free (p);
   ^~~~
/export/gnu/import/git/gitlab/x86-gcc/gcc/testsuite/gcc.dg/pr69471-1.c:8:10:
warning: incompatible implicit declaration of built-in function
‘malloc’
   return malloc (p);
          ^~~~~~
/export/gnu/import/git/gitlab/x86-gcc/gcc/testsuite/gcc.dg/pr69471-1.c:8:10:
note: include ‘<stdlib.h>’ or provide a declaration of ‘malloc’
[hjl@gnu-cfl-1 gcc]$

Without only -fno-builtin-malloc:
[hjl@gnu-cfl-1 gcc]$ gcc -S -Wno-implicit-function-declaration
-Wno-int-conversion
/export/gnu/import/git/gitlab/x86-gcc/gcc/testsuite/gcc.dg/pr69471-1.c
-fno-builtin-malloc
/export/gnu/import/git/gitlab/x86-gcc/gcc/testsuite/gcc.dg/pr69471-1.c:
In function ‘foo’:
/export/gnu/import/git/gitlab/x86-gcc/gcc/testsuite/gcc.dg/pr69471-1.c:7:3:
warning: incompatible implicit declaration of built-in function ‘free’
   free (p);
   ^~~~
/export/gnu/import/git/gitlab/x86-gcc/gcc/testsuite/gcc.dg/pr69471-1.c:7:3:
note: include ‘<stdlib.h>’ or provide a declaration of ‘free’
/export/gnu/import/git/gitlab/x86-gcc/gcc/testsuite/gcc.dg/pr69471-1.c:1:1:
+#include <stdlib.h>
 /* { dg-do compile } */
/export/gnu/import/git/gitlab/x86-gcc/gcc/testsuite/gcc.dg/pr69471-1.c:7:3:
   free (p);
   ^~~~
[hjl@gnu-cfl-1 gcc]$

With -fno-builtin-free -fno-builtin-malloc

[hjl@gnu-cfl-1 gcc]$ gcc -S -Wno-implicit-function-declaration
-Wno-int-conversion
/export/gnu/import/git/gitlab/x86-gcc/gcc/testsuite/gcc.dg/pr69471-1.c
-fno-builtin-free -fno-builtin-malloc
[hjl@gnu-cfl-1 gcc]$

if  -fno-builtin-free is overridden by -fno-builtin-malloc, compile will fail.


H.J.
-- 
H.J.

Re: PING^1: [PATCH] driver: Also prune joined switches with negation

[Jakub Jelinek]

On Wed, Feb 13, 2019 at 03:08:01PM -0800, H.J. Lu wrote:
> > How does this test verify that both -fno-builtin-* options are in effect?
> > That is, how does it fail if you remove either or both of those options?
> >
> 
> Without -fno-builtin-free -fno-builtin-malloc

As you haven't discovered any ubsan/asan etc. failures, I guess I'll add
a test for similar thing with -fsanitize=signed-integer-overflow -fsanitize=shift
and similar (two -fsanitize-recover=, two -fno-sanitize=, two
-fno-sanitize-recover=).  But I can do that incrementally.
There is
c-c++-common/ubsan/attrib-2.c:/* { dg-options "-fsanitize=undefined -fsanitize=float-divide-by-zero -fsanitize=float-cast-overflow" } */
but it doesn't actually test all of those are instrumented.

	Jakub

V2 [PATCH] driver: Also prune joined switches with negation

[H.J. Lu]

On Thu, Feb 14, 2019 at 12:15:53AM +0100, Jakub Jelinek wrote:
> On Wed, Feb 13, 2019 at 03:08:01PM -0800, H.J. Lu wrote:
> > > How does this test verify that both -fno-builtin-* options are in effect?
> > > That is, how does it fail if you remove either or both of those options?
> > >
> > 
> > Without -fno-builtin-free -fno-builtin-malloc
> 
> As you haven't discovered any ubsan/asan etc. failures, I guess I'll add
> a test for similar thing with -fsanitize=signed-integer-overflow -fsanitize=shift
> and similar (two -fsanitize-recover=, two -fno-sanitize=, two
> -fno-sanitize-recover=).  But I can do that incrementally.
> There is
> c-c++-common/ubsan/attrib-2.c:/* { dg-options "-fsanitize=undefined -fsanitize=float-divide-by-zero -fsanitize=float-cast-overflow" } */
> but it doesn't actually test all of those are instrumented.
> 

Tested on x86-64.  OK for trunk?

Thanks.

H.J.
---
When -march=native is passed to host_detect_local_cpu to the backend,
it overrides all command lines after it.  That means

$ gcc -march=native -march=skylake-avx512

is the treated as

$ gcc -march=skylake-avx512 -march=native

Prune joined switches with Negative and RejectNegative to allow
-march=skylake-avx512 to override previous -march=native on command-line.

gcc/

	PR driver/69471
	* opts-common.c (prune_options): Also prune joined switches
	with Negative and RejectNegative.
	* config/i386/i386.opt (march=): Add Negative(march=).
	(mtune=): Add Negative(mtune=).
	* doc/options.texi: Document Negative used together with Joined
	and RejectNegative.

gcc/testsuite/

	PR driver/69471
	* gcc.dg/pr69471-1.c: New test.
	* gcc.dg/pr69471-2.c: Likewise.
---
 gcc/config/i386/i386.opt         |  4 ++--
 gcc/doc/options.texi             |  5 ++++-
 gcc/opts-common.c                | 11 ++++++++---
 gcc/testsuite/gcc.dg/pr69471-1.c |  9 +++++++++
 gcc/testsuite/gcc.dg/pr69471-2.c |  8 ++++++++
 5 files changed, 31 insertions(+), 6 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/pr69471-1.c
 create mode 100644 gcc/testsuite/gcc.dg/pr69471-2.c

diff --git a/gcc/config/i386/i386.opt b/gcc/config/i386/i386.opt
index 9b93241f790..b7998ee7363 100644
--- a/gcc/config/i386/i386.opt
+++ b/gcc/config/i386/i386.opt
@@ -253,7 +253,7 @@ EnumValue
 Enum(ix86_align_data) String(cacheline) Value(ix86_align_data_type_cacheline)
 
 march=
-Target RejectNegative Joined Var(ix86_arch_string)
+Target RejectNegative Negative(march=) Joined Var(ix86_arch_string)
 Generate code for given CPU.
 
 masm=
@@ -510,7 +510,7 @@ Target Report Mask(TLS_DIRECT_SEG_REFS)
 Use direct references against %gs when accessing tls data.
 
 mtune=
-Target RejectNegative Joined Var(ix86_tune_string)
+Target RejectNegative Negative(mtune=) Joined Var(ix86_tune_string)
 Schedule code for given CPU.
 
 mtune-ctrl=
diff --git a/gcc/doc/options.texi b/gcc/doc/options.texi
index 0081243acab..6b29d56de62 100644
--- a/gcc/doc/options.texi
+++ b/gcc/doc/options.texi
@@ -227,7 +227,10 @@ options, their @code{Negative} properties should form a circular chain.
 For example, if options @option{-@var{a}}, @option{-@var{b}} and
 @option{-@var{c}} are mutually exclusive, their respective @code{Negative}
 properties should be @samp{Negative(@var{b})}, @samp{Negative(@var{c})}
-and @samp{Negative(@var{a})}.
+and @samp{Negative(@var{a})}.  @code{Negative} can be used together
+with @code{Joined} if there is no @code{RejectNegative} property.
+@code{Negative} is ignored if there is @code{Joined} without
+@code{RejectNegative}.
 
 @item Joined
 @itemx Separate
diff --git a/gcc/opts-common.c b/gcc/opts-common.c
index ee8898b22ec..edbb3ac9b6d 100644
--- a/gcc/opts-common.c
+++ b/gcc/opts-common.c
@@ -1015,7 +1015,9 @@ prune_options (struct cl_decoded_option **decoded_options,
 	    goto keep;
 
 	  /* Skip joined switches.  */
-	  if ((option->flags & CL_JOINED))
+	  if ((option->flags & CL_JOINED)
+	      && (!option->cl_reject_negative
+		  || (unsigned int) option->neg_index != opt_idx))
 	    goto keep;
 
 	  for (j = i + 1; j < old_decoded_options_count; j++)
@@ -1027,8 +1029,11 @@ prune_options (struct cl_decoded_option **decoded_options,
 		continue;
 	      if (cl_options[next_opt_idx].neg_index < 0)
 		continue;
-	      if ((cl_options[next_opt_idx].flags & CL_JOINED))
-		  continue;
+	      if ((cl_options[next_opt_idx].flags & CL_JOINED)
+		  && (!cl_options[next_opt_idx].cl_reject_negative
+		      || ((unsigned int) cl_options[next_opt_idx].neg_index
+			  != next_opt_idx)))
+		continue;
 	      if (cancel_option (opt_idx, next_opt_idx, next_opt_idx))
 		break;
 	    }
diff --git a/gcc/testsuite/gcc.dg/pr69471-1.c b/gcc/testsuite/gcc.dg/pr69471-1.c
new file mode 100644
index 00000000000..3eac3b5bdbc
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr69471-1.c
@@ -0,0 +1,9 @@
+/* { dg-do compile } */
+/* { dg-options "-Wno-implicit-function-declaration -Wno-int-conversion -fno-builtin-free -fno-builtin-malloc" } */
+
+void *
+foo (void * p)
+{
+  free (p);
+  return malloc (p);
+}
diff --git a/gcc/testsuite/gcc.dg/pr69471-2.c b/gcc/testsuite/gcc.dg/pr69471-2.c
new file mode 100644
index 00000000000..d5799604b36
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr69471-2.c
@@ -0,0 +1,8 @@
+/* { dg-do compile } */
+/* { dg-options "-gstabs2 -gdwarf-4 -gstabs3" } */
+/* { dg-error "conflicts with prior selectio" "" { target *-*-* } 0 } */
+
+void
+foo (void)
+{
+}
-- 
2.20.1

Re: V2 [PATCH] driver: Also prune joined switches with negation

[Jakub Jelinek]

On Wed, Feb 13, 2019 at 06:27:51PM -0800, H.J. Lu wrote:
> --- a/gcc/doc/options.texi
> +++ b/gcc/doc/options.texi
> @@ -227,7 +227,10 @@ options, their @code{Negative} properties should form a circular chain.
>  For example, if options @option{-@var{a}}, @option{-@var{b}} and
>  @option{-@var{c}} are mutually exclusive, their respective @code{Negative}
>  properties should be @samp{Negative(@var{b})}, @samp{Negative(@var{c})}
> -and @samp{Negative(@var{a})}.
> +and @samp{Negative(@var{a})}.  @code{Negative} can be used together
> +with @code{Joined} if there is no @code{RejectNegative} property.
> +@code{Negative} is ignored if there is @code{Joined} without
> +@code{RejectNegative}.

I think this doesn't describe what is implemented.

Something like:
 the option name with the leading ``-'' removed.  This chain action will
 propagate through the @code{Negative} property of the option to be
-turned off.
+turned off.  The driver will prune options, removing those that are
+turned off by some later option.  This pruning is not done for options
+with @code{Joined} or @code{JoinedOrMissing} properties, unless the
+options have either @code{RejectNegative} property or the @code{Negative}
+property mentions an option other than itself.

 As a consequence, if you have a group of mutually-exclusive
 options, their @code{Negative} properties should form a circular chain.

?

Otherwise LGTM, but Joseph is the options machinery maintainer, so I'll
defer to him here.

	Jakub

[PATCH] Add testcases for multiple -fsanitize=, -fno-sanitize= or -fno-sanitize-recover= options

[Jakub Jelinek]

Hi!

The following patch adds testcase coverage to make sure
-f{,no-}sanitize{,-recover}= options are all passed to the compiler backend
from the driver.

All these tests were broken by the earlier option handling patch from H.J.:
https://gcc.gnu.org/ml/gcc-patches/2019-02/msg00492.html
and as nothing in the testsuite revealed the patch broke this, I think we
want to cover this in the testsuite.

Tested on x86_64-linux with
make check-gcc check-c++-all RUNTESTFLAGS='--target_board=unix\{-m32,-m64\} ubsan.exp=opts*'
with current trunk (all tests PASS) and with trunk patched with the above
patch (all tests FAIL).  Ok for trunk?

2019-02-14  Jakub Jelinek  <jakub@redhat.com>

	* c-c++-common/ubsan/opts-1.c: New test.
	* c-c++-common/ubsan/opts-2.c: New test.
	* c-c++-common/ubsan/opts-3.c: New test.
	* c-c++-common/ubsan/opts-4.c: New test.

--- gcc/testsuite/c-c++-common/ubsan/opts-1.c.jj	2019-02-14 11:31:33.144895232 +0100
+++ gcc/testsuite/c-c++-common/ubsan/opts-1.c	2019-02-14 11:33:23.049077585 +0100
@@ -0,0 +1,22 @@
+/* { dg-do compile } */
+/* { dg-options "-fsanitize=undefined -fsanitize=shift -fsanitize=float-divide-by-zero -fdump-tree-optimized" } */
+/* { dg-final { scan-tree-dump-times "__ubsan_handle_divrem_overflow" 2 "optimized" } } */
+/* { dg-final { scan-tree-dump-times "__ubsan_handle_shift_out_of_bounds" 1 "optimized" } } */
+
+int
+foo (int x, int y)
+{
+  return x / y;
+}
+
+int
+bar (int x, int y)
+{
+  return x << y;
+}
+
+float
+baz (float x, float y)
+{
+  return x / y;
+}
--- gcc/testsuite/c-c++-common/ubsan/opts-2.c.jj	2019-02-14 11:33:29.806965829 +0100
+++ gcc/testsuite/c-c++-common/ubsan/opts-2.c	2019-02-14 11:34:03.169414166 +0100
@@ -0,0 +1,22 @@
+/* { dg-do compile } */
+/* { dg-options "-fsanitize=undefined -fno-sanitize=shift -fsanitize=float-divide-by-zero -fdump-tree-optimized" } */
+/* { dg-final { scan-tree-dump-times "__ubsan_handle_divrem_overflow" 2 "optimized" } } */
+/* { dg-final { scan-tree-dump-not "__ubsan_handle_shift_out_of_bounds" "optimized" } } */
+
+int
+foo (int x, int y)
+{
+  return x / y;
+}
+
+int
+bar (int x, int y)
+{
+  return x << y;
+}
+
+float
+baz (float x, float y)
+{
+  return x / y;
+}
--- gcc/testsuite/c-c++-common/ubsan/opts-3.c.jj	2019-02-14 11:34:10.538292322 +0100
+++ gcc/testsuite/c-c++-common/ubsan/opts-3.c	2019-02-14 11:34:35.512879358 +0100
@@ -0,0 +1,22 @@
+/* { dg-do compile } */
+/* { dg-options "-fsanitize=undefined -fno-sanitize=shift -fno-sanitize=float-divide-by-zero -fdump-tree-optimized" } */
+/* { dg-final { scan-tree-dump-times "__ubsan_handle_divrem_overflow" 1 "optimized" } } */
+/* { dg-final { scan-tree-dump-not "__ubsan_handle_shift_out_of_bounds" "optimized" } } */
+
+int
+foo (int x, int y)
+{
+  return x / y;
+}
+
+int
+bar (int x, int y)
+{
+  return x << y;
+}
+
+float
+baz (float x, float y)
+{
+  return x / y;
+}
--- gcc/testsuite/c-c++-common/ubsan/opts-4.c.jj	2019-02-14 11:40:35.771922337 +0100
+++ gcc/testsuite/c-c++-common/ubsan/opts-4.c	2019-02-14 11:40:29.220030674 +0100
@@ -0,0 +1,26 @@
+/* { dg-do compile } */
+/* { dg-options "-fsanitize=undefined -fno-sanitize-recover=integer-divide-by-zero -fno-sanitize-recover=shift -fdump-tree-optimized" } */
+/* { dg-final { scan-tree-dump-times "__ubsan_handle_divrem_overflow_abort" 1 "optimized" } } */
+/* { dg-final { scan-tree-dump-times "__ubsan_handle_shift_out_of_bounds_abort" 1 "optimized" } } */
+/* { dg-final { scan-tree-dump-times "__ubsan_handle_type_mismatch_v1" 1 "optimized" } } */
+/* { dg-final { scan-tree-dump-not "__ubsan_handle_type_mismatch_v1_abort" "optimized" } } */
+
+int
+foo (int x, int y)
+{
+  return x / y;
+}
+
+int
+bar (int x, int y)
+{
+  return x << y;
+}
+
+enum E { E0, E1, E2, E3 };
+
+enum E
+baz (enum E *x)
+{
+  return *x;
+}

	Jakub

Re: [PATCH] Add testcases for multiple -fsanitize=, -fno-sanitize= or -fno-sanitize-recover= options

[H.J. Lu]

On Thu, Feb 14, 2019 at 3:09 AM Jakub Jelinek <jakub@redhat.com> wrote:
>
> Hi!
>
> The following patch adds testcase coverage to make sure
> -f{,no-}sanitize{,-recover}= options are all passed to the compiler backend
> from the driver.
>
> All these tests were broken by the earlier option handling patch from H.J.:
> https://gcc.gnu.org/ml/gcc-patches/2019-02/msg00492.html
> and as nothing in the testsuite revealed the patch broke this, I think we
> want to cover this in the testsuite.
>
> Tested on x86_64-linux with
> make check-gcc check-c++-all RUNTESTFLAGS='--target_board=unix\{-m32,-m64\} ubsan.exp=opts*'
> with current trunk (all tests PASS) and with trunk patched with the above
> patch (all tests FAIL).  Ok for trunk?
>
> 2019-02-14  Jakub Jelinek  <jakub@redhat.com>
>
>         * c-c++-common/ubsan/opts-1.c: New test.
>         * c-c++-common/ubsan/opts-2.c: New test.
>         * c-c++-common/ubsan/opts-3.c: New test.
>         * c-c++-common/ubsan/opts-4.c: New test.

I got

UNRESOLVED: c-c++-common/ubsan/opts-1.c   -O2 -flto
-fuse-linker-plugin -fno-fat-lto-objects   scan-tree-dump-times
optimized "__ubsan_handle_divrem_overflow" 2
UNRESOLVED: c-c++-common/ubsan/opts-1.c   -O2 -flto
-fuse-linker-plugin -fno-fat-lto-objects   scan-tree-dump-times
optimized "__ubsan_handle_shift_out_of_bounds" 1
UNRESOLVED: c-c++-common/ubsan/opts-2.c   -O2 -flto
-fuse-linker-plugin -fno-fat-lto-objects   scan-tree-dump-times
optimized "__ubsan_handle_divrem_overflow" 2
UNRESOLVED: c-c++-common/ubsan/opts-2.c   -O2 -flto
-fuse-linker-plugin -fno-fat-lto-objects   scan-tree-dump-not
optimized "__ubsan_handle_shift_out_of_bounds"
UNRESOLVED: c-c++-common/ubsan/opts-3.c   -O2 -flto
-fuse-linker-plugin -fno-fat-lto-objects   scan-tree-dump-times
optimized "__ubsan_handle_divrem_overflow" 1
UNRESOLVED: c-c++-common/ubsan/opts-3.c   -O2 -flto
-fuse-linker-plugin -fno-fat-lto-objects   scan-tree-dump-not
optimized "__ubsan_handle_shift_out_of_bounds"
UNRESOLVED: c-c++-common/ubsan/opts-4.c   -O2 -flto
-fuse-linker-plugin -fno-fat-lto-objects   scan-tree-dump-times
optimized "__ubsan_handle_divrem_overflow_abort" 1
UNRESOLVED: c-c++-common/ubsan/opts-4.c   -O2 -flto
-fuse-linker-plugin -fno-fat-lto-objects   scan-tree-dump-times
optimized "__ubsan_handle_shift_out_of_bounds_abort" 1
UNRESOLVED: c-c++-common/ubsan/opts-4.c   -O2 -flto
-fuse-linker-plugin -fno-fat-lto-objects   scan-tree-dump-times
optimized "__ubsan_handle_type_mismatch_v1" 1
UNRESOLVED: c-c++-common/ubsan/opts-4.c   -O2 -flto
-fuse-linker-plugin -fno-fat-lto-objects   scan-tree-dump-not
optimized "__ubsan_handle_type_mismatch_v1_abort"
UNRESOLVED: c-c++-common/ubsan/opts-1.c   -O2 -flto
-fuse-linker-plugin -fno-fat-lto-objects   scan-tree-dump-times
optimized "__ubsan_handle_divrem_overflow" 2
UNRESOLVED: c-c++-common/ubsan/opts-1.c   -O2 -flto
-fuse-linker-plugin -fno-fat-lto-objects   scan-tree-dump-times
optimized "__ubsan_handle_shift_out_of_bounds" 1
UNRESOLVED: c-c++-common/ubsan/opts-2.c   -O2 -flto
-fuse-linker-plugin -fno-fat-lto-objects   scan-tree-dump-times
optimized "__ubsan_handle_divrem_overflow" 2
UNRESOLVED: c-c++-common/ubsan/opts-2.c   -O2 -flto
-fuse-linker-plugin -fno-fat-lto-objects   scan-tree-dump-not
optimized "__ubsan_handle_shift_out_of_bounds"
UNRESOLVED: c-c++-common/ubsan/opts-3.c   -O2 -flto
-fuse-linker-plugin -fno-fat-lto-objects   scan-tree-dump-times
optimized "__ubsan_handle_divrem_overflow" 1
UNRESOLVED: c-c++-common/ubsan/opts-3.c   -O2 -flto
-fuse-linker-plugin -fno-fat-lto-objects   scan-tree-dump-not
optimized "__ubsan_handle_shift_out_of_bounds"
UNRESOLVED: c-c++-common/ubsan/opts-4.c   -O2 -flto
-fuse-linker-plugin -fno-fat-lto-objects   scan-tree-dump-times
optimized "__ubsan_handle_divrem_overflow_abort" 1
UNRESOLVED: c-c++-common/ubsan/opts-4.c   -O2 -flto
-fuse-linker-plugin -fno-fat-lto-objects   scan-tree-dump-times
optimized "__ubsan_handle_shift_out_of_bounds_abort" 1
UNRESOLVED: c-c++-common/ubsan/opts-4.c   -O2 -flto
-fuse-linker-plugin -fno-fat-lto-objects   scan-tree-dump-times
optimized "__ubsan_handle_type_mismatch_v1" 1
UNRESOLVED: c-c++-common/ubsan/opts-4.c   -O2 -flto
-fuse-linker-plugin -fno-fat-lto-objects   scan-tree-dump-not
optimized "__ubsan_handle_type_mismatch_v1_abort"

since  -flto suppresses -fdump-tree-optimized.

H.J.
> --- gcc/testsuite/c-c++-common/ubsan/opts-1.c.jj        2019-02-14 11:31:33.144895232 +0100
> +++ gcc/testsuite/c-c++-common/ubsan/opts-1.c   2019-02-14 11:33:23.049077585 +0100
> @@ -0,0 +1,22 @@
> +/* { dg-do compile } */
> +/* { dg-options "-fsanitize=undefined -fsanitize=shift -fsanitize=float-divide-by-zero -fdump-tree-optimized" } */
> +/* { dg-final { scan-tree-dump-times "__ubsan_handle_divrem_overflow" 2 "optimized" } } */
> +/* { dg-final { scan-tree-dump-times "__ubsan_handle_shift_out_of_bounds" 1 "optimized" } } */
> +
> +int
> +foo (int x, int y)
> +{
> +  return x / y;
> +}
> +
> +int
> +bar (int x, int y)
> +{
> +  return x << y;
> +}
> +
> +float
> +baz (float x, float y)
> +{
> +  return x / y;
> +}
> --- gcc/testsuite/c-c++-common/ubsan/opts-2.c.jj        2019-02-14 11:33:29.806965829 +0100
> +++ gcc/testsuite/c-c++-common/ubsan/opts-2.c   2019-02-14 11:34:03.169414166 +0100
> @@ -0,0 +1,22 @@
> +/* { dg-do compile } */
> +/* { dg-options "-fsanitize=undefined -fno-sanitize=shift -fsanitize=float-divide-by-zero -fdump-tree-optimized" } */
> +/* { dg-final { scan-tree-dump-times "__ubsan_handle_divrem_overflow" 2 "optimized" } } */
> +/* { dg-final { scan-tree-dump-not "__ubsan_handle_shift_out_of_bounds" "optimized" } } */
> +
> +int
> +foo (int x, int y)
> +{
> +  return x / y;
> +}
> +
> +int
> +bar (int x, int y)
> +{
> +  return x << y;
> +}
> +
> +float
> +baz (float x, float y)
> +{
> +  return x / y;
> +}
> --- gcc/testsuite/c-c++-common/ubsan/opts-3.c.jj        2019-02-14 11:34:10.538292322 +0100
> +++ gcc/testsuite/c-c++-common/ubsan/opts-3.c   2019-02-14 11:34:35.512879358 +0100
> @@ -0,0 +1,22 @@
> +/* { dg-do compile } */
> +/* { dg-options "-fsanitize=undefined -fno-sanitize=shift -fno-sanitize=float-divide-by-zero -fdump-tree-optimized" } */
> +/* { dg-final { scan-tree-dump-times "__ubsan_handle_divrem_overflow" 1 "optimized" } } */
> +/* { dg-final { scan-tree-dump-not "__ubsan_handle_shift_out_of_bounds" "optimized" } } */
> +
> +int
> +foo (int x, int y)
> +{
> +  return x / y;
> +}
> +
> +int
> +bar (int x, int y)
> +{
> +  return x << y;
> +}
> +
> +float
> +baz (float x, float y)
> +{
> +  return x / y;
> +}
> --- gcc/testsuite/c-c++-common/ubsan/opts-4.c.jj        2019-02-14 11:40:35.771922337 +0100
> +++ gcc/testsuite/c-c++-common/ubsan/opts-4.c   2019-02-14 11:40:29.220030674 +0100
> @@ -0,0 +1,26 @@
> +/* { dg-do compile } */
> +/* { dg-options "-fsanitize=undefined -fno-sanitize-recover=integer-divide-by-zero -fno-sanitize-recover=shift -fdump-tree-optimized" } */
> +/* { dg-final { scan-tree-dump-times "__ubsan_handle_divrem_overflow_abort" 1 "optimized" } } */
> +/* { dg-final { scan-tree-dump-times "__ubsan_handle_shift_out_of_bounds_abort" 1 "optimized" } } */
> +/* { dg-final { scan-tree-dump-times "__ubsan_handle_type_mismatch_v1" 1 "optimized" } } */
> +/* { dg-final { scan-tree-dump-not "__ubsan_handle_type_mismatch_v1_abort" "optimized" } } */
> +
> +int
> +foo (int x, int y)
> +{
> +  return x / y;
> +}
> +
> +int
> +bar (int x, int y)
> +{
> +  return x << y;
> +}
> +
> +enum E { E0, E1, E2, E3 };
> +
> +enum E
> +baz (enum E *x)
> +{
> +  return *x;
> +}
>
>         Jakub



-- 
H.J.

Re: V2 [PATCH] driver: Also prune joined switches with negation

[H.J. Lu]

On Thu, Feb 14, 2019 at 12:03:30PM +0100, Jakub Jelinek wrote:
> On Wed, Feb 13, 2019 at 06:27:51PM -0800, H.J. Lu wrote:
> > --- a/gcc/doc/options.texi
> > +++ b/gcc/doc/options.texi
> > @@ -227,7 +227,10 @@ options, their @code{Negative} properties should form a circular chain.
> >  For example, if options @option{-@var{a}}, @option{-@var{b}} and
> >  @option{-@var{c}} are mutually exclusive, their respective @code{Negative}
> >  properties should be @samp{Negative(@var{b})}, @samp{Negative(@var{c})}
> > -and @samp{Negative(@var{a})}.
> > +and @samp{Negative(@var{a})}.  @code{Negative} can be used together
> > +with @code{Joined} if there is no @code{RejectNegative} property.
> > +@code{Negative} is ignored if there is @code{Joined} without
> > +@code{RejectNegative}.
> 
> I think this doesn't describe what is implemented.
> 
> Something like:
>  the option name with the leading ``-'' removed.  This chain action will
>  propagate through the @code{Negative} property of the option to be
> -turned off.
> +turned off.  The driver will prune options, removing those that are
> +turned off by some later option.  This pruning is not done for options
> +with @code{Joined} or @code{JoinedOrMissing} properties, unless the
> +options have either @code{RejectNegative} property or the @code{Negative}
> +property mentions an option other than itself.
> 
>  As a consequence, if you have a group of mutually-exclusive
>  options, their @code{Negative} properties should form a circular chain.
> 
> ?
> 
> Otherwise LGTM, but Joseph is the options machinery maintainer, so I'll
> defer to him here.
> 

Here is the updated patch with a "-march=native -march=knl" testcase.

Thanks.

H.J.
---
When -march=native is passed to host_detect_local_cpu to the backend,
it overrides all command lines after it.  That means

$ gcc -march=native -march=skylake-avx512

is the treated as

$ gcc -march=skylake-avx512 -march=native

Prune joined switches with Negative and RejectNegative to allow
-march=skylake-avx512 to override previous -march=native on command-line.

gcc/

	PR driver/69471
	* opts-common.c (prune_options): Also prune joined switches
	with Negative and RejectNegative.
	* config/i386/i386.opt (march=): Add Negative(march=).
	(mtune=): Add Negative(mtune=).
	* doc/options.texi: Document Negative used together with Joined
	and RejectNegative.

gcc/testsuite/

	PR driver/69471
	* gcc.dg/pr69471-1.c: New test.
	* gcc.dg/pr69471-2.c: Likewise.
	* gcc.target/i386/pr69471-3.c: Likewise.
---
 gcc/config/i386/i386.opt                  |  4 ++--
 gcc/doc/options.texi                      |  6 +++++-
 gcc/opts-common.c                         | 11 ++++++++---
 gcc/testsuite/gcc.dg/pr69471-1.c          |  9 +++++++++
 gcc/testsuite/gcc.dg/pr69471-2.c          |  8 ++++++++
 gcc/testsuite/gcc.target/i386/pr69471-3.c | 11 +++++++++++
 6 files changed, 43 insertions(+), 6 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/pr69471-1.c
 create mode 100644 gcc/testsuite/gcc.dg/pr69471-2.c
 create mode 100644 gcc/testsuite/gcc.target/i386/pr69471-3.c

diff --git a/gcc/config/i386/i386.opt b/gcc/config/i386/i386.opt
index 9b93241f790..b7998ee7363 100644
--- a/gcc/config/i386/i386.opt
+++ b/gcc/config/i386/i386.opt
@@ -253,7 +253,7 @@ EnumValue
 Enum(ix86_align_data) String(cacheline) Value(ix86_align_data_type_cacheline)
 
 march=
-Target RejectNegative Joined Var(ix86_arch_string)
+Target RejectNegative Negative(march=) Joined Var(ix86_arch_string)
 Generate code for given CPU.
 
 masm=
@@ -510,7 +510,7 @@ Target Report Mask(TLS_DIRECT_SEG_REFS)
 Use direct references against %gs when accessing tls data.
 
 mtune=
-Target RejectNegative Joined Var(ix86_tune_string)
+Target RejectNegative Negative(mtune=) Joined Var(ix86_tune_string)
 Schedule code for given CPU.
 
 mtune-ctrl=
diff --git a/gcc/doc/options.texi b/gcc/doc/options.texi
index 0081243acab..1c83d241488 100644
--- a/gcc/doc/options.texi
+++ b/gcc/doc/options.texi
@@ -220,7 +220,11 @@ property is used.
 The option will turn off another option @var{othername}, which is
 the option name with the leading ``-'' removed.  This chain action will
 propagate through the @code{Negative} property of the option to be
-turned off.
+turned off.  The driver will prune options, removing those that are
+turned off by some later option.  This pruning is not done for options
+with @code{Joined} or @code{JoinedOrMissing} properties, unless the
+options have either @code{RejectNegative} property or the @code{Negative}
+property mentions an option other than itself.
 
 As a consequence, if you have a group of mutually-exclusive
 options, their @code{Negative} properties should form a circular chain.
diff --git a/gcc/opts-common.c b/gcc/opts-common.c
index ee8898b22ec..edbb3ac9b6d 100644
--- a/gcc/opts-common.c
+++ b/gcc/opts-common.c
@@ -1015,7 +1015,9 @@ prune_options (struct cl_decoded_option **decoded_options,
 	    goto keep;
 
 	  /* Skip joined switches.  */
-	  if ((option->flags & CL_JOINED))
+	  if ((option->flags & CL_JOINED)
+	      && (!option->cl_reject_negative
+		  || (unsigned int) option->neg_index != opt_idx))
 	    goto keep;
 
 	  for (j = i + 1; j < old_decoded_options_count; j++)
@@ -1027,8 +1029,11 @@ prune_options (struct cl_decoded_option **decoded_options,
 		continue;
 	      if (cl_options[next_opt_idx].neg_index < 0)
 		continue;
-	      if ((cl_options[next_opt_idx].flags & CL_JOINED))
-		  continue;
+	      if ((cl_options[next_opt_idx].flags & CL_JOINED)
+		  && (!cl_options[next_opt_idx].cl_reject_negative
+		      || ((unsigned int) cl_options[next_opt_idx].neg_index
+			  != next_opt_idx)))
+		continue;
 	      if (cancel_option (opt_idx, next_opt_idx, next_opt_idx))
 		break;
 	    }
diff --git a/gcc/testsuite/gcc.dg/pr69471-1.c b/gcc/testsuite/gcc.dg/pr69471-1.c
new file mode 100644
index 00000000000..3eac3b5bdbc
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr69471-1.c
@@ -0,0 +1,9 @@
+/* { dg-do compile } */
+/* { dg-options "-Wno-implicit-function-declaration -Wno-int-conversion -fno-builtin-free -fno-builtin-malloc" } */
+
+void *
+foo (void * p)
+{
+  free (p);
+  return malloc (p);
+}
diff --git a/gcc/testsuite/gcc.dg/pr69471-2.c b/gcc/testsuite/gcc.dg/pr69471-2.c
new file mode 100644
index 00000000000..d5799604b36
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr69471-2.c
@@ -0,0 +1,8 @@
+/* { dg-do compile } */
+/* { dg-options "-gstabs2 -gdwarf-4 -gstabs3" } */
+/* { dg-error "conflicts with prior selectio" "" { target *-*-* } 0 } */
+
+void
+foo (void)
+{
+}
diff --git a/gcc/testsuite/gcc.target/i386/pr69471-3.c b/gcc/testsuite/gcc.target/i386/pr69471-3.c
new file mode 100644
index 00000000000..3826f969090
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr69471-3.c
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-options "-march=native -march=knl" } */
+
+/* NB: We want to verify that -march=native -march=processor is the same
+   as -march=processor.  Since it is very unlikely that GCC will be built
+   on KNL, -march=native will have -mno-avx512er and -march=knl should
+   enable AVX512ER.  */
+
+#ifndef __AVX512ER__
+# error __AVX512ER__ is not defined
+#endif
-- 
2.20.1

[PATCH] Add testcases for multiple -fsanitize=, -fno-sanitize= or -fno-sanitize-recover= options (take 2)

[Jakub Jelinek]

On Thu, Feb 14, 2019 at 05:48:29AM -0800, H.J. Lu wrote:
> I got
> 
> UNRESOLVED: c-c++-common/ubsan/opts-1.c   -O2 -flto
> -fuse-linker-plugin -fno-fat-lto-objects   scan-tree-dump-times
> optimized "__ubsan_handle_divrem_overflow" 2

Ah, yes, UNRESOLVED doesn't show up visible when running tests by hand,
rather than doing test_summary.  Here is an updated patch that adds the
needed dg-skip-if directives.  Ok for trunk?

2019-02-14  Jakub Jelinek  <jakub@redhat.com>

	* c-c++-common/ubsan/opts-1.c: New test.
	* c-c++-common/ubsan/opts-2.c: New test.
	* c-c++-common/ubsan/opts-3.c: New test.
	* c-c++-common/ubsan/opts-4.c: New test.

--- gcc/testsuite/c-c++-common/ubsan/opts-1.c.jj	2019-02-14 11:31:33.144895232 +0100
+++ gcc/testsuite/c-c++-common/ubsan/opts-1.c	2019-02-14 11:33:23.049077585 +0100
@@ -0,0 +1,23 @@
+/* { dg-do compile } */
+/* { dg-options "-fsanitize=undefined -fsanitize=shift -fsanitize=float-divide-by-zero -fdump-tree-optimized" } */
+/* { dg-skip-if "" { *-*-* } { "-flto -fno-fat-lto-objects" } } */
+/* { dg-final { scan-tree-dump-times "__ubsan_handle_divrem_overflow" 2 "optimized" } } */
+/* { dg-final { scan-tree-dump-times "__ubsan_handle_shift_out_of_bounds" 1 "optimized" } } */
+
+int
+foo (int x, int y)
+{
+  return x / y;
+}
+
+int
+bar (int x, int y)
+{
+  return x << y;
+}
+
+float
+baz (float x, float y)
+{
+  return x / y;
+}
--- gcc/testsuite/c-c++-common/ubsan/opts-2.c.jj	2019-02-14 11:33:29.806965829 +0100
+++ gcc/testsuite/c-c++-common/ubsan/opts-2.c	2019-02-14 11:34:03.169414166 +0100
@@ -0,0 +1,23 @@
+/* { dg-do compile } */
+/* { dg-options "-fsanitize=undefined -fno-sanitize=shift -fsanitize=float-divide-by-zero -fdump-tree-optimized" } */
+/* { dg-skip-if "" { *-*-* } { "-flto -fno-fat-lto-objects" } } */
+/* { dg-final { scan-tree-dump-times "__ubsan_handle_divrem_overflow" 2 "optimized" } } */
+/* { dg-final { scan-tree-dump-not "__ubsan_handle_shift_out_of_bounds" "optimized" } } */
+
+int
+foo (int x, int y)
+{
+  return x / y;
+}
+
+int
+bar (int x, int y)
+{
+  return x << y;
+}
+
+float
+baz (float x, float y)
+{
+  return x / y;
+}
--- gcc/testsuite/c-c++-common/ubsan/opts-3.c.jj	2019-02-14 11:34:10.538292322 +0100
+++ gcc/testsuite/c-c++-common/ubsan/opts-3.c	2019-02-14 11:34:35.512879358 +0100
@@ -0,0 +1,23 @@
+/* { dg-do compile } */
+/* { dg-options "-fsanitize=undefined -fno-sanitize=shift -fno-sanitize=float-divide-by-zero -fdump-tree-optimized" } */
+/* { dg-skip-if "" { *-*-* } { "-flto -fno-fat-lto-objects" } } */
+/* { dg-final { scan-tree-dump-times "__ubsan_handle_divrem_overflow" 1 "optimized" } } */
+/* { dg-final { scan-tree-dump-not "__ubsan_handle_shift_out_of_bounds" "optimized" } } */
+
+int
+foo (int x, int y)
+{
+  return x / y;
+}
+
+int
+bar (int x, int y)
+{
+  return x << y;
+}
+
+float
+baz (float x, float y)
+{
+  return x / y;
+}
--- gcc/testsuite/c-c++-common/ubsan/opts-4.c.jj	2019-02-14 11:40:35.771922337 +0100
+++ gcc/testsuite/c-c++-common/ubsan/opts-4.c	2019-02-14 11:40:29.220030674 +0100
@@ -0,0 +1,27 @@
+/* { dg-do compile } */
+/* { dg-options "-fsanitize=undefined -fno-sanitize-recover=integer-divide-by-zero -fno-sanitize-recover=shift -fdump-tree-optimized" } */
+/* { dg-skip-if "" { *-*-* } { "-flto -fno-fat-lto-objects" } } */
+/* { dg-final { scan-tree-dump-times "__ubsan_handle_divrem_overflow_abort" 1 "optimized" } } */
+/* { dg-final { scan-tree-dump-times "__ubsan_handle_shift_out_of_bounds_abort" 1 "optimized" } } */
+/* { dg-final { scan-tree-dump-times "__ubsan_handle_type_mismatch_v1" 1 "optimized" } } */
+/* { dg-final { scan-tree-dump-not "__ubsan_handle_type_mismatch_v1_abort" "optimized" } } */
+
+int
+foo (int x, int y)
+{
+  return x / y;
+}
+
+int
+bar (int x, int y)
+{
+  return x << y;
+}
+
+enum E { E0, E1, E2, E3 };
+
+enum E
+baz (enum E *x)
+{
+  return *x;
+}


	Jakub

Re: [PATCH] Add testcases for multiple -fsanitize=, -fno-sanitize= or -fno-sanitize-recover= options (take 2)

[Mike Stump]

On Feb 14, 2019, at 6:15 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> Ah, yes, UNRESOLVED doesn't show up visible when running tests by hand,
> rather than doing test_summary.  Here is an updated patch that adds the
> needed dg-skip-if directives.  Ok for trunk?

Ok.

PING^1: V2 [PATCH] driver: Also prune joined switches with negation

[H.J. Lu]

On Thu, Feb 14, 2019 at 5:51 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Thu, Feb 14, 2019 at 12:03:30PM +0100, Jakub Jelinek wrote:
> > On Wed, Feb 13, 2019 at 06:27:51PM -0800, H.J. Lu wrote:
> > > --- a/gcc/doc/options.texi
> > > +++ b/gcc/doc/options.texi
> > > @@ -227,7 +227,10 @@ options, their @code{Negative} properties should form a circular chain.
> > >  For example, if options @option{-@var{a}}, @option{-@var{b}} and
> > >  @option{-@var{c}} are mutually exclusive, their respective @code{Negative}
> > >  properties should be @samp{Negative(@var{b})}, @samp{Negative(@var{c})}
> > > -and @samp{Negative(@var{a})}.
> > > +and @samp{Negative(@var{a})}.  @code{Negative} can be used together
> > > +with @code{Joined} if there is no @code{RejectNegative} property.
> > > +@code{Negative} is ignored if there is @code{Joined} without
> > > +@code{RejectNegative}.
> >
> > I think this doesn't describe what is implemented.
> >
> > Something like:
> >  the option name with the leading ``-'' removed.  This chain action will
> >  propagate through the @code{Negative} property of the option to be
> > -turned off.
> > +turned off.  The driver will prune options, removing those that are
> > +turned off by some later option.  This pruning is not done for options
> > +with @code{Joined} or @code{JoinedOrMissing} properties, unless the
> > +options have either @code{RejectNegative} property or the @code{Negative}
> > +property mentions an option other than itself.
> >
> >  As a consequence, if you have a group of mutually-exclusive
> >  options, their @code{Negative} properties should form a circular chain.
> >
> > ?
> >
> > Otherwise LGTM, but Joseph is the options machinery maintainer, so I'll
> > defer to him here.
> >
>
> Here is the updated patch with a "-march=native -march=knl" testcase.
>
> Thanks.
>
> H.J.
> ---
> When -march=native is passed to host_detect_local_cpu to the backend,
> it overrides all command lines after it.  That means
>
> $ gcc -march=native -march=skylake-avx512
>
> is the treated as
>
> $ gcc -march=skylake-avx512 -march=native
>
> Prune joined switches with Negative and RejectNegative to allow
> -march=skylake-avx512 to override previous -march=native on command-line.
>
> gcc/
>
>         PR driver/69471
>         * opts-common.c (prune_options): Also prune joined switches
>         with Negative and RejectNegative.
>         * config/i386/i386.opt (march=): Add Negative(march=).
>         (mtune=): Add Negative(mtune=).
>         * doc/options.texi: Document Negative used together with Joined
>         and RejectNegative.
>
> gcc/testsuite/
>
>         PR driver/69471
>         * gcc.dg/pr69471-1.c: New test.
>         * gcc.dg/pr69471-2.c: Likewise.
>         * gcc.target/i386/pr69471-3.c: Likewise.

PING:

https://gcc.gnu.org/ml/gcc-patches/2019-02/msg01071.html


-- 
H.J.

Re: PING^1: V2 [PATCH] driver: Also prune joined switches with negation

[Jakub Jelinek]

On Thu, Feb 21, 2019 at 10:40:09AM -0800, H.J. Lu wrote:
> > Prune joined switches with Negative and RejectNegative to allow
> > -march=skylake-avx512 to override previous -march=native on command-line.
> >
> > gcc/
> >
> >         PR driver/69471
> >         * opts-common.c (prune_options): Also prune joined switches
> >         with Negative and RejectNegative.
> >         * config/i386/i386.opt (march=): Add Negative(march=).
> >         (mtune=): Add Negative(mtune=).
> >         * doc/options.texi: Document Negative used together with Joined
> >         and RejectNegative.
> >
> > gcc/testsuite/
> >
> >         PR driver/69471
> >         * gcc.dg/pr69471-1.c: New test.
> >         * gcc.dg/pr69471-2.c: Likewise.
> >         * gcc.target/i386/pr69471-3.c: Likewise.
> 
> PING:
> 
> https://gcc.gnu.org/ml/gcc-patches/2019-02/msg01071.html

The latest version looks good to me, so if Joseph won't object in next 48
hours, it is ok for trunk.  I guess we want to ask maintainers of other
backends who support -m*=native to decide what they want afterwards.

	Jakub

Re: [PATCH] driver: Also prune joined switches with negation

[Kyrill Tkachov]

On 9/25/19 12:10 PM, Kyrill Tkachov wrote:
>
> On 9/24/19 7:47 PM, Matt Turner wrote:
> > When -march=native is passed to host_detect_local_cpu to the backend,
> > it overrides all command lines after it.  That means
> >
> > $ gcc -march=native -march=armv8-a
> >
> > is treated as
> >
> > $ gcc -march=armv8-a -march=native
> >
> > Prune joined switches with Negative and RejectNegative to allow
> > -march=armv8-a to override previous -march=native on command-line.
> >
> > This is the same fix as was applied for i386 in SVN revision 269164
> > but for
> > aarch64 and arm.
> >
> > gcc/
> >
> >         PR driver/69471
> >         * config/aarch64/aarch64.opt (march=): Add Negative(march=).
> >         (mtune=): Add Negative(mtune=). (mcpu=): Add Negative(mcpu=).
> >         * config/arm/arm.opt: Likewise.
>
>
> Thanks.
>
> This is ok for arm. LGTM for aarch64 but you'll need an aarch64
> maintainer to approve.
>
> I've bootstrapped and tested this patch on arm-none-linux-gnueabihf and
> aarch64-none-linux-gnu and there's no fallout.
>
> I can commit it for you once the aarch64 part is
>
I've now committed this to trunk with a slightly adjusted ChangeLog

2019-09-26  Matt Turner  <mattst88@gmail.com>

     PR driver/69471
     * config/aarch64/aarch64.opt (march=): Add Negative(march=).
     (mtune=): Add Negative(mtune=).
     (mcpu=): Add Negative(mcpu=).
     * config/arm/arm.opt: Likewise.

as r276148. Backports will come a bit later after it's had some testing 
on trunk.

I don't think this patch is above the threshold for a copyright 
assignment, but if you intend to write larger patches in the future 
you'll want to get that sorted out.

Thanks!

Kyrill


> Kyrill
>
>
> > ---
> >  gcc/config/aarch64/aarch64.opt | 6 +++---
> >  gcc/config/arm/arm.opt         | 6 +++---
> >  2 files changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/gcc/config/aarch64/aarch64.opt
> > b/gcc/config/aarch64/aarch64.opt
> > index 865b6a6d8ca..fc43428b32a 100644
> > --- a/gcc/config/aarch64/aarch64.opt
> > +++ b/gcc/config/aarch64/aarch64.opt
> > @@ -119,15 +119,15 @@ EnumValue
> >  Enum(aarch64_tls_size) String(48) Value(48)
> >
> >  march=
> > -Target RejectNegative ToLower Joined Var(aarch64_arch_string)
> > +Target RejectNegative Negative(march=) ToLower Joined
> > Var(aarch64_arch_string)
> >  Use features of architecture ARCH.
> >
> >  mcpu=
> > -Target RejectNegative ToLower Joined Var(aarch64_cpu_string)
> > +Target RejectNegative Negative(mcpu=) ToLower Joined
> > Var(aarch64_cpu_string)
> >  Use features of and optimize for CPU.
> >
> >  mtune=
> > -Target RejectNegative ToLower Joined Var(aarch64_tune_string)
> > +Target RejectNegative Negative(mtune=) ToLower Joined
> > Var(aarch64_tune_string)
> >  Optimize for CPU.
> >
> >  mabi=
> > diff --git a/gcc/config/arm/arm.opt b/gcc/config/arm/arm.opt
> > index 452f0cf6d67..76c10ab62a2 100644
> > --- a/gcc/config/arm/arm.opt
> > +++ b/gcc/config/arm/arm.opt
> > @@ -82,7 +82,7 @@ mapcs-stack-check
> >  Target Report Mask(APCS_STACK) Undocumented
> >
> >  march=
> > -Target RejectNegative ToLower Joined Var(arm_arch_string)
> > +Target RejectNegative Negative(march=) ToLower Joined
> > Var(arm_arch_string)
> >  Specify the name of the target architecture.
> >
> >  ; Other arm_arch values are loaded from arm-tables.opt
> > @@ -107,7 +107,7 @@ Target Report Mask(CALLER_INTERWORKING)
> >  Thumb: Assume function pointers may go to non-Thumb aware code.
> >
> >  mcpu=
> > -Target RejectNegative ToLower Joined Var(arm_cpu_string)
> > +Target RejectNegative Negative(mcpu=) ToLower Joined 
> Var(arm_cpu_string)
> >  Specify the name of the target CPU.
> >
> >  mfloat-abi=
> > @@ -232,7 +232,7 @@ Target Report Mask(TPCS_LEAF_FRAME)
> >  Thumb: Generate (leaf) stack frames even if not needed.
> >
> >  mtune=
> > -Target RejectNegative ToLower Joined Var(arm_tune_string)
> > +Target RejectNegative Negative(mtune=) ToLower Joined
> > Var(arm_tune_string)
> >  Tune code for the given processor.
> >
> >  mprint-tune-info
> > --
> > 2.21.0
> >

Re: [PATCH] driver: Also prune joined switches with negation

[Kyrill Tkachov]

On 9/24/19 7:47 PM, Matt Turner wrote:
> When -march=native is passed to host_detect_local_cpu to the backend,
> it overrides all command lines after it.  That means
>
> $ gcc -march=native -march=armv8-a
>
> is treated as
>
> $ gcc -march=armv8-a -march=native
>
> Prune joined switches with Negative and RejectNegative to allow
> -march=armv8-a to override previous -march=native on command-line.
>
> This is the same fix as was applied for i386 in SVN revision 269164 
> but for
> aarch64 and arm.
>
> gcc/
>
>         PR driver/69471
>         * config/aarch64/aarch64.opt (march=): Add Negative(march=).
>         (mtune=): Add Negative(mtune=). (mcpu=): Add Negative(mcpu=).
>         * config/arm/arm.opt: Likewise.


Thanks.

This is ok for arm. LGTM for aarch64 but you'll need an aarch64 
maintainer to approve.

I've bootstrapped and tested this patch on arm-none-linux-gnueabihf and 
aarch64-none-linux-gnu and there's no fallout.

I can commit it for you once the aarch64 part is

Kyrill


> ---
>  gcc/config/aarch64/aarch64.opt | 6 +++---
>  gcc/config/arm/arm.opt         | 6 +++---
>  2 files changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/gcc/config/aarch64/aarch64.opt 
> b/gcc/config/aarch64/aarch64.opt
> index 865b6a6d8ca..fc43428b32a 100644
> --- a/gcc/config/aarch64/aarch64.opt
> +++ b/gcc/config/aarch64/aarch64.opt
> @@ -119,15 +119,15 @@ EnumValue
>  Enum(aarch64_tls_size) String(48) Value(48)
>
>  march=
> -Target RejectNegative ToLower Joined Var(aarch64_arch_string)
> +Target RejectNegative Negative(march=) ToLower Joined 
> Var(aarch64_arch_string)
>  Use features of architecture ARCH.
>
>  mcpu=
> -Target RejectNegative ToLower Joined Var(aarch64_cpu_string)
> +Target RejectNegative Negative(mcpu=) ToLower Joined 
> Var(aarch64_cpu_string)
>  Use features of and optimize for CPU.
>
>  mtune=
> -Target RejectNegative ToLower Joined Var(aarch64_tune_string)
> +Target RejectNegative Negative(mtune=) ToLower Joined 
> Var(aarch64_tune_string)
>  Optimize for CPU.
>
>  mabi=
> diff --git a/gcc/config/arm/arm.opt b/gcc/config/arm/arm.opt
> index 452f0cf6d67..76c10ab62a2 100644
> --- a/gcc/config/arm/arm.opt
> +++ b/gcc/config/arm/arm.opt
> @@ -82,7 +82,7 @@ mapcs-stack-check
>  Target Report Mask(APCS_STACK) Undocumented
>
>  march=
> -Target RejectNegative ToLower Joined Var(arm_arch_string)
> +Target RejectNegative Negative(march=) ToLower Joined 
> Var(arm_arch_string)
>  Specify the name of the target architecture.
>
>  ; Other arm_arch values are loaded from arm-tables.opt
> @@ -107,7 +107,7 @@ Target Report Mask(CALLER_INTERWORKING)
>  Thumb: Assume function pointers may go to non-Thumb aware code.
>
>  mcpu=
> -Target RejectNegative ToLower Joined Var(arm_cpu_string)
> +Target RejectNegative Negative(mcpu=) ToLower Joined Var(arm_cpu_string)
>  Specify the name of the target CPU.
>
>  mfloat-abi=
> @@ -232,7 +232,7 @@ Target Report Mask(TPCS_LEAF_FRAME)
>  Thumb: Generate (leaf) stack frames even if not needed.
>
>  mtune=
> -Target RejectNegative ToLower Joined Var(arm_tune_string)
> +Target RejectNegative Negative(mtune=) ToLower Joined 
> Var(arm_tune_string)
>  Tune code for the given processor.
>
>  mprint-tune-info
> -- 
> 2.21.0
>

Re: [PATCH] driver: Also prune joined switches with negation

[Matt Turner]

On Tue, Sep 24, 2019 at 1:24 AM Kyrill Tkachov
<kyrylo.tkachov@foss.arm.com> wrote:
>
> Hi Matt,
>
> On 9/24/19 5:04 AM, Matt Turner wrote:
> > When -march=native is passed to host_detect_local_cpu to the backend,
> > it overrides all command lines after it.  That means
> >
> > $ gcc -march=native -march=armv8-a
> >
> > is treated as
> >
> > $ gcc -march=armv8-a -march=native
> >
> > Prune joined switches with Negative and RejectNegative to allow
> > -march=armv8-a to override previous -march=native on command-line.
> >
> > This is the same fix as was applied for i386 in SVN revision 269164
> > but for
> > aarch64 and arm.
> >
> The fix is ok for arm and LGTM for aarch64 FWIW.

Thanks!

> How has this been tested?

The problem was noticed in this bug report:

   https://bugs.gentoo.org/693522

I remembered seeing the i386 fix and I separately encountered the
problem on ARM when building the pixman library which has iwMMXt code
which requires march=iwmmxt (Could I bribe someone into fixing that by
giving gcc an -miwmmxt flag?)

I verified the fix works by patching gcc and seeing that nss (the
package from the Gentoo bug report) successfully builds with
CFLAGS="-march=native -O2 -pipe"

SVN revision 269164 also added some tests to the gcc test suite, but I
am not sufficiently familiar with building gcc and running the test
suite to verify that any test I speculatively add actually works.

> However...
>
>
> > gcc/
> >
> >         PR driver/69471
> >         * config/aarch64/aarch64.opt (march=): Add Negative(march=).
> >         (mtune=): Add Negative(mtune=).
> >         * config/arm/arm.opt: Likewise.
> > ---
> >  gcc/config/aarch64/aarch64.opt | 5 +++--
> >  gcc/config/arm/arm.opt         | 4 ++--
> >  2 files changed, 5 insertions(+), 4 deletions(-)
> >
> > diff --git a/gcc/config/aarch64/aarch64.opt
> > b/gcc/config/aarch64/aarch64.opt
> > index 865b6a6d8ca..908dca23b3c 100644
> > --- a/gcc/config/aarch64/aarch64.opt
> > +++ b/gcc/config/aarch64/aarch64.opt
> > @@ -119,7 +119,8 @@ EnumValue
> >  Enum(aarch64_tls_size) String(48) Value(48)
> >
> >  march=
> > -Target RejectNegative ToLower Joined Var(aarch64_arch_string)
> > +Target RejectNegative Negative(march=) ToLower Joined
> > Var(aarch64_arch_string)
> > +
> >  Use features of architecture ARCH.
> >
> >  mcpu=
>
>
> ... Looks like we'll need something similar for -mcpu. On arm and
> aarch64 the -mcpu is the most commonly used option and that can also
> take a "native" value that would suffer from the same issue I presume.

Thank you. I've sent a second version with this addressed in reply to
my initial patch.

If the patch is okay, I think we'd appreciate it if it were backported
to the gcc-8 branch as well.

[PATCH] driver: Also prune joined switches with negation

[Matt Turner]

When -march=native is passed to host_detect_local_cpu to the backend,
it overrides all command lines after it.  That means

$ gcc -march=native -march=armv8-a

is treated as

$ gcc -march=armv8-a -march=native

Prune joined switches with Negative and RejectNegative to allow
-march=armv8-a to override previous -march=native on command-line.

This is the same fix as was applied for i386 in SVN revision 269164 but for
aarch64 and arm.

gcc/

	PR driver/69471
	* config/aarch64/aarch64.opt (march=): Add Negative(march=).
	(mtune=): Add Negative(mtune=). (mcpu=): Add Negative(mcpu=).
	* config/arm/arm.opt: Likewise.
---
 gcc/config/aarch64/aarch64.opt | 6 +++---
 gcc/config/arm/arm.opt         | 6 +++---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/gcc/config/aarch64/aarch64.opt b/gcc/config/aarch64/aarch64.opt
index 865b6a6d8ca..fc43428b32a 100644
--- a/gcc/config/aarch64/aarch64.opt
+++ b/gcc/config/aarch64/aarch64.opt
@@ -119,15 +119,15 @@ EnumValue
 Enum(aarch64_tls_size) String(48) Value(48)
 
 march=
-Target RejectNegative ToLower Joined Var(aarch64_arch_string)
+Target RejectNegative Negative(march=) ToLower Joined Var(aarch64_arch_string)
 Use features of architecture ARCH.
 
 mcpu=
-Target RejectNegative ToLower Joined Var(aarch64_cpu_string)
+Target RejectNegative Negative(mcpu=) ToLower Joined Var(aarch64_cpu_string)
 Use features of and optimize for CPU.
 
 mtune=
-Target RejectNegative ToLower Joined Var(aarch64_tune_string)
+Target RejectNegative Negative(mtune=) ToLower Joined Var(aarch64_tune_string)
 Optimize for CPU.
 
 mabi=
diff --git a/gcc/config/arm/arm.opt b/gcc/config/arm/arm.opt
index 452f0cf6d67..76c10ab62a2 100644
--- a/gcc/config/arm/arm.opt
+++ b/gcc/config/arm/arm.opt
@@ -82,7 +82,7 @@ mapcs-stack-check
 Target Report Mask(APCS_STACK) Undocumented
 
 march=
-Target RejectNegative ToLower Joined Var(arm_arch_string)
+Target RejectNegative Negative(march=) ToLower Joined Var(arm_arch_string)
 Specify the name of the target architecture.
 
 ; Other arm_arch values are loaded from arm-tables.opt
@@ -107,7 +107,7 @@ Target Report Mask(CALLER_INTERWORKING)
 Thumb: Assume function pointers may go to non-Thumb aware code.
 
 mcpu=
-Target RejectNegative ToLower Joined Var(arm_cpu_string)
+Target RejectNegative Negative(mcpu=) ToLower Joined Var(arm_cpu_string)
 Specify the name of the target CPU.
 
 mfloat-abi=
@@ -232,7 +232,7 @@ Target Report Mask(TPCS_LEAF_FRAME)
 Thumb: Generate (leaf) stack frames even if not needed.
 
 mtune=
-Target RejectNegative ToLower Joined Var(arm_tune_string)
+Target RejectNegative Negative(mtune=) ToLower Joined Var(arm_tune_string)
 Tune code for the given processor.
 
 mprint-tune-info
-- 
2.21.0

Re: [PATCH] driver: Also prune joined switches with negation

[Kyrill Tkachov]

Hi Matt,

On 9/24/19 5:04 AM, Matt Turner wrote:
> When -march=native is passed to host_detect_local_cpu to the backend,
> it overrides all command lines after it.  That means
>
> $ gcc -march=native -march=armv8-a
>
> is treated as
>
> $ gcc -march=armv8-a -march=native
>
> Prune joined switches with Negative and RejectNegative to allow
> -march=armv8-a to override previous -march=native on command-line.
>
> This is the same fix as was applied for i386 in SVN revision 269164 
> but for
> aarch64 and arm.
>
The fix is ok for arm and LGTM for aarch64 FWIW.

How has this been tested?

However...


> gcc/
>
>         PR driver/69471
>         * config/aarch64/aarch64.opt (march=): Add Negative(march=).
>         (mtune=): Add Negative(mtune=).
>         * config/arm/arm.opt: Likewise.
> ---
>  gcc/config/aarch64/aarch64.opt | 5 +++--
>  gcc/config/arm/arm.opt         | 4 ++--
>  2 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/gcc/config/aarch64/aarch64.opt 
> b/gcc/config/aarch64/aarch64.opt
> index 865b6a6d8ca..908dca23b3c 100644
> --- a/gcc/config/aarch64/aarch64.opt
> +++ b/gcc/config/aarch64/aarch64.opt
> @@ -119,7 +119,8 @@ EnumValue
>  Enum(aarch64_tls_size) String(48) Value(48)
>
>  march=
> -Target RejectNegative ToLower Joined Var(aarch64_arch_string)
> +Target RejectNegative Negative(march=) ToLower Joined 
> Var(aarch64_arch_string)
> +
>  Use features of architecture ARCH.
>
>  mcpu=


... Looks like we'll need something similar for -mcpu. On arm and 
aarch64 the -mcpu is the most commonly used option and that can also 
take a "native" value that would suffer from the same issue I presume.

Thanks,

Kyrill


> @@ -127,7 +128,7 @@ Target RejectNegative ToLower Joined 
> Var(aarch64_cpu_string)
>  Use features of and optimize for CPU.
>
>  mtune=
> -Target RejectNegative ToLower Joined Var(aarch64_tune_string)
> +Target RejectNegative Negative(mtune=) ToLower Joined 
> Var(aarch64_tune_string)
>  Optimize for CPU.
>
>  mabi=
> diff --git a/gcc/config/arm/arm.opt b/gcc/config/arm/arm.opt
> index 452f0cf6d67..e3ead5c95d1 100644
> --- a/gcc/config/arm/arm.opt
> +++ b/gcc/config/arm/arm.opt
> @@ -82,7 +82,7 @@ mapcs-stack-check
>  Target Report Mask(APCS_STACK) Undocumented
>
>  march=
> -Target RejectNegative ToLower Joined Var(arm_arch_string)
> +Target RejectNegative Negative(march=) ToLower Joined 
> Var(arm_arch_string)
>  Specify the name of the target architecture.
>
>  ; Other arm_arch values are loaded from arm-tables.opt
> @@ -232,7 +232,7 @@ Target Report Mask(TPCS_LEAF_FRAME)
>  Thumb: Generate (leaf) stack frames even if not needed.
>
>  mtune=
> -Target RejectNegative ToLower Joined Var(arm_tune_string)
> +Target RejectNegative Negative(mtune=) ToLower Joined 
> Var(arm_tune_string)
>  Tune code for the given processor.
>
>  mprint-tune-info
> -- 
> 2.21.0
>

[PATCH] driver: Also prune joined switches with negation

[Matt Turner]

When -march=native is passed to host_detect_local_cpu to the backend,
it overrides all command lines after it.  That means

$ gcc -march=native -march=armv8-a

is treated as

$ gcc -march=armv8-a -march=native

Prune joined switches with Negative and RejectNegative to allow
-march=armv8-a to override previous -march=native on command-line.

This is the same fix as was applied for i386 in SVN revision 269164 but for
aarch64 and arm.

gcc/

	PR driver/69471
	* config/aarch64/aarch64.opt (march=): Add Negative(march=).
	(mtune=): Add Negative(mtune=).
	* config/arm/arm.opt: Likewise.
---
 gcc/config/aarch64/aarch64.opt | 5 +++--
 gcc/config/arm/arm.opt         | 4 ++--
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/gcc/config/aarch64/aarch64.opt b/gcc/config/aarch64/aarch64.opt
index 865b6a6d8ca..908dca23b3c 100644
--- a/gcc/config/aarch64/aarch64.opt
+++ b/gcc/config/aarch64/aarch64.opt
@@ -119,7 +119,8 @@ EnumValue
 Enum(aarch64_tls_size) String(48) Value(48)
 
 march=
-Target RejectNegative ToLower Joined Var(aarch64_arch_string)
+Target RejectNegative Negative(march=) ToLower Joined Var(aarch64_arch_string)
+
 Use features of architecture ARCH.
 
 mcpu=
@@ -127,7 +128,7 @@ Target RejectNegative ToLower Joined Var(aarch64_cpu_string)
 Use features of and optimize for CPU.
 
 mtune=
-Target RejectNegative ToLower Joined Var(aarch64_tune_string)
+Target RejectNegative Negative(mtune=) ToLower Joined Var(aarch64_tune_string)
 Optimize for CPU.
 
 mabi=
diff --git a/gcc/config/arm/arm.opt b/gcc/config/arm/arm.opt
index 452f0cf6d67..e3ead5c95d1 100644
--- a/gcc/config/arm/arm.opt
+++ b/gcc/config/arm/arm.opt
@@ -82,7 +82,7 @@ mapcs-stack-check
 Target Report Mask(APCS_STACK) Undocumented
 
 march=
-Target RejectNegative ToLower Joined Var(arm_arch_string)
+Target RejectNegative Negative(march=) ToLower Joined Var(arm_arch_string)
 Specify the name of the target architecture.
 
 ; Other arm_arch values are loaded from arm-tables.opt
@@ -232,7 +232,7 @@ Target Report Mask(TPCS_LEAF_FRAME)
 Thumb: Generate (leaf) stack frames even if not needed.
 
 mtune=
-Target RejectNegative ToLower Joined Var(arm_tune_string)
+Target RejectNegative Negative(mtune=) ToLower Joined Var(arm_tune_string)
 Tune code for the given processor.
 
 mprint-tune-info
-- 
2.21.0