From fbbc4c24fd7ba87e0c47cd965ae624afba6fa375 Mon Sep 17 00:00:00 2001 From: Jakub Jelinek Date: Wed, 15 Jan 2020 00:30:10 +0100 Subject: [PATCH] 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, %2, %0 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\t{%3, %2, %0|%0, %2, %3} vfmadd213\t{%3, %1, %0|%0, %1, %3}" 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 PR target/93009 * config/i386/sse.md (*fma_fmadd__bcst_1, *fma_fmsub__bcst_1, *fma_fnmadd__bcst_1, *fma_fnmsub__bcst_1): Use just a single alternative instead of two, make operands 1 and 2 commutative. * gcc.target/i386/avx512vl-pr93009.c: New test. --- gcc/ChangeLog | 11 ++++++ gcc/config/i386/sse.md | 32 ++++++++-------- gcc/testsuite/ChangeLog | 5 +++ .../gcc.target/i386/avx512vl-pr93009.c | 38 +++++++++++++++++++ 4 files changed, 70 insertions(+), 16 deletions(-) create mode 100644 gcc/testsuite/gcc.target/i386/avx512vl-pr93009.c diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 33ca91a6467..356bc63c6d0 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,14 @@ +2020-01-15 Jakub Jelinek + + PR target/93009 + * config/i386/sse.md + (*fma_fmadd__bcst_1, + *fma_fmsub__bcst_1, + *fma_fnmadd__bcst_1, + *fma_fnmsub__bcst_1): Use + just a single alternative instead of two, make operands 1 and 2 + commutative. + 2020-01-14 Jan Hubicka PR lto/91576 diff --git a/gcc/config/i386/sse.md b/gcc/config/i386/sse.md index f4c8a3911c7..b8d41b7ed54 100644 --- a/gcc/config/i386/sse.md +++ b/gcc/config/i386/sse.md @@ -4172,12 +4172,12 @@ (set_attr "mode" "")]) (define_insn "*fma_fmadd__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: 3 "memory_operand" "m,m"))))] + (match_operand: 3 "memory_operand" "m"))))] "TARGET_AVX512F && " "vfmadd213\t{%3, %2, %0|%0, %2, %3}" [(set_attr "type" "ssemuladd") @@ -4289,13 +4289,13 @@ (set_attr "mode" "")]) (define_insn "*fma_fmsub__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: 3 "memory_operand" "m,m")))))] + (match_operand: 3 "memory_operand" "m")))))] "TARGET_AVX512F && " "vfmsub213\t{%3, %2, %0|%0, %2, %3}" [(set_attr "type" "ssemuladd") @@ -4411,13 +4411,13 @@ (set_attr "mode" "")]) (define_insn "*fma_fnmadd__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: 3 "memory_operand" "m,m"))))] + (match_operand: 3 "memory_operand" "m"))))] "TARGET_AVX512F && " "vfnmadd213\t{%3, %2, %0|%0, %2, %3}" [(set_attr "type" "ssemuladd") @@ -4535,14 +4535,14 @@ (set_attr "mode" "")]) (define_insn "*fma_fnmsub__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: 3 "memory_operand" "m,m")))))] + (match_operand: 3 "memory_operand" "m")))))] "TARGET_AVX512F && " "vfnmsub213\t{%3, %2, %0|%0, %2, %3}" [(set_attr "type" "ssemuladd") diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index dc42601794b..ea37d69fa88 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2020-01-15 Jakub Jelinek + + PR target/93009 + * gcc.target/i386/avx512vl-pr93009.c: New test. + 2020-01-14 Jan Hubicka 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 00000000000..4dfc4a9f3c3 --- /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 (); +} -- 2.30.2