aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJakub Jelinek <jakub@redhat.com>2020-01-15 00:30:10 +0100
committerJakub Jelinek <jakub@redhat.com>2020-01-15 00:30:10 +0100
commitfbbc4c24fd7ba87e0c47cd965ae624afba6fa375 (patch)
tree8175b3ddc03ae580e8dde005c60d4b41c09c0aa3
parentf474fbd5e3cca37ebc886a4950827e93d1c665c8 (diff)
downloadgcc-fbbc4c24fd7ba87e0c47cd965ae624afba6fa375.zip
gcc-fbbc4c24fd7ba87e0c47cd965ae624afba6fa375.tar.gz
gcc-fbbc4c24fd7ba87e0c47cd965ae624afba6fa375.tar.bz2
i386: Fix wrong-code x86 issue with avx512{f,vl} fma PR93009
As mentioned in the PR, the following testcase is miscompiled with avx512vl. The reason is that the fma *_bcst_1 define_insns have two alternatives: "=v,v" "0,v" "v,0" "m,m" and use the same vfmadd213* %3<avx512bcst>, %2, %0<sd_mask_op4> pattern. If the first alternative is chosen, everything is ok, but if the second alternative is chosen, %2 and %0 are the same register, so instead of doing dest=dest*another+membcst we do dest=dest*dest+membcst. Now, to fix this, either we'd need separate: "vfmadd213<ssemodesuffix>\t{%3<avx512bcst>, %2, %0<sd_mask_op4>|%0<sd_mask_op4>, %2, %3<avx512bcst>} vfmadd213<ssemodesuffix>\t{%3<avx512bcst>, %1, %0<sd_mask_op4>|%0<sd_mask_op4>, %1, %3<avx512bcst>}" where for the second alternative, we'd just use %1 instead of %2, but what I think is actually cleaner is just use a single alternative and make the two multiplication operands commutative, which they really are. 2020-01-15 Jakub Jelinek <jakub@redhat.com> PR target/93009 * config/i386/sse.md (*<sd_mask_codefor>fma_fmadd_<mode><sd_maskz_name>_bcst_1, *<sd_mask_codefor>fma_fmsub_<mode><sd_maskz_name>_bcst_1, *<sd_mask_codefor>fma_fnmadd_<mode><sd_maskz_name>_bcst_1, *<sd_mask_codefor>fma_fnmsub_<mode><sd_maskz_name>_bcst_1): Use just a single alternative instead of two, make operands 1 and 2 commutative. * gcc.target/i386/avx512vl-pr93009.c: New test.
-rw-r--r--gcc/ChangeLog11
-rw-r--r--gcc/config/i386/sse.md32
-rw-r--r--gcc/testsuite/ChangeLog5
-rw-r--r--gcc/testsuite/gcc.target/i386/avx512vl-pr93009.c38
4 files changed, 70 insertions, 16 deletions
diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 33ca91a..356bc63 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,3 +1,14 @@
+2020-01-15 Jakub Jelinek <jakub@redhat.com>
+
+ PR target/93009
+ * config/i386/sse.md
+ (*<sd_mask_codefor>fma_fmadd_<mode><sd_maskz_name>_bcst_1,
+ *<sd_mask_codefor>fma_fmsub_<mode><sd_maskz_name>_bcst_1,
+ *<sd_mask_codefor>fma_fnmadd_<mode><sd_maskz_name>_bcst_1,
+ *<sd_mask_codefor>fma_fnmsub_<mode><sd_maskz_name>_bcst_1): Use
+ just a single alternative instead of two, make operands 1 and 2
+ commutative.
+
2020-01-14 Jan Hubicka <hubicka@ucw.cz>
PR lto/91576
diff --git a/gcc/config/i386/sse.md b/gcc/config/i386/sse.md
index f4c8a39..b8d41b7 100644
--- a/gcc/config/i386/sse.md
+++ b/gcc/config/i386/sse.md
@@ -4172,12 +4172,12 @@
(set_attr "mode" "<MODE>")])
(define_insn "*<sd_mask_codefor>fma_fmadd_<mode><sd_maskz_name>_bcst_1"
- [(set (match_operand:VF_AVX512 0 "register_operand" "=v,v")
+ [(set (match_operand:VF_AVX512 0 "register_operand" "=v")
(fma:VF_AVX512
- (match_operand:VF_AVX512 1 "register_operand" "0,v")
- (match_operand:VF_AVX512 2 "register_operand" "v,0")
+ (match_operand:VF_AVX512 1 "register_operand" "%0")
+ (match_operand:VF_AVX512 2 "register_operand" "v")
(vec_duplicate:VF_AVX512
- (match_operand:<ssescalarmode> 3 "memory_operand" "m,m"))))]
+ (match_operand:<ssescalarmode> 3 "memory_operand" "m"))))]
"TARGET_AVX512F && <sd_mask_mode512bit_condition>"
"vfmadd213<ssemodesuffix>\t{%3<avx512bcst>, %2, %0<sd_mask_op4>|%0<sd_mask_op4>, %2, %3<avx512bcst>}"
[(set_attr "type" "ssemuladd")
@@ -4289,13 +4289,13 @@
(set_attr "mode" "<MODE>")])
(define_insn "*<sd_mask_codefor>fma_fmsub_<mode><sd_maskz_name>_bcst_1"
- [(set (match_operand:VF_AVX512 0 "register_operand" "=v,v")
+ [(set (match_operand:VF_AVX512 0 "register_operand" "=v")
(fma:VF_AVX512
- (match_operand:VF_AVX512 1 "register_operand" "0,v")
- (match_operand:VF_AVX512 2 "register_operand" "v,0")
+ (match_operand:VF_AVX512 1 "register_operand" "%0")
+ (match_operand:VF_AVX512 2 "register_operand" "v")
(neg:VF_AVX512
(vec_duplicate:VF_AVX512
- (match_operand:<ssescalarmode> 3 "memory_operand" "m,m")))))]
+ (match_operand:<ssescalarmode> 3 "memory_operand" "m")))))]
"TARGET_AVX512F && <sd_mask_mode512bit_condition>"
"vfmsub213<ssemodesuffix>\t{%3<avx512bcst>, %2, %0<sd_mask_op4>|%0<sd_mask_op4>, %2, %3<avx512bcst>}"
[(set_attr "type" "ssemuladd")
@@ -4411,13 +4411,13 @@
(set_attr "mode" "<MODE>")])
(define_insn "*<sd_mask_codefor>fma_fnmadd_<mode><sd_maskz_name>_bcst_1"
- [(set (match_operand:VF_AVX512 0 "register_operand" "=v,v")
+ [(set (match_operand:VF_AVX512 0 "register_operand" "=v")
(fma:VF_AVX512
(neg:VF_AVX512
- (match_operand:VF_AVX512 1 "register_operand" "0,v"))
- (match_operand:VF_AVX512 2 "register_operand" "v,0")
+ (match_operand:VF_AVX512 1 "register_operand" "%0"))
+ (match_operand:VF_AVX512 2 "register_operand" "v")
(vec_duplicate:VF_AVX512
- (match_operand:<ssescalarmode> 3 "memory_operand" "m,m"))))]
+ (match_operand:<ssescalarmode> 3 "memory_operand" "m"))))]
"TARGET_AVX512F && <sd_mask_mode512bit_condition>"
"vfnmadd213<ssemodesuffix>\t{%3<avx512bcst>, %2, %0<sd_mask_op4>|%0<sd_mask_op4>, %2, %3<avx512bcst>}"
[(set_attr "type" "ssemuladd")
@@ -4535,14 +4535,14 @@
(set_attr "mode" "<MODE>")])
(define_insn "*<sd_mask_codefor>fma_fnmsub_<mode><sd_maskz_name>_bcst_1"
- [(set (match_operand:VF_AVX512 0 "register_operand" "=v,v")
+ [(set (match_operand:VF_AVX512 0 "register_operand" "=v")
(fma:VF_AVX512
(neg:VF_AVX512
- (match_operand:VF_AVX512 1 "register_operand" "0,v"))
- (match_operand:VF_AVX512 2 "register_operand" "v,0")
+ (match_operand:VF_AVX512 1 "register_operand" "%0"))
+ (match_operand:VF_AVX512 2 "register_operand" "v")
(neg:VF_AVX512
(vec_duplicate:VF_AVX512
- (match_operand:<ssescalarmode> 3 "memory_operand" "m,m")))))]
+ (match_operand:<ssescalarmode> 3 "memory_operand" "m")))))]
"TARGET_AVX512F && <sd_mask_mode512bit_condition>"
"vfnmsub213<ssemodesuffix>\t{%3<avx512bcst>, %2, %0<sd_mask_op4>|%0<sd_mask_op4>, %2, %3<avx512bcst>}"
[(set_attr "type" "ssemuladd")
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index dc42601..ea37d69 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,8 @@
+2020-01-15 Jakub Jelinek <jakub@redhat.com>
+
+ PR target/93009
+ * gcc.target/i386/avx512vl-pr93009.c: New test.
+
2020-01-14 Jan Hubicka <hubicka@ucw.cz>
PR lto/91576
diff --git a/gcc/testsuite/gcc.target/i386/avx512vl-pr93009.c b/gcc/testsuite/gcc.target/i386/avx512vl-pr93009.c
new file mode 100644
index 0000000..4dfc4a9
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/avx512vl-pr93009.c
@@ -0,0 +1,38 @@
+/* PR target/93009 */
+/* { dg-do run { target { avx512vl } } } */
+/* { dg-options "-O2 -mavx512vl" } */
+
+#define AVX512VL
+#define AVX512F_LEN 512
+#define AVX512F_LEN_HALF 256
+
+#include "avx512f-check.h"
+
+typedef double v2df __attribute__((vector_size (16)));
+
+__attribute__((noipa)) v2df
+foo (v2df x, v2df y, double *z)
+{
+ return x * y + (v2df) { z[0], z[0] };
+}
+
+__attribute__((noipa)) v2df
+bar (v2df x, v2df y, double *z)
+{
+ return y * x + (v2df) { z[0], z[0] };
+}
+
+static void
+test_256 (void)
+{
+}
+
+static void
+test_128 (void)
+{
+ double z = 5.0;
+ v2df x = foo ((v2df) { 1.0, 2.0 }, (v2df) { 3.0, 4.0 }, &z);
+ v2df y = bar ((v2df) { 6.0, 7.0 }, (v2df) { 8.0, 9.0 }, &z);
+ if (x[0] != 8.0 || x[1] != 13.0 || y[0] != 53.0 || y[1] != 68.0)
+ abort ();
+}