Re: [PATCH] i386: Guard 128 bit VAES builtins with AVX512VL

[Hongtao Liu <crazylht@gmail.com>]

On Tue, Jul 11, 2023 at 11:40 AM Haochen Jiang via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Hi all,
>
> Currently on trunk, both usage of intrin and builtin for 128 bit VAES
> ISA will result in ICE since we did not check AVX512VL until pattern,
> which is not user expected. This patch aims to fix that ICE and throw
> an error under this scenario.
>
> Regtested on x86-64-linux-gnu{-m32,}. Ok for trunk?
>
Ok.
> BRs,
> Haochen
>
> Since commit 24a8acc, 128 bit intrin is enabled for VAES. However,
> AVX512VL is not checked until we reached into pattern, which reports an
> ICE.
>
> Added an AVX512VL guard at builtin to report error when checking ISA
> flags.
>
> gcc/ChangeLog:
>
>         * config/i386/i386-builtins.cc (ix86_init_mmx_sse_builtins):
>         Add OPTION_MASK_ISA_AVX512VL.
>         * config/i386/i386-expand.cc (ix86_check_builtin_isa_match):
>         Ditto.
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.target/i386/avx512vl-vaes-1.c: New test.
> ---
>  gcc/config/i386/i386-builtins.cc                | 12 ++++++++----
>  gcc/config/i386/i386-expand.cc                  |  4 +++-
>  gcc/testsuite/gcc.target/i386/avx512vl-vaes-1.c | 12 ++++++++++++
>  3 files changed, 23 insertions(+), 5 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/i386/avx512vl-vaes-1.c
>
> diff --git a/gcc/config/i386/i386-builtins.cc b/gcc/config/i386/i386-builtins.cc
> index 28f404da288..e436ca4e5b1 100644
> --- a/gcc/config/i386/i386-builtins.cc
> +++ b/gcc/config/i386/i386-builtins.cc
> @@ -662,19 +662,23 @@ ix86_init_mmx_sse_builtins (void)
>                VOID_FTYPE_UNSIGNED_UNSIGNED, IX86_BUILTIN_MWAIT);
>
>    /* AES */
> -  def_builtin_const (OPTION_MASK_ISA_AES | OPTION_MASK_ISA_SSE2,
> +  def_builtin_const (OPTION_MASK_ISA_AES | OPTION_MASK_ISA_SSE2
> +                    | OPTION_MASK_ISA_AVX512VL,
>                      OPTION_MASK_ISA2_VAES,
>                      "__builtin_ia32_aesenc128",
>                      V2DI_FTYPE_V2DI_V2DI, IX86_BUILTIN_AESENC128);
> -  def_builtin_const (OPTION_MASK_ISA_AES | OPTION_MASK_ISA_SSE2,
> +  def_builtin_const (OPTION_MASK_ISA_AES | OPTION_MASK_ISA_SSE2
> +                    | OPTION_MASK_ISA_AVX512VL,
>                      OPTION_MASK_ISA2_VAES,
>                      "__builtin_ia32_aesenclast128",
>                      V2DI_FTYPE_V2DI_V2DI, IX86_BUILTIN_AESENCLAST128);
> -  def_builtin_const (OPTION_MASK_ISA_AES | OPTION_MASK_ISA_SSE2,
> +  def_builtin_const (OPTION_MASK_ISA_AES | OPTION_MASK_ISA_SSE2
> +                    | OPTION_MASK_ISA_AVX512VL,
>                      OPTION_MASK_ISA2_VAES,
>                      "__builtin_ia32_aesdec128",
>                      V2DI_FTYPE_V2DI_V2DI, IX86_BUILTIN_AESDEC128);
> -  def_builtin_const (OPTION_MASK_ISA_AES | OPTION_MASK_ISA_SSE2,
> +  def_builtin_const (OPTION_MASK_ISA_AES | OPTION_MASK_ISA_SSE2
> +                    | OPTION_MASK_ISA_AVX512VL,
>                      OPTION_MASK_ISA2_VAES,
>                      "__builtin_ia32_aesdeclast128",
>                      V2DI_FTYPE_V2DI_V2DI, IX86_BUILTIN_AESDECLAST128);
> diff --git a/gcc/config/i386/i386-expand.cc b/gcc/config/i386/i386-expand.cc
> index 567248d6830..9a04bf4455b 100644
> --- a/gcc/config/i386/i386-expand.cc
> +++ b/gcc/config/i386/i386-expand.cc
> @@ -12626,6 +12626,7 @@ ix86_check_builtin_isa_match (unsigned int fcode,
>         OPTION_MASK_ISA2_AVXIFMA
>       (OPTION_MASK_ISA_AVX512VL | OPTION_MASK_ISA2_AVX512BF16) or
>         OPTION_MASK_ISA2_AVXNECONVERT
> +     OPTION_MASK_ISA_AES or (OPTION_MASK_ISA_AVX512VL | OPTION_MASK_ISA2_VAES)
>       where for each such pair it is sufficient if either of the ISAs is
>       enabled, plus if it is ored with other options also those others.
>       OPTION_MASK_ISA_MMX in bisa is satisfied also if TARGET_MMX_WITH_SSE.  */
> @@ -12649,7 +12650,8 @@ ix86_check_builtin_isa_match (unsigned int fcode,
>                  OPTION_MASK_ISA2_AVXIFMA);
>    SHARE_BUILTIN (OPTION_MASK_ISA_AVX512VL, OPTION_MASK_ISA2_AVX512BF16, 0,
>                  OPTION_MASK_ISA2_AVXNECONVERT);
> -  SHARE_BUILTIN (OPTION_MASK_ISA_AES, 0, 0, OPTION_MASK_ISA2_VAES);
> +  SHARE_BUILTIN (OPTION_MASK_ISA_AES, 0, OPTION_MASK_ISA_AVX512VL,
> +                OPTION_MASK_ISA2_VAES);
>    isa = tmp_isa;
>    isa2 = tmp_isa2;
>
> diff --git a/gcc/testsuite/gcc.target/i386/avx512vl-vaes-1.c b/gcc/testsuite/gcc.target/i386/avx512vl-vaes-1.c
> new file mode 100644
> index 00000000000..fabb170a031
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/i386/avx512vl-vaes-1.c
> @@ -0,0 +1,12 @@
> +/* { dg-do compile } */
> +/* { dg-options "-mvaes -mno-avx512vl -mno-aes" } */
> +
> +#include <immintrin.h>
> +
> +typedef long long v2di __attribute__((vector_size (16)));
> +
> +v2di
> +f1 (v2di x, v2di y)
> +{
> +  return __builtin_ia32_aesenc128 (x, y); /* { dg-error "needs isa option" } */
> +}
> --
> 2.31.1
>


-- 
BR,
Hongtao