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

Martin Liška mliska@suse.cz
Tue Apr 16 08:32:00 GMT 2019


On 4/15/19 5:09 PM, H.J. Lu wrote:
> 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.

Then arch=axy is the solution for that.

> 2. -march= also implies -mtune, which may not be wanted.

Why is that not intended. Using target (or target_clone), I would expect the
fastest possible version of the function for defined ISA (or ARCH). Then
implying -mtune is reasonable for me.

> 3. -march=icelake-client enables more than just AVX512XXX.

Yes, but if you take a look at number of AVX512 ISA extensions (18), then
it looks more easier to me to use arch=xyz as it leads to reasonable number
of architectures.

Martin

> 
> H.J.
> 
> 



More information about the Gcc-patches mailing list