| Message ID | 20140317115336.GC65059@msticlxl57.ims.intel.com |
|---|---|
| State | New |
| Headers | show |
On Mon, Mar 17, 2014 at 4:53 AM, Kirill Yukhin <kirill.yukhin@gmail.com> wrote: > Hello, > Patch in the bottom allows to use ymmXX and zmmXX > register names in inline asm statements as well as > in `register` variables definitions. > > New tests pass. > Bootstrap pass. > > Is it ok for trunk? > Do we need to backport it to 4.8? > > gcc/ > * config/i386/i386.h (ADDITIONAL_REGISTER_NAMES): Add > ymm and zmm register names. > > testsuite/ > * gcc.target/i386/avx-additional-reg-names.c: New. > * gcc.target/i386/avx512f-additional-reg-names.c: Ditto. > > -- > Thanks, K > > commit c3884af93c105115bc1e4d02fa824d24420c5bbf > Author: Kirill Yukhin <kirill.yukhin@intel.com> > Date: Mon Mar 17 14:56:06 2014 +0400 > > [AVX, AVX-512]. Extend ADDITIONAL_REGISTER_NAMES to Ymms and Zmms. > --- > gcc/config/i386/i386.h | 28 +++++++++++++++++----- > .../gcc.target/i386/avx-additional-reg-names.c | 9 +++++++ > .../gcc.target/i386/avx512f-additional-reg-names.c | 9 +++++++ > 3 files changed, 40 insertions(+), 6 deletions(-) > > diff --git a/gcc/config/i386/i386.h b/gcc/config/i386/i386.h > index c80878b..c5c1d58 100644 > --- a/gcc/config/i386/i386.h > +++ b/gcc/config/i386/i386.h > @@ -2016,12 +2016,28 @@ do { \ > /* Table of additional register names to use in user input. */ > > #define ADDITIONAL_REGISTER_NAMES \ > -{ { "eax", 0 }, { "edx", 1 }, { "ecx", 2 }, { "ebx", 3 }, \ > - { "esi", 4 }, { "edi", 5 }, { "ebp", 6 }, { "esp", 7 }, \ > - { "rax", 0 }, { "rdx", 1 }, { "rcx", 2 }, { "rbx", 3 }, \ > - { "rsi", 4 }, { "rdi", 5 }, { "rbp", 6 }, { "rsp", 7 }, \ > - { "al", 0 }, { "dl", 1 }, { "cl", 2 }, { "bl", 3 }, \ > - { "ah", 0 }, { "dh", 1 }, { "ch", 2 }, { "bh", 3 } } > +{ { "eax", 0 }, { "edx", 1 }, { "ecx", 2 }, { "ebx", 3 }, \ > + { "esi", 4 }, { "edi", 5 }, { "ebp", 6 }, { "esp", 7 }, \ > + { "rax", 0 }, { "rdx", 1 }, { "rcx", 2 }, { "rbx", 3 }, \ > + { "rsi", 4 }, { "rdi", 5 }, { "rbp", 6 }, { "rsp", 7 }, \ > + { "al", 0 }, { "dl", 1 }, { "cl", 2 }, { "bl", 3 }, \ > + { "ah", 0 }, { "dh", 1 }, { "ch", 2 }, { "bh", 3 }, \ > + { "ymm0", 21}, { "ymm1", 22}, { "ymm2", 23}, { "ymm3", 24}, \ > + { "ymm4", 25}, { "ymm5", 26}, { "ymm6", 27}, { "ymm7", 28}, \ > + { "ymm8", 45}, { "ymm9", 46}, { "ymm10", 47}, { "ymm11", 48}, \ > + { "ymm12", 49}, { "ymm13", 50}, { "ymm14", 51}, { "ymm15", 52}, \ > + { "ymm16", 53}, { "ymm17", 54}, { "ymm18", 55}, { "ymm19", 56}, \ > + { "ymm20", 57}, { "ymm21", 58}, { "ymm22", 59}, { "ymm23", 60}, \ > + { "ymm24", 61}, { "ymm25", 62}, { "ymm26", 63}, { "ymm27", 64}, \ > + { "ymm28", 65}, { "ymm29", 66}, { "ymm30", 67}, { "ymm31", 68}, \ > + { "zmm0", 21}, { "zmm1", 22}, { "zmm2", 23}, { "zmm3", 24}, \ > + { "zmm4", 25}, { "zmm5", 26}, { "zmm6", 27}, { "zmm7", 28}, \ > + { "zmm8", 45}, { "zmm9", 46}, { "zmm10", 47}, { "zmm11", 48}, \ > + { "zmm12", 49}, { "zmm13", 50}, { "zmm14", 51}, { "zmm15", 52}, \ > + { "zmm16", 53}, { "zmm17", 54}, { "zmm18", 55}, { "zmm19", 56}, \ > + { "zmm20", 57}, { "zmm21", 58}, { "zmm22", 59}, { "zmm23", 60}, \ > + { "zmm24", 61}, { "zmm25", 62}, { "zmm26", 63}, { "zmm27", 64}, \ > + { "zmm28", 65}, { "zmm29", 66}, { "zmm30", 67}, { "zmm31", 68} } > > /* Note we are omitting these since currently I don't know how > to get gcc to use these, since they want the same but different > diff --git a/gcc/testsuite/gcc.target/i386/avx-additional-reg-names.c b/gcc/testsuite/gcc.target/i386/avx-additional-reg-names.c > new file mode 100644 > index 0000000..d984bff > --- /dev/null > +++ b/gcc/testsuite/gcc.target/i386/avx-additional-reg-names.c > @@ -0,0 +1,9 @@ > +/* { dg-do compile } */ > +/* { dg-options "-mavx" } */ > + > +void foo () > +{ > + register int ymm_var asm ("ymm4"); > + > + __asm__ __volatile__("vxorpd %%ymm0, %%ymm0, %%ymm7\n" : : : "ymm7" ); > +} Doesn't GCC generate the same code with xmm? > diff --git a/gcc/testsuite/gcc.target/i386/avx512f-additional-reg-names.c b/gcc/testsuite/gcc.target/i386/avx512f-additional-reg-names.c > new file mode 100644 > index 0000000..1bd428a > --- /dev/null > +++ b/gcc/testsuite/gcc.target/i386/avx512f-additional-reg-names.c > @@ -0,0 +1,9 @@ > +/* { dg-do compile } */ > +/* { dg-options "-mavx512f" } */ > + > +void foo () > +{ > + register int zmm_var asm ("zmm9"); > + > + __asm__ __volatile__("vxorpd %%zmm0, %%zmm0, %%zmm7\n" : : : "zmm7" ); > +} Doesn't GCC generate the same code with xmm?
On Mon, Mar 17, 2014 at 4:12 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >> Patch in the bottom allows to use ymmXX and zmmXX >> register names in inline asm statements as well as >> in `register` variables definitions. >> >> New tests pass. >> Bootstrap pass. >> >> Is it ok for trunk? >> Do we need to backport it to 4.8? >> >> gcc/ >> * config/i386/i386.h (ADDITIONAL_REGISTER_NAMES): Add >> ymm and zmm register names. >> >> testsuite/ >> * gcc.target/i386/avx-additional-reg-names.c: New. >> * gcc.target/i386/avx512f-additional-reg-names.c: Ditto. > Doesn't GCC generate the same code with xmm? > >> diff --git a/gcc/testsuite/gcc.target/i386/avx512f-additional-reg-names.c b/gcc/testsuite/gcc.target/i386/avx512f-additional-reg-names.c >> new file mode 100644 >> index 0000000..1bd428a >> --- /dev/null >> +++ b/gcc/testsuite/gcc.target/i386/avx512f-additional-reg-names.c >> @@ -0,0 +1,9 @@ >> +/* { dg-do compile } */ >> +/* { dg-options "-mavx512f" } */ >> + >> +void foo () >> +{ >> + register int zmm_var asm ("zmm9"); >> + >> + __asm__ __volatile__("vxorpd %%zmm0, %%zmm0, %%zmm7\n" : : : "zmm7" ); >> +} > > Doesn't GCC generate the same code with xmm? It does, but the situation is the same as with %eax vs. %rax names. So, I think the patch is OK for mainline, and similar patch involving only %ymm names for AVX-enabled branches. Uros.
On Mon, Mar 17, 2014 at 9:52 AM, Uros Bizjak <ubizjak@gmail.com> wrote: > On Mon, Mar 17, 2014 at 4:12 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > >>> Patch in the bottom allows to use ymmXX and zmmXX >>> register names in inline asm statements as well as >>> in `register` variables definitions. >>> >>> New tests pass. >>> Bootstrap pass. >>> >>> Is it ok for trunk? >>> Do we need to backport it to 4.8? >>> >>> gcc/ >>> * config/i386/i386.h (ADDITIONAL_REGISTER_NAMES): Add >>> ymm and zmm register names. >>> >>> testsuite/ >>> * gcc.target/i386/avx-additional-reg-names.c: New. >>> * gcc.target/i386/avx512f-additional-reg-names.c: Ditto. > >> Doesn't GCC generate the same code with xmm? >> >>> diff --git a/gcc/testsuite/gcc.target/i386/avx512f-additional-reg-names.c b/gcc/testsuite/gcc.target/i386/avx512f-additional-reg-names.c >>> new file mode 100644 >>> index 0000000..1bd428a >>> --- /dev/null >>> +++ b/gcc/testsuite/gcc.target/i386/avx512f-additional-reg-names.c >>> @@ -0,0 +1,9 @@ >>> +/* { dg-do compile } */ >>> +/* { dg-options "-mavx512f" } */ >>> + >>> +void foo () >>> +{ >>> + register int zmm_var asm ("zmm9"); >>> + >>> + __asm__ __volatile__("vxorpd %%zmm0, %%zmm0, %%zmm7\n" : : : "zmm7" ); >>> +} >> >> Doesn't GCC generate the same code with xmm? > > It does, but the situation is the same as with %eax vs. %rax names. > So, I think the patch is OK for mainline, and similar patch involving > only %ymm names for AVX-enabled branches. > If I want to write codes with asm statements which can be compiled with GCC 4.6 and above, I will use xmm instead of ymm. It makes ymm less attractive.
On Mon, Mar 17, 2014 at 10:11 AM, H.J. Lu <hjl.tools@gmail.com> wrote: > On Mon, Mar 17, 2014 at 9:52 AM, Uros Bizjak <ubizjak@gmail.com> wrote: >> On Mon, Mar 17, 2014 at 4:12 PM, H.J. Lu <hjl.tools@gmail.com> wrote: >> >>>> Patch in the bottom allows to use ymmXX and zmmXX >>>> register names in inline asm statements as well as >>>> in `register` variables definitions. >>>> >>>> New tests pass. >>>> Bootstrap pass. >>>> >>>> Is it ok for trunk? >>>> Do we need to backport it to 4.8? >>>> >>>> gcc/ >>>> * config/i386/i386.h (ADDITIONAL_REGISTER_NAMES): Add >>>> ymm and zmm register names. >>>> >>>> testsuite/ >>>> * gcc.target/i386/avx-additional-reg-names.c: New. >>>> * gcc.target/i386/avx512f-additional-reg-names.c: Ditto. >> >>> Doesn't GCC generate the same code with xmm? >>> >>>> diff --git a/gcc/testsuite/gcc.target/i386/avx512f-additional-reg-names.c b/gcc/testsuite/gcc.target/i386/avx512f-additional-reg-names.c >>>> new file mode 100644 >>>> index 0000000..1bd428a >>>> --- /dev/null >>>> +++ b/gcc/testsuite/gcc.target/i386/avx512f-additional-reg-names.c >>>> @@ -0,0 +1,9 @@ >>>> +/* { dg-do compile } */ >>>> +/* { dg-options "-mavx512f" } */ >>>> + >>>> +void foo () >>>> +{ >>>> + register int zmm_var asm ("zmm9"); >>>> + >>>> + __asm__ __volatile__("vxorpd %%zmm0, %%zmm0, %%zmm7\n" : : : "zmm7" ); >>>> +} >>> >>> Doesn't GCC generate the same code with xmm? >> >> It does, but the situation is the same as with %eax vs. %rax names. >> So, I think the patch is OK for mainline, and similar patch involving >> only %ymm names for AVX-enabled branches. >> > > If I want to write codes with asm statements which can > be compiled with GCC 4.6 and above, I will use xmm > instead of ymm. It makes ymm less attractive. > BTW, in glibc, there are asm volatile ("vmovdqa64 %0, %%zmm0" : : "x" (zmm) : "xmm0" ); and asm volatile ("vmovdqa %0, %%ymm0" : : "x" (ymm) : "xmm0" );
On 17 Mar 17:52, Uros Bizjak wrote: > On Mon, Mar 17, 2014 at 4:12 PM, H.J. Lu <hjl.tools@gmail.com> wrote: > > >> Is it ok for trunk? > >> Do we need to backport it to 4.8? > It does, but the situation is the same as with %eax vs. %rax names. > So, I think the patch is OK for mainline, and similar patch involving > only %ymm names for AVX-enabled branches. Thanks, Uroš! Couple of questions. AVX-enabled branches are 4.8 and 4.7? I suspect that 4.6 is out of support. Second. I didn't understood point of HJ at all. Did you? (I'll try to reach him via internal IM). -- Thanks, K
On 17 Mar 10:16, H.J. Lu wrote: > BTW, in glibc, there are > > asm volatile ("vmovdqa64 %0, %%zmm0" : : "x" (zmm) : "xmm0" ); Maybe. But I belive that this is much more clear to have instead: asm volatile ("vmovdqa64 %0, %%zmm0" : : "x" (zmm) : "zmm0" ); -- Thanks, K
On Mon, Mar 17, 2014 at 10:37 AM, Kirill Yukhin <kirill.yukhin@gmail.com> wrote: > On 17 Mar 10:16, H.J. Lu wrote: >> BTW, in glibc, there are >> >> asm volatile ("vmovdqa64 %0, %%zmm0" : : "x" (zmm) : "xmm0" ); > Maybe. But I belive that this is much more clear to have instead: > asm volatile ("vmovdqa64 %0, %%zmm0" : : "x" (zmm) : "zmm0" ); > My issue is this is a user-visible change. Code using ymm which works with GCC 4.9 won't work with the installed GCC 4.6/4.7/4.8. This change introduces GCC portability issues without significant benefit.
On Mon, Mar 17, 2014 at 11:26:58AM -0700, H.J. Lu wrote: > On Mon, Mar 17, 2014 at 10:37 AM, Kirill Yukhin <kirill.yukhin@gmail.com> wrote: > > On 17 Mar 10:16, H.J. Lu wrote: > >> BTW, in glibc, there are > >> > >> asm volatile ("vmovdqa64 %0, %%zmm0" : : "x" (zmm) : "xmm0" ); > > Maybe. But I belive that this is much more clear to have instead: > > asm volatile ("vmovdqa64 %0, %%zmm0" : : "x" (zmm) : "zmm0" ); > > > > My issue is this is a user-visible change. Code using ymm which > works with GCC 4.9 won't work with the installed GCC 4.6/4.7/4.8. > This change introduces GCC portability issues without significant > benefit. It is up to the user to decide if they want to be portable to older compilers or not. But it is useful and more intuitive if we allow specifying also the ymm and zmm forms. Jakub
diff --git a/gcc/config/i386/i386.h b/gcc/config/i386/i386.h index c80878b..c5c1d58 100644 --- a/gcc/config/i386/i386.h +++ b/gcc/config/i386/i386.h @@ -2016,12 +2016,28 @@ do { \ /* Table of additional register names to use in user input. */ #define ADDITIONAL_REGISTER_NAMES \ -{ { "eax", 0 }, { "edx", 1 }, { "ecx", 2 }, { "ebx", 3 }, \ - { "esi", 4 }, { "edi", 5 }, { "ebp", 6 }, { "esp", 7 }, \ - { "rax", 0 }, { "rdx", 1 }, { "rcx", 2 }, { "rbx", 3 }, \ - { "rsi", 4 }, { "rdi", 5 }, { "rbp", 6 }, { "rsp", 7 }, \ - { "al", 0 }, { "dl", 1 }, { "cl", 2 }, { "bl", 3 }, \ - { "ah", 0 }, { "dh", 1 }, { "ch", 2 }, { "bh", 3 } } +{ { "eax", 0 }, { "edx", 1 }, { "ecx", 2 }, { "ebx", 3 }, \ + { "esi", 4 }, { "edi", 5 }, { "ebp", 6 }, { "esp", 7 }, \ + { "rax", 0 }, { "rdx", 1 }, { "rcx", 2 }, { "rbx", 3 }, \ + { "rsi", 4 }, { "rdi", 5 }, { "rbp", 6 }, { "rsp", 7 }, \ + { "al", 0 }, { "dl", 1 }, { "cl", 2 }, { "bl", 3 }, \ + { "ah", 0 }, { "dh", 1 }, { "ch", 2 }, { "bh", 3 }, \ + { "ymm0", 21}, { "ymm1", 22}, { "ymm2", 23}, { "ymm3", 24}, \ + { "ymm4", 25}, { "ymm5", 26}, { "ymm6", 27}, { "ymm7", 28}, \ + { "ymm8", 45}, { "ymm9", 46}, { "ymm10", 47}, { "ymm11", 48}, \ + { "ymm12", 49}, { "ymm13", 50}, { "ymm14", 51}, { "ymm15", 52}, \ + { "ymm16", 53}, { "ymm17", 54}, { "ymm18", 55}, { "ymm19", 56}, \ + { "ymm20", 57}, { "ymm21", 58}, { "ymm22", 59}, { "ymm23", 60}, \ + { "ymm24", 61}, { "ymm25", 62}, { "ymm26", 63}, { "ymm27", 64}, \ + { "ymm28", 65}, { "ymm29", 66}, { "ymm30", 67}, { "ymm31", 68}, \ + { "zmm0", 21}, { "zmm1", 22}, { "zmm2", 23}, { "zmm3", 24}, \ + { "zmm4", 25}, { "zmm5", 26}, { "zmm6", 27}, { "zmm7", 28}, \ + { "zmm8", 45}, { "zmm9", 46}, { "zmm10", 47}, { "zmm11", 48}, \ + { "zmm12", 49}, { "zmm13", 50}, { "zmm14", 51}, { "zmm15", 52}, \ + { "zmm16", 53}, { "zmm17", 54}, { "zmm18", 55}, { "zmm19", 56}, \ + { "zmm20", 57}, { "zmm21", 58}, { "zmm22", 59}, { "zmm23", 60}, \ + { "zmm24", 61}, { "zmm25", 62}, { "zmm26", 63}, { "zmm27", 64}, \ + { "zmm28", 65}, { "zmm29", 66}, { "zmm30", 67}, { "zmm31", 68} } /* Note we are omitting these since currently I don't know how to get gcc to use these, since they want the same but different diff --git a/gcc/testsuite/gcc.target/i386/avx-additional-reg-names.c b/gcc/testsuite/gcc.target/i386/avx-additional-reg-names.c new file mode 100644 index 0000000..d984bff --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/avx-additional-reg-names.c @@ -0,0 +1,9 @@ +/* { dg-do compile } */ +/* { dg-options "-mavx" } */ + +void foo () +{ + register int ymm_var asm ("ymm4"); + + __asm__ __volatile__("vxorpd %%ymm0, %%ymm0, %%ymm7\n" : : : "ymm7" ); +} diff --git a/gcc/testsuite/gcc.target/i386/avx512f-additional-reg-names.c b/gcc/testsuite/gcc.target/i386/avx512f-additional-reg-names.c new file mode 100644 index 0000000..1bd428a --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/avx512f-additional-reg-names.c @@ -0,0 +1,9 @@ +/* { dg-do compile } */ +/* { dg-options "-mavx512f" } */ + +void foo () +{ + register int zmm_var asm ("zmm9"); + + __asm__ __volatile__("vxorpd %%zmm0, %%zmm0, %%zmm7\n" : : : "zmm7" ); +}
Hello, Patch in the bottom allows to use ymmXX and zmmXX register names in inline asm statements as well as in `register` variables definitions. New tests pass. Bootstrap pass. Is it ok for trunk? Do we need to backport it to 4.8? gcc/ * config/i386/i386.h (ADDITIONAL_REGISTER_NAMES): Add ymm and zmm register names. testsuite/ * gcc.target/i386/avx-additional-reg-names.c: New. * gcc.target/i386/avx512f-additional-reg-names.c: Ditto. -- Thanks, K commit c3884af93c105115bc1e4d02fa824d24420c5bbf Author: Kirill Yukhin <kirill.yukhin@intel.com> Date: Mon Mar 17 14:56:06 2014 +0400 [AVX, AVX-512]. Extend ADDITIONAL_REGISTER_NAMES to Ymms and Zmms. --- gcc/config/i386/i386.h | 28 +++++++++++++++++----- .../gcc.target/i386/avx-additional-reg-names.c | 9 +++++++ .../gcc.target/i386/avx512f-additional-reg-names.c | 9 +++++++ 3 files changed, 40 insertions(+), 6 deletions(-)