[PATCH] Add support for missing AVX512* ISAs (PR target/89929).

[Martin Liška]

On 4/11/19 6:30 PM, H.J. Lu wrote:
> On Thu, Apr 11, 2019 at 1:38 AM Martin Liška <mliska@suse.cz> wrote:
>>
>> Hi.
>>
>> The patch is adding missing AVX512 ISAs for target and target_clone
>> attributes.
>>
>> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
>>
>> Ready to be installed?
>> Thanks,
>> Martin
>>
>> gcc/ChangeLog:
>>
>> 2019-04-10  Martin Liska  <mliska@suse.cz>
>>
>>         PR target/89929
>>         * config/i386/i386.c (get_builtin_code_for_version): Add
>>         support for missing AVX512 ISAs.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 2019-04-10  Martin Liska  <mliska@suse.cz>
>>
>>         PR target/89929
>>         * g++.target/i386/mv28.C: New test.
>>         * gcc.target/i386/mvc14.c: New test.
>> ---
>>  gcc/config/i386/i386.c                | 34 ++++++++++++++++++++++++++-
>>  gcc/testsuite/g++.target/i386/mv28.C  | 30 +++++++++++++++++++++++
>>  gcc/testsuite/gcc.target/i386/mvc14.c | 16 +++++++++++++
>>  3 files changed, 79 insertions(+), 1 deletion(-)
>>  create mode 100644 gcc/testsuite/g++.target/i386/mv28.C
>>  create mode 100644 gcc/testsuite/gcc.target/i386/mvc14.c
>>
>>
> 

Hi.

> Since any ISAs beyond AVX512F may be enabled individually, we
> can't simply assign priorities to them.   For GFNI, we can have
> 
> 1. GFNI
> 2.  GFNI + AVX
> 3.  GFNI + AVX512F
> 4. GFNI + AVX512F + AVX512VL

Makes sense to me! I'm considering syntax extension where one would be
able to come up with a priority. Eg.

__attribute__((target("gfni,avx512bw", priority((3)))))

Without that the ISA combinations are probably not comparable in a reasonable way.

> 
> For this code,  GFNI + AVX512BW is ignored:
> 
> [hjl@gnu-cfl-1 pr89929]$ cat z.ii
> __attribute__((target("gfni")))
> int foo(int i) {
>         return 1;
> }
> __attribute__((target("gfni,avx512bw")))
> int foo(int i) {
>         return 4;
> }
> __attribute__((target("default")))
> int foo(int i) {
>         return 3;
> }
> int bar ()
> {
>         return foo(2);
> }

For 'target' attribute it works for me:

1) $ cat z.c && ./xg++ -B. z.c -c 
#include <x86intrin.h>
volatile __m512i x1, x2;
volatile __mmask64 m64;

__attribute__((target("gfni")))
int foo(int i) {
    x1 = _mm512_gf2p8affineinv_epi64_epi8(x1, x2, 3);
    return 1;
}
__attribute__((target("gfni,avx512bw")))
int foo(int i) {
    return 4;
}
__attribute__((target("default")))
int foo(int i) {
  return 3;
}
int bar ()
{
        return foo(2);
}
In file included from ./include/immintrin.h:117,
                 from ./include/x86intrin.h:32,
                 from z.c:1:
z.c: In function ‘int foo(int)’:
z.c:7:10: error: ‘__builtin_ia32_vgf2p8affineinvqb_v64qi’ needs isa option -m32 -mgfni -mavx512f
    7 |     x1 = _mm512_gf2p8affineinv_epi64_epi8(x1, x2, 3);
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
z.c:7:10: note: the ABI for passing parameters with 64-byte alignment has changed in GCC 4.6

2) $ cat z.c && ./xg++ -B. z.c -c 
#include <x86intrin.h>
volatile __m512i x1, x2;
volatile __mmask64 m64;

__attribute__((target("gfni")))
int foo(int i) {
    return 1;
}
__attribute__((target("gfni,avx512bw")))
int foo(int i) {
    x1 = _mm512_gf2p8affineinv_epi64_epi8(x1, x2, 3);
    return 4;
}
__attribute__((target("default")))
int foo(int i) {
  return 3;
}
int bar ()
{
        return foo(2);
}

[OK]

Btw. is it really correct the '-m32' in: 'needs isa option -m32' ?

Similar applies to target_clone attribute where we'll have to come up with
a syntax that will allow multiple ISA to be combined. Something like:

__attribute__((target_clones("gfni+avx512bw")))

? Priorities can be maybe implemented by order?

Martin

> [hjl@gnu-cfl-1 pr89929]$
> /export/build/gnu/tools-build/gcc-wip-debug/build-x86_64-linux/gcc/xgcc
> -B/export/build/gnu/tools-build/gcc-wip-debug/build-x86_64-linux/gcc/
> -S z.ii -O2
> [hjl@gnu-cfl-1 pr89929]$ cat z.s
>         .file "z.ii"
>         .text
>         .p2align 4
>         .globl _Z3fooi.gfni
>         .type _Z3fooi.gfni, @function
> _Z3fooi.gfni:
> .LFB0:
>         .cfi_startproc
>         movl $1, %eax
>         ret
>         .cfi_endproc
> .LFE0:
>         .size _Z3fooi.gfni, .-_Z3fooi.gfni
>         .p2align 4
>         .globl _Z3fooi
>         .type _Z3fooi, @function
> _Z3fooi:
> .LFB2:
>         .cfi_startproc
>         movl $3, %eax
>         ret
>         .cfi_endproc
> .LFE2:
>         .size _Z3fooi, .-_Z3fooi
>         .p2align 4
>         .globl _Z3fooi.avx512bw_gfni
>         .type _Z3fooi.avx512bw_gfni, @function
> _Z3fooi.avx512bw_gfni:
> .LFB1:
>         .cfi_startproc
>         movl $4, %eax
>         ret
>         .cfi_endproc
> .LFE1:
>         .size _Z3fooi.avx512bw_gfni, .-_Z3fooi.avx512bw_gfni
>         .section .text._Z3fooi.resolver,"axG",@progbits,_Z3fooi.resolver,comdat
>         .p2align 4
>         .weak _Z3fooi.resolver
>         .type _Z3fooi.resolver, @function
> _Z3fooi.resolver:
> .LFB5:
>         .cfi_startproc
>         subq $8, %rsp
>         .cfi_def_cfa_offset 16
>         call __cpu_indicator_init
>         movl $_Z3fooi, %eax
>         movl $_Z3fooi.gfni, %edx
>         testb $1, __cpu_features2(%rip)
>         cmovne %rdx, %rax
>         addq $8, %rsp
>         .cfi_def_cfa_offset 8
>         ret
>         .cfi_endproc
> .LFE5:
>         .size _Z3fooi.resolver, .-_Z3fooi.resolver
>         .globl _Z7_Z3fooii
>         .type _Z7_Z3fooii, @gnu_indirect_function
>         .set _Z7_Z3fooii,_Z3fooi.resolver
>         .text
>         .p2align 4
>         .globl _Z3barv
>         .type _Z3barv, @function
> _Z3barv:
> .LFB3:
>         .cfi_startproc
>         movl $2, %edi
>         jmp _Z7_Z3fooii
>         .cfi_endproc
> .LFE3:
>         .size _Z3barv, .-_Z3barv
>         .ident "GCC: (GNU) 9.0.1 20190411 (experimental)"
>         .section .note.GNU-stack,"",@progbits
> [hjl@gnu-cfl-1 pr89929]$
> 
> For AVX512 ISAs, we need a different scheme for priorities.
> 
>