[PATCH] Don't set x86_prefetch_sse based on -mtune= option

[PATCH] Don't set x86_prefetch_sse based on -mtune= option

[Jakub Jelinek]

Hi!

gcc -O2 -march=i386 -mtune=pentium4,
which ought to mean generate code which will run on any i386 compatible CPU,
but tune for P4, might use SSE prefetch instructions, which IMHO is a big
no no.  Only -march= should influence selection of instructions which aren't
available on all CPUs, -mtune= should only be about scheduling and/or
selection among instructions which are available on the -march= CPU.
Certainly e.g. VIA C3 SIGILLs on the prefetch instructions present
in -O2 -march=i386 -mtune=pentium4 compiled gcc.

Not to mention that for invalid -mtune= value this if accesses memory
past the end of processor_alias_table array.

Ok for HEAD/3.4/3.3?

2004-07-07  Jakub Jelinek  <jakub@redhat.com>

	* config/i386/i386.c (override_options): Don't set x86_prefetch_sse
	from -mtune= option.

--- gcc/config/i386/i386.c.jj	2004-06-23 23:04:17.000000000 +0200
+++ gcc/config/i386/i386.c	2004-07-07 18:32:50.771416824 +0200
@@ -1310,8 +1310,6 @@ override_options (void)
 	  error ("CPU you selected does not support x86-64 instruction set");
 	break;
       }
-  if (processor_alias_table[i].flags & PTA_PREFETCH_SSE)
-    x86_prefetch_sse = true;
   if (i == pta_size)
     error ("bad value (%s) for -mtune= switch", ix86_tune_string);
 

	Jakub

Re: [PATCH] Don't set x86_prefetch_sse based on -mtune= option

[Richard Henderson]

On Wed, Jul 07, 2004 at 12:42:35PM -0400, Jakub Jelinek wrote:
> 	* config/i386/i386.c (override_options): Don't set x86_prefetch_sse
> 	from -mtune= option.

Ok.


r~

Re: [PATCH] Don't set x86_prefetch_sse based on -mtune= option

[Jan Hubicka]

> Hi!
> 
> gcc -O2 -march=i386 -mtune=pentium4,
> which ought to mean generate code which will run on any i386 compatible CPU,
> but tune for P4, might use SSE prefetch instructions, which IMHO is a big
> no no.  Only -march= should influence selection of instructions which aren't
> available on all CPUs, -mtune= should only be about scheduling and/or
> selection among instructions which are available on the -march= CPU.
> Certainly e.g. VIA C3 SIGILLs on the prefetch instructions present
> in -O2 -march=i386 -mtune=pentium4 compiled gcc.

