[committed] Require sse for testcase on i686.

Message ID 20180706104640.GA18510@arm.com
State New
Headers show
Series
  • [committed] Require sse for testcase on i686.
Related show

Commit Message

Tamar Christina July 6, 2018, 10:46 a.m.
Hi All,

This fixes an ABI warning generated on i686-pc-linux-gnu when using
`vector_size` with no sse enabled explicitly.

Regtested single test on x86_64-pc-linux-gnu with -m32 and no issues.

Committed under the GCC obvious rule.

Thanks,
Tamar

gcc/testsuite/
2018-07-06  Tamar Christina  <tamar.christina@arm.com>

	PR target/84711
	* gcc.dg/vect/pr84711.c: Add -msse for i686 targets.

--

Comments

Jakub Jelinek July 7, 2018, 7:50 a.m. | #1
On Fri, Jul 06, 2018 at 11:46:43AM +0100, Tamar Christina wrote:
> This fixes an ABI warning generated on i686-pc-linux-gnu when using
> `vector_size` with no sse enabled explicitly.
> 
> Regtested single test on x86_64-pc-linux-gnu with -m32 and no issues.
> 
> Committed under the GCC obvious rule.

That is insufficient, I get the FAIL on i686-linux.
You don't really need dg-require-effective-target sse, it is a dg-do compile
time only test and on i?86/x86_64 failures with old assemblers would show up
only when assembling.
But -msse really should be used on all i?86-*-* and x86_64-*-*.

You'd normally get the -msse etc. automatically, but dg-options
is undesirable in gcc.dg/vect/ tests where all those predefined options
are lost that way, dg-additional-options should be used instead (and you can
say use there -O2 -fno-tree-vectorize or whatever you want).

So, if you have spare cycles, please test such change whether it still FAILs
on arm with your patch reverted after such test changes.

> gcc/testsuite/
> 2018-07-06  Tamar Christina  <tamar.christina@arm.com>
> 
> 	PR target/84711
> 	* gcc.dg/vect/pr84711.c: Add -msse for i686 targets.

In the meantime, I've committed following fix as obvious:

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

	PR target/84711
	* gcc.dg/vect/pr84711.c: Remove unnecessary sse
	dg-require-effective-target.  Add -msse not just on i386-*, but
	on all i?86-* and x86_64-*.

--- gcc/testsuite/gcc.dg/vect/pr84711.c.jj	2018-07-06 23:35:44.952791972 +0200
+++ gcc/testsuite/gcc.dg/vect/pr84711.c	2018-07-07 09:43:27.068785902 +0200
@@ -1,8 +1,7 @@
 /* { dg-do compile } */
 /* { dg-require-effective-target vect_int } */
-/* { dg-require-effective-target sse { target i386*-*-* } } */
 /* { dg-options "-O2" } */
-/* { dg-additional-options "-msse" { target i386*-*-* } } */
+/* { dg-additional-options "-msse" { target i?86-*-* x86_64-*-* } } */
 
 typedef int v4si
   __attribute__ ((vector_size (16)));


	Jakub
Tamar Christina July 7, 2018, 11:07 a.m. | #2
Hi Jakub,

> 
> On Fri, Jul 06, 2018 at 11:46:43AM +0100, Tamar Christina wrote:
> > This fixes an ABI warning generated on i686-pc-linux-gnu when using
> > `vector_size` with no sse enabled explicitly.
> >
> > Regtested single test on x86_64-pc-linux-gnu with -m32 and no issues.
> >
> > Committed under the GCC obvious rule.
> 
> That is insufficient, I get the FAIL on i686-linux.
> You don't really need dg-require-effective-target sse, it is a dg-do compile
> time only test and on i?86/x86_64 failures with old assemblers would show up
> only when assembling.
> But -msse really should be used on all i?86-*-* and x86_64-*-*.

Ah I wasn't are of that, I initially didn't add it to x86-64 since the test wasn't generating the ABI warning there. But this is good to know for the future.

