On 4/12/19 4:12 PM, H.J. Lu wrote:
> 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.
I've just created a PR for that:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90096
Martin
>
>> 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.
>
