[i386] Fix ICE with a md builtin call (PR target/69255)

On Mon, Sep 05, 2016 at 08:58:12PM +0200, Uros Bizjak wrote:
> ... perhaps we can emit a warning here and expand the builtin as a
> call? This way, we will mirror the case when builtin requires some
> ISA, e.g.:
> 
> void foo ()
> {
>   __builtin_ia32_stmxcsr();
> }
> 
> $ gcc -S -mno-sse dd.c
> dd.c: In function ‘foo’:
> dd.c:3:3: warning: implicit declaration of function
> ‘__builtin_ia32_stmxcsr’ [-Wimplicit-function-declaration]
>    __builtin_ia32_stmxcsr();
>    ^~~~~~~~~~~~~~~~~~~~~~

I'm not sure it is a good idea to emit a warning, we are in the backend
and we don't know what kind of warning/error the FE would emit if the
builtin isn't implicitly prototyped.  E.g. C++ would certainly error out,
C with -pedantic-errors would too, etc., it is simply too hard to replicate
such behavior this late.

Using expand_call works too (only tested on the testcases so far, but can test
it fully), just had to add some expected warnings for the return and
argument type -Wpsabi stuff.

2016-09-05  Jakub Jelinek  <jakub@redhat.com>

	PR target/69255
	* config/i386/i386.c (ix86_expand_builtin): For builtin with
	unsupported or unknown ISA, use expand_call.

	* gcc.target/i386/pr69255-1.c: New test.
	* gcc.target/i386/pr69255-2.c: New test.
	* gcc.target/i386/pr69255-3.c: New test.



	Jakub

On Tue, Sep 6, 2016 at 5:06 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Mon, Sep 05, 2016 at 08:58:12PM +0200, Uros Bizjak wrote:
>> ... perhaps we can emit a warning here and expand the builtin as a
>> call? This way, we will mirror the case when builtin requires some
>> ISA, e.g.:
>>
>> void foo ()
>> {
>>   __builtin_ia32_stmxcsr();
>> }
>>
>> $ gcc -S -mno-sse dd.c
>> dd.c: In function ‘foo’:
>> dd.c:3:3: warning: implicit declaration of function
>> ‘__builtin_ia32_stmxcsr’ [-Wimplicit-function-declaration]
>>    __builtin_ia32_stmxcsr();
>>    ^~~~~~~~~~~~~~~~~~~~~~
>
> I'm not sure it is a good idea to emit a warning, we are in the backend
> and we don't know what kind of warning/error the FE would emit if the
> builtin isn't implicitly prototyped.  E.g. C++ would certainly error out,
> C with -pedantic-errors would too, etc., it is simply too hard to replicate
> such behavior this late.

OK.

> Using expand_call works too (only tested on the testcases so far, but can test
> it fully), just had to add some expected warnings for the return and
> argument type -Wpsabi stuff.

Yes, this is what compiling testcase without using #pragmas warns.

> 2016-09-05  Jakub Jelinek  <jakub@redhat.com>
>
>         PR target/69255
>         * config/i386/i386.c (ix86_expand_builtin): For builtin with
>         unsupported or unknown ISA, use expand_call.
>
>         * gcc.target/i386/pr69255-1.c: New test.
>         * gcc.target/i386/pr69255-2.c: New test.
>         * gcc.target/i386/pr69255-3.c: New test.

OK for mainline and release branches.

Thanks,
Uros.

> --- gcc/config/i386/i386.c.jj   2016-09-05 19:27:03.287671306 +0200
> +++ gcc/config/i386/i386.c      2016-09-06 16:55:35.524605779 +0200
> @@ -36107,7 +36107,7 @@ ix86_expand_builtin (tree exp, rtx targe
>           error ("%qE needs isa option %s", fndecl, opts);
>           free (opts);
>         }
> -      return const0_rtx;
> +      return expand_call (exp, target, ignore);
>      }
>
>    switch (fcode)
> --- gcc/testsuite/gcc.target/i386/pr69255-1.c.jj        2016-09-06 16:54:01.793783159 +0200
> +++ gcc/testsuite/gcc.target/i386/pr69255-1.c   2016-09-06 16:59:53.317370510 +0200
> @@ -0,0 +1,17 @@
> +/* PR target/69255 */
> +/* { dg-do compile } */
> +/* { dg-options "-mno-avx512f" } */
> +
> +#pragma GCC target "avx512vl"
> +#pragma GCC target "no-avx512vl"
> +__attribute__ ((__vector_size__ (32))) long long a;
> +__attribute__ ((__vector_size__ (16))) int b;
> +
> +void
> +foo (const long long *p)
> +{
> +  a = __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 *-*-* } 13 } */
> +/* { dg-warning "AVX vector argument without AVX enabled changes the ABI" "" { target *-*-* } 13 } */
> --- gcc/testsuite/gcc.target/i386/pr69255-2.c.jj        2016-09-06 16:54:01.793783159 +0200
> +++ gcc/testsuite/gcc.target/i386/pr69255-2.c   2016-09-06 17:00:04.229233625 +0200
> @@ -0,0 +1,17 @@
> +/* PR target/69255 */
> +/* { dg-do compile } */
> +/* { dg-options "-mno-avx512f" } */
> +
> +#pragma GCC target "avx512vl"
> +#pragma GCC target ""
> +__attribute__ ((__vector_size__ (32))) long long a;
> +__attribute__ ((__vector_size__ (16))) int b;
> +
> +void
> +foo (const long long *p)
> +{
> +  __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 *-*-* } 13 } */
> +/* { dg-warning "AVX vector argument without AVX enabled changes the ABI" "" { target *-*-* } 13 } */
> --- gcc/testsuite/gcc.target/i386/pr69255-3.c.jj        2016-09-06 16:54:01.794783146 +0200
> +++ gcc/testsuite/gcc.target/i386/pr69255-3.c   2016-09-06 17:00:14.915099574 +0200
> @@ -0,0 +1,17 @@
> +/* PR target/69255 */
> +/* { dg-do compile } */
> +/* { dg-options "-mno-avx512f" } */
> +
> +#pragma GCC target "avx512vl"
> +#pragma GCC target ""
> +__attribute__ ((__vector_size__ (32))) long long a;
> +__attribute__ ((__vector_size__ (16))) int b;
> +
> +void
> +foo (const long long *p, __attribute__ ((__vector_size__ (32))) long long *q)
> +{
> +  *q = __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 *-*-* } 13 } */
> +/* { dg-warning "AVX vector argument without AVX enabled changes the ABI" "" { target *-*-* } 13 } */
>
>
>         Jakub