On Thu, Oct 15, 2020 at 4:14 PM Hongtao Liu <[email protected]> wrote:
>
> On Thu, Oct 15, 2020 at 1:37 AM Segher Boessenkool
> <[email protected]> wrote:
> >
> > Hi!
> >
> > On Wed, Oct 14, 2020 at 01:43:45PM +0800, Hongtao Liu wrote:
> > > On Wed, Oct 14, 2020 at 4:01 AM Segher Boessenkool
> > > <[email protected]> wrote:
> > > > On Tue, Oct 13, 2020 at 04:40:53PM +0800, Hongtao Liu wrote:
> > > > > For rtx like
> > > > > (vec_select:V2SI (subreg:V4SI (inner:V2SI) 0)
> > > > > (parallel [(const_int 0) (const_int 1)]))
> > > > > it could be simplified as inner.
> > > >
> > > > You could even simplify any vec_select of a subreg of X to just a
> > > > vec_select of X, by changing the selection vector a bit (well, only do
> > >
> > > Yes, when SUBREG_BYTE of trueop0 is not 0, we need to add offset to
> > > selection.
> >
> > Exactly.
> >
> > > > this if that is a constant vector, I suppose). Not just for paradoxical
> > > > subregs either, just for *all* subregs.
> > >
> > > Yes, and only when X has the same inner mode and more elements.
> >
> > No, for *all*. The mode of the first argument of vec_select does not
> > have to equal its result mode.
> >
> > Any (constant indices) vec_select of a subreg can be written as just a
> > vec_select.
> >
> > > + /* Simplify vec_select of a subreg of X to just a vec_select of X
> > > + when available. */
> >
> > What does "when available" mean here?
> >
>
> When X has same component mode as vec_select, i'll add this to comments.
>
> > > + int l2;
> > > + if (GET_CODE (trueop0) == SUBREG
> > > + && (GET_MODE_INNER (mode)
> > > + == GET_MODE_INNER (GET_MODE (XEXP (trueop0, 0))))
> >
> > Don't use unnecessary parens here please, it makes it harder to read
> > (there are quite enough parens already :-) )
> >
>
> Yes.
>
> > > + gcc_assert (can_div_trunc_p (SUBREG_BYTE (trueop0),
> > > + GET_MODE_SIZE (GET_MODE_INNER
> > > (mode)),
> > > + &subreg_offset));
> >
> > Why is this needed?
>
> I only found this interface for poly_uint64 division to get subreg_offset.
>
> >
> > > + bool success = true;
> > > + for (int i = 0;i != l1; i++)
> >
> > (space after ; )
> >
> > > + {
> > > + rtx j = XVECEXP (trueop1, 0, i);
> >
> > (i and j and k ususally are integers, not rtx)
> >
> > > + if (!CONST_INT_P (j)
> > > + || known_ge (UINTVAL (j), l2 - subreg_offset))
> > > + {
> > > + success = false;
> > > + break;
> > > + }
> > > + }
> >
> > You don't have to test if the input RTL is valid. You can assume it is.
> >
>
> This test is for something like (vec_select:v2di (subreg:v4di
> (reg:v2di) 0)(parallel [ (const_int 2) (const_int 3)])).
> const_int 2 here is out of range. Are you meaning the upper rtx wouldn't
> exist?
>
Remove the test.
> > > + if (success)
> > > + {
> > > + rtx par = trueop1;
> > > + if (subreg_offset)
> > > + {
> > > + rtvec vec = rtvec_alloc (l1);
> > > + for (int i = 0; i < l1; i++)
> > > + RTVEC_ELT (vec, i)
> > > + = GEN_INT (INTVAL (XVECEXP (trueop1, 0, i)
> > > + + subreg_offset));
> > > + par = gen_rtx_PARALLEL (VOIDmode, vec);
> > > + }
> > > + return gen_rtx_VEC_SELECT (mode, XEXP (trueop0, 0), par);
> > > + }
> > > + }
> >
> > subreg_offset will differ in meaning if big-endian; is this correct
> Yes.
> > there, do all the stars align so this code works out fine there as well?
> >
>
> i found it's a bit tricky to adjust selection index for target
> BYTES_BIG_ENDIA != WORDS_BIG_ENDIAN.
> Especially for component mode smaller than word, Any interface to handle this?
>
Use subreg_lsb to get offset from least significant bits, suppose this
could be independent of big/little-endian.
> > Looks fine otherwise, thanks :-)
> >
> >
> > Segher
>
>
>
> --
> BR,
> Hongtao
Update Patch.
--
BR,
Hongtao
From 2b56fed24fe7ef7f49bcac131bbc7f51f36b0a6a Mon Sep 17 00:00:00 2001
From: liuhongt <[email protected]>
Date: Tue, 13 Oct 2020 15:35:29 +0800
Subject: [PATCH] Simplify vec_select of a subreg of X to just a vec_select of
X.
gcc/ChangeLog
PR rtl-optimization/97249
* simplify-rtx.c (simplify_binary_operation_1): Simplify
vec_select of a subreg of X to a vec_select of X when
available.
gcc/testsuite/ChangeLog
* gcc.target/i386/pr97249-1.c: New test.
---
gcc/simplify-rtx.c | 43 +++++++++++++++++++++++
gcc/testsuite/gcc.target/i386/pr97249-1.c | 30 ++++++++++++++++
2 files changed, 73 insertions(+)
create mode 100644 gcc/testsuite/gcc.target/i386/pr97249-1.c
diff --git a/gcc/simplify-rtx.c b/gcc/simplify-rtx.c
index 869f0d11b2e..ff831bcec8f 100644
--- a/gcc/simplify-rtx.c
+++ b/gcc/simplify-rtx.c
@@ -4170,6 +4170,49 @@ simplify_binary_operation_1 (enum rtx_code code, machine_mode mode,
return subop1;
}
}
+
+ /* Simplify vec_select of a subreg of X to just a vec_select of X
+ when X has same component mode as vec_select. */
+ int l2;
+ if (GET_CODE (trueop0) == SUBREG
+ && GET_MODE_INNER (mode)
+ == GET_MODE_INNER (GET_MODE (XEXP (trueop0, 0)))
+ && (GET_MODE_NUNITS (GET_MODE (trueop0))).is_constant (&l0)
+ && (GET_MODE_NUNITS (mode)).is_constant (&l1)
+ && (GET_MODE_NUNITS (GET_MODE (XEXP (trueop0, 0))))
+ .is_constant (&l2)
+ && known_le (l1, l2))
+ {
+ unsigned HOST_WIDE_INT subreg_offset = 0;
+ gcc_assert (known_eq (XVECLEN (trueop1, 0), l1));
+ gcc_assert (can_div_trunc_p (exact_div (subreg_lsb (trueop0), BITS_PER_UNIT),
+ GET_MODE_SIZE (GET_MODE_INNER (mode)),
+ &subreg_offset));
+ bool success = true;
+ for (int i = 0; i != l1; i++)
+ {
+ rtx idx = XVECEXP (trueop1, 0, i);
+ if (!CONST_INT_P (idx))
+ {
+ success = false;
+ break;
+ }
+ }
+ if (success)
+ {
+ rtx par = trueop1;
+ if (subreg_offset)
+ {
+ rtvec vec = rtvec_alloc (l1);
+ for (int i = 0; i < l1; i++)
+ RTVEC_ELT (vec, i)
+ = GEN_INT (INTVAL (XVECEXP (trueop1, 0, i)
+ + subreg_offset));
+ par = gen_rtx_PARALLEL (VOIDmode, vec);
+ }
+ return gen_rtx_VEC_SELECT (mode, XEXP (trueop0, 0), par);
+ }
+ }
}
if (XVECLEN (trueop1, 0) == 1
diff --git a/gcc/testsuite/gcc.target/i386/pr97249-1.c b/gcc/testsuite/gcc.target/i386/pr97249-1.c
new file mode 100644
index 00000000000..4478a34a9f8
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/pr97249-1.c
@@ -0,0 +1,30 @@
+/* PR target/97249 */
+/* { dg-do compile } */
+/* { dg-options "-mavx2 -O3 -masm=att" } */
+/* { dg-final { scan-assembler-times {(?n)vpmovzxbw[ \t]+\(.*%xmm[0-9]} 2 } } */
+/* { dg-final { scan-assembler-times {(?n)vpmovzxwd[ \t]+\(.*%xmm[0-9]} 2 } } */
+/* { dg-final { scan-assembler-times {(?n)vpmovzxdq[ \t]+\(.*%xmm[0-9]} 2 } } */
+
+void
+foo (unsigned char* p1, unsigned char* p2, short* __restrict p3)
+{
+ for (int i = 0 ; i != 8; i++)
+ p3[i] = p1[i] + p2[i];
+ return;
+}
+
+void
+foo1 (unsigned short* p1, unsigned short* p2, int* __restrict p3)
+{
+ for (int i = 0 ; i != 4; i++)
+ p3[i] = p1[i] + p2[i];
+ return;
+}
+
+void
+foo2 (unsigned int* p1, unsigned int* p2, long long* __restrict p3)
+{
+ for (int i = 0 ; i != 2; i++)
+ p3[i] = (long long)p1[i] + (long long)p2[i];
+ return;
+}
--
2.18.1
