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

[Uros Bizjak <ubizjak at gmail dot com>]

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