diff mbox

Fix PR81175, make gather builtins pure

Message ID 20170627102725.GR2123@tucnak
State New
Headers show

Commit Message

Jakub Jelinek June 27, 2017, 10:27 a.m. UTC
On Tue, Jun 27, 2017 at 12:08:59PM +0200, Uros Bizjak wrote:
> > Ok for trunk if it passes bootstrap/regtest?
> >
> > 2017-06-27  Jakub Jelinek  <jakub@redhat.com>
> >
> >         PR target/81175
> >         * config/i386/i386.c (ix86_init_mmx_sse_builtins): Use def_builtin
> >         rather than def_builtin_pure for __builtin_ia32_gatherpf*.
> 
> Looks obvious to me, especially after the above explanation.

Thanks.  Apparently the same commit regressed another testcase, where we
want to check the expansion of the gather builtin without the right ISA
enabled, but that doesn't trigger anymore because it is optimized away.

Fixed thusly, ok for trunk?  Perhaps we should add another testcase to check
similarly gatherpf builtin without the lhs, but we'd need different options.

2017-06-27  Jakub Jelinek  <jakub@redhat.com>

	PR target/81175
	* gcc.target/i386/pr69255-2.c (foo): Use the return value of the
	gather.



	Jakub

Comments

Jakub Jelinek July 4, 2017, 8:35 a.m. UTC | #1
Hi!

On Tue, Jun 27, 2017 at 12:27:25PM +0200, Jakub Jelinek wrote:
> Fixed thusly, ok for trunk?  Perhaps we should add another testcase to check
> similarly gatherpf builtin without the lhs, but we'd need different options.

I'd like to ping this patch, ok for trunk?

> 2017-06-27  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR target/81175
> 	* gcc.target/i386/pr69255-2.c (foo): Use the return value of the
> 	gather.
> 
> --- gcc/testsuite/gcc.target/i386/pr69255-2.c.jj	2017-05-05 09:19:48.000000000 +0200
> +++ gcc/testsuite/gcc.target/i386/pr69255-2.c	2017-06-27 12:20:31.697944761 +0200
> @@ -12,7 +12,8 @@ __attribute__ ((__vector_size__ (16))) i
>  void
>  foo (const long long *p)
>  {
> -  __builtin_ia32_gather3siv4di (a, p, b, 1, 1);		/* { dg-error "needs isa option -m32 -mavx512vl" } */
> +  volatile __attribute__ ((__vector_size__ (32))) long long c;
> +  c = __builtin_ia32_gather3siv4di (a, p, b, 1, 1);		/* { dg-error "needs isa option -m32 -mavx512vl" } */
>    /* { dg-warning "AVX vector return without AVX enabled changes the ABI" "" { target *-*-* } .-1 } */
>    /* { dg-warning "AVX vector argument without AVX enabled changes the ABI" "" { target *-*-* } .-2 } */
>  }

	Jakub
Uros Bizjak July 4, 2017, 8:38 a.m. UTC | #2
On Tue, Jul 4, 2017 at 10:35 AM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> On Tue, Jun 27, 2017 at 12:27:25PM +0200, Jakub Jelinek wrote:
>> Fixed thusly, ok for trunk?  Perhaps we should add another testcase to check
>> similarly gatherpf builtin without the lhs, but we'd need different options.
>
> I'd like to ping this patch, ok for trunk?
>
>> 2017-06-27  Jakub Jelinek  <jakub@redhat.com>
>>
>>       PR target/81175
>>       * gcc.target/i386/pr69255-2.c (foo): Use the return value of the
>>       gather.

OK.

Thanks,
Uros.

>> --- gcc/testsuite/gcc.target/i386/pr69255-2.c.jj      2017-05-05 09:19:48.000000000 +0200
>> +++ gcc/testsuite/gcc.target/i386/pr69255-2.c 2017-06-27 12:20:31.697944761 +0200
>> @@ -12,7 +12,8 @@ __attribute__ ((__vector_size__ (16))) i
>>  void
>>  foo (const long long *p)
>>  {
>> -  __builtin_ia32_gather3siv4di (a, p, b, 1, 1);              /* { dg-error "needs isa option -m32 -mavx512vl" } */
>> +  volatile __attribute__ ((__vector_size__ (32))) long long c;
>> +  c = __builtin_ia32_gather3siv4di (a, p, b, 1, 1);          /* { dg-error "needs isa option -m32 -mavx512vl" } */
>>    /* { dg-warning "AVX vector return without AVX enabled changes the ABI" "" { target *-*-* } .-1 } */
>>    /* { dg-warning "AVX vector argument without AVX enabled changes the ABI" "" { target *-*-* } .-2 } */
>>  }
>
>         Jakub
Richard Biener July 4, 2017, 8:45 a.m. UTC | #3
On Tue, 4 Jul 2017, Jakub Jelinek wrote:

> Hi!
> 
> On Tue, Jun 27, 2017 at 12:27:25PM +0200, Jakub Jelinek wrote:
> > Fixed thusly, ok for trunk?  Perhaps we should add another testcase to check
> > similarly gatherpf builtin without the lhs, but we'd need different options.
> 
> I'd like to ping this patch, ok for trunk?

Ok (I thought it was quite obvious).

Thanks,
Richard.

> > 2017-06-27  Jakub Jelinek  <jakub@redhat.com>
> > 
> > 	PR target/81175
> > 	* gcc.target/i386/pr69255-2.c (foo): Use the return value of the
> > 	gather.
> > 
> > --- gcc/testsuite/gcc.target/i386/pr69255-2.c.jj	2017-05-05 09:19:48.000000000 +0200
> > +++ gcc/testsuite/gcc.target/i386/pr69255-2.c	2017-06-27 12:20:31.697944761 +0200
> > @@ -12,7 +12,8 @@ __attribute__ ((__vector_size__ (16))) i
> >  void
> >  foo (const long long *p)
> >  {
> > -  __builtin_ia32_gather3siv4di (a, p, b, 1, 1);		/* { dg-error "needs isa option -m32 -mavx512vl" } */
> > +  volatile __attribute__ ((__vector_size__ (32))) long long c;
> > +  c = __builtin_ia32_gather3siv4di (a, p, b, 1, 1);		/* { dg-error "needs isa option -m32 -mavx512vl" } */
> >    /* { dg-warning "AVX vector return without AVX enabled changes the ABI" "" { target *-*-* } .-1 } */
> >    /* { dg-warning "AVX vector argument without AVX enabled changes the ABI" "" { target *-*-* } .-2 } */
> >  }
diff mbox

Patch

--- gcc/testsuite/gcc.target/i386/pr69255-2.c.jj	2017-05-05 09:19:48.000000000 +0200
+++ gcc/testsuite/gcc.target/i386/pr69255-2.c	2017-06-27 12:20:31.697944761 +0200
@@ -12,7 +12,8 @@  __attribute__ ((__vector_size__ (16))) i
 void
 foo (const long long *p)
 {
-  __builtin_ia32_gather3siv4di (a, p, b, 1, 1);		/* { dg-error "needs isa option -m32 -mavx512vl" } */
+  volatile __attribute__ ((__vector_size__ (32))) long long c;
+  c = __builtin_ia32_gather3siv4di (a, p, b, 1, 1);		/* { dg-error "needs isa option -m32 -mavx512vl" } */
   /* { dg-warning "AVX vector return without AVX enabled changes the ABI" "" { target *-*-* } .-1 } */
   /* { dg-warning "AVX vector argument without AVX enabled changes the ABI" "" { target *-*-* } .-2 } */
 }