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

H.J. Lu hjl.tools@gmail.com
Mon Apr 15 15:50:00 GMT 2019


On Mon, Apr 15, 2019 at 12:26 AM Martin Liška <mliska@suse.cz> wrote:
>
> On 4/12/19 4:12 PM, H.J. Lu wrote:
> > On Fri, Apr 12, 2019 at 4:41 AM Martin Liška <mliska@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 <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' ?
> >
> > It does look odd.
>
> Then let me take a look at this.
>
> >
> >> 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.
> >
>
> Well, isn't that what we currently support, e.g.:
>
> $ cat mvc11.c && gcc mvc11.c -c
> __attribute__((target_clones("arch=sandybridge", "arch=cascadelake", "default"))) int
> foo (void)
> {
>   return 0;
> }
>
> int
> main ()
> {
>   foo ();
> }
>
> If so, we can provide a new warning that will tell that for AVX512* on should use 'arch=xyz'
> instead?
>

1. We don't have one option to enable AVX512F and AVX512CD, which are common to
Skylake server and KNL.
2. -march= also implies -mtune, which may not be wanted.
3. -march=icelake-client enables more than just AVX512XXX.

H.J.


-- 
H.J.



More information about the Gcc-patches mailing list