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

Hi!

As the testcase shows, if we want to diagnose a md builtin not enabled in
the current ISA, we call error and then return const0_rtx.  That isn't a
good choice if the result is BLKmode, which can happen for vector modes
that aren't enabled in the current ISA.  In that case, returning target
is better.  In the PR I've also mentioned DECL_MODE issues, but looking at
expr.c, I see there:
      /* DECL_MODE might change when TYPE_MODE depends on attribute target
         settings for VECTOR_TYPE_P that might switch for the function.  */
      if (currently_expanding_to_rtl
          && code == VAR_DECL && MEM_P (decl_rtl)
          && VECTOR_TYPE_P (type) && exp && DECL_MODE (exp) != mode)
        decl_rtl = change_address (decl_rtl, TYPE_MODE (type), 0);
      else
        decl_rtl = copy_rtx (decl_rtl);
so we should be fine.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

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, return target if non-NULL, instead of
	const0_rtx.

	* 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 Mon, Sep 5, 2016 at 7:14 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> As the testcase shows, if we want to diagnose a md builtin not enabled in
> the current ISA, we call error and then return const0_rtx.  That isn't a
> good choice if the result is BLKmode, which can happen for vector modes
> that aren't enabled in the current ISA.  In that case, returning target
> is better.  In the PR I've also mentioned DECL_MODE issues, but looking at
> expr.c, I see there:
>       /* DECL_MODE might change when TYPE_MODE depends on attribute target
>          settings for VECTOR_TYPE_P that might switch for the function.  */
>       if (currently_expanding_to_rtl
>           && code == VAR_DECL && MEM_P (decl_rtl)
>           && VECTOR_TYPE_P (type) && exp && DECL_MODE (exp) != mode)
>         decl_rtl = change_address (decl_rtl, TYPE_MODE (type), 0);
>       else
>         decl_rtl = copy_rtx (decl_rtl);
> so we should be fine.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Hm...

... 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();
   ^~~~~~~~~~~~~~~~~~~~~~

Uros.

> 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, return target if non-NULL, instead of
>         const0_rtx.
>
>         * gcc.target/i386/pr69255-1.c: New test.
>         * gcc.target/i386/pr69255-2.c: New test.
>         * gcc.target/i386/pr69255-3.c: New test.
>
> --- gcc/config/i386/i386.c.jj   2016-09-02 18:18:10.000000000 +0200
> +++ gcc/config/i386/i386.c      2016-09-05 12:41:03.341382125 +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 target ? target : const0_rtx;
>      }
>
>    switch (fcode)
> --- gcc/testsuite/gcc.target/i386/pr69255-1.c.jj        2016-09-05 12:50:29.455307440 +0200
> +++ gcc/testsuite/gcc.target/i386/pr69255-1.c   2016-09-05 12:50:06.000000000 +0200
> @@ -0,0 +1,16 @@
> +/* 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 } */
> --- gcc/testsuite/gcc.target/i386/pr69255-2.c.jj        2016-09-05 12:50:32.931264038 +0200
> +++ gcc/testsuite/gcc.target/i386/pr69255-2.c   2016-09-05 12:49:57.000000000 +0200
> @@ -0,0 +1,14 @@
> +/* 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" } */
> +}
> --- gcc/testsuite/gcc.target/i386/pr69255-3.c.jj        2016-09-05 12:50:35.951226330 +0200
> +++ gcc/testsuite/gcc.target/i386/pr69255-3.c   2016-09-05 12:49:13.000000000 +0200
> @@ -0,0 +1,16 @@
> +/* 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 } */
>
>         Jakub