diff mbox

PATCH: PR target/59588: Don't check/change generic/i686 tuning

Message ID 20131224203809.GA6048@intel.com
State New
Headers show

Commit Message

H.J. Lu Dec. 24, 2013, 8:38 p.m. UTC 
Hi Honza,

We have combined generic32 and generic64 into generic.  There is no need
to check "generic" anymore.  Also we shouldn't change -mtune=i686 into
-mtune=generic.  OK to install?

Thanks.

H.J.
---
gcc/

2013-12-24   H.J. Lu  <hongjiu.lu@intel.com>

	PR target/59588
	* config/i386/i386.c (ix86_option_override_internal): Don't
	check generic tuning.  Don't change i686 tuning.

gcc/testsuite/

2013-12-24   H.J. Lu  <hongjiu.lu@intel.com>

	PR target/59588
	* gcc.target/i386/pr59588-1.c: New file.
	* gcc.target/i386/pr59588-2.c: Likewise.

Comments

Jan Hubicka Dec. 26, 2013, 12:38 p.m. UTC | #1 
> Hi Honza,
> 
> We have combined generic32 and generic64 into generic.  There is no need
> to check "generic" anymore.  Also we shouldn't change -mtune=i686 into
> -mtune=generic.  OK to install?

The i686->generic change was intended to get generic optimized code
for i686-linux configuration rather than pentiumpro.  I think it still makes
sense to use this, since it is what most 32bit distros still configure for?

Honza
H.J. Lu Dec. 26, 2013, 1:40 p.m. UTC | #2 
On Thu, Dec 26, 2013 at 4:38 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
>> Hi Honza,
>>
>> We have combined generic32 and generic64 into generic.  There is no need
>> to check "generic" anymore.  Also we shouldn't change -mtune=i686 into
>> -mtune=generic.  OK to install?
>
> The i686->generic change was intended to get generic optimized code
> for i686-linux configuration rather than pentiumpro.  I think it still makes
> sense to use this, since it is what most 32bit distros still configure for?
>

Should -mtune=i686 define __tune_i686__?  If not, how can
it be defined? Don't we default -mtune to generic for
i686-linux?
Jan Hubicka Dec. 26, 2013, 3:45 p.m. UTC | #3 
> On Thu, Dec 26, 2013 at 4:38 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
> >> Hi Honza,
> >>
> >> We have combined generic32 and generic64 into generic.  There is no need
> >> to check "generic" anymore.  Also we shouldn't change -mtune=i686 into
> >> -mtune=generic.  OK to install?
> >
> > The i686->generic change was intended to get generic optimized code
> > for i686-linux configuration rather than pentiumpro.  I think it still makes
> > sense to use this, since it is what most 32bit distros still configure for?
> >
> 
> Should -mtune=i686 define __tune_i686__?  If not, how can
> it be defined? Don't we default -mtune to generic for
> i686-linux?

If i686-linux defaults to -mtune=generic, then I think it is all fine.
i686 is bit misbehaved since it was used as both CPU name for PPro (that does not
make much sense) and for the overall architecture...

Honza
> 
> -- 
> H.J.
H.J. Lu Dec. 26, 2013, 4:06 p.m. UTC | #4 
On Thu, Dec 26, 2013 at 7:45 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
>> On Thu, Dec 26, 2013 at 4:38 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
>> >> Hi Honza,
>> >>
>> >> We have combined generic32 and generic64 into generic.  There is no need
>> >> to check "generic" anymore.  Also we shouldn't change -mtune=i686 into
>> >> -mtune=generic.  OK to install?
>> >
>> > The i686->generic change was intended to get generic optimized code
>> > for i686-linux configuration rather than pentiumpro.  I think it still makes
>> > sense to use this, since it is what most 32bit distros still configure for?
>> >
>>
>> Should -mtune=i686 define __tune_i686__?  If not, how can
>> it be defined? Don't we default -mtune to generic for
>> i686-linux?
>
> If i686-linux defaults to -mtune=generic, then I think it is all fine.

We have defaulted
[hjl@gnu-6 gcc]$ ./xgcc -B./ -S /tmp/x.i -v
Reading specs from ./specs
COLLECT_GCC=./xgcc
Target: i686-linux
Configured with: /export/gnu/import/git/gcc/configure
--enable-languages=c,c++ --disable-bootstrap i686-linux
--prefix=/usr/gcc-4.9.0 --with-local-prefix=/usr/local
--enable-targets=all --with-fpmath=sse : (reconfigured)
/export/gnu/import/git/gcc/configure --enable-languages=c,c++
--disable-bootstrap i686-linux --prefix=/usr/gcc-4.9.0
--with-local-prefix=/usr/local --enable-targets=all --with-fpmath=sse
Thread model: posix
gcc version 4.9.0 20131224 (experimental) (GCC)
COLLECT_GCC_OPTIONS='-B' './' '-S' '-v' '-mtune=generic' '-march=pentium4'
 ./cc1 -fpreprocessed /tmp/x.i -quiet -dumpbase x.i -mtune=generic
