Re: [match.pd] [SVE] Add pattern to transform svrev(svrev(v)) --> v

[Richard Biener <richard.guenther@gmail.com>]

On Wed, Apr 19, 2023 at 11:21 AM Prathamesh Kulkarni
<prathamesh.kulkarni@linaro.org> wrote:
>
> On Tue, 11 Apr 2023 at 19:36, Prathamesh Kulkarni
> <prathamesh.kulkarni@linaro.org> wrote:
> >
> > On Tue, 11 Apr 2023 at 14:17, Richard Biener <richard.guenther@gmail.com> wrote:
> > >
> > > On Wed, Apr 5, 2023 at 10:39 AM Prathamesh Kulkarni via Gcc-patches
> > > <gcc-patches@gcc.gnu.org> wrote:
> > > >
> > > > Hi,
> > > > For the following test:
> > > >
> > > > svint32_t f(svint32_t v)
> > > > {
> > > >   return svrev_s32 (svrev_s32 (v));
> > > > }
> > > >
> > > > We generate 2 rev instructions instead of nop:
> > > > f:
> > > >         rev     z0.s, z0.s
> > > >         rev     z0.s, z0.s
> > > >         ret
> > > >
> > > > The attached patch tries to fix that by trying to recognize the following
> > > > pattern in match.pd:
> > > > v1 = VEC_PERM_EXPR (v0, v0, mask)
> > > > v2 = VEC_PERM_EXPR (v1, v1, mask)
> > > > -->
> > > > v2 = v0
> > > > if mask is { nelts - 1, nelts - 2, nelts - 3, ... }
> > > >
> > > > Code-gen with patch:
> > > > f:
> > > >         ret
> > > >
> > > > Bootstrap+test passes on aarch64-linux-gnu, and SVE bootstrap in progress.
> > > > Does it look OK for stage-1 ?
> > >
> > > I didn't look at the patch but tree-ssa-forwprop.cc:simplify_permutation should
> > > handle two consecutive permutes with the is_combined_permutation_identity
> > > which might need tweaking for VLA vectors
> > Hi Richard,
> > Thanks for the suggestions. The attached patch modifies
> > is_combined_permutation_identity
> > to recognize the above pattern.
> > Does it look OK ?
> > Bootstrap+test in progress on aarch64-linux-gnu and x86_64-linux-gnu.
> Hi,
> ping https://gcc.gnu.org/pipermail/gcc-patches/2023-April/615502.html

Can you instead of def_stmt pass in a bool whether rhs1 is equal to rhs2
and amend the function comment accordingly, say,

  tem = VEC_PERM <op0, op1, MASK1>;
  res = VEC_PERM <tem, tem, MASK2>;

SAME_P specifies whether op0 and op1 compare equal.  */

+  if (def_stmt)
+    gcc_checking_assert (is_gimple_assign (def_stmt)
+                        && gimple_assign_rhs_code (def_stmt) == VEC_PERM_EXPR);
this is then unnecessary

   mask = fold_ternary (VEC_PERM_EXPR, TREE_TYPE (mask1), mask1, mask1, mask2);
+
+  /* For VLA masks, check for the following pattern:
+     v1 = VEC_PERM_EXPR (v0, v0, mask)
+     v2 = VEC_PERM_EXPR (v1, v1, mask)
+     -->
+     v2 = v0

you are not using 'mask' so please defer fold_ternary until after your
special-case.

+  if (operand_equal_p (mask1, mask2, 0)
+      && !VECTOR_CST_NELTS (mask1).is_constant ()
+      && def_stmt
+      && operand_equal_p (gimple_assign_rhs1 (def_stmt),
+                         gimple_assign_rhs2 (def_stmt), 0))
+    {
+      vec_perm_builder builder;
+      if (tree_to_vec_perm_builder (&builder, mask1))
+       {
+         poly_uint64 nelts = TYPE_VECTOR_SUBPARTS (TREE_TYPE (mask1));
+         vec_perm_indices sel (builder, 1, nelts);
+         if (sel.series_p (0, 1, nelts - 1, -1))
+           return 1;
+       }
+      return 0;

I'm defering to Richard whether this is the correct way to check for a vector
reversing mask (I wonder how constructing such mask is even possible)

Richard.

> Thanks,
> Prathamesh
> >
> > Thanks,
> > Prathamesh
> > >
> > > Richard.
> > >
> > > >
> > > > Thanks,
> > > > Prathamesh