* [PATCH] x86: Allow CONST_VECTOR for vector load in combine @ 2021-08-22 12:53 H.J. Lu 2021-08-23 7:23 ` Hongtao Liu 0 siblings, 1 reply; 7+ messages in thread From: H.J. Lu @ 2021-08-22 12:53 UTC (permalink / raw) To: gcc-patches; +Cc: Uros Bizjak, liuhongt In vetor move pattern, replace nonimmediate_or_sse_const_operand with nonimmediate_or_sse_const_vector_operand to allow vector load from non-uniform CONST_VECTOR. Non-uniform CONST_VECTOR is enabled only in the combine pass since other RTL optimizers work better with constant pool. gcc/ PR target/43147 * config/i386/i386-expand.c (ix86_constant_broadcast): New function. Extracted from ix86_broadcast_from_constant. (ix86_broadcast_from_constant): Call ix86_constant_broadcast. (non_uniform_const_vector_p): New function. * config/i386/i386-protos.h (non_uniform_const_vector_p): New prototype. * config/i386/predicates.md (nonimmediate_or_sse_const_vector_operand): New predicate. * config/i386/sse.md (mov<mode>_internal): Replace nonimmediate_or_sse_const_operand with nonimmediate_or_sse_const_vector_operand. gcc/testsuite/ PR target/43147 * gcc.target/i386/pr43147.c: New test. --- gcc/config/i386/i386-expand.c | 74 ++++++++++++++----------- gcc/config/i386/i386-protos.h | 1 + gcc/config/i386/predicates.md | 7 +++ gcc/config/i386/sse.md | 2 +- gcc/testsuite/gcc.target/i386/pr43147.c | 15 +++++ 5 files changed, 67 insertions(+), 32 deletions(-) create mode 100644 gcc/testsuite/gcc.target/i386/pr43147.c diff --git a/gcc/config/i386/i386-expand.c b/gcc/config/i386/i386-expand.c index 9bf13dbfa92..1d8f3110310 100644 --- a/gcc/config/i386/i386-expand.c +++ b/gcc/config/i386/i386-expand.c @@ -453,31 +453,12 @@ ix86_expand_move (machine_mode mode, rtx operands[]) emit_insn (gen_rtx_SET (op0, op1)); } -/* OP is a memref of CONST_VECTOR, return scalar constant mem - if CONST_VECTOR is a vec_duplicate, else return NULL. */ +/* CONSTANT is a CONST_VECTOR, return scalar constant if CONST_VECTOR is + a vec_duplicate, else return nullptr. */ + static rtx -ix86_broadcast_from_constant (machine_mode mode, rtx op) +ix86_constant_broadcast (rtx constant, machine_mode mode) { - int nunits = GET_MODE_NUNITS (mode); - if (nunits < 2) - return nullptr; - - /* Don't use integer vector broadcast if we can't move from GPR to SSE - register directly. */ - if (!TARGET_INTER_UNIT_MOVES_TO_VEC - && INTEGRAL_MODE_P (mode)) - return nullptr; - - /* Convert CONST_VECTOR to a non-standard SSE constant integer - broadcast only if vector broadcast is available. */ - if (!(TARGET_AVX2 - || (TARGET_AVX - && (GET_MODE_INNER (mode) == SImode - || GET_MODE_INNER (mode) == DImode)) - || FLOAT_MODE_P (mode)) - || standard_sse_constant_p (op, mode)) - return nullptr; - /* Don't broadcast from a 64-bit integer constant in 32-bit mode. We can still put 64-bit integer constant in memory when avx512 embed broadcast is available. */ @@ -486,13 +467,6 @@ ix86_broadcast_from_constant (machine_mode mode, rtx op) || (GET_MODE_SIZE (mode) < 64 && !TARGET_AVX512VL))) return nullptr; - if (GET_MODE_INNER (mode) == TImode) - return nullptr; - - rtx constant = get_pool_constant (XEXP (op, 0)); - if (GET_CODE (constant) != CONST_VECTOR) - return nullptr; - /* There could be some rtx like (mem/u/c:V16QI (symbol_ref/u:DI ("*.LC1"))) but with "*.LC1" refer to V2DI constant vector. */ @@ -506,7 +480,7 @@ ix86_broadcast_from_constant (machine_mode mode, rtx op) rtx first = XVECEXP (constant, 0, 0); - for (int i = 1; i < nunits; ++i) + for (int i = 1; i < GET_MODE_NUNITS (mode); ++i) { rtx tmp = XVECEXP (constant, 0, i); /* Vector duplicate value. */ @@ -517,6 +491,44 @@ ix86_broadcast_from_constant (machine_mode mode, rtx op) return first; } +/* OP is a memref of CONST_VECTOR, return scalar constant mem + if CONST_VECTOR is a vec_duplicate, else return NULL. */ +static rtx +ix86_broadcast_from_constant (machine_mode mode, rtx op) +{ + int nunits = GET_MODE_NUNITS (mode); + if (nunits < 2) + return nullptr; + + /* Don't use integer vector broadcast if we can't move from GPR to SSE + register directly. */ + if (!TARGET_INTER_UNIT_MOVES_TO_VEC + && INTEGRAL_MODE_P (mode)) + return nullptr; + + if (GET_MODE_INNER (mode) == TImode) + return nullptr; + + rtx constant = get_pool_constant (XEXP (op, 0)); + if (GET_CODE (constant) != CONST_VECTOR) + return nullptr; + + return ix86_constant_broadcast (constant, mode); +} + +/* Return true if OP is a non-uniform CONST_VECTOR. */ + +bool +non_uniform_const_vector_p (rtx op, machine_mode mode) +{ + /* Allow non-uniform CONST_VECTOR only in the combine pass since other + RTL optimizers work better with constant pool. */ + return (current_pass + && current_pass->tv_id == TV_COMBINE + && GET_CODE (op) == CONST_VECTOR + && ix86_constant_broadcast (op, mode) == nullptr); +} + void ix86_expand_vector_move (machine_mode mode, rtx operands[]) { diff --git a/gcc/config/i386/i386-protos.h b/gcc/config/i386/i386-protos.h index 2fd13074c81..91d4c7cefb8 100644 --- a/gcc/config/i386/i386-protos.h +++ b/gcc/config/i386/i386-protos.h @@ -58,6 +58,7 @@ extern int standard_80387_constant_p (rtx); extern const char *standard_80387_constant_opcode (rtx); extern rtx standard_80387_constant_rtx (int); extern int standard_sse_constant_p (rtx, machine_mode); +extern bool non_uniform_const_vector_p (rtx, machine_mode); extern const char *standard_sse_constant_opcode (rtx_insn *, rtx *); extern bool ix86_standard_x87sse_constant_load_p (const rtx_insn *, rtx); extern bool ix86_pre_reload_split (void); diff --git a/gcc/config/i386/predicates.md b/gcc/config/i386/predicates.md index 9321f332ef9..435b22855c8 100644 --- a/gcc/config/i386/predicates.md +++ b/gcc/config/i386/predicates.md @@ -1176,6 +1176,13 @@ (define_predicate "nonimmediate_or_sse_const_operand" (ior (match_operand 0 "nonimmediate_operand") (match_test "standard_sse_constant_p (op, mode)"))) +;; Return true when OP is nonimmediate, standard SSE constant, or +;; non-uniform CONST_VECTOR. +(define_predicate "nonimmediate_or_sse_const_vector_operand" + (ior (match_operand 0 "nonimmediate_operand") + (match_test "standard_sse_constant_p (op, mode)") + (match_test "non_uniform_const_vector_p (op, mode)"))) + ;; Return true if OP is a register or a zero. (define_predicate "reg_or_0_operand" (ior (match_operand 0 "register_operand") diff --git a/gcc/config/i386/sse.md b/gcc/config/i386/sse.md index 13889687793..416ddcfcacc 100644 --- a/gcc/config/i386/sse.md +++ b/gcc/config/i386/sse.md @@ -1075,7 +1075,7 @@ (define_expand "mov<mode>" (define_insn "mov<mode>_internal" [(set (match_operand:VMOVE 0 "nonimmediate_operand" "=v,v ,v ,m") - (match_operand:VMOVE 1 "nonimmediate_or_sse_const_operand" + (match_operand:VMOVE 1 "nonimmediate_or_sse_const_vector_operand" " C,<sseconstm1>,vm,v"))] "TARGET_SSE && (register_operand (operands[0], <MODE>mode) diff --git a/gcc/testsuite/gcc.target/i386/pr43147.c b/gcc/testsuite/gcc.target/i386/pr43147.c new file mode 100644 index 00000000000..3c30f917c06 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr43147.c @@ -0,0 +1,15 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -msse2" } */ +/* { dg-final { scan-assembler "movaps" } } */ +/* { dg-final { scan-assembler-not "shufps" } } */ + +#include <x86intrin.h> + +__m128 +foo (void) +{ + __m128 m = _mm_set_ps(1.0f, 2.0f, 3.0f, 4.0f); + m = _mm_shuffle_ps(m, m, 0xC9); + m = _mm_shuffle_ps(m, m, 0x2D); + return m; +} -- 2.31.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] x86: Allow CONST_VECTOR for vector load in combine 2021-08-22 12:53 [PATCH] x86: Allow CONST_VECTOR for vector load in combine H.J. Lu @ 2021-08-23 7:23 ` Hongtao Liu 2021-08-23 13:14 ` [PATCH v2] " H.J. Lu 0 siblings, 1 reply; 7+ messages in thread From: Hongtao Liu @ 2021-08-23 7:23 UTC (permalink / raw) To: H.J. Lu; +Cc: GCC Patches, liuhongt On Sun, Aug 22, 2021 at 8:54 PM H.J. Lu via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > In vetor move pattern, replace nonimmediate_or_sse_const_operand with > nonimmediate_or_sse_const_vector_operand to allow vector load from > non-uniform CONST_VECTOR. Non-uniform CONST_VECTOR is enabled only in > the combine pass since other RTL optimizers work better with constant > pool. > > gcc/ > > PR target/43147 > * config/i386/i386-expand.c (ix86_constant_broadcast): New > function. Extracted from ix86_broadcast_from_constant. > (ix86_broadcast_from_constant): Call ix86_constant_broadcast. > (non_uniform_const_vector_p): New function. > * config/i386/i386-protos.h (non_uniform_const_vector_p): New > prototype. > * config/i386/predicates.md > (nonimmediate_or_sse_const_vector_operand): New predicate. > * config/i386/sse.md (mov<mode>_internal): Replace > nonimmediate_or_sse_const_operand with > nonimmediate_or_sse_const_vector_operand. > > gcc/testsuite/ > > PR target/43147 > * gcc.target/i386/pr43147.c: New test. > --- > gcc/config/i386/i386-expand.c | 74 ++++++++++++++----------- > gcc/config/i386/i386-protos.h | 1 + > gcc/config/i386/predicates.md | 7 +++ > gcc/config/i386/sse.md | 2 +- > gcc/testsuite/gcc.target/i386/pr43147.c | 15 +++++ > 5 files changed, 67 insertions(+), 32 deletions(-) > create mode 100644 gcc/testsuite/gcc.target/i386/pr43147.c > > diff --git a/gcc/config/i386/i386-expand.c b/gcc/config/i386/i386-expand.c > index 9bf13dbfa92..1d8f3110310 100644 > --- a/gcc/config/i386/i386-expand.c > +++ b/gcc/config/i386/i386-expand.c > @@ -453,31 +453,12 @@ ix86_expand_move (machine_mode mode, rtx operands[]) > emit_insn (gen_rtx_SET (op0, op1)); > } > > -/* OP is a memref of CONST_VECTOR, return scalar constant mem > - if CONST_VECTOR is a vec_duplicate, else return NULL. */ > +/* CONSTANT is a CONST_VECTOR, return scalar constant if CONST_VECTOR is > + a vec_duplicate, else return nullptr. */ > + > static rtx > -ix86_broadcast_from_constant (machine_mode mode, rtx op) > +ix86_constant_broadcast (rtx constant, machine_mode mode) > { > - int nunits = GET_MODE_NUNITS (mode); > - if (nunits < 2) > - return nullptr; > - > - /* Don't use integer vector broadcast if we can't move from GPR to SSE > - register directly. */ > - if (!TARGET_INTER_UNIT_MOVES_TO_VEC > - && INTEGRAL_MODE_P (mode)) > - return nullptr; > - > - /* Convert CONST_VECTOR to a non-standard SSE constant integer > - broadcast only if vector broadcast is available. */ > - if (!(TARGET_AVX2 > - || (TARGET_AVX > - && (GET_MODE_INNER (mode) == SImode > - || GET_MODE_INNER (mode) == DImode)) > - || FLOAT_MODE_P (mode)) > - || standard_sse_constant_p (op, mode)) > - return nullptr; > - This part is dropped in the new ix86_broadcast_from_constant,why? > /* Don't broadcast from a 64-bit integer constant in 32-bit mode. > We can still put 64-bit integer constant in memory when > avx512 embed broadcast is available. */ > @@ -486,13 +467,6 @@ ix86_broadcast_from_constant (machine_mode mode, rtx op) > || (GET_MODE_SIZE (mode) < 64 && !TARGET_AVX512VL))) > return nullptr; > > - if (GET_MODE_INNER (mode) == TImode) > - return nullptr; > - > - rtx constant = get_pool_constant (XEXP (op, 0)); > - if (GET_CODE (constant) != CONST_VECTOR) > - return nullptr; > - > /* There could be some rtx like > (mem/u/c:V16QI (symbol_ref/u:DI ("*.LC1"))) > but with "*.LC1" refer to V2DI constant vector. */ > @@ -506,7 +480,7 @@ ix86_broadcast_from_constant (machine_mode mode, rtx op) > > rtx first = XVECEXP (constant, 0, 0); > > - for (int i = 1; i < nunits; ++i) > + for (int i = 1; i < GET_MODE_NUNITS (mode); ++i) > { > rtx tmp = XVECEXP (constant, 0, i); > /* Vector duplicate value. */ > @@ -517,6 +491,44 @@ ix86_broadcast_from_constant (machine_mode mode, rtx op) > return first; > } > > +/* OP is a memref of CONST_VECTOR, return scalar constant mem > + if CONST_VECTOR is a vec_duplicate, else return NULL. */ > +static rtx > +ix86_broadcast_from_constant (machine_mode mode, rtx op) > +{ > + int nunits = GET_MODE_NUNITS (mode); > + if (nunits < 2) > + return nullptr; > + > + /* Don't use integer vector broadcast if we can't move from GPR to SSE > + register directly. */ > + if (!TARGET_INTER_UNIT_MOVES_TO_VEC > + && INTEGRAL_MODE_P (mode)) > + return nullptr; > + > + if (GET_MODE_INNER (mode) == TImode) > + return nullptr; > + > + rtx constant = get_pool_constant (XEXP (op, 0)); > + if (GET_CODE (constant) != CONST_VECTOR) > + return nullptr; > + > + return ix86_constant_broadcast (constant, mode); > +} > + > +/* Return true if OP is a non-uniform CONST_VECTOR. */ > + Drop empty line. > +bool > +non_uniform_const_vector_p (rtx op, machine_mode mode) > +{ > + /* Allow non-uniform CONST_VECTOR only in the combine pass since other > + RTL optimizers work better with constant pool. */ > + return (current_pass > + && current_pass->tv_id == TV_COMBINE > + && GET_CODE (op) == CONST_VECTOR > + && ix86_constant_broadcast (op, mode) == nullptr); > +} I'm not sure it's a good idea to add a specific pass condition to the backend's predicate. If it's for combine, should we add define_split alternatively? > + > void > ix86_expand_vector_move (machine_mode mode, rtx operands[]) > { > diff --git a/gcc/config/i386/i386-protos.h b/gcc/config/i386/i386-protos.h > index 2fd13074c81..91d4c7cefb8 100644 > --- a/gcc/config/i386/i386-protos.h > +++ b/gcc/config/i386/i386-protos.h > @@ -58,6 +58,7 @@ extern int standard_80387_constant_p (rtx); > extern const char *standard_80387_constant_opcode (rtx); > extern rtx standard_80387_constant_rtx (int); > extern int standard_sse_constant_p (rtx, machine_mode); > +extern bool non_uniform_const_vector_p (rtx, machine_mode); > extern const char *standard_sse_constant_opcode (rtx_insn *, rtx *); > extern bool ix86_standard_x87sse_constant_load_p (const rtx_insn *, rtx); > extern bool ix86_pre_reload_split (void); > diff --git a/gcc/config/i386/predicates.md b/gcc/config/i386/predicates.md > index 9321f332ef9..435b22855c8 100644 > --- a/gcc/config/i386/predicates.md > +++ b/gcc/config/i386/predicates.md > @@ -1176,6 +1176,13 @@ (define_predicate "nonimmediate_or_sse_const_operand" > (ior (match_operand 0 "nonimmediate_operand") > (match_test "standard_sse_constant_p (op, mode)"))) > > +;; Return true when OP is nonimmediate, standard SSE constant, or > +;; non-uniform CONST_VECTOR. > +(define_predicate "nonimmediate_or_sse_const_vector_operand" > + (ior (match_operand 0 "nonimmediate_operand") > + (match_test "standard_sse_constant_p (op, mode)") > + (match_test "non_uniform_const_vector_p (op, mode)"))) > + > ;; Return true if OP is a register or a zero. > (define_predicate "reg_or_0_operand" > (ior (match_operand 0 "register_operand") > diff --git a/gcc/config/i386/sse.md b/gcc/config/i386/sse.md > index 13889687793..416ddcfcacc 100644 > --- a/gcc/config/i386/sse.md > +++ b/gcc/config/i386/sse.md > @@ -1075,7 +1075,7 @@ (define_expand "mov<mode>" > (define_insn "mov<mode>_internal" > [(set (match_operand:VMOVE 0 "nonimmediate_operand" > "=v,v ,v ,m") > - (match_operand:VMOVE 1 "nonimmediate_or_sse_const_operand" > + (match_operand:VMOVE 1 "nonimmediate_or_sse_const_vector_operand" > " C,<sseconstm1>,vm,v"))] > "TARGET_SSE > && (register_operand (operands[0], <MODE>mode) > diff --git a/gcc/testsuite/gcc.target/i386/pr43147.c b/gcc/testsuite/gcc.target/i386/pr43147.c > new file mode 100644 > index 00000000000..3c30f917c06 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/i386/pr43147.c > @@ -0,0 +1,15 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -msse2" } */ > +/* { dg-final { scan-assembler "movaps" } } */ > +/* { dg-final { scan-assembler-not "shufps" } } */ > + > +#include <x86intrin.h> > + > +__m128 > +foo (void) > +{ > + __m128 m = _mm_set_ps(1.0f, 2.0f, 3.0f, 4.0f); > + m = _mm_shuffle_ps(m, m, 0xC9); > + m = _mm_shuffle_ps(m, m, 0x2D); > + return m; > +} > -- > 2.31.1 > -- BR, Hongtao ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2] x86: Allow CONST_VECTOR for vector load in combine 2021-08-23 7:23 ` Hongtao Liu @ 2021-08-23 13:14 ` H.J. Lu 2021-08-24 1:57 ` Hongtao Liu 0 siblings, 1 reply; 7+ messages in thread From: H.J. Lu @ 2021-08-23 13:14 UTC (permalink / raw) To: Hongtao Liu; +Cc: GCC Patches On Mon, Aug 23, 2021 at 03:23:26PM +0800, Hongtao Liu wrote: > On Sun, Aug 22, 2021 at 8:54 PM H.J. Lu via Gcc-patches > <gcc-patches@gcc.gnu.org> wrote: > > > > In vetor move pattern, replace nonimmediate_or_sse_const_operand with > > nonimmediate_or_sse_const_vector_operand to allow vector load from > > non-uniform CONST_VECTOR. Non-uniform CONST_VECTOR is enabled only in > > the combine pass since other RTL optimizers work better with constant > > pool. > > > > gcc/ > > > > PR target/43147 > > * config/i386/i386-expand.c (ix86_constant_broadcast): New > > function. Extracted from ix86_broadcast_from_constant. > > (ix86_broadcast_from_constant): Call ix86_constant_broadcast. > > (non_uniform_const_vector_p): New function. > > * config/i386/i386-protos.h (non_uniform_const_vector_p): New > > prototype. > > * config/i386/predicates.md > > (nonimmediate_or_sse_const_vector_operand): New predicate. > > * config/i386/sse.md (mov<mode>_internal): Replace > > nonimmediate_or_sse_const_operand with > > nonimmediate_or_sse_const_vector_operand. > > > > gcc/testsuite/ > > > > PR target/43147 > > * gcc.target/i386/pr43147.c: New test. > > --- > > gcc/config/i386/i386-expand.c | 74 ++++++++++++++----------- > > gcc/config/i386/i386-protos.h | 1 + > > gcc/config/i386/predicates.md | 7 +++ > > gcc/config/i386/sse.md | 2 +- > > gcc/testsuite/gcc.target/i386/pr43147.c | 15 +++++ > > 5 files changed, 67 insertions(+), 32 deletions(-) > > create mode 100644 gcc/testsuite/gcc.target/i386/pr43147.c > > > > diff --git a/gcc/config/i386/i386-expand.c b/gcc/config/i386/i386-expand.c > > index 9bf13dbfa92..1d8f3110310 100644 > > --- a/gcc/config/i386/i386-expand.c > > +++ b/gcc/config/i386/i386-expand.c > > @@ -453,31 +453,12 @@ ix86_expand_move (machine_mode mode, rtx operands[]) > > emit_insn (gen_rtx_SET (op0, op1)); > > } > > > > -/* OP is a memref of CONST_VECTOR, return scalar constant mem > > - if CONST_VECTOR is a vec_duplicate, else return NULL. */ > > +/* CONSTANT is a CONST_VECTOR, return scalar constant if CONST_VECTOR is > > + a vec_duplicate, else return nullptr. */ > > + > > static rtx > > -ix86_broadcast_from_constant (machine_mode mode, rtx op) > > +ix86_constant_broadcast (rtx constant, machine_mode mode) > > { > > - int nunits = GET_MODE_NUNITS (mode); > > - if (nunits < 2) > > - return nullptr; > > - > > - /* Don't use integer vector broadcast if we can't move from GPR to SSE > > - register directly. */ > > - if (!TARGET_INTER_UNIT_MOVES_TO_VEC > > - && INTEGRAL_MODE_P (mode)) > > - return nullptr; > > - > > - /* Convert CONST_VECTOR to a non-standard SSE constant integer > > - broadcast only if vector broadcast is available. */ > > - if (!(TARGET_AVX2 > > - || (TARGET_AVX > > - && (GET_MODE_INNER (mode) == SImode > > - || GET_MODE_INNER (mode) == DImode)) > > - || FLOAT_MODE_P (mode)) > > - || standard_sse_constant_p (op, mode)) > > - return nullptr; > > - > This part is dropped in the new ix86_broadcast_from_constant,why? Restored. > > /* Don't broadcast from a 64-bit integer constant in 32-bit mode. > > We can still put 64-bit integer constant in memory when > > avx512 embed broadcast is available. */ > > @@ -486,13 +467,6 @@ ix86_broadcast_from_constant (machine_mode mode, rtx op) > > || (GET_MODE_SIZE (mode) < 64 && !TARGET_AVX512VL))) > > return nullptr; > > > > - if (GET_MODE_INNER (mode) == TImode) > > - return nullptr; > > - > > - rtx constant = get_pool_constant (XEXP (op, 0)); > > - if (GET_CODE (constant) != CONST_VECTOR) > > - return nullptr; > > - > > /* There could be some rtx like > > (mem/u/c:V16QI (symbol_ref/u:DI ("*.LC1"))) > > but with "*.LC1" refer to V2DI constant vector. */ > > @@ -506,7 +480,7 @@ ix86_broadcast_from_constant (machine_mode mode, rtx op) > > > > rtx first = XVECEXP (constant, 0, 0); > > > > - for (int i = 1; i < nunits; ++i) > > + for (int i = 1; i < GET_MODE_NUNITS (mode); ++i) > > { > > rtx tmp = XVECEXP (constant, 0, i); > > /* Vector duplicate value. */ > > @@ -517,6 +491,44 @@ ix86_broadcast_from_constant (machine_mode mode, rtx op) > > return first; > > } > > > > +/* OP is a memref of CONST_VECTOR, return scalar constant mem > > + if CONST_VECTOR is a vec_duplicate, else return NULL. */ > > +static rtx > > +ix86_broadcast_from_constant (machine_mode mode, rtx op) > > +{ > > + int nunits = GET_MODE_NUNITS (mode); > > + if (nunits < 2) > > + return nullptr; > > + > > + /* Don't use integer vector broadcast if we can't move from GPR to SSE > > + register directly. */ > > + if (!TARGET_INTER_UNIT_MOVES_TO_VEC > > + && INTEGRAL_MODE_P (mode)) > > + return nullptr; > > + > > + if (GET_MODE_INNER (mode) == TImode) > > + return nullptr; > > + > > + rtx constant = get_pool_constant (XEXP (op, 0)); > > + if (GET_CODE (constant) != CONST_VECTOR) > > + return nullptr; > > + > > + return ix86_constant_broadcast (constant, mode); > > +} > > + > > +/* Return true if OP is a non-uniform CONST_VECTOR. */ > > + > Drop empty line. Removed. > > +bool > > +non_uniform_const_vector_p (rtx op, machine_mode mode) > > +{ > > + /* Allow non-uniform CONST_VECTOR only in the combine pass since other > > + RTL optimizers work better with constant pool. */ > > + return (current_pass > > + && current_pass->tv_id == TV_COMBINE > > + && GET_CODE (op) == CONST_VECTOR > > + && ix86_constant_broadcast (op, mode) == nullptr); > > +} > I'm not sure it's a good idea to add a specific pass condition to the > backend's predicate. > If it's for combine, should we add define_split alternatively? I tried. RTL optimizers won't optimize non-uniform CONST_VECTOR which leads extra moves: FAIL: gcc.target/i386/fnabs.c scan-assembler-times \tv?orp[sd][ \t] 2 FAIL: gcc.target/i386/fuse-caller-save-xmm.c scan-assembler-times addpd\\t\\.?[Ll]C0.*, %xmm0 1 FAIL: gcc.target/i386/fuse-caller-save-xmm.c scan-assembler-times movapd\t%xmm0, %xmm1 1 Only all bits 0/1 CONST_VECTOR is allowed as an operand for load and all other CONST_VECTORs are placed in constant pool. My patch allows non-uniform CONST_VECTOR in the combine pass so that it can generate load from non-uniform CONST_VECTOR. Since non-uniform CONST_VECTOR isn't allowed in other RTL passes, it will be put in constant pool. H.J. --- In vetor move pattern, replace nonimmediate_or_sse_const_operand with nonimmediate_or_sse_const_vector_operand to allow vector load from non-uniform CONST_VECTOR. Non-uniform CONST_VECTOR is enabled only in the combine pass since other RTL optimizers and IRA/LRA work better with constant pool. gcc/ PR target/43147 * config/i386/i386-expand.c (ix86_constant_broadcast): New function. Extracted from ix86_broadcast_from_constant. (ix86_broadcast_from_constant): Call ix86_constant_broadcast. (non_uniform_const_vector_p): New function. * config/i386/i386-protos.h (non_uniform_const_vector_p): New prototype. * config/i386/predicates.md (nonimmediate_or_sse_const_vector_operand): New predicate. * config/i386/sse.md (mov<mode>_internal): Replace nonimmediate_or_sse_const_operand with nonimmediate_or_sse_const_vector_operand. gcc/testsuite/ PR target/43147 * gcc.target/i386/pr43147.c: New test. --- gcc/config/i386/i386-expand.c | 79 ++++++++++++++++--------- gcc/config/i386/i386-protos.h | 1 + gcc/config/i386/predicates.md | 7 +++ gcc/config/i386/sse.md | 2 +- gcc/testsuite/gcc.target/i386/pr43147.c | 15 +++++ 5 files changed, 74 insertions(+), 30 deletions(-) create mode 100644 gcc/testsuite/gcc.target/i386/pr43147.c diff --git a/gcc/config/i386/i386-expand.c b/gcc/config/i386/i386-expand.c index 9bf13dbfa92..599f66582d5 100644 --- a/gcc/config/i386/i386-expand.c +++ b/gcc/config/i386/i386-expand.c @@ -453,6 +453,44 @@ ix86_expand_move (machine_mode mode, rtx operands[]) emit_insn (gen_rtx_SET (op0, op1)); } +/* CONSTANT is a CONST_VECTOR, return scalar constant if CONST_VECTOR is + a vec_duplicate, else return nullptr. */ + +static rtx +ix86_constant_broadcast (rtx constant, machine_mode mode) +{ + /* Don't broadcast from a 64-bit integer constant in 32-bit mode. + We can still put 64-bit integer constant in memory when + avx512 embed broadcast is available. */ + if (GET_MODE_INNER (mode) == DImode && !TARGET_64BIT + && (!TARGET_AVX512F + || (GET_MODE_SIZE (mode) < 64 && !TARGET_AVX512VL))) + return nullptr; + + /* There could be some rtx like + (mem/u/c:V16QI (symbol_ref/u:DI ("*.LC1"))) + but with "*.LC1" refer to V2DI constant vector. */ + if (GET_MODE (constant) != mode) + { + constant = simplify_subreg (mode, constant, GET_MODE (constant), + 0); + if (constant == nullptr || GET_CODE (constant) != CONST_VECTOR) + return nullptr; + } + + rtx first = XVECEXP (constant, 0, 0); + + for (int i = 1; i < GET_MODE_NUNITS (mode); ++i) + { + rtx tmp = XVECEXP (constant, 0, i); + /* Vector duplicate value. */ + if (!rtx_equal_p (tmp, first)) + return nullptr; + } + + return first; +} + /* OP is a memref of CONST_VECTOR, return scalar constant mem if CONST_VECTOR is a vec_duplicate, else return NULL. */ static rtx @@ -478,14 +516,6 @@ ix86_broadcast_from_constant (machine_mode mode, rtx op) || standard_sse_constant_p (op, mode)) return nullptr; - /* Don't broadcast from a 64-bit integer constant in 32-bit mode. - We can still put 64-bit integer constant in memory when - avx512 embed broadcast is available. */ - if (GET_MODE_INNER (mode) == DImode && !TARGET_64BIT - && (!TARGET_AVX512F - || (GET_MODE_SIZE (mode) < 64 && !TARGET_AVX512VL))) - return nullptr; - if (GET_MODE_INNER (mode) == TImode) return nullptr; @@ -493,28 +523,19 @@ ix86_broadcast_from_constant (machine_mode mode, rtx op) if (GET_CODE (constant) != CONST_VECTOR) return nullptr; - /* There could be some rtx like - (mem/u/c:V16QI (symbol_ref/u:DI ("*.LC1"))) - but with "*.LC1" refer to V2DI constant vector. */ - if (GET_MODE (constant) != mode) - { - constant = simplify_subreg (mode, constant, GET_MODE (constant), - 0); - if (constant == nullptr || GET_CODE (constant) != CONST_VECTOR) - return nullptr; - } - - rtx first = XVECEXP (constant, 0, 0); - - for (int i = 1; i < nunits; ++i) - { - rtx tmp = XVECEXP (constant, 0, i); - /* Vector duplicate value. */ - if (!rtx_equal_p (tmp, first)) - return nullptr; - } + return ix86_constant_broadcast (constant, mode); +} - return first; +/* Return true if OP is a non-uniform CONST_VECTOR. */ +bool +non_uniform_const_vector_p (rtx op, machine_mode mode) +{ + /* Allow non-uniform CONST_VECTOR only in the combine pass since other + RTL optimizers and IRA/LRA work better with constant pool. */ + return (current_pass + && current_pass->tv_id == TV_COMBINE + && GET_CODE (op) == CONST_VECTOR + && ix86_constant_broadcast (op, mode) == nullptr); } void diff --git a/gcc/config/i386/i386-protos.h b/gcc/config/i386/i386-protos.h index 2fd13074c81..91d4c7cefb8 100644 --- a/gcc/config/i386/i386-protos.h +++ b/gcc/config/i386/i386-protos.h @@ -58,6 +58,7 @@ extern int standard_80387_constant_p (rtx); extern const char *standard_80387_constant_opcode (rtx); extern rtx standard_80387_constant_rtx (int); extern int standard_sse_constant_p (rtx, machine_mode); +extern bool non_uniform_const_vector_p (rtx, machine_mode); extern const char *standard_sse_constant_opcode (rtx_insn *, rtx *); extern bool ix86_standard_x87sse_constant_load_p (const rtx_insn *, rtx); extern bool ix86_pre_reload_split (void); diff --git a/gcc/config/i386/predicates.md b/gcc/config/i386/predicates.md index 9321f332ef9..435b22855c8 100644 --- a/gcc/config/i386/predicates.md +++ b/gcc/config/i386/predicates.md @@ -1176,6 +1176,13 @@ (define_predicate "nonimmediate_or_sse_const_operand" (ior (match_operand 0 "nonimmediate_operand") (match_test "standard_sse_constant_p (op, mode)"))) +;; Return true when OP is nonimmediate, standard SSE constant, or +;; non-uniform CONST_VECTOR. +(define_predicate "nonimmediate_or_sse_const_vector_operand" + (ior (match_operand 0 "nonimmediate_operand") + (match_test "standard_sse_constant_p (op, mode)") + (match_test "non_uniform_const_vector_p (op, mode)"))) + ;; Return true if OP is a register or a zero. (define_predicate "reg_or_0_operand" (ior (match_operand 0 "register_operand") diff --git a/gcc/config/i386/sse.md b/gcc/config/i386/sse.md index 95f95823ea3..a7919eca6c3 100644 --- a/gcc/config/i386/sse.md +++ b/gcc/config/i386/sse.md @@ -1075,7 +1075,7 @@ (define_expand "mov<mode>" (define_insn "mov<mode>_internal" [(set (match_operand:VMOVE 0 "nonimmediate_operand" "=v,v ,v ,m") - (match_operand:VMOVE 1 "nonimmediate_or_sse_const_operand" + (match_operand:VMOVE 1 "nonimmediate_or_sse_const_vector_operand" " C,<sseconstm1>,vm,v"))] "TARGET_SSE && (register_operand (operands[0], <MODE>mode) diff --git a/gcc/testsuite/gcc.target/i386/pr43147.c b/gcc/testsuite/gcc.target/i386/pr43147.c new file mode 100644 index 00000000000..3c30f917c06 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr43147.c @@ -0,0 +1,15 @@ +/* { dg-do compile } */ +/* { dg-options "-O2 -msse2" } */ +/* { dg-final { scan-assembler "movaps" } } */ +/* { dg-final { scan-assembler-not "shufps" } } */ + +#include <x86intrin.h> + +__m128 +foo (void) +{ + __m128 m = _mm_set_ps(1.0f, 2.0f, 3.0f, 4.0f); + m = _mm_shuffle_ps(m, m, 0xC9); + m = _mm_shuffle_ps(m, m, 0x2D); + return m; +} -- 2.31.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] x86: Allow CONST_VECTOR for vector load in combine 2021-08-23 13:14 ` [PATCH v2] " H.J. Lu @ 2021-08-24 1:57 ` Hongtao Liu 2021-08-24 1:58 ` H.J. Lu 2021-08-24 16:14 ` Segher Boessenkool 0 siblings, 2 replies; 7+ messages in thread From: Hongtao Liu @ 2021-08-24 1:57 UTC (permalink / raw) To: H.J. Lu; +Cc: GCC Patches, Segher Boessenkool, Richard Sandiford On Mon, Aug 23, 2021 at 9:14 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > On Mon, Aug 23, 2021 at 03:23:26PM +0800, Hongtao Liu wrote: > > On Sun, Aug 22, 2021 at 8:54 PM H.J. Lu via Gcc-patches > > <gcc-patches@gcc.gnu.org> wrote: > > > > > > In vetor move pattern, replace nonimmediate_or_sse_const_operand with > > > nonimmediate_or_sse_const_vector_operand to allow vector load from > > > non-uniform CONST_VECTOR. Non-uniform CONST_VECTOR is enabled only in > > > the combine pass since other RTL optimizers work better with constant > > > pool. > > > > > > gcc/ > > > > > > PR target/43147 > > > * config/i386/i386-expand.c (ix86_constant_broadcast): New > > > function. Extracted from ix86_broadcast_from_constant. > > > (ix86_broadcast_from_constant): Call ix86_constant_broadcast. > > > (non_uniform_const_vector_p): New function. > > > * config/i386/i386-protos.h (non_uniform_const_vector_p): New > > > prototype. > > > * config/i386/predicates.md > > > (nonimmediate_or_sse_const_vector_operand): New predicate. > > > * config/i386/sse.md (mov<mode>_internal): Replace > > > nonimmediate_or_sse_const_operand with > > > nonimmediate_or_sse_const_vector_operand. > > > > > > gcc/testsuite/ > > > > > > PR target/43147 > > > * gcc.target/i386/pr43147.c: New test. > > > --- > > > gcc/config/i386/i386-expand.c | 74 ++++++++++++++----------- > > > gcc/config/i386/i386-protos.h | 1 + > > > gcc/config/i386/predicates.md | 7 +++ > > > gcc/config/i386/sse.md | 2 +- > > > gcc/testsuite/gcc.target/i386/pr43147.c | 15 +++++ > > > 5 files changed, 67 insertions(+), 32 deletions(-) > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr43147.c > > > > > > diff --git a/gcc/config/i386/i386-expand.c b/gcc/config/i386/i386-expand.c > > > index 9bf13dbfa92..1d8f3110310 100644 > > > --- a/gcc/config/i386/i386-expand.c > > > +++ b/gcc/config/i386/i386-expand.c > > > @@ -453,31 +453,12 @@ ix86_expand_move (machine_mode mode, rtx operands[]) > > > emit_insn (gen_rtx_SET (op0, op1)); > > > } > > > > > > -/* OP is a memref of CONST_VECTOR, return scalar constant mem > > > - if CONST_VECTOR is a vec_duplicate, else return NULL. */ > > > +/* CONSTANT is a CONST_VECTOR, return scalar constant if CONST_VECTOR is > > > + a vec_duplicate, else return nullptr. */ > > > + > > > static rtx > > > -ix86_broadcast_from_constant (machine_mode mode, rtx op) > > > +ix86_constant_broadcast (rtx constant, machine_mode mode) > > > { > > > - int nunits = GET_MODE_NUNITS (mode); > > > - if (nunits < 2) > > > - return nullptr; > > > - > > > - /* Don't use integer vector broadcast if we can't move from GPR to SSE > > > - register directly. */ > > > - if (!TARGET_INTER_UNIT_MOVES_TO_VEC > > > - && INTEGRAL_MODE_P (mode)) > > > - return nullptr; > > > - > > > - /* Convert CONST_VECTOR to a non-standard SSE constant integer > > > - broadcast only if vector broadcast is available. */ > > > - if (!(TARGET_AVX2 > > > - || (TARGET_AVX > > > - && (GET_MODE_INNER (mode) == SImode > > > - || GET_MODE_INNER (mode) == DImode)) > > > - || FLOAT_MODE_P (mode)) > > > - || standard_sse_constant_p (op, mode)) > > > - return nullptr; > > > - > > This part is dropped in the new ix86_broadcast_from_constant,why? > > > Restored. > > > > /* Don't broadcast from a 64-bit integer constant in 32-bit mode. > > > We can still put 64-bit integer constant in memory when > > > avx512 embed broadcast is available. */ > > > @@ -486,13 +467,6 @@ ix86_broadcast_from_constant (machine_mode mode, rtx op) > > > || (GET_MODE_SIZE (mode) < 64 && !TARGET_AVX512VL))) > > > return nullptr; > > > > > > - if (GET_MODE_INNER (mode) == TImode) > > > - return nullptr; > > > - > > > - rtx constant = get_pool_constant (XEXP (op, 0)); > > > - if (GET_CODE (constant) != CONST_VECTOR) > > > - return nullptr; > > > - > > > /* There could be some rtx like > > > (mem/u/c:V16QI (symbol_ref/u:DI ("*.LC1"))) > > > but with "*.LC1" refer to V2DI constant vector. */ > > > @@ -506,7 +480,7 @@ ix86_broadcast_from_constant (machine_mode mode, rtx op) > > > > > > rtx first = XVECEXP (constant, 0, 0); > > > > > > - for (int i = 1; i < nunits; ++i) > > > + for (int i = 1; i < GET_MODE_NUNITS (mode); ++i) > > > { > > > rtx tmp = XVECEXP (constant, 0, i); > > > /* Vector duplicate value. */ > > > @@ -517,6 +491,44 @@ ix86_broadcast_from_constant (machine_mode mode, rtx op) > > > return first; > > > } > > > > > > +/* OP is a memref of CONST_VECTOR, return scalar constant mem > > > + if CONST_VECTOR is a vec_duplicate, else return NULL. */ > > > +static rtx > > > +ix86_broadcast_from_constant (machine_mode mode, rtx op) > > > +{ > > > + int nunits = GET_MODE_NUNITS (mode); > > > + if (nunits < 2) > > > + return nullptr; > > > + > > > + /* Don't use integer vector broadcast if we can't move from GPR to SSE > > > + register directly. */ > > > + if (!TARGET_INTER_UNIT_MOVES_TO_VEC > > > + && INTEGRAL_MODE_P (mode)) > > > + return nullptr; > > > + > > > + if (GET_MODE_INNER (mode) == TImode) > > > + return nullptr; > > > + > > > + rtx constant = get_pool_constant (XEXP (op, 0)); > > > + if (GET_CODE (constant) != CONST_VECTOR) > > > + return nullptr; > > > + > > > + return ix86_constant_broadcast (constant, mode); > > > +} > > > + > > > +/* Return true if OP is a non-uniform CONST_VECTOR. */ > > > + > > Drop empty line. > > Removed. > > > > +bool > > > +non_uniform_const_vector_p (rtx op, machine_mode mode) > > > +{ > > > + /* Allow non-uniform CONST_VECTOR only in the combine pass since other > > > + RTL optimizers work better with constant pool. */ > > > + return (current_pass > > > + && current_pass->tv_id == TV_COMBINE > > > + && GET_CODE (op) == CONST_VECTOR > > > + && ix86_constant_broadcast (op, mode) == nullptr); > > > +} > > I'm not sure it's a good idea to add a specific pass condition to the > > backend's predicate. > > If it's for combine, should we add define_split alternatively? > > I tried. RTL optimizers won't optimize non-uniform CONST_VECTOR which > leads extra moves: > > FAIL: gcc.target/i386/fnabs.c scan-assembler-times \tv?orp[sd][ \t] 2 > FAIL: gcc.target/i386/fuse-caller-save-xmm.c scan-assembler-times addpd\\t\\.?[Ll]C0.*, %xmm0 1 > FAIL: gcc.target/i386/fuse-caller-save-xmm.c scan-assembler-times movapd\t%xmm0, %xmm1 1 > > Only all bits 0/1 CONST_VECTOR is allowed as an operand for load and all > other CONST_VECTORs are placed in constant pool. My patch allows > non-uniform CONST_VECTOR in the combine pass so that it can generate load > from non-uniform CONST_VECTOR. Since non-uniform CONST_VECTOR isn't > allowed in other RTL passes, it will be put in constant pool. > Trying 5 -> 7: 5: r85:V4SF=[`*.LC0'] REG_EQUAL const_vector 7: r84:V4SF=vec_select(vec_concat(r85:V4SF,r85:V4SF),parallel) REG_DEAD r85:V4SF REG_EQUAL const_vector Failed to match this instruction: (set (reg:V4SF 84) (const_vector:V4SF [ (const_double:SF 3.0e+0 [0x0.cp+2]) (const_double:SF 2.0e+0 [0x0.8p+2]) (const_double:SF 4.0e+0 [0x0.8p+3]) (const_double:SF 1.0e+0 [0x0.8p+1]) ])) (insn 5 2 7 2 (set (reg:V4SF 85) (mem/u/c:V4SF (symbol_ref/u:DI ("*.LC0") [flags 0x2]) [0 S16 A128])) "/export/users/liuhongt/install/git_trunk_master_native/lib/gcc/x86_64-pc-linux-gnu/12.0.0/include/xmmintrin.h":746:19 1600 {movv4sf_internal} (expr_list:REG_EQUAL (const_vector:V4SF [ (const_double:SF 4.0e+0 [0x0.8p+3]) (const_double:SF 3.0e+0 [0x0.cp+2]) (const_double:SF 2.0e+0 [0x0.8p+2]) (const_double:SF 1.0e+0 [0x0.8p+1]) ]) (nil))) (insn 7 5 11 2 (set (reg:V4SF 84) (vec_select:V4SF (vec_concat:V8SF (reg:V4SF 85) (reg:V4SF 85)) (parallel [ (const_int 1 [0x1]) (const_int 2 [0x2]) (const_int 4 [0x4]) (const_int 7 [0x7]) ]))) "/export/users/liuhongt/install/git_trunk_master_native/lib/gcc/x86_64-pc-linux-gnu/12.0.0/include/xmmintrin.h":746:19 3015 {sse_shufps_v4sf} (expr_list:REG_DEAD (reg:V4SF 85) (expr_list:REG_EQUAL (const_vector:V4SF [ (const_double:SF 3.0e+0 [0x0.cp+2]) (const_double:SF 2.0e+0 [0x0.8p+2]) (const_double:SF 4.0e+0 [0x0.8p+3]) (const_double:SF 1.0e+0 [0x0.8p+1]) ]) (nil)))) I think pass_combine should be extended to force illegitimate constant to constant pool and recog load insn again, It looks like a general optimization that better not do it in the backend. > H.J. > --- > In vetor move pattern, replace nonimmediate_or_sse_const_operand with > nonimmediate_or_sse_const_vector_operand to allow vector load from > non-uniform CONST_VECTOR. Non-uniform CONST_VECTOR is enabled only in > the combine pass since other RTL optimizers and IRA/LRA work better with > constant pool. > > gcc/ > > PR target/43147 > * config/i386/i386-expand.c (ix86_constant_broadcast): New > function. Extracted from ix86_broadcast_from_constant. > (ix86_broadcast_from_constant): Call ix86_constant_broadcast. > (non_uniform_const_vector_p): New function. > * config/i386/i386-protos.h (non_uniform_const_vector_p): New > prototype. > * config/i386/predicates.md > (nonimmediate_or_sse_const_vector_operand): New predicate. > * config/i386/sse.md (mov<mode>_internal): Replace > nonimmediate_or_sse_const_operand with > nonimmediate_or_sse_const_vector_operand. > > gcc/testsuite/ > > PR target/43147 > * gcc.target/i386/pr43147.c: New test. > --- > gcc/config/i386/i386-expand.c | 79 ++++++++++++++++--------- > gcc/config/i386/i386-protos.h | 1 + > gcc/config/i386/predicates.md | 7 +++ > gcc/config/i386/sse.md | 2 +- > gcc/testsuite/gcc.target/i386/pr43147.c | 15 +++++ > 5 files changed, 74 insertions(+), 30 deletions(-) > create mode 100644 gcc/testsuite/gcc.target/i386/pr43147.c > > diff --git a/gcc/config/i386/i386-expand.c b/gcc/config/i386/i386-expand.c > index 9bf13dbfa92..599f66582d5 100644 > --- a/gcc/config/i386/i386-expand.c > +++ b/gcc/config/i386/i386-expand.c > @@ -453,6 +453,44 @@ ix86_expand_move (machine_mode mode, rtx operands[]) > emit_insn (gen_rtx_SET (op0, op1)); > } > > +/* CONSTANT is a CONST_VECTOR, return scalar constant if CONST_VECTOR is > + a vec_duplicate, else return nullptr. */ > + > +static rtx > +ix86_constant_broadcast (rtx constant, machine_mode mode) > +{ > + /* Don't broadcast from a 64-bit integer constant in 32-bit mode. > + We can still put 64-bit integer constant in memory when > + avx512 embed broadcast is available. */ > + if (GET_MODE_INNER (mode) == DImode && !TARGET_64BIT > + && (!TARGET_AVX512F > + || (GET_MODE_SIZE (mode) < 64 && !TARGET_AVX512VL))) > + return nullptr; > + > + /* There could be some rtx like > + (mem/u/c:V16QI (symbol_ref/u:DI ("*.LC1"))) > + but with "*.LC1" refer to V2DI constant vector. */ > + if (GET_MODE (constant) != mode) > + { > + constant = simplify_subreg (mode, constant, GET_MODE (constant), > + 0); > + if (constant == nullptr || GET_CODE (constant) != CONST_VECTOR) > + return nullptr; > + } > + > + rtx first = XVECEXP (constant, 0, 0); > + > + for (int i = 1; i < GET_MODE_NUNITS (mode); ++i) > + { > + rtx tmp = XVECEXP (constant, 0, i); > + /* Vector duplicate value. */ > + if (!rtx_equal_p (tmp, first)) > + return nullptr; > + } > + > + return first; > +} > + > /* OP is a memref of CONST_VECTOR, return scalar constant mem > if CONST_VECTOR is a vec_duplicate, else return NULL. */ > static rtx > @@ -478,14 +516,6 @@ ix86_broadcast_from_constant (machine_mode mode, rtx op) > || standard_sse_constant_p (op, mode)) > return nullptr; > > - /* Don't broadcast from a 64-bit integer constant in 32-bit mode. > - We can still put 64-bit integer constant in memory when > - avx512 embed broadcast is available. */ > - if (GET_MODE_INNER (mode) == DImode && !TARGET_64BIT > - && (!TARGET_AVX512F > - || (GET_MODE_SIZE (mode) < 64 && !TARGET_AVX512VL))) > - return nullptr; > - > if (GET_MODE_INNER (mode) == TImode) > return nullptr; > > @@ -493,28 +523,19 @@ ix86_broadcast_from_constant (machine_mode mode, rtx op) > if (GET_CODE (constant) != CONST_VECTOR) > return nullptr; > > - /* There could be some rtx like > - (mem/u/c:V16QI (symbol_ref/u:DI ("*.LC1"))) > - but with "*.LC1" refer to V2DI constant vector. */ > - if (GET_MODE (constant) != mode) > - { > - constant = simplify_subreg (mode, constant, GET_MODE (constant), > - 0); > - if (constant == nullptr || GET_CODE (constant) != CONST_VECTOR) > - return nullptr; > - } > - > - rtx first = XVECEXP (constant, 0, 0); > - > - for (int i = 1; i < nunits; ++i) > - { > - rtx tmp = XVECEXP (constant, 0, i); > - /* Vector duplicate value. */ > - if (!rtx_equal_p (tmp, first)) > - return nullptr; > - } > + return ix86_constant_broadcast (constant, mode); > +} > > - return first; > +/* Return true if OP is a non-uniform CONST_VECTOR. */ > +bool > +non_uniform_const_vector_p (rtx op, machine_mode mode) > +{ > + /* Allow non-uniform CONST_VECTOR only in the combine pass since other > + RTL optimizers and IRA/LRA work better with constant pool. */ > + return (current_pass > + && current_pass->tv_id == TV_COMBINE > + && GET_CODE (op) == CONST_VECTOR > + && ix86_constant_broadcast (op, mode) == nullptr); > } > > void > diff --git a/gcc/config/i386/i386-protos.h b/gcc/config/i386/i386-protos.h > index 2fd13074c81..91d4c7cefb8 100644 > --- a/gcc/config/i386/i386-protos.h > +++ b/gcc/config/i386/i386-protos.h > @@ -58,6 +58,7 @@ extern int standard_80387_constant_p (rtx); > extern const char *standard_80387_constant_opcode (rtx); > extern rtx standard_80387_constant_rtx (int); > extern int standard_sse_constant_p (rtx, machine_mode); > +extern bool non_uniform_const_vector_p (rtx, machine_mode); > extern const char *standard_sse_constant_opcode (rtx_insn *, rtx *); > extern bool ix86_standard_x87sse_constant_load_p (const rtx_insn *, rtx); > extern bool ix86_pre_reload_split (void); > diff --git a/gcc/config/i386/predicates.md b/gcc/config/i386/predicates.md > index 9321f332ef9..435b22855c8 100644 > --- a/gcc/config/i386/predicates.md > +++ b/gcc/config/i386/predicates.md > @@ -1176,6 +1176,13 @@ (define_predicate "nonimmediate_or_sse_const_operand" > (ior (match_operand 0 "nonimmediate_operand") > (match_test "standard_sse_constant_p (op, mode)"))) > > +;; Return true when OP is nonimmediate, standard SSE constant, or > +;; non-uniform CONST_VECTOR. > +(define_predicate "nonimmediate_or_sse_const_vector_operand" > + (ior (match_operand 0 "nonimmediate_operand") > + (match_test "standard_sse_constant_p (op, mode)") > + (match_test "non_uniform_const_vector_p (op, mode)"))) > + > ;; Return true if OP is a register or a zero. > (define_predicate "reg_or_0_operand" > (ior (match_operand 0 "register_operand") > diff --git a/gcc/config/i386/sse.md b/gcc/config/i386/sse.md > index 95f95823ea3..a7919eca6c3 100644 > --- a/gcc/config/i386/sse.md > +++ b/gcc/config/i386/sse.md > @@ -1075,7 +1075,7 @@ (define_expand "mov<mode>" > (define_insn "mov<mode>_internal" > [(set (match_operand:VMOVE 0 "nonimmediate_operand" > "=v,v ,v ,m") > - (match_operand:VMOVE 1 "nonimmediate_or_sse_const_operand" > + (match_operand:VMOVE 1 "nonimmediate_or_sse_const_vector_operand" > " C,<sseconstm1>,vm,v"))] > "TARGET_SSE > && (register_operand (operands[0], <MODE>mode) > diff --git a/gcc/testsuite/gcc.target/i386/pr43147.c b/gcc/testsuite/gcc.target/i386/pr43147.c > new file mode 100644 > index 00000000000..3c30f917c06 > --- /dev/null > +++ b/gcc/testsuite/gcc.target/i386/pr43147.c > @@ -0,0 +1,15 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -msse2" } */ > +/* { dg-final { scan-assembler "movaps" } } */ > +/* { dg-final { scan-assembler-not "shufps" } } */ > + > +#include <x86intrin.h> > + > +__m128 > +foo (void) > +{ > + __m128 m = _mm_set_ps(1.0f, 2.0f, 3.0f, 4.0f); > + m = _mm_shuffle_ps(m, m, 0xC9); > + m = _mm_shuffle_ps(m, m, 0x2D); > + return m; > +} > -- > 2.31.1 > -- BR, Hongtao ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] x86: Allow CONST_VECTOR for vector load in combine 2021-08-24 1:57 ` Hongtao Liu @ 2021-08-24 1:58 ` H.J. Lu 2021-08-24 16:14 ` Segher Boessenkool 1 sibling, 0 replies; 7+ messages in thread From: H.J. Lu @ 2021-08-24 1:58 UTC (permalink / raw) To: Hongtao Liu; +Cc: GCC Patches, Segher Boessenkool, Richard Sandiford On Mon, Aug 23, 2021 at 6:52 PM Hongtao Liu <crazylht@gmail.com> wrote: > > On Mon, Aug 23, 2021 at 9:14 PM H.J. Lu <hjl.tools@gmail.com> wrote: > > > > On Mon, Aug 23, 2021 at 03:23:26PM +0800, Hongtao Liu wrote: > > > On Sun, Aug 22, 2021 at 8:54 PM H.J. Lu via Gcc-patches > > > <gcc-patches@gcc.gnu.org> wrote: > > > > > > > > In vetor move pattern, replace nonimmediate_or_sse_const_operand with > > > > nonimmediate_or_sse_const_vector_operand to allow vector load from > > > > non-uniform CONST_VECTOR. Non-uniform CONST_VECTOR is enabled only in > > > > the combine pass since other RTL optimizers work better with constant > > > > pool. > > > > > > > > gcc/ > > > > > > > > PR target/43147 > > > > * config/i386/i386-expand.c (ix86_constant_broadcast): New > > > > function. Extracted from ix86_broadcast_from_constant. > > > > (ix86_broadcast_from_constant): Call ix86_constant_broadcast. > > > > (non_uniform_const_vector_p): New function. > > > > * config/i386/i386-protos.h (non_uniform_const_vector_p): New > > > > prototype. > > > > * config/i386/predicates.md > > > > (nonimmediate_or_sse_const_vector_operand): New predicate. > > > > * config/i386/sse.md (mov<mode>_internal): Replace > > > > nonimmediate_or_sse_const_operand with > > > > nonimmediate_or_sse_const_vector_operand. > > > > > > > > gcc/testsuite/ > > > > > > > > PR target/43147 > > > > * gcc.target/i386/pr43147.c: New test. > > > > --- > > > > gcc/config/i386/i386-expand.c | 74 ++++++++++++++----------- > > > > gcc/config/i386/i386-protos.h | 1 + > > > > gcc/config/i386/predicates.md | 7 +++ > > > > gcc/config/i386/sse.md | 2 +- > > > > gcc/testsuite/gcc.target/i386/pr43147.c | 15 +++++ > > > > 5 files changed, 67 insertions(+), 32 deletions(-) > > > > create mode 100644 gcc/testsuite/gcc.target/i386/pr43147.c > > > > > > > > diff --git a/gcc/config/i386/i386-expand.c b/gcc/config/i386/i386-expand.c > > > > index 9bf13dbfa92..1d8f3110310 100644 > > > > --- a/gcc/config/i386/i386-expand.c > > > > +++ b/gcc/config/i386/i386-expand.c > > > > @@ -453,31 +453,12 @@ ix86_expand_move (machine_mode mode, rtx operands[]) > > > > emit_insn (gen_rtx_SET (op0, op1)); > > > > } > > > > > > > > -/* OP is a memref of CONST_VECTOR, return scalar constant mem > > > > - if CONST_VECTOR is a vec_duplicate, else return NULL. */ > > > > +/* CONSTANT is a CONST_VECTOR, return scalar constant if CONST_VECTOR is > > > > + a vec_duplicate, else return nullptr. */ > > > > + > > > > static rtx > > > > -ix86_broadcast_from_constant (machine_mode mode, rtx op) > > > > +ix86_constant_broadcast (rtx constant, machine_mode mode) > > > > { > > > > - int nunits = GET_MODE_NUNITS (mode); > > > > - if (nunits < 2) > > > > - return nullptr; > > > > - > > > > - /* Don't use integer vector broadcast if we can't move from GPR to SSE > > > > - register directly. */ > > > > - if (!TARGET_INTER_UNIT_MOVES_TO_VEC > > > > - && INTEGRAL_MODE_P (mode)) > > > > - return nullptr; > > > > - > > > > - /* Convert CONST_VECTOR to a non-standard SSE constant integer > > > > - broadcast only if vector broadcast is available. */ > > > > - if (!(TARGET_AVX2 > > > > - || (TARGET_AVX > > > > - && (GET_MODE_INNER (mode) == SImode > > > > - || GET_MODE_INNER (mode) == DImode)) > > > > - || FLOAT_MODE_P (mode)) > > > > - || standard_sse_constant_p (op, mode)) > > > > - return nullptr; > > > > - > > > This part is dropped in the new ix86_broadcast_from_constant,why? > > > > > > Restored. > > > > > > /* Don't broadcast from a 64-bit integer constant in 32-bit mode. > > > > We can still put 64-bit integer constant in memory when > > > > avx512 embed broadcast is available. */ > > > > @@ -486,13 +467,6 @@ ix86_broadcast_from_constant (machine_mode mode, rtx op) > > > > || (GET_MODE_SIZE (mode) < 64 && !TARGET_AVX512VL))) > > > > return nullptr; > > > > > > > > - if (GET_MODE_INNER (mode) == TImode) > > > > - return nullptr; > > > > - > > > > - rtx constant = get_pool_constant (XEXP (op, 0)); > > > > - if (GET_CODE (constant) != CONST_VECTOR) > > > > - return nullptr; > > > > - > > > > /* There could be some rtx like > > > > (mem/u/c:V16QI (symbol_ref/u:DI ("*.LC1"))) > > > > but with "*.LC1" refer to V2DI constant vector. */ > > > > @@ -506,7 +480,7 @@ ix86_broadcast_from_constant (machine_mode mode, rtx op) > > > > > > > > rtx first = XVECEXP (constant, 0, 0); > > > > > > > > - for (int i = 1; i < nunits; ++i) > > > > + for (int i = 1; i < GET_MODE_NUNITS (mode); ++i) > > > > { > > > > rtx tmp = XVECEXP (constant, 0, i); > > > > /* Vector duplicate value. */ > > > > @@ -517,6 +491,44 @@ ix86_broadcast_from_constant (machine_mode mode, rtx op) > > > > return first; > > > > } > > > > > > > > +/* OP is a memref of CONST_VECTOR, return scalar constant mem > > > > + if CONST_VECTOR is a vec_duplicate, else return NULL. */ > > > > +static rtx > > > > +ix86_broadcast_from_constant (machine_mode mode, rtx op) > > > > +{ > > > > + int nunits = GET_MODE_NUNITS (mode); > > > > + if (nunits < 2) > > > > + return nullptr; > > > > + > > > > + /* Don't use integer vector broadcast if we can't move from GPR to SSE > > > > + register directly. */ > > > > + if (!TARGET_INTER_UNIT_MOVES_TO_VEC > > > > + && INTEGRAL_MODE_P (mode)) > > > > + return nullptr; > > > > + > > > > + if (GET_MODE_INNER (mode) == TImode) > > > > + return nullptr; > > > > + > > > > + rtx constant = get_pool_constant (XEXP (op, 0)); > > > > + if (GET_CODE (constant) != CONST_VECTOR) > > > > + return nullptr; > > > > + > > > > + return ix86_constant_broadcast (constant, mode); > > > > +} > > > > + > > > > +/* Return true if OP is a non-uniform CONST_VECTOR. */ > > > > + > > > Drop empty line. > > > > Removed. > > > > > > +bool > > > > +non_uniform_const_vector_p (rtx op, machine_mode mode) > > > > +{ > > > > + /* Allow non-uniform CONST_VECTOR only in the combine pass since other > > > > + RTL optimizers work better with constant pool. */ > > > > + return (current_pass > > > > + && current_pass->tv_id == TV_COMBINE > > > > + && GET_CODE (op) == CONST_VECTOR > > > > + && ix86_constant_broadcast (op, mode) == nullptr); > > > > +} > > > I'm not sure it's a good idea to add a specific pass condition to the > > > backend's predicate. > > > If it's for combine, should we add define_split alternatively? > > > > I tried. RTL optimizers won't optimize non-uniform CONST_VECTOR which > > leads extra moves: > > > > FAIL: gcc.target/i386/fnabs.c scan-assembler-times \tv?orp[sd][ \t] 2 > > FAIL: gcc.target/i386/fuse-caller-save-xmm.c scan-assembler-times addpd\\t\\.?[Ll]C0.*, %xmm0 1 > > FAIL: gcc.target/i386/fuse-caller-save-xmm.c scan-assembler-times movapd\t%xmm0, %xmm1 1 > > > > Only all bits 0/1 CONST_VECTOR is allowed as an operand for load and all > > other CONST_VECTORs are placed in constant pool. My patch allows > > non-uniform CONST_VECTOR in the combine pass so that it can generate load > > from non-uniform CONST_VECTOR. Since non-uniform CONST_VECTOR isn't > > allowed in other RTL passes, it will be put in constant pool. > > > Trying 5 -> 7: > 5: r85:V4SF=[`*.LC0'] > REG_EQUAL const_vector > 7: r84:V4SF=vec_select(vec_concat(r85:V4SF,r85:V4SF),parallel) > REG_DEAD r85:V4SF > REG_EQUAL const_vector > Failed to match this instruction: > (set (reg:V4SF 84) > (const_vector:V4SF [ > (const_double:SF 3.0e+0 [0x0.cp+2]) > (const_double:SF 2.0e+0 [0x0.8p+2]) > (const_double:SF 4.0e+0 [0x0.8p+3]) > (const_double:SF 1.0e+0 [0x0.8p+1]) > ])) > > (insn 5 2 7 2 (set (reg:V4SF 85) > (mem/u/c:V4SF (symbol_ref/u:DI ("*.LC0") [flags 0x2]) [0 S16 > A128])) "/export/users/liuhongt/install/git_trunk_master_native/lib/gcc/x86_64-pc-linux-gnu/12.0.0/include/xmmintrin.h":746:19 > 1600 {movv4sf_internal} > (expr_list:REG_EQUAL (const_vector:V4SF [ > (const_double:SF 4.0e+0 [0x0.8p+3]) > (const_double:SF 3.0e+0 [0x0.cp+2]) > (const_double:SF 2.0e+0 [0x0.8p+2]) > (const_double:SF 1.0e+0 [0x0.8p+1]) > ]) > (nil))) > (insn 7 5 11 2 (set (reg:V4SF 84) > (vec_select:V4SF (vec_concat:V8SF (reg:V4SF 85) > (reg:V4SF 85)) > (parallel [ > (const_int 1 [0x1]) > (const_int 2 [0x2]) > (const_int 4 [0x4]) > (const_int 7 [0x7]) > ]))) > "/export/users/liuhongt/install/git_trunk_master_native/lib/gcc/x86_64-pc-linux-gnu/12.0.0/include/xmmintrin.h":746:19 > 3015 {sse_shufps_v4sf} > (expr_list:REG_DEAD (reg:V4SF 85) > (expr_list:REG_EQUAL (const_vector:V4SF [ > (const_double:SF 3.0e+0 [0x0.cp+2]) > (const_double:SF 2.0e+0 [0x0.8p+2]) > (const_double:SF 4.0e+0 [0x0.8p+3]) > (const_double:SF 1.0e+0 [0x0.8p+1]) > ]) > (nil)))) > > I think pass_combine should be extended to force illegitimate constant > to constant pool and recog load insn again, It looks like a general > optimization that better not do it in the backend. It sounds reasonable. I updated the bug report. > > H.J. > > --- > > In vetor move pattern, replace nonimmediate_or_sse_const_operand with > > nonimmediate_or_sse_const_vector_operand to allow vector load from > > non-uniform CONST_VECTOR. Non-uniform CONST_VECTOR is enabled only in > > the combine pass since other RTL optimizers and IRA/LRA work better with > > constant pool. > > > > gcc/ > > > > PR target/43147 > > * config/i386/i386-expand.c (ix86_constant_broadcast): New > > function. Extracted from ix86_broadcast_from_constant. > > (ix86_broadcast_from_constant): Call ix86_constant_broadcast. > > (non_uniform_const_vector_p): New function. > > * config/i386/i386-protos.h (non_uniform_const_vector_p): New > > prototype. > > * config/i386/predicates.md > > (nonimmediate_or_sse_const_vector_operand): New predicate. > > * config/i386/sse.md (mov<mode>_internal): Replace > > nonimmediate_or_sse_const_operand with > > nonimmediate_or_sse_const_vector_operand. > > > > gcc/testsuite/ > > > > PR target/43147 > > * gcc.target/i386/pr43147.c: New test. > > --- > > gcc/config/i386/i386-expand.c | 79 ++++++++++++++++--------- > > gcc/config/i386/i386-protos.h | 1 + > > gcc/config/i386/predicates.md | 7 +++ > > gcc/config/i386/sse.md | 2 +- > > gcc/testsuite/gcc.target/i386/pr43147.c | 15 +++++ > > 5 files changed, 74 insertions(+), 30 deletions(-) > > create mode 100644 gcc/testsuite/gcc.target/i386/pr43147.c > > > > diff --git a/gcc/config/i386/i386-expand.c b/gcc/config/i386/i386-expand.c > > index 9bf13dbfa92..599f66582d5 100644 > > --- a/gcc/config/i386/i386-expand.c > > +++ b/gcc/config/i386/i386-expand.c > > @@ -453,6 +453,44 @@ ix86_expand_move (machine_mode mode, rtx operands[]) > > emit_insn (gen_rtx_SET (op0, op1)); > > } > > > > +/* CONSTANT is a CONST_VECTOR, return scalar constant if CONST_VECTOR is > > + a vec_duplicate, else return nullptr. */ > > + > > +static rtx > > +ix86_constant_broadcast (rtx constant, machine_mode mode) > > +{ > > + /* Don't broadcast from a 64-bit integer constant in 32-bit mode. > > + We can still put 64-bit integer constant in memory when > > + avx512 embed broadcast is available. */ > > + if (GET_MODE_INNER (mode) == DImode && !TARGET_64BIT > > + && (!TARGET_AVX512F > > + || (GET_MODE_SIZE (mode) < 64 && !TARGET_AVX512VL))) > > + return nullptr; > > + > > + /* There could be some rtx like > > + (mem/u/c:V16QI (symbol_ref/u:DI ("*.LC1"))) > > + but with "*.LC1" refer to V2DI constant vector. */ > > + if (GET_MODE (constant) != mode) > > + { > > + constant = simplify_subreg (mode, constant, GET_MODE (constant), > > + 0); > > + if (constant == nullptr || GET_CODE (constant) != CONST_VECTOR) > > + return nullptr; > > + } > > + > > + rtx first = XVECEXP (constant, 0, 0); > > + > > + for (int i = 1; i < GET_MODE_NUNITS (mode); ++i) > > + { > > + rtx tmp = XVECEXP (constant, 0, i); > > + /* Vector duplicate value. */ > > + if (!rtx_equal_p (tmp, first)) > > + return nullptr; > > + } > > + > > + return first; > > +} > > + > > /* OP is a memref of CONST_VECTOR, return scalar constant mem > > if CONST_VECTOR is a vec_duplicate, else return NULL. */ > > static rtx > > @@ -478,14 +516,6 @@ ix86_broadcast_from_constant (machine_mode mode, rtx op) > > || standard_sse_constant_p (op, mode)) > > return nullptr; > > > > - /* Don't broadcast from a 64-bit integer constant in 32-bit mode. > > - We can still put 64-bit integer constant in memory when > > - avx512 embed broadcast is available. */ > > - if (GET_MODE_INNER (mode) == DImode && !TARGET_64BIT > > - && (!TARGET_AVX512F > > - || (GET_MODE_SIZE (mode) < 64 && !TARGET_AVX512VL))) > > - return nullptr; > > - > > if (GET_MODE_INNER (mode) == TImode) > > return nullptr; > > > > @@ -493,28 +523,19 @@ ix86_broadcast_from_constant (machine_mode mode, rtx op) > > if (GET_CODE (constant) != CONST_VECTOR) > > return nullptr; > > > > - /* There could be some rtx like > > - (mem/u/c:V16QI (symbol_ref/u:DI ("*.LC1"))) > > - but with "*.LC1" refer to V2DI constant vector. */ > > - if (GET_MODE (constant) != mode) > > - { > > - constant = simplify_subreg (mode, constant, GET_MODE (constant), > > - 0); > > - if (constant == nullptr || GET_CODE (constant) != CONST_VECTOR) > > - return nullptr; > > - } > > - > > - rtx first = XVECEXP (constant, 0, 0); > > - > > - for (int i = 1; i < nunits; ++i) > > - { > > - rtx tmp = XVECEXP (constant, 0, i); > > - /* Vector duplicate value. */ > > - if (!rtx_equal_p (tmp, first)) > > - return nullptr; > > - } > > + return ix86_constant_broadcast (constant, mode); > > +} > > > > - return first; > > +/* Return true if OP is a non-uniform CONST_VECTOR. */ > > +bool > > +non_uniform_const_vector_p (rtx op, machine_mode mode) > > +{ > > + /* Allow non-uniform CONST_VECTOR only in the combine pass since other > > + RTL optimizers and IRA/LRA work better with constant pool. */ > > + return (current_pass > > + && current_pass->tv_id == TV_COMBINE > > + && GET_CODE (op) == CONST_VECTOR > > + && ix86_constant_broadcast (op, mode) == nullptr); > > } > > > > void > > diff --git a/gcc/config/i386/i386-protos.h b/gcc/config/i386/i386-protos.h > > index 2fd13074c81..91d4c7cefb8 100644 > > --- a/gcc/config/i386/i386-protos.h > > +++ b/gcc/config/i386/i386-protos.h > > @@ -58,6 +58,7 @@ extern int standard_80387_constant_p (rtx); > > extern const char *standard_80387_constant_opcode (rtx); > > extern rtx standard_80387_constant_rtx (int); > > extern int standard_sse_constant_p (rtx, machine_mode); > > +extern bool non_uniform_const_vector_p (rtx, machine_mode); > > extern const char *standard_sse_constant_opcode (rtx_insn *, rtx *); > > extern bool ix86_standard_x87sse_constant_load_p (const rtx_insn *, rtx); > > extern bool ix86_pre_reload_split (void); > > diff --git a/gcc/config/i386/predicates.md b/gcc/config/i386/predicates.md > > index 9321f332ef9..435b22855c8 100644 > > --- a/gcc/config/i386/predicates.md > > +++ b/gcc/config/i386/predicates.md > > @@ -1176,6 +1176,13 @@ (define_predicate "nonimmediate_or_sse_const_operand" > > (ior (match_operand 0 "nonimmediate_operand") > > (match_test "standard_sse_constant_p (op, mode)"))) > > > > +;; Return true when OP is nonimmediate, standard SSE constant, or > > +;; non-uniform CONST_VECTOR. > > +(define_predicate "nonimmediate_or_sse_const_vector_operand" > > + (ior (match_operand 0 "nonimmediate_operand") > > + (match_test "standard_sse_constant_p (op, mode)") > > + (match_test "non_uniform_const_vector_p (op, mode)"))) > > + > > ;; Return true if OP is a register or a zero. > > (define_predicate "reg_or_0_operand" > > (ior (match_operand 0 "register_operand") > > diff --git a/gcc/config/i386/sse.md b/gcc/config/i386/sse.md > > index 95f95823ea3..a7919eca6c3 100644 > > --- a/gcc/config/i386/sse.md > > +++ b/gcc/config/i386/sse.md > > @@ -1075,7 +1075,7 @@ (define_expand "mov<mode>" > > (define_insn "mov<mode>_internal" > > [(set (match_operand:VMOVE 0 "nonimmediate_operand" > > "=v,v ,v ,m") > > - (match_operand:VMOVE 1 "nonimmediate_or_sse_const_operand" > > + (match_operand:VMOVE 1 "nonimmediate_or_sse_const_vector_operand" > > " C,<sseconstm1>,vm,v"))] > > "TARGET_SSE > > && (register_operand (operands[0], <MODE>mode) > > diff --git a/gcc/testsuite/gcc.target/i386/pr43147.c b/gcc/testsuite/gcc.target/i386/pr43147.c > > new file mode 100644 > > index 00000000000..3c30f917c06 > > --- /dev/null > > +++ b/gcc/testsuite/gcc.target/i386/pr43147.c > > @@ -0,0 +1,15 @@ > > +/* { dg-do compile } */ > > +/* { dg-options "-O2 -msse2" } */ > > +/* { dg-final { scan-assembler "movaps" } } */ > > +/* { dg-final { scan-assembler-not "shufps" } } */ > > + > > +#include <x86intrin.h> > > + > > +__m128 > > +foo (void) > > +{ > > + __m128 m = _mm_set_ps(1.0f, 2.0f, 3.0f, 4.0f); > > + m = _mm_shuffle_ps(m, m, 0xC9); > > + m = _mm_shuffle_ps(m, m, 0x2D); > > + return m; > > +} > > -- > > 2.31.1 > > > > > -- > BR, > Hongtao -- H.J. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] x86: Allow CONST_VECTOR for vector load in combine 2021-08-24 1:57 ` Hongtao Liu 2021-08-24 1:58 ` H.J. Lu @ 2021-08-24 16:14 ` Segher Boessenkool 2021-08-24 17:03 ` H.J. Lu 1 sibling, 1 reply; 7+ messages in thread From: Segher Boessenkool @ 2021-08-24 16:14 UTC (permalink / raw) To: Hongtao Liu; +Cc: H.J. Lu, GCC Patches, Richard Sandiford On Tue, Aug 24, 2021 at 09:57:52AM +0800, Hongtao Liu wrote: > Trying 5 -> 7: > 5: r85:V4SF=[`*.LC0'] > REG_EQUAL const_vector > 7: r84:V4SF=vec_select(vec_concat(r85:V4SF,r85:V4SF),parallel) > REG_DEAD r85:V4SF > REG_EQUAL const_vector > Failed to match this instruction: > (set (reg:V4SF 84) > (const_vector:V4SF [ > (const_double:SF 3.0e+0 [0x0.cp+2]) > (const_double:SF 2.0e+0 [0x0.8p+2]) > (const_double:SF 4.0e+0 [0x0.8p+3]) > (const_double:SF 1.0e+0 [0x0.8p+1]) > ])) > > (insn 5 2 7 2 (set (reg:V4SF 85) > (mem/u/c:V4SF (symbol_ref/u:DI ("*.LC0") [flags 0x2]) [0 S16 > A128])) "/export/users/liuhongt/install/git_trunk_master_native/lib/gcc/x86_64-pc-linux-gnu/12.0.0/include/xmmintrin.h":746:19 > 1600 {movv4sf_internal} > (expr_list:REG_EQUAL (const_vector:V4SF [ > (const_double:SF 4.0e+0 [0x0.8p+3]) > (const_double:SF 3.0e+0 [0x0.cp+2]) > (const_double:SF 2.0e+0 [0x0.8p+2]) > (const_double:SF 1.0e+0 [0x0.8p+1]) > ]) > (nil))) > (insn 7 5 11 2 (set (reg:V4SF 84) > (vec_select:V4SF (vec_concat:V8SF (reg:V4SF 85) > (reg:V4SF 85)) > (parallel [ > (const_int 1 [0x1]) > (const_int 2 [0x2]) > (const_int 4 [0x4]) > (const_int 7 [0x7]) > ]))) > "/export/users/liuhongt/install/git_trunk_master_native/lib/gcc/x86_64-pc-linux-gnu/12.0.0/include/xmmintrin.h":746:19 > 3015 {sse_shufps_v4sf} > (expr_list:REG_DEAD (reg:V4SF 85) > (expr_list:REG_EQUAL (const_vector:V4SF [ > (const_double:SF 3.0e+0 [0x0.cp+2]) > (const_double:SF 2.0e+0 [0x0.8p+2]) > (const_double:SF 4.0e+0 [0x0.8p+3]) > (const_double:SF 1.0e+0 [0x0.8p+1]) > ]) > (nil)))) > > I think pass_combine should be extended to force illegitimate constant > to constant pool and recog load insn again, It looks like a general > optimization that better not do it in the backend. Patches welcome. You should do this like change_zero_ext is done, and perhaps make sure you do not introduce new is_just_move insns that can make 2->2 combinations do the wrong thing. Also somehow make this not take exponential time? It looks like this should onle be done in cases where change_zero_ext is not, and the reverse, so this will work fine with a little attention to detail. gl;hf, Segher ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] x86: Allow CONST_VECTOR for vector load in combine 2021-08-24 16:14 ` Segher Boessenkool @ 2021-08-24 17:03 ` H.J. Lu 0 siblings, 0 replies; 7+ messages in thread From: H.J. Lu @ 2021-08-24 17:03 UTC (permalink / raw) To: Segher Boessenkool; +Cc: Hongtao Liu, GCC Patches, Richard Sandiford On Tue, Aug 24, 2021 at 9:16 AM Segher Boessenkool <segher@kernel.crashing.org> wrote: > > On Tue, Aug 24, 2021 at 09:57:52AM +0800, Hongtao Liu wrote: > > Trying 5 -> 7: > > 5: r85:V4SF=[`*.LC0'] > > REG_EQUAL const_vector > > 7: r84:V4SF=vec_select(vec_concat(r85:V4SF,r85:V4SF),parallel) > > REG_DEAD r85:V4SF > > REG_EQUAL const_vector > > Failed to match this instruction: > > (set (reg:V4SF 84) > > (const_vector:V4SF [ > > (const_double:SF 3.0e+0 [0x0.cp+2]) > > (const_double:SF 2.0e+0 [0x0.8p+2]) > > (const_double:SF 4.0e+0 [0x0.8p+3]) > > (const_double:SF 1.0e+0 [0x0.8p+1]) > > ])) > > > > (insn 5 2 7 2 (set (reg:V4SF 85) > > (mem/u/c:V4SF (symbol_ref/u:DI ("*.LC0") [flags 0x2]) [0 S16 > > A128])) "/export/users/liuhongt/install/git_trunk_master_native/lib/gcc/x86_64-pc-linux-gnu/12.0.0/include/xmmintrin.h":746:19 > > 1600 {movv4sf_internal} > > (expr_list:REG_EQUAL (const_vector:V4SF [ > > (const_double:SF 4.0e+0 [0x0.8p+3]) > > (const_double:SF 3.0e+0 [0x0.cp+2]) > > (const_double:SF 2.0e+0 [0x0.8p+2]) > > (const_double:SF 1.0e+0 [0x0.8p+1]) > > ]) > > (nil))) > > (insn 7 5 11 2 (set (reg:V4SF 84) > > (vec_select:V4SF (vec_concat:V8SF (reg:V4SF 85) > > (reg:V4SF 85)) > > (parallel [ > > (const_int 1 [0x1]) > > (const_int 2 [0x2]) > > (const_int 4 [0x4]) > > (const_int 7 [0x7]) > > ]))) > > "/export/users/liuhongt/install/git_trunk_master_native/lib/gcc/x86_64-pc-linux-gnu/12.0.0/include/xmmintrin.h":746:19 > > 3015 {sse_shufps_v4sf} > > (expr_list:REG_DEAD (reg:V4SF 85) > > (expr_list:REG_EQUAL (const_vector:V4SF [ > > (const_double:SF 3.0e+0 [0x0.cp+2]) > > (const_double:SF 2.0e+0 [0x0.8p+2]) > > (const_double:SF 4.0e+0 [0x0.8p+3]) > > (const_double:SF 1.0e+0 [0x0.8p+1]) > > ]) > > (nil)))) > > > > I think pass_combine should be extended to force illegitimate constant > > to constant pool and recog load insn again, It looks like a general > > optimization that better not do it in the backend. > > Patches welcome. You should do this like change_zero_ext is done, and > perhaps make sure you do not introduce new is_just_move insns that can > make 2->2 combinations do the wrong thing. > > Also somehow make this not take exponential time? It looks like this > should onle be done in cases where change_zero_ext is not, and the > reverse, so this will work fine with a little attention to detail. > The combine patch is here: https://gcc.gnu.org/pipermail/gcc-patches/2021-August/578017.html Thanks. -- H.J. ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-08-24 17:03 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-08-22 12:53 [PATCH] x86: Allow CONST_VECTOR for vector load in combine H.J. Lu 2021-08-23 7:23 ` Hongtao Liu 2021-08-23 13:14 ` [PATCH v2] " H.J. Lu 2021-08-24 1:57 ` Hongtao Liu 2021-08-24 1:58 ` H.J. Lu 2021-08-24 16:14 ` Segher Boessenkool 2021-08-24 17:03 ` H.J. Lu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).