On Tue, Apr 16, 2019 at 10:07 AM Jakub Jelinek <ja...@redhat.com> wrote:
>
> Hi!
>
> Most of the x86 builtins don't have OPTION_MASK_ISA_64BIT in their bisa
> mask, but when printing an error if some builtin is used from a function
> without right isa flags, we always print that case as -m32, so we end up
> with bogus messages like
> needs isa option -m32 -mavx512vl
> It doesn't really need -m32, it only needs -mavx512vl.
> A couple of builtins have OPTION_MASK_ISA_64BIT set, those actually require
> (either solely or perhaps with some other option) -m64 or -mx32 option.
>
> The following patch adjusts this diagnostics, so that it never prints -m32
> in the needs isa option list. For the builtins that do require
> OPTION_MASK_ISA_64BIT, I believe the usual case is that for -m32 such
> builtins are not registered as builtins at all, and when building with -m64
> we print it requires e.g. -m64 -mavx512f (I think it is unnecessary to write
> something like ( -m64 or -mx32 ) and -mavx512f) and when building with -mx32
> it prints -mx32 -mavx512f.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2019-04-16 Jakub Jelinek <ja...@redhat.com>
>
> PR target/90096
> * config/i386/i386.c (ix86_target_string): Add ADD_ABI_P argument,
> only
> print -m64/-mx32/-m32 if it is true.
> (ix86_debug_options, ix86_function_specific_print): Pass true as
> ADD_ABI_P to ix86_target_string.
> (ix86_expand_builtin): Adjust ix86_target_string caller, pass true as
> ADD_ABI_P only if OPTION_MASK_ISA_64BIT is set in bisa and in that
> case
> or into it OPTION_MASK_ISA_ABI_64 or OPTION_MASK_ISA_ABI_X32.
>
> * gcc.target/i386/pr90096.c: New test.
> * gcc.target/i386/pr69255-1.c: Adjust expected diagnostics.
> * gcc.target/i386/pr69255-2.c: Likewise.
> * gcc.target/i386/pr69255-3.c: Likewise.
OK.
Thanks,
Uros.
> --- gcc/config/i386/i386.c.jj 2019-04-08 10:11:33.000000000 +0200
> +++ gcc/config/i386/i386.c 2019-04-15 18:35:19.934878176 +0200
> @@ -838,7 +838,7 @@ enum ix86_function_specific_strings
>
> static char *ix86_target_string (HOST_WIDE_INT, HOST_WIDE_INT, int, int,
> const char *, const char *, enum fpmath_unit,
> - bool);
> + bool, bool);
> static void ix86_function_specific_save (struct cl_target_option *,
> struct gcc_options *opts);
> static void ix86_function_specific_restore (struct gcc_options *opts,
> @@ -2928,7 +2928,7 @@ static char *
> ix86_target_string (HOST_WIDE_INT isa, HOST_WIDE_INT isa2,
> int flags, int flags2,
> const char *arch, const char *tune,
> - enum fpmath_unit fpmath, bool add_nl_p)
> + enum fpmath_unit fpmath, bool add_nl_p, bool add_abi_p)
> {
> struct ix86_target_opts
> {
> @@ -3095,19 +3095,20 @@ ix86_target_string (HOST_WIDE_INT isa, H
> }
>
> /* Add -m32/-m64/-mx32. */
> - if ((isa & OPTION_MASK_ISA_64BIT) != 0)
> + if (add_abi_p)
> {
> - if ((isa & OPTION_MASK_ABI_64) != 0)
> - abi = "-m64";
> + if ((isa & OPTION_MASK_ISA_64BIT) != 0)
> + {
> + if ((isa & OPTION_MASK_ABI_64) != 0)
> + abi = "-m64";
> + else
> + abi = "-mx32";
> + }
> else
> - abi = "-mx32";
> - isa &= ~ (OPTION_MASK_ISA_64BIT
> - | OPTION_MASK_ABI_64
> - | OPTION_MASK_ABI_X32);
> + abi = "-m32";
> + opts[num++][0] = abi;
> }
> - else
> - abi = "-m32";
> - opts[num++][0] = abi;
> + isa &= ~(OPTION_MASK_ISA_64BIT | OPTION_MASK_ABI_64 | OPTION_MASK_ABI_X32);
>
> /* Pick out the options in isa2 options. */
> for (i = 0; i < ARRAY_SIZE (isa2_opts); i++)
> @@ -3269,7 +3270,7 @@ ix86_debug_options (void)
> char *opts = ix86_target_string (ix86_isa_flags, ix86_isa_flags2,
> target_flags, ix86_target_flags,
> ix86_arch_string,ix86_tune_string,
> - ix86_fpmath, true);
> + ix86_fpmath, true, true);
>
> if (opts)
> {
> @@ -5121,7 +5122,7 @@ ix86_function_specific_print (FILE *file
> char *target_string
> = ix86_target_string (ptr->x_ix86_isa_flags, ptr->x_ix86_isa_flags2,
> ptr->x_target_flags, ptr->x_ix86_target_flags,
> - NULL, NULL, ptr->x_ix86_fpmath, false);
> + NULL, NULL, ptr->x_ix86_fpmath, false, true);
>
> gcc_assert (ptr->arch < PROCESSOR_max);
> fprintf (file, "%*sarch = %d (%s)\n",
> @@ -36709,8 +36710,13 @@ ix86_expand_builtin (tree exp, rtx targe
> isa |= (OPTION_MASK_ISA_FMA | OPTION_MASK_ISA_FMA4);
> if ((bisa & isa) != bisa || (bisa2 & isa2) != bisa2)
> {
> + bool add_abi_p = bisa & OPTION_MASK_ISA_64BIT;
> + if (TARGET_ABI_X32)
> + bisa |= OPTION_MASK_ABI_X32;
> + else
> + bisa |= OPTION_MASK_ABI_64;
> char *opts = ix86_target_string (bisa, bisa2, 0, 0, NULL, NULL,
> - (enum fpmath_unit) 0, false);
> + (enum fpmath_unit) 0, false,
> add_abi_p);
> if (!opts)
> error ("%qE needs unknown isa option", fndecl);
> else
> --- gcc/testsuite/gcc.target/i386/pr90096.c.jj 2019-04-15 18:46:27.838965628
> +0200
> +++ gcc/testsuite/gcc.target/i386/pr90096.c 2019-04-15 18:56:51.327806993
> +0200
> @@ -0,0 +1,24 @@
> +/* PR target/90096 */
> +/* { dg-do compile } */
> +/* { dg-options "-O0 -mno-gfni -mno-avx512f -Wno-psabi" } */
> +
> +#include <x86intrin.h>
> +
> +volatile __m512i x1, x2;
> +volatile __mmask64 m64;
> +
> +void
> +foo (int i)
> +{
> + x1 = _mm512_gf2p8affineinv_epi64_epi8 (x1, x2, 3); /* { dg-error "needs
> isa option -mgfni -mavx512f" } */
> +}
> +
> +#ifdef __x86_64__
> +unsigned long long
> +bar (__m128 *p)
> +{
> + return _mm_cvtt_roundss_u64 (*p, _MM_FROUND_TO_ZERO |_MM_FROUND_NO_EXC);
> + /* { dg-error "needs isa option -m64 -mavx512f" "" { target lp64 } .-1 } */
> + /* { dg-error "needs isa option -mx32 -mavx512f" "" { target x32 } .-1 } */
> +}
> +#endif
> --- gcc/testsuite/gcc.target/i386/pr69255-1.c.jj 2017-05-05
> 09:19:48.886720160 +0200
> +++ gcc/testsuite/gcc.target/i386/pr69255-1.c 2019-04-15 18:46:49.461612349
> +0200
> @@ -12,7 +12,7 @@ __attribute__ ((__vector_size__ (16))) i
> void
> foo (const long long *p)
> {
> - a = __builtin_ia32_gather3siv4di (a, p, b, 1, 1); /* { dg-error "needs
> isa option -m32 -mavx512vl" } */
> + a = __builtin_ia32_gather3siv4di (a, p, b, 1, 1); /* { dg-error "needs
> isa option -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 } */
> }
> --- gcc/testsuite/gcc.target/i386/pr69255-2.c.jj 2017-07-04
> 10:44:03.397540963 +0200
> +++ gcc/testsuite/gcc.target/i386/pr69255-2.c 2019-04-15 18:47:26.015015117
> +0200
> @@ -13,7 +13,7 @@ void
> foo (const long long *p)
> {
> volatile __attribute__ ((__vector_size__ (32))) long long c;
> - c = __builtin_ia32_gather3siv4di (a, p, b, 1, 1); /* { dg-error
> "needs isa option -m32 -mavx512vl" } */
> + c = __builtin_ia32_gather3siv4di (a, p, b, 1, 1); /* { dg-error
> "needs isa option -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 } */
> }
> --- gcc/testsuite/gcc.target/i386/pr69255-3.c.jj 2017-05-05
> 09:19:48.915719780 +0200
> +++ gcc/testsuite/gcc.target/i386/pr69255-3.c 2019-04-15 18:47:36.066850891
> +0200
> @@ -12,7 +12,7 @@ __attribute__ ((__vector_size__ (16))) i
> 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" } */
> + *q = __builtin_ia32_gather3siv4di (a, p, b, 1, 1); /* { dg-error "needs
> isa option -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
