* [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).