On Wed, Feb 13, 2019 at 12:02 AM Jakub Jelinek <ja...@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.
From 6ba21bba3f2ce2fdf8f1d8b080d8b8b748025d89 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.to...@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

Reply via email to