On Fri, Apr 12, 2019 at 4:41 AM Martin Liška <mli...@suse.cz> wrote:
>
> On 4/11/19 6:30 PM, H.J. Lu wrote:
> > On Thu, Apr 11, 2019 at 1:38 AM Martin Liška <mli...@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 <mli...@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 <mli...@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' ?
It does look odd.
> 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?
>
I am thinking -misa=processor which will enable ISAs for
processor. It differs from -march=. -misa= doesn't set
-mtune.
--
H.J.