> 
> You'd normally get the -msse etc. automatically, but dg-options
> is undesirable in gcc.dg/vect/ tests where all those predefined options
> are lost that way, dg-additional-options should be used instead (and you can
> say use there -O2 -fno-tree-vectorize or whatever you want).
> 
> So, if you have spare cycles, please test such change whether it still FAILs
> on arm with your patch reverted after such test changes.

Will do. Sorry for the mess, not very use to the x86 options yet.

Thanks,
Tamar

> 
> > gcc/testsuite/
> > 2018-07-06  Tamar Christina  <tamar.christina@arm.com>
> >
> >       PR target/84711
> >       * gcc.dg/vect/pr84711.c: Add -msse for i686 targets.
> 
> In the meantime, I've committed following fix as obvious:
> 
> 2018-07-07  Jakub Jelinek  <jakub@redhat.com>
> 
>         PR target/84711
>         * gcc.dg/vect/pr84711.c: Remove unnecessary sse
>         dg-require-effective-target.  Add -msse not just on i386-*, but
>         on all i?86-* and x86_64-*.
> 
> --- gcc/testsuite/gcc.dg/vect/pr84711.c.jj      2018-07-06 23:35:44.952791972 +0200
> +++ gcc/testsuite/gcc.dg/vect/pr84711.c 2018-07-07 09:43:27.068785902 +0200
> @@ -1,8 +1,7 @@
>  /* { dg-do compile } */
>  /* { dg-require-effective-target vect_int } */
> -/* { dg-require-effective-target sse { target i386*-*-* } } */
>  /* { dg-options "-O2" } */
> -/* { dg-additional-options "-msse" { target i386*-*-* } } */
> +/* { dg-additional-options "-msse" { target i?86-*-* x86_64-*-* } } */
> 
>  typedef int v4si
>    __attribute__ ((vector_size (16)));
> 
> 
>         Jakub
>
Jakub Jelinek July 7, 2018, 11:14 a.m. | #3
On Sat, Jul 07, 2018 at 11:07:28AM +0000, Tamar Christina wrote:
> > On Fri, Jul 06, 2018 at 11:46:43AM +0100, Tamar Christina wrote:
> > > This fixes an ABI warning generated on i686-pc-linux-gnu when using
> > > `vector_size` with no sse enabled explicitly.
> > >
> > > Regtested single test on x86_64-pc-linux-gnu with -m32 and no issues.
> > >
> > > Committed under the GCC obvious rule.
> > 
> > That is insufficient, I get the FAIL on i686-linux.
> > You don't really need dg-require-effective-target sse, it is a dg-do compile
> > time only test and on i?86/x86_64 failures with old assemblers would show up
> > only when assembling.
> > But -msse really should be used on all i?86-*-* and x86_64-*-*.
> 
> Ah I wasn't are of that, I initially didn't add it to x86-64 since the
> test wasn't generating the ABI warning there.  But this is good to know
> for the future.

The thing is that i?86-*-* and x86_64-*-* can be both multilib, and only the
-m64 and -mx32 multilibs default to -msse2, -m32 does not.
In gcc.target/ia32 one uses typically ia32 effective target for the -m32
i?86/x86_64 multilib, but in this case it doesn't hurt to add -msse to all.

	Jakub

Patch

diff --git a/gcc/testsuite/gcc.dg/vect/pr84711.c b/gcc/testsuite/gcc.dg/vect/pr84711.c
index dbe61bef0dc35cdbcacf7a9217951888b64a99f1..87763aaabed22a043680223b466b3f43ca70744e 100644
--- a/gcc/testsuite/gcc.dg/vect/pr84711.c
+++ b/gcc/testsuite/gcc.dg/vect/pr84711.c
@@ -1,6 +1,8 @@ 
 /* { dg-do compile } */
 /* { dg-require-effective-target vect_int } */
+/* { dg-require-effective-target sse { target i386*-*-* } } */
 /* { dg-options "-O2" } */
+/* { dg-additional-options "-msse" { target i386*-*-* } } */
 
 typedef int v4si
   __attribute__ ((vector_size (16)));