-march=pentium4 -auxbase x -version -o x.s
GNU C (GCC) version 4.9.0 20131224 (experimental) (i686-linux)
    compiled by GNU C version 4.8.2 20131212 (Red Hat 4.8.2-7), GMP
version 5.1.1, MPFR version 3.1.1, MPC version 1.0.1
GGC heuristics: --param ggc-min-expand=30 --param ggc-min-heapsize=4096
GNU C (GCC) version 4.9.0 20131224 (experimental) (i686-linux)
    compiled by GNU C version 4.8.2 20131212 (Red Hat 4.8.2-7), GMP
version 5.1.1, MPFR version 3.1.1, MPC version 1.0.1
GGC heuristics: --param ggc-min-expand=30 --param ggc-min-heapsize=4096
Compiler executable checksum: 8d0a04c49875a54ef44488e5406c52dd
COMPILER_PATH=./
LIBRARY_PATH=./:/lib/../lib/:/usr/lib/../lib/:/lib/:/usr/lib/
COLLECT_GCC_OPTIONS='-B' './' '-S' '-v' '-mtune=generic' '-march=pentium4'
[hjl@gnu-6 gcc]$

I will check in my patch.

> i686 is bit misbehaved since it was used as both CPU name for PPro (that does not
> make much sense) and for the overall architecture...
>

Thanks.
diff mbox

Patch

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index f5d9ce5..b95a620 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -3332,23 +3332,13 @@  ix86_option_override_internal (bool main_args_p,
   /* Need to check -mtune=generic first.  */
   if (opts->x_ix86_tune_string)
     {
-      if (!strcmp (opts->x_ix86_tune_string, "generic")
-	  || !strcmp (opts->x_ix86_tune_string, "i686")
-	  /* As special support for cross compilers we read -mtune=native
+      /* As special support for cross compilers we read -mtune=native
 	     as -mtune=generic.  With native compilers we won't see the
 	     -mtune=native, as it was changed by the driver.  */
-	  || !strcmp (opts->x_ix86_tune_string, "native"))
+      if (!strcmp (opts->x_ix86_tune_string, "native"))
 	{
 	  opts->x_ix86_tune_string = "generic";
 	}
-      /* If this call is for setting the option attribute, allow the
-	 generic that was previously set.  */
-      else if (!main_args_p
-	       && !strcmp (opts->x_ix86_tune_string, "generic"))
-	;
-      else if (!strncmp (opts->x_ix86_tune_string, "generic", 7))
-        error ("bad value (%s) for %stune=%s %s",
-	       opts->x_ix86_tune_string, prefix, suffix, sw);
       else if (!strcmp (opts->x_ix86_tune_string, "x86-64"))
         warning (OPT_Wdeprecated, "%stune=x86-64%s is deprecated; use "
                  "%stune=k8%s or %stune=generic%s instead as appropriate",
@@ -3366,9 +3356,7 @@  ix86_option_override_internal (bool main_args_p,
 
       /* opts->x_ix86_tune_string is set to opts->x_ix86_arch_string
 	 or defaulted.  We need to use a sensible tune option.  */
-      if (!strcmp (opts->x_ix86_tune_string, "generic")
-	  || !strcmp (opts->x_ix86_tune_string, "x86-64")
-	  || !strcmp (opts->x_ix86_tune_string, "i686"))
+      if (!strcmp (opts->x_ix86_tune_string, "x86-64"))
 	{
 	  opts->x_ix86_tune_string = "generic";
 	}
@@ -3648,7 +3636,7 @@  ix86_option_override_internal (bool main_args_p,
   else if (!strcmp (opts->x_ix86_arch_string, "intel"))
     error ("intel CPU can be used only for %stune=%s %s",
 	   prefix, suffix, sw);
-  else if (!strncmp (opts->x_ix86_arch_string, "generic", 7) || i == pta_size)
+  else if (i == pta_size)
     error ("bad value (%s) for %sarch=%s %s",
 	   opts->x_ix86_arch_string, prefix, suffix, sw);
 
diff --git a/gcc/testsuite/gcc.target/i386/pr59588-1.c b/gcc/testsuite/gcc.target/i386/pr59588-1.c
new file mode 100644
index 0000000..391f2aa
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr59588-1.c
@@ -0,0 +1,7 @@ 
+/* { dg-do preprocess } */
+/* { dg-require-effective-target ia32 } */
+/* { dg-options "-march=i686" } */
+
+#ifndef __tune_i686__
+#error "__tune_i686__ should defined for this test"
+#endif
diff --git a/gcc/testsuite/gcc.target/i386/pr59588-2.c b/gcc/testsuite/gcc.target/i386/pr59588-2.c
new file mode 100644
index 0000000..bb5f12a
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr59588-2.c
@@ -0,0 +1,7 @@ 
+/* { dg-do preprocess } */
+/* { dg-require-effective-target ia32 } */
+/* { dg-options "-mtune=i686" } */
+
+#ifndef __tune_i686__
+#error "__tune_i686__ should defined for this test"
+#endif