This is interesting as Intel has even patent pending on the idea of
replacing instructions interpreted as NOPs by earlier CPUs by something
ignoreable in future revisions.  SSE prefetches should be precisely one
of these.  It looks like VIA has screwed up :(

Honza

Re: [PATCH] Don't set x86_prefetch_sse based on -mtune= option

[Jakub Jelinek]

On Wed, Jul 07, 2004 at 10:16:49PM +0200, Jan Hubicka wrote:
> > Hi!
> > 
> > gcc -O2 -march=i386 -mtune=pentium4,
> > which ought to mean generate code which will run on any i386 compatible CPU,
> > but tune for P4, might use SSE prefetch instructions, which IMHO is a big
> > no no.  Only -march= should influence selection of instructions which aren't
> > available on all CPUs, -mtune= should only be about scheduling and/or
> > selection among instructions which are available on the -march= CPU.
> > Certainly e.g. VIA C3 SIGILLs on the prefetch instructions present
> > in -O2 -march=i386 -mtune=pentium4 compiled gcc.
> 
> This is interesting as Intel has even patent pending on the idea of
> replacing instructions interpreted as NOPs by earlier CPUs by something
> ignoreable in future revisions.  SSE prefetches should be precisely one
> of these.  It looks like VIA has screwed up :(

If it is just VIA C3 which SIGILLs on them, we could perhaps set
x86_prefetch_sse = true if tune CPU has PTA_PREFETCH_SSE and
TARGET_CMOVE.
So -march=i386 -mtune=pentium4 would not generate prefetches but
-march=i686 -mtune=pentium4 would.  The latter can't run on pre-cmov/sse
VIAs.

	Jakub

Re: [PATCH] Don't set x86_prefetch_sse based on -mtune= option

[Paolo Bonzini]

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

> If it is just VIA C3 which SIGILLs on them, we could perhaps set
> x86_prefetch_sse = true if tune CPU has PTA_PREFETCH_SSE and
> TARGET_CMOVE.
> So -march=i386 -mtune=pentium4 would not generate prefetches but
> -march=i686 -mtune=pentium4 would.  The latter can't run on pre-cmov/sse
> VIAs.

Like this?  This ends up enabling SSE prefetches when tuning for any 
processor that does have them (Athlon, K8, Pentium 4, Nocona), as long 
as the -march points to a Pentium Pro (ok) or a CPU with SSE prefetches.

It also updates i386-pf-sse.c accordingly (it is failing now).

Ok for mainline?

Paolo


[-- Attachment #2: enable-sse-prefetch-for-i686.patch --]
[-- Type: text/plain, Size: 1890 bytes --]

gcc/ChangeLog:
2004-07-08  Paolo Bonzini  <bonzini@gnu.org>
	    Jakub Jelinek  <jakub@redhat.com>

	* config/i386/i386.c (override_options): Enable
	SSE prefetches with -mtune, as long as we are
	compiling for i686 or higher.  All i686 processors
	accept SSE prefetches as NOPS, some i586's don't.

gcc/testsuite/ChangeLog:
2004-07-08  Paolo Bonzini  <bonzini@gnu.org>

	* gcc.misc-tests/i386-pf-sse.c: Pass -march=pentium2.

Index: config/i386/i386.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/i386/i386.c,v
retrieving revision 1.685
diff -u -r1.685 i386.c
--- config/i386/i386.c	8 Jul 2004 05:53:38 -0000	1.685
+++ config/i386/i386.c	8 Jul 2004 09:34:52 -0000
@@ -1311,6 +1311,13 @@
   if (i == pta_size)
     error ("bad value (%s) for -mtune= switch", ix86_tune_string);
 
+  /* Intel CPUs have always interpreted SSE prefetch instructions as
+     NOPs; so, we can enable SSE prefetch instructions even when
+     -mtune (rather than -march) points us to a processor that has them.
+     However, the VIA C3 gives a SIGILL, so we only do that for i686 and
+     higher processors.  */
+  if (TARGET_CMOVE && (processor_alias_table[i].flags & PTA_PREFETCH_SSE))
+    x86_prefetch_sse = true;
   if (optimize_size)
     ix86_cost = &size_cost;
   else
Index: testsuite/gcc.misc-tests/i386-pf-sse-1.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/testsuite/gcc.misc-tests/i386-pf-sse-1.c,v
retrieving revision 1.1
diff -u -r1.1 i386-pf-sse-1.c
--- testsuite/gcc.misc-tests/i386-pf-sse-1.c	15 Jan 2002 17:30:28 -0000	1.1
+++ testsuite/gcc.misc-tests/i386-pf-sse-1.c	8 Jul 2004 09:34:52 -0000
@@ -2,6 +2,7 @@
    variants that use SSE prefetch instructions.  */
 
 /* { dg-do compile { target i?86-*-* } } */
+/* { dg-options "-march=pentium2" } */
 
 char *msg = "howdy there";
 

Re: [PATCH] Don't set x86_prefetch_sse based on -mtune= option

[Paolo Bonzini]

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

> If it is just VIA C3 which SIGILLs on them, we could perhaps set
> x86_prefetch_sse = true if tune CPU has PTA_PREFETCH_SSE and
> TARGET_CMOVE.
> So -march=i386 -mtune=pentium4 would not generate prefetches but
> -march=i686 -mtune=pentium4 would.  The latter can't run on pre-cmov/sse
> VIAs.

Like this?  This ends up enabling SSE prefetches when tuning for any 
processor that does have them (Athlon, K8, Pentium 4, Nocona), as long 
as the -march points to a Pentium Pro (ok) or a CPU with SSE prefetches.

It also updates i386-pf-sse.c accordingly (it is failing now).

Ok for mainline?

Paolo


[-- Attachment #2: enable-sse-prefetch-for-i686.patch --]
[-- Type: text/plain, Size: 1890 bytes --]

gcc/ChangeLog:
2004-07-08  Paolo Bonzini  <bonzini@gnu.org>
	    Jakub Jelinek  <jakub@redhat.com>

	* config/i386/i386.c (override_options): Enable
	SSE prefetches with -mtune, as long as we are
	compiling for i686 or higher.  All i686 processors
	accept SSE prefetches as NOPS, some i586's don't.

gcc/testsuite/ChangeLog:
2004-07-08  Paolo Bonzini  <bonzini@gnu.org>

	* gcc.misc-tests/i386-pf-sse.c: Pass -march=pentium2.

Index: config/i386/i386.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/config/i386/i386.c,v
retrieving revision 1.685
diff -u -r1.685 i386.c
--- config/i386/i386.c	8 Jul 2004 05:53:38 -0000	1.685
+++ config/i386/i386.c	8 Jul 2004 09:34:52 -0000
@@ -1311,6 +1311,13 @@
   if (i == pta_size)
     error ("bad value (%s) for -mtune= switch", ix86_tune_string);
 
+  /* Intel CPUs have always interpreted SSE prefetch instructions as
+     NOPs; so, we can enable SSE prefetch instructions even when
+     -mtune (rather than -march) points us to a processor that has them.
+     However, the VIA C3 gives a SIGILL, so we only do that for i686 and
+     higher processors.  */
+  if (TARGET_CMOVE && (processor_alias_table[i].flags & PTA_PREFETCH_SSE))
+    x86_prefetch_sse = true;
   if (optimize_size)
     ix86_cost = &size_cost;
   else
Index: testsuite/gcc.misc-tests/i386-pf-sse-1.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/testsuite/gcc.misc-tests/i386-pf-sse-1.c,v
retrieving revision 1.1
diff -u -r1.1 i386-pf-sse-1.c
--- testsuite/gcc.misc-tests/i386-pf-sse-1.c	15 Jan 2002 17:30:28 -0000	1.1
+++ testsuite/gcc.misc-tests/i386-pf-sse-1.c	8 Jul 2004 09:34:52 -0000
@@ -2,6 +2,7 @@
    variants that use SSE prefetch instructions.  */
 
 /* { dg-do compile { target i?86-*-* } } */
+/* { dg-options "-march=pentium2" } */
 
 char *msg = "howdy there";
 

[PATCH] Don't set x86_prefetch_sse based on -mtune= option (followup)

[Jakub Jelinek]

On Thu, Jul 08, 2004 at 11:47:10AM +0200, Paolo Bonzini wrote:
> >If it is just VIA C3 which SIGILLs on them, we could perhaps set
> >x86_prefetch_sse = true if tune CPU has PTA_PREFETCH_SSE and
> >TARGET_CMOVE.
> >So -march=i386 -mtune=pentium4 would not generate prefetches but
> >-march=i686 -mtune=pentium4 would.  The latter can't run on pre-cmov/sse
> >VIAs.
> 
> Like this?  This ends up enabling SSE prefetches when tuning for any 
> processor that does have them (Athlon, K8, Pentium 4, Nocona), as long 
> as the -march points to a Pentium Pro (ok) or a CPU with SSE prefetches.

Something like that, though I think it is better to move that check into the
loop.  Otherwise there is risk somebody reuses i for something else in
between.

> It also updates i386-pf-sse.c accordingly (it is failing now).

Sorry about that.  Though, the right fix is IMHO different, see below.
Ok to commit?

2004-07-08  Paolo Bonzini  <bonzini@gnu.org>
            Jakub Jelinek  <jakub@redhat.com>

	* config/i386/i386.c (override_options): Enable
	SSE prefetches with -mtune, as long as we are
	compiling for i686 or higher.  All i686 processors
	accept SSE prefetches as NOPS, some i586's don't.

2004-07-08  Jakub Jelinek  <jakub@redhat.com>

	* gcc.mist-tests/i386-prefetch.exp (PREFETCH_SSE): Change all
	-march=i386 into -march=i686.  Add -march=i686 -mtune=x and
	-march=x for pentium3, pentium3m, pentium-m, pentium4m,
	prescott and c3-2.
	(PREFETCH_3DNOW): Add -march=c3.

--- gcc/config/i386/i386.c.jj	2004-07-08 12:18:01.000000000 +0200
+++ gcc/config/i386/i386.c	2004-07-08 12:20:03.019534891 +0200
@@ -1306,6 +1306,14 @@ override_options (void)
 	ix86_tune = processor_alias_table[i].processor;
 	if (TARGET_64BIT && !(processor_alias_table[i].flags & PTA_64BIT))
 	  error ("CPU you selected does not support x86-64 instruction set");
+
+	/* Intel CPUs have always interpreted SSE prefetch instructions as
+	   NOPs; so, we can enable SSE prefetch instructions even when
+	   -mtune (rather than -march) points us to a processor that has them.
+	   However, the VIA C3 gives a SIGILL, so we only do that for i686 and
+	   higher processors.  */
+	if (TARGET_CMOVE && (processor_alias_table[i].flags & PTA_PREFETCH_SSE))
+	  x86_prefetch_sse = true;
 	break;
       }
   if (i == pta_size)
--- gcc/testsuite/gcc.misc-tests/i386-prefetch.exp.jj	2004-07-08 12:18:02.719791777 +0200
+++ gcc/testsuite/gcc.misc-tests/i386-prefetch.exp	2004-07-08 12:18:49.716487247 +0200
@@ -1,4 +1,4 @@
-#   Copyright (C) 2002 Free Software Foundation, Inc.
+#   Copyright (C) 2002, 2004 Free Software Foundation, Inc.
 
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -46,16 +46,27 @@ set PREFETCH_NONE [list \
 # instructions as nops.
 
 set PREFETCH_SSE [list \
-	{ -march=i386 -mtune=pentium3 } \
-	{ -march=i386 -mtune=pentium4 } \
-	{ -march=i386 -mtune=athlon } \
-	{ -march=i386 -mtune=athlon-4 } \
+	{ -march=i686 -mtune=pentium3 } \
+	{ -march=i686 -mtune=pentium3m } \
+	{ -march=i686 -mtune=pentium-m } \
+	{ -march=i686 -mtune=pentium4 } \
+	{ -march=i686 -mtune=pentium4m } \
+	{ -march=i686 -mtune=prescott } \
+	{ -march=i686 -mtune=athlon } \
+	{ -march=i686 -mtune=athlon-4 } \
+	{ -march=i686 -mtune=c3-2 } \
 	{ -march=pentium3 } \
-	{ -march=pentium4 } ]
+	{ -march=pentium3m } \
+	{ -march=pentium-m } \
+	{ -march=pentium4 } \
+	{ -march=pentium4m } \
+	{ -march=prescott } \
+	{ -march=c3-2 } ]
 
 # Generate 3DNow! prefetch instructions for the following.
 
 set PREFETCH_3DNOW [list \
+	{ -march=c3 } \
 	{ -march=k6-2 } \
 	{ -march=k6-3 } ]
 

	Jakub

Re: [PATCH] Don't set x86_prefetch_sse based on -mtune= option (followup)

[Paolo Bonzini]

> Sorry about that.  Though, the right fix is IMHO different, see below.

You're right.

Paolo

Re: [PATCH] Don't set x86_prefetch_sse based on -mtune= option (followup)

[Paolo Bonzini]

> Sorry about that.  Though, the right fix is IMHO different, see below.

You're right.

Paolo

Re: [PATCH] Don't set x86_prefetch_sse based on -mtune= option (followup)

[Richard Henderson]

Ok.

r~