| Message ID | 20181019210437.29826-1-hjl.tools@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] i386: Enable AVX512 memory broadcast for FP mul | expand |
On Fri, Oct 19, 2018 at 11:08 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > Many AVX512 vector operations can broadcast from a scalar memory source. > This patch enables memory broadcast for FP mul operations. > > gcc/ > > PR target/72782 > * config/i386/sse.md (*mul<mode>3<mask_name>_bcst_1): New. > (*mul<mode>3<mask_name>_bcst_2): Likewise. > > gcc/testsuite/ > > PR target/72782 > * gcc.target/i386/avx512f-mul-df-zmm-1.c: New test. > * gcc.target/i386/avx512f-mul-sf-zmm-1.c: Likewise. > * gcc.target/i386/avx512f-mul-sf-zmm-2.c: Likewise. > * gcc.target/i386/avx512f-mul-sf-zmm-3.c: Likewise. > * gcc.target/i386/avx512f-mul-sf-zmm-4.c: Likewise. > * gcc.target/i386/avx512f-mul-sf-zmm-5.c: Likewise. > * gcc.target/i386/avx512f-mul-sf-zmm-6.c: Likewise. > * gcc.target/i386/avx512vl-mul-sf-xmm-1.c: Likewise. > * gcc.target/i386/avx512vl-mul-sf-ymm-1.c: Likewise. > --- > gcc/config/i386/sse.md | 24 +++++++++++++++++++ > .../gcc.target/i386/avx512f-mul-df-zmm-1.c | 12 ++++++++++ > .../gcc.target/i386/avx512f-mul-sf-zmm-1.c | 12 ++++++++++ > .../gcc.target/i386/avx512f-mul-sf-zmm-2.c | 12 ++++++++++ > .../gcc.target/i386/avx512f-mul-sf-zmm-3.c | 12 ++++++++++ > .../gcc.target/i386/avx512f-mul-sf-zmm-4.c | 12 ++++++++++ > .../gcc.target/i386/avx512f-mul-sf-zmm-5.c | 12 ++++++++++ > .../gcc.target/i386/avx512f-mul-sf-zmm-6.c | 12 ++++++++++ > .../gcc.target/i386/avx512vl-mul-sf-xmm-1.c | 12 ++++++++++ > .../gcc.target/i386/avx512vl-mul-sf-ymm-1.c | 12 ++++++++++ > 10 files changed, 132 insertions(+) > create mode 100644 gcc/testsuite/gcc.target/i386/avx512f-mul-df-zmm-1.c > create mode 100644 gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-1.c > create mode 100644 gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-2.c > create mode 100644 gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-3.c > create mode 100644 gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-4.c > create mode 100644 gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-5.c > create mode 100644 gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-6.c > create mode 100644 gcc/testsuite/gcc.target/i386/avx512vl-mul-sf-xmm-1.c > create mode 100644 gcc/testsuite/gcc.target/i386/avx512vl-mul-sf-ymm-1.c > > diff --git a/gcc/config/i386/sse.md b/gcc/config/i386/sse.md > index 411c78ae8d3..a73659e6bd2 100644 > --- a/gcc/config/i386/sse.md > +++ b/gcc/config/i386/sse.md > @@ -1754,6 +1754,30 @@ > (set_attr "btver2_decode" "direct,double") > (set_attr "mode" "<MODE>")]) > > +(define_insn "*mul<mode>3<mask_name>_bcst_1" > + [(set (match_operand:VF_AVX512 0 "register_operand" "=v") > + (mult:VF_AVX512 > + (match_operand:VF_AVX512 1 "register_operand" "v") > + (vec_duplicate:VF_AVX512 > + (match_operand:<ssescalarmode> 2 "memory_operand" "m"))))] > + "TARGET_AVX512F && <mask_mode512bit_condition>" > + "vmul<ssemodesuffix>\t{%2<avx512bcst>, %1, %0<mask_operand3>|%0<mask_operand3>, %1, %2<<avx512bcst>>}" > + [(set_attr "prefix" "evex") > + (set_attr "type" "ssemul") > + (set_attr "mode" "<MODE>")]) > + > +(define_insn "*mul<mode>3<mask_name>_bcst_2" > + [(set (match_operand:VF_AVX512 0 "register_operand" "=v") > + (mult:VF_AVX512 > + (vec_duplicate:VF_AVX512 > + (match_operand:<ssescalarmode> 1 "memory_operand" "m")) > + (match_operand:VF_AVX512 2 "register_operand" "v")))] > + "TARGET_AVX512F && <mask_mode512bit_condition>" > + "vmul<ssemodesuffix>\t{%1<avx512bcst>, %2, %0<mask_operand3>|%0<mask_operand3>, %2, %1<<avx512bcst>>}" > + [(set_attr "prefix" "evex") > + (set_attr "type" "ssemul") > + (set_attr "mode" "<MODE>")]) Do we really need two patterns here? IIRC, the compiler canonicalizes commutative binops so that they have memory operand in the second place. We have vec_duplicate here, so this may not be the case, but please investigate if we really need two patterns for commutative binops. Uros. > (define_insn "<sse>_vm<multdiv_mnemonic><mode>3<mask_scalar_name><round_scalar_name>" > [(set (match_operand:VF_128 0 "register_operand" "=x,v") > (vec_merge:VF_128 > diff --git a/gcc/testsuite/gcc.target/i386/avx512f-mul-df-zmm-1.c b/gcc/testsuite/gcc.target/i386/avx512f-mul-df-zmm-1.c > new file mode 100644 > index 00000000000..e3c51986fe2 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/i386/avx512f-mul-df-zmm-1.c > @@ -0,0 +1,12 @@ > +/* { dg-do compile } */ > +/* { dg-options "-mavx512f -O2" } */ > +/* { dg-final { scan-assembler-times "vmulpd\[ \\t\]+\\(%(?:eax|rdi|edi)\\)\\\{1to\[1-8\]+\\\}, %zmm\[0-9\]+, %zmm0" 1 } } */ > +/* { dg-final { scan-assembler-not "vbroadcastsd\[^\n\]*%zmm\[0-9\]+" } } */ > + > +#define type __m512d > +#define vec 512 > +#define op mul > +#define suffix pd > +#define SCALAR double > + > +#include "avx512-binop-1.h" > diff --git a/gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-1.c b/gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-1.c > new file mode 100644 > index 00000000000..14bccca276a > --- /dev/null > +++ b/gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-1.c > @@ -0,0 +1,12 @@ > +/* { dg-do compile } */ > +/* { dg-options "-mavx512f -O2" } */ > +/* { dg-final { scan-assembler-times "vmulps\[ \\t\]+\\(%(?:eax|rdi|edi)\\)\\\{1to\[1-8\]+\\\}, %zmm\[0-9\]+, %zmm0" 1 } } */ > +/* { dg-final { scan-assembler-not "vbroadcastss\[^\n\]*%zmm\[0-9\]+" } } */ > + > +#define type __m512 > +#define vec 512 > +#define op mul > +#define suffix ps > +#define SCALAR float > + > +#include "avx512-binop-1.h" > diff --git a/gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-2.c b/gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-2.c > new file mode 100644 > index 00000000000..8293324084b > --- /dev/null > +++ b/gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-2.c > @@ -0,0 +1,12 @@ > +/* { dg-do compile } */ > +/* { dg-options "-mavx512f -O2" } */ > +/* { dg-final { scan-assembler-times "vmulps\[ \\t\]+\\(%(?:eax|rdi|edi)\\)\\\{1to\[1-8\]+\\\}, %zmm\[0-9\]+, %zmm0" 1 } } */ > +/* { dg-final { scan-assembler-not "vbroadcastss\[^\n\]*%zmm\[0-9\]+" } } */ > + > +#define type __m512 > +#define vec 512 > +#define op mul > +#define suffix ps > +#define SCALAR float > + > +#include "avx512-binop-2.h" > diff --git a/gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-3.c b/gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-3.c > new file mode 100644 > index 00000000000..cb768db2cb4 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-3.c > @@ -0,0 +1,12 @@ > +/* { dg-do compile } */ > +/* { dg-options "-mavx512f -O2" } */ > +/* { dg-final { scan-assembler-times "vbroadcastss\[^\n\]*%zmm\[0-9\]+" 1 } } */ > +/* { dg-final { scan-assembler-times "vmulps\[^\n\]*%zmm\[0-9\]+" 1 } } */ > + > +#define type __m512 > +#define vec 512 > +#define op mul > +#define suffix ps > +#define SCALAR float > + > +#include "avx512-binop-3.h" > diff --git a/gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-4.c b/gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-4.c > new file mode 100644 > index 00000000000..7626192ed3d > --- /dev/null > +++ b/gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-4.c > @@ -0,0 +1,12 @@ > +/* { dg-do compile { target { ! ia32 } } } */ > +/* { dg-options "-mavx512f -O2" } */ > +/* { dg-final { scan-assembler-times "vbroadcastss\[^\n\]*%zmm\[0-9\]+" 1 } } */ > +/* { dg-final { scan-assembler-times "vmulps\[ \\t\]+%zmm\[0-9\]+, %zmm\[0-9\]+, %zmm0" 1 } } */ > + > +#define type __m512 > +#define vec 512 > +#define op mul > +#define suffix ps > +#define SCALAR float > + > +#include "avx512-binop-4.h" > diff --git a/gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-5.c b/gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-5.c > new file mode 100644 > index 00000000000..b2ad8054d98 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-5.c > @@ -0,0 +1,12 @@ > +/* { dg-do compile } */ > +/* { dg-options "-mavx512f -O2" } */ > +/* { dg-final { scan-assembler-times "vmulps\[ \\t\]+\[^\n\]*\\\{1to\[1-8\]+\\\}, %zmm\[0-9\]+, %zmm0" 1 } } */ > +/* { dg-final { scan-assembler-not "vbroadcastss\[^\n\]*%zmm\[0-9\]+" } } */ > + > +#define type __m512 > +#define vec 512 > +#define op mul > +#define suffix ps > +#define SCALAR float > + > +#include "avx512-binop-5.h" > diff --git a/gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-6.c b/gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-6.c > new file mode 100644 > index 00000000000..d8dde860fe7 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-6.c > @@ -0,0 +1,12 @@ > +/* { dg-do compile } */ > +/* { dg-options "-mavx512f -O2" } */ > +/* { dg-final { scan-assembler-times "vmulps\[ \\t\]+\[^\n\]*\\\{1to\[1-8\]+\\\}, %zmm\[0-9\]+, %zmm0" 1 } } */ > +/* { dg-final { scan-assembler-not "vbroadcastss\[^\n\]*%zmm\[0-9\]+" } } */ > + > +#define type __m512 > +#define vec 512 > +#define op mul > +#define suffix ps > +#define SCALAR float > + > +#include "avx512-binop-6.h" > diff --git a/gcc/testsuite/gcc.target/i386/avx512vl-mul-sf-xmm-1.c b/gcc/testsuite/gcc.target/i386/avx512vl-mul-sf-xmm-1.c > new file mode 100644 > index 00000000000..395cccdc78b > --- /dev/null > +++ b/gcc/testsuite/gcc.target/i386/avx512vl-mul-sf-xmm-1.c > @@ -0,0 +1,12 @@ > +/* { dg-do compile } */ > +/* { dg-options "-mfma -mavx512vl -O2" } */ > +/* { dg-final { scan-assembler-times "vmulps\[ \\t\]+\\(%(?:eax|rdi|edi)\\)\\\{1to\[1-8\]+\\\}, %xmm\[0-9\]+, %xmm0" 1 } } */ > +/* { dg-final { scan-assembler-not "vbroadcastss\[^\n\]*%xmm\[0-9\]+" } } */ > + > +#define type __m128 > +#define vec > +#define op mul > +#define suffix ps > +#define SCALAR float > + > +#include "avx512-binop-1.h" > diff --git a/gcc/testsuite/gcc.target/i386/avx512vl-mul-sf-ymm-1.c b/gcc/testsuite/gcc.target/i386/avx512vl-mul-sf-ymm-1.c > new file mode 100644 > index 00000000000..2a1989381bf > --- /dev/null > +++ b/gcc/testsuite/gcc.target/i386/avx512vl-mul-sf-ymm-1.c > @@ -0,0 +1,12 @@ > +/* { dg-do compile } */ > +/* { dg-options "-mfma -mavx512vl -O2" } */ > +/* { dg-final { scan-assembler-times "vmulps\[ \\t\]+\\(%(?:eax|rdi|edi)\\)\\\{1to\[1-8\]+\\\}, %ymm\[0-9\]+, %ymm0" 1 } } */ > +/* { dg-final { scan-assembler-not "vbroadcastss\[^\n\]*%ymm\[0-9\]+" } } */ > + > +#define type __m256 > +#define vec 256 > +#define op mul > +#define suffix ps > +#define SCALAR float > + > +#include "avx512-binop-1.h" > -- > 2.17.2 >
On 10/20/18, Uros Bizjak <ubizjak@gmail.com> wrote: > On Fri, Oct 19, 2018 at 11:08 PM H.J. Lu <hjl.tools@gmail.com> wrote: >> >> Many AVX512 vector operations can broadcast from a scalar memory source. >> This patch enables memory broadcast for FP mul operations. >> >> gcc/ >> >> PR target/72782 >> * config/i386/sse.md (*mul<mode>3<mask_name>_bcst_1): New. >> (*mul<mode>3<mask_name>_bcst_2): Likewise. >> >> gcc/testsuite/ >> >> PR target/72782 >> * gcc.target/i386/avx512f-mul-df-zmm-1.c: New test. >> * gcc.target/i386/avx512f-mul-sf-zmm-1.c: Likewise. >> * gcc.target/i386/avx512f-mul-sf-zmm-2.c: Likewise. >> * gcc.target/i386/avx512f-mul-sf-zmm-3.c: Likewise. >> * gcc.target/i386/avx512f-mul-sf-zmm-4.c: Likewise. >> * gcc.target/i386/avx512f-mul-sf-zmm-5.c: Likewise. >> * gcc.target/i386/avx512f-mul-sf-zmm-6.c: Likewise. >> * gcc.target/i386/avx512vl-mul-sf-xmm-1.c: Likewise. >> * gcc.target/i386/avx512vl-mul-sf-ymm-1.c: Likewise. >> --- >> gcc/config/i386/sse.md | 24 +++++++++++++++++++ >> .../gcc.target/i386/avx512f-mul-df-zmm-1.c | 12 ++++++++++ >> .../gcc.target/i386/avx512f-mul-sf-zmm-1.c | 12 ++++++++++ >> .../gcc.target/i386/avx512f-mul-sf-zmm-2.c | 12 ++++++++++ >> .../gcc.target/i386/avx512f-mul-sf-zmm-3.c | 12 ++++++++++ >> .../gcc.target/i386/avx512f-mul-sf-zmm-4.c | 12 ++++++++++ >> .../gcc.target/i386/avx512f-mul-sf-zmm-5.c | 12 ++++++++++ >> .../gcc.target/i386/avx512f-mul-sf-zmm-6.c | 12 ++++++++++ >> .../gcc.target/i386/avx512vl-mul-sf-xmm-1.c | 12 ++++++++++ >> .../gcc.target/i386/avx512vl-mul-sf-ymm-1.c | 12 ++++++++++ >> 10 files changed, 132 insertions(+) >> create mode 100644 gcc/testsuite/gcc.target/i386/avx512f-mul-df-zmm-1.c >> create mode 100644 gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-1.c >> create mode 100644 gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-2.c >> create mode 100644 gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-3.c >> create mode 100644 gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-4.c >> create mode 100644 gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-5.c >> create mode 100644 gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-6.c >> create mode 100644 gcc/testsuite/gcc.target/i386/avx512vl-mul-sf-xmm-1.c >> create mode 100644 gcc/testsuite/gcc.target/i386/avx512vl-mul-sf-ymm-1.c >> >> diff --git a/gcc/config/i386/sse.md b/gcc/config/i386/sse.md >> index 411c78ae8d3..a73659e6bd2 100644 >> --- a/gcc/config/i386/sse.md >> +++ b/gcc/config/i386/sse.md >> @@ -1754,6 +1754,30 @@ >> (set_attr "btver2_decode" "direct,double") >> (set_attr "mode" "<MODE>")]) >> >> +(define_insn "*mul<mode>3<mask_name>_bcst_1" >> + [(set (match_operand:VF_AVX512 0 "register_operand" "=v") >> + (mult:VF_AVX512 >> + (match_operand:VF_AVX512 1 "register_operand" "v") >> + (vec_duplicate:VF_AVX512 >> + (match_operand:<ssescalarmode> 2 "memory_operand" "m"))))] >> + "TARGET_AVX512F && <mask_mode512bit_condition>" >> + "vmul<ssemodesuffix>\t{%2<avx512bcst>, %1, >> %0<mask_operand3>|%0<mask_operand3>, %1, %2<<avx512bcst>>}" >> + [(set_attr "prefix" "evex") >> + (set_attr "type" "ssemul") >> + (set_attr "mode" "<MODE>")]) >> + >> +(define_insn "*mul<mode>3<mask_name>_bcst_2" >> + [(set (match_operand:VF_AVX512 0 "register_operand" "=v") >> + (mult:VF_AVX512 >> + (vec_duplicate:VF_AVX512 >> + (match_operand:<ssescalarmode> 1 "memory_operand" "m")) >> + (match_operand:VF_AVX512 2 "register_operand" "v")))] >> + "TARGET_AVX512F && <mask_mode512bit_condition>" >> + "vmul<ssemodesuffix>\t{%1<avx512bcst>, %2, >> %0<mask_operand3>|%0<mask_operand3>, %2, %1<<avx512bcst>>}" >> + [(set_attr "prefix" "evex") >> + (set_attr "type" "ssemul") >> + (set_attr "mode" "<MODE>")]) > > Do we really need two patterns here? IIRC, the compiler canonicalizes > commutative binops so that they have memory operand in the second > place. We have vec_duplicate here, so this may not be the case, but > please investigate if we really need two patterns for commutative > binops. > Only one pattern is needed. For (set (reg:V16SF 89) (vec_duplicate:V16SF (reg:SF 91))) (set (reg:V16SF 95) (mult:V16SF (reg:V16SF 87) (reg:V16SF 89))) combiner prefers (set (reg:V16SF 95) (mult:V16SF (vec_duplicate:V16SF (reg:SF 91)) (reg:V16SF 87))) instead of (set (reg:V16SF 95) (mult:V16SF (reg:V16SF 87) (vec_duplicate:V16SF (reg:SF 91)))) commutation is performed at (set (reg:V16SF 95) (mult:V16SF (reg:V16SF 87) (reg:V16SF 89))) Here is the updated patch. OK for trunk? Thanks.
On Sat, Oct 20, 2018 at 11:47 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > On 10/20/18, Uros Bizjak <ubizjak@gmail.com> wrote: > > On Fri, Oct 19, 2018 at 11:08 PM H.J. Lu <hjl.tools@gmail.com> wrote: > >> > >> Many AVX512 vector operations can broadcast from a scalar memory source. > >> This patch enables memory broadcast for FP mul operations. > >> > >> gcc/ > >> > >> PR target/72782 > >> * config/i386/sse.md (*mul<mode>3<mask_name>_bcst_1): New. > >> (*mul<mode>3<mask_name>_bcst_2): Likewise. > >> > >> gcc/testsuite/ > >> > >> PR target/72782 > >> * gcc.target/i386/avx512f-mul-df-zmm-1.c: New test. > >> * gcc.target/i386/avx512f-mul-sf-zmm-1.c: Likewise. > >> * gcc.target/i386/avx512f-mul-sf-zmm-2.c: Likewise. > >> * gcc.target/i386/avx512f-mul-sf-zmm-3.c: Likewise. > >> * gcc.target/i386/avx512f-mul-sf-zmm-4.c: Likewise. > >> * gcc.target/i386/avx512f-mul-sf-zmm-5.c: Likewise. > >> * gcc.target/i386/avx512f-mul-sf-zmm-6.c: Likewise. > >> * gcc.target/i386/avx512vl-mul-sf-xmm-1.c: Likewise. > >> * gcc.target/i386/avx512vl-mul-sf-ymm-1.c: Likewise. > >> --- > >> gcc/config/i386/sse.md | 24 +++++++++++++++++++ > >> .../gcc.target/i386/avx512f-mul-df-zmm-1.c | 12 ++++++++++ > >> .../gcc.target/i386/avx512f-mul-sf-zmm-1.c | 12 ++++++++++ > >> .../gcc.target/i386/avx512f-mul-sf-zmm-2.c | 12 ++++++++++ > >> .../gcc.target/i386/avx512f-mul-sf-zmm-3.c | 12 ++++++++++ > >> .../gcc.target/i386/avx512f-mul-sf-zmm-4.c | 12 ++++++++++ > >> .../gcc.target/i386/avx512f-mul-sf-zmm-5.c | 12 ++++++++++ > >> .../gcc.target/i386/avx512f-mul-sf-zmm-6.c | 12 ++++++++++ > >> .../gcc.target/i386/avx512vl-mul-sf-xmm-1.c | 12 ++++++++++ > >> .../gcc.target/i386/avx512vl-mul-sf-ymm-1.c | 12 ++++++++++ > >> 10 files changed, 132 insertions(+) > >> create mode 100644 gcc/testsuite/gcc.target/i386/avx512f-mul-df-zmm-1.c > >> create mode 100644 gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-1.c > >> create mode 100644 gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-2.c > >> create mode 100644 gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-3.c > >> create mode 100644 gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-4.c > >> create mode 100644 gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-5.c > >> create mode 100644 gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-6.c > >> create mode 100644 gcc/testsuite/gcc.target/i386/avx512vl-mul-sf-xmm-1.c > >> create mode 100644 gcc/testsuite/gcc.target/i386/avx512vl-mul-sf-ymm-1.c > >> > >> diff --git a/gcc/config/i386/sse.md b/gcc/config/i386/sse.md > >> index 411c78ae8d3..a73659e6bd2 100644 > >> --- a/gcc/config/i386/sse.md > >> +++ b/gcc/config/i386/sse.md > >> @@ -1754,6 +1754,30 @@ > >> (set_attr "btver2_decode" "direct,double") > >> (set_attr "mode" "<MODE>")]) > >> > >> +(define_insn "*mul<mode>3<mask_name>_bcst_1" > >> + [(set (match_operand:VF_AVX512 0 "register_operand" "=v") > >> + (mult:VF_AVX512 > >> + (match_operand:VF_AVX512 1 "register_operand" "v") > >> + (vec_duplicate:VF_AVX512 > >> + (match_operand:<ssescalarmode> 2 "memory_operand" "m"))))] > >> + "TARGET_AVX512F && <mask_mode512bit_condition>" > >> + "vmul<ssemodesuffix>\t{%2<avx512bcst>, %1, > >> %0<mask_operand3>|%0<mask_operand3>, %1, %2<<avx512bcst>>}" > >> + [(set_attr "prefix" "evex") > >> + (set_attr "type" "ssemul") > >> + (set_attr "mode" "<MODE>")]) > >> + > >> +(define_insn "*mul<mode>3<mask_name>_bcst_2" > >> + [(set (match_operand:VF_AVX512 0 "register_operand" "=v") > >> + (mult:VF_AVX512 > >> + (vec_duplicate:VF_AVX512 > >> + (match_operand:<ssescalarmode> 1 "memory_operand" "m")) > >> + (match_operand:VF_AVX512 2 "register_operand" "v")))] > >> + "TARGET_AVX512F && <mask_mode512bit_condition>" > >> + "vmul<ssemodesuffix>\t{%1<avx512bcst>, %2, > >> %0<mask_operand3>|%0<mask_operand3>, %2, %1<<avx512bcst>>}" > >> + [(set_attr "prefix" "evex") > >> + (set_attr "type" "ssemul") > >> + (set_attr "mode" "<MODE>")]) > > > > Do we really need two patterns here? IIRC, the compiler canonicalizes > > commutative binops so that they have memory operand in the second > > place. We have vec_duplicate here, so this may not be the case, but > > please investigate if we really need two patterns for commutative > > binops. > > > > Only one pattern is needed. For > > (set (reg:V16SF 89) (vec_duplicate:V16SF (reg:SF 91))) > (set (reg:V16SF 95) (mult:V16SF (reg:V16SF 87) (reg:V16SF 89))) > > combiner prefers > > (set (reg:V16SF 95) > (mult:V16SF > (vec_duplicate:V16SF (reg:SF 91)) > (reg:V16SF 87))) > > instead of > > (set (reg:V16SF 95) > (mult:V16SF > (reg:V16SF 87) > (vec_duplicate:V16SF (reg:SF 91)))) > > commutation is performed at > > (set (reg:V16SF 95) (mult:V16SF (reg:V16SF 87) (reg:V16SF 89))) > > Here is the updated patch. OK for trunk? No need for a big comment, this is due to RTX operator precedence in commutative operators.. OK with the above change. Please also remove plus part from *<plusminus_insn><mode>3<mask_name>_bcst_1 and rename it together with *add<mode>3<mask_name>_bcst_2 to ..._bcst, without suffix. Thanks, Uros. > > -- > H.J.
On 10/21/18, Uros Bizjak <ubizjak@gmail.com> wrote: > On Sat, Oct 20, 2018 at 11:47 PM H.J. Lu <hjl.tools@gmail.com> wrote: >> >> On 10/20/18, Uros Bizjak <ubizjak@gmail.com> wrote: >> > On Fri, Oct 19, 2018 at 11:08 PM H.J. Lu <hjl.tools@gmail.com> wrote: >> >> >> >> Many AVX512 vector operations can broadcast from a scalar memory >> >> source. >> >> This patch enables memory broadcast for FP mul operations. >> >> >> >> gcc/ >> >> >> >> PR target/72782 >> >> * config/i386/sse.md (*mul<mode>3<mask_name>_bcst_1): New. >> >> (*mul<mode>3<mask_name>_bcst_2): Likewise. >> >> >> >> gcc/testsuite/ >> >> >> >> PR target/72782 >> >> * gcc.target/i386/avx512f-mul-df-zmm-1.c: New test. >> >> * gcc.target/i386/avx512f-mul-sf-zmm-1.c: Likewise. >> >> * gcc.target/i386/avx512f-mul-sf-zmm-2.c: Likewise. >> >> * gcc.target/i386/avx512f-mul-sf-zmm-3.c: Likewise. >> >> * gcc.target/i386/avx512f-mul-sf-zmm-4.c: Likewise. >> >> * gcc.target/i386/avx512f-mul-sf-zmm-5.c: Likewise. >> >> * gcc.target/i386/avx512f-mul-sf-zmm-6.c: Likewise. >> >> * gcc.target/i386/avx512vl-mul-sf-xmm-1.c: Likewise. >> >> * gcc.target/i386/avx512vl-mul-sf-ymm-1.c: Likewise. >> >> --- >> >> gcc/config/i386/sse.md | 24 >> >> +++++++++++++++++++ >> >> .../gcc.target/i386/avx512f-mul-df-zmm-1.c | 12 ++++++++++ >> >> .../gcc.target/i386/avx512f-mul-sf-zmm-1.c | 12 ++++++++++ >> >> .../gcc.target/i386/avx512f-mul-sf-zmm-2.c | 12 ++++++++++ >> >> .../gcc.target/i386/avx512f-mul-sf-zmm-3.c | 12 ++++++++++ >> >> .../gcc.target/i386/avx512f-mul-sf-zmm-4.c | 12 ++++++++++ >> >> .../gcc.target/i386/avx512f-mul-sf-zmm-5.c | 12 ++++++++++ >> >> .../gcc.target/i386/avx512f-mul-sf-zmm-6.c | 12 ++++++++++ >> >> .../gcc.target/i386/avx512vl-mul-sf-xmm-1.c | 12 ++++++++++ >> >> .../gcc.target/i386/avx512vl-mul-sf-ymm-1.c | 12 ++++++++++ >> >> 10 files changed, 132 insertions(+) >> >> create mode 100644 >> >> gcc/testsuite/gcc.target/i386/avx512f-mul-df-zmm-1.c >> >> create mode 100644 >> >> gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-1.c >> >> create mode 100644 >> >> gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-2.c >> >> create mode 100644 >> >> gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-3.c >> >> create mode 100644 >> >> gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-4.c >> >> create mode 100644 >> >> gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-5.c >> >> create mode 100644 >> >> gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-6.c >> >> create mode 100644 >> >> gcc/testsuite/gcc.target/i386/avx512vl-mul-sf-xmm-1.c >> >> create mode 100644 >> >> gcc/testsuite/gcc.target/i386/avx512vl-mul-sf-ymm-1.c >> >> >> >> diff --git a/gcc/config/i386/sse.md b/gcc/config/i386/sse.md >> >> index 411c78ae8d3..a73659e6bd2 100644 >> >> --- a/gcc/config/i386/sse.md >> >> +++ b/gcc/config/i386/sse.md >> >> @@ -1754,6 +1754,30 @@ >> >> (set_attr "btver2_decode" "direct,double") >> >> (set_attr "mode" "<MODE>")]) >> >> >> >> +(define_insn "*mul<mode>3<mask_name>_bcst_1" >> >> + [(set (match_operand:VF_AVX512 0 "register_operand" "=v") >> >> + (mult:VF_AVX512 >> >> + (match_operand:VF_AVX512 1 "register_operand" "v") >> >> + (vec_duplicate:VF_AVX512 >> >> + (match_operand:<ssescalarmode> 2 "memory_operand" >> >> "m"))))] >> >> + "TARGET_AVX512F && <mask_mode512bit_condition>" >> >> + "vmul<ssemodesuffix>\t{%2<avx512bcst>, %1, >> >> %0<mask_operand3>|%0<mask_operand3>, %1, %2<<avx512bcst>>}" >> >> + [(set_attr "prefix" "evex") >> >> + (set_attr "type" "ssemul") >> >> + (set_attr "mode" "<MODE>")]) >> >> + >> >> +(define_insn "*mul<mode>3<mask_name>_bcst_2" >> >> + [(set (match_operand:VF_AVX512 0 "register_operand" "=v") >> >> + (mult:VF_AVX512 >> >> + (vec_duplicate:VF_AVX512 >> >> + (match_operand:<ssescalarmode> 1 "memory_operand" "m")) >> >> + (match_operand:VF_AVX512 2 "register_operand" "v")))] >> >> + "TARGET_AVX512F && <mask_mode512bit_condition>" >> >> + "vmul<ssemodesuffix>\t{%1<avx512bcst>, %2, >> >> %0<mask_operand3>|%0<mask_operand3>, %2, %1<<avx512bcst>>}" >> >> + [(set_attr "prefix" "evex") >> >> + (set_attr "type" "ssemul") >> >> + (set_attr "mode" "<MODE>")]) >> > >> > Do we really need two patterns here? IIRC, the compiler canonicalizes >> > commutative binops so that they have memory operand in the second >> > place. We have vec_duplicate here, so this may not be the case, but >> > please investigate if we really need two patterns for commutative >> > binops. >> > >> >> Only one pattern is needed. For >> >> (set (reg:V16SF 89) (vec_duplicate:V16SF (reg:SF 91))) >> (set (reg:V16SF 95) (mult:V16SF (reg:V16SF 87) (reg:V16SF 89))) >> >> combiner prefers >> >> (set (reg:V16SF 95) >> (mult:V16SF >> (vec_duplicate:V16SF (reg:SF 91)) >> (reg:V16SF 87))) >> >> instead of >> >> (set (reg:V16SF 95) >> (mult:V16SF >> (reg:V16SF 87) >> (vec_duplicate:V16SF (reg:SF 91)))) >> >> commutation is performed at >> >> (set (reg:V16SF 95) (mult:V16SF (reg:V16SF 87) (reg:V16SF 89))) >> >> Here is the updated patch. OK for trunk? > > No need for a big comment, this is due to RTX operator precedence in > commutative operators.. > > OK with the above change. Checked in. > Please also remove plus part from > > *<plusminus_insn><mode>3<mask_name>_bcst_1 > > and rename it together with > > *add<mode>3<mask_name>_bcst_2 > > to ..._bcst, without suffix. > This is the patch I am checking in.
diff --git a/gcc/config/i386/sse.md b/gcc/config/i386/sse.md index 411c78ae8d3..a73659e6bd2 100644 --- a/gcc/config/i386/sse.md +++ b/gcc/config/i386/sse.md @@ -1754,6 +1754,30 @@ (set_attr "btver2_decode" "direct,double") (set_attr "mode" "<MODE>")]) +(define_insn "*mul<mode>3<mask_name>_bcst_1" + [(set (match_operand:VF_AVX512 0 "register_operand" "=v") + (mult:VF_AVX512 + (match_operand:VF_AVX512 1 "register_operand" "v") + (vec_duplicate:VF_AVX512 + (match_operand:<ssescalarmode> 2 "memory_operand" "m"))))] + "TARGET_AVX512F && <mask_mode512bit_condition>" + "vmul<ssemodesuffix>\t{%2<avx512bcst>, %1, %0<mask_operand3>|%0<mask_operand3>, %1, %2<<avx512bcst>>}" + [(set_attr "prefix" "evex") + (set_attr "type" "ssemul") + (set_attr "mode" "<MODE>")]) + +(define_insn "*mul<mode>3<mask_name>_bcst_2" + [(set (match_operand:VF_AVX512 0 "register_operand" "=v") + (mult:VF_AVX512 + (vec_duplicate:VF_AVX512 + (match_operand:<ssescalarmode> 1 "memory_operand" "m")) + (match_operand:VF_AVX512 2 "register_operand" "v")))] + "TARGET_AVX512F && <mask_mode512bit_condition>" + "vmul<ssemodesuffix>\t{%1<avx512bcst>, %2, %0<mask_operand3>|%0<mask_operand3>, %2, %1<<avx512bcst>>}" + [(set_attr "prefix" "evex") + (set_attr "type" "ssemul") + (set_attr "mode" "<MODE>")]) + (define_insn "<sse>_vm<multdiv_mnemonic><mode>3<mask_scalar_name><round_scalar_name>" [(set (match_operand:VF_128 0 "register_operand" "=x,v") (vec_merge:VF_128 diff --git a/gcc/testsuite/gcc.target/i386/avx512f-mul-df-zmm-1.c b/gcc/testsuite/gcc.target/i386/avx512f-mul-df-zmm-1.c new file mode 100644 index 00000000000..e3c51986fe2 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/avx512f-mul-df-zmm-1.c @@ -0,0 +1,12 @@ +/* { dg-do compile } */ +/* { dg-options "-mavx512f -O2" } */ +/* { dg-final { scan-assembler-times "vmulpd\[ \\t\]+\\(%(?:eax|rdi|edi)\\)\\\{1to\[1-8\]+\\\}, %zmm\[0-9\]+, %zmm0" 1 } } */ +/* { dg-final { scan-assembler-not "vbroadcastsd\[^\n\]*%zmm\[0-9\]+" } } */ + +#define type __m512d +#define vec 512 +#define op mul +#define suffix pd +#define SCALAR double + +#include "avx512-binop-1.h" diff --git a/gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-1.c b/gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-1.c new file mode 100644 index 00000000000..14bccca276a --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-1.c @@ -0,0 +1,12 @@ +/* { dg-do compile } */ +/* { dg-options "-mavx512f -O2" } */ +/* { dg-final { scan-assembler-times "vmulps\[ \\t\]+\\(%(?:eax|rdi|edi)\\)\\\{1to\[1-8\]+\\\}, %zmm\[0-9\]+, %zmm0" 1 } } */ +/* { dg-final { scan-assembler-not "vbroadcastss\[^\n\]*%zmm\[0-9\]+" } } */ + +#define type __m512 +#define vec 512 +#define op mul +#define suffix ps +#define SCALAR float + +#include "avx512-binop-1.h" diff --git a/gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-2.c b/gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-2.c new file mode 100644 index 00000000000..8293324084b --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-2.c @@ -0,0 +1,12 @@ +/* { dg-do compile } */ +/* { dg-options "-mavx512f -O2" } */ +/* { dg-final { scan-assembler-times "vmulps\[ \\t\]+\\(%(?:eax|rdi|edi)\\)\\\{1to\[1-8\]+\\\}, %zmm\[0-9\]+, %zmm0" 1 } } */ +/* { dg-final { scan-assembler-not "vbroadcastss\[^\n\]*%zmm\[0-9\]+" } } */ + +#define type __m512 +#define vec 512 +#define op mul +#define suffix ps +#define SCALAR float + +#include "avx512-binop-2.h" diff --git a/gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-3.c b/gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-3.c new file mode 100644 index 00000000000..cb768db2cb4 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-3.c @@ -0,0 +1,12 @@ +/* { dg-do compile } */ +/* { dg-options "-mavx512f -O2" } */ +/* { dg-final { scan-assembler-times "vbroadcastss\[^\n\]*%zmm\[0-9\]+" 1 } } */ +/* { dg-final { scan-assembler-times "vmulps\[^\n\]*%zmm\[0-9\]+" 1 } } */ + +#define type __m512 +#define vec 512 +#define op mul +#define suffix ps +#define SCALAR float + +#include "avx512-binop-3.h" diff --git a/gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-4.c b/gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-4.c new file mode 100644 index 00000000000..7626192ed3d --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-4.c @@ -0,0 +1,12 @@ +/* { dg-do compile { target { ! ia32 } } } */ +/* { dg-options "-mavx512f -O2" } */ +/* { dg-final { scan-assembler-times "vbroadcastss\[^\n\]*%zmm\[0-9\]+" 1 } } */ +/* { dg-final { scan-assembler-times "vmulps\[ \\t\]+%zmm\[0-9\]+, %zmm\[0-9\]+, %zmm0" 1 } } */ + +#define type __m512 +#define vec 512 +#define op mul +#define suffix ps +#define SCALAR float + +#include "avx512-binop-4.h" diff --git a/gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-5.c b/gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-5.c new file mode 100644 index 00000000000..b2ad8054d98 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-5.c @@ -0,0 +1,12 @@ +/* { dg-do compile } */ +/* { dg-options "-mavx512f -O2" } */ +/* { dg-final { scan-assembler-times "vmulps\[ \\t\]+\[^\n\]*\\\{1to\[1-8\]+\\\}, %zmm\[0-9\]+, %zmm0" 1 } } */ +/* { dg-final { scan-assembler-not "vbroadcastss\[^\n\]*%zmm\[0-9\]+" } } */ + +#define type __m512 +#define vec 512 +#define op mul +#define suffix ps +#define SCALAR float + +#include "avx512-binop-5.h" diff --git a/gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-6.c b/gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-6.c new file mode 100644 index 00000000000..d8dde860fe7 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/avx512f-mul-sf-zmm-6.c @@ -0,0 +1,12 @@ +/* { dg-do compile } */ +/* { dg-options "-mavx512f -O2" } */ +/* { dg-final { scan-assembler-times "vmulps\[ \\t\]+\[^\n\]*\\\{1to\[1-8\]+\\\}, %zmm\[0-9\]+, %zmm0" 1 } } */ +/* { dg-final { scan-assembler-not "vbroadcastss\[^\n\]*%zmm\[0-9\]+" } } */ + +#define type __m512 +#define vec 512 +#define op mul +#define suffix ps +#define SCALAR float + +#include "avx512-binop-6.h" diff --git a/gcc/testsuite/gcc.target/i386/avx512vl-mul-sf-xmm-1.c b/gcc/testsuite/gcc.target/i386/avx512vl-mul-sf-xmm-1.c new file mode 100644 index 00000000000..395cccdc78b --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/avx512vl-mul-sf-xmm-1.c @@ -0,0 +1,12 @@ +/* { dg-do compile } */ +/* { dg-options "-mfma -mavx512vl -O2" } */ +/* { dg-final { scan-assembler-times "vmulps\[ \\t\]+\\(%(?:eax|rdi|edi)\\)\\\{1to\[1-8\]+\\\}, %xmm\[0-9\]+, %xmm0" 1 } } */ +/* { dg-final { scan-assembler-not "vbroadcastss\[^\n\]*%xmm\[0-9\]+" } } */ + +#define type __m128 +#define vec +#define op mul +#define suffix ps +#define SCALAR float + +#include "avx512-binop-1.h" diff --git a/gcc/testsuite/gcc.target/i386/avx512vl-mul-sf-ymm-1.c b/gcc/testsuite/gcc.target/i386/avx512vl-mul-sf-ymm-1.c new file mode 100644 index 00000000000..2a1989381bf --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/avx512vl-mul-sf-ymm-1.c @@ -0,0 +1,12 @@ +/* { dg-do compile } */ +/* { dg-options "-mfma -mavx512vl -O2" } */ +/* { dg-final { scan-assembler-times "vmulps\[ \\t\]+\\(%(?:eax|rdi|edi)\\)\\\{1to\[1-8\]+\\\}, %ymm\[0-9\]+, %ymm0" 1 } } */ +/* { dg-final { scan-assembler-not "vbroadcastss\[^\n\]*%ymm\[0-9\]+" } } */ + +#define type __m256 +#define vec 256 +#define op mul +#define suffix ps +#define SCALAR float + +#include "avx512-binop-1.h"