[PATCH] RISC-V: Support simplify (-1-x) for vector.

[PATCH] RISC-V: Support simplify (-1-x) for vector.

[yanzhang.wang]

From: Yanzhang Wang <yanzhang.wang@intel.com>

The pattern is enabled for scalar but not for vector. The patch try to
make it consistent and will convert below code,

shortcut_for_riscv_vrsub_case_1_32:
        vl1re32.v       v1,0(a1)
        vsetvli zero,a2,e32,m1,ta,ma
        vrsub.vi        v1,v1,-1
        vs1r.v  v1,0(a0)
        ret

to,

shortcut_for_riscv_vrsub_case_1_32:
        vl1re32.v       v1,0(a1)
        vsetvli zero,a2,e32,m1,ta,ma
        vnot.v  v1,v1
        vs1r.v  v1,0(a0)
        ret

gcc/ChangeLog:

	* simplify-rtx.cc (simplify_context::simplify_binary_operation_1):
    Get -1 with mode.

gcc/testsuite/ChangeLog:

	* gcc.target/riscv/rvv/base/simplify-vrsub.c: New test.

Signed-off-by: Yanzhang Wang <yanzhang.wang@intel.com>
---
 gcc/simplify-rtx.cc                            |  2 +-
 .../gcc.target/riscv/rvv/base/simplify-vrsub.c | 18 ++++++++++++++++++
 2 files changed, 19 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/simplify-vrsub.c

diff --git a/gcc/simplify-rtx.cc b/gcc/simplify-rtx.cc
index d7315d82aa3..eb1ac120832 100644
--- a/gcc/simplify-rtx.cc
+++ b/gcc/simplify-rtx.cc
@@ -3071,7 +3071,7 @@ simplify_context::simplify_binary_operation_1 (rtx_code code,
       /* (-1 - a) is ~a, unless the expression contains symbolic
 	 constants, in which case not retaining additions and
 	 subtractions could cause invalid assembly to be produced.  */
-      if (trueop0 == constm1_rtx
+      if (trueop0 == CONSTM1_RTX (mode)
 	  && !contains_symbolic_reference_p (op1))
 	return simplify_gen_unary (NOT, mode, op1, mode);
 
diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/simplify-vrsub.c b/gcc/testsuite/gcc.target/riscv/rvv/base/simplify-vrsub.c
new file mode 100644
index 00000000000..df87ed94ea4
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/rvv/base/simplify-vrsub.c
@@ -0,0 +1,18 @@
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gcv -mabi=lp64 -O3 -Wno-psabi" } */
+
+#include "riscv_vector.h"
+
+#define VRSUB_WITH_LMUL(LMUL, DTYPE)                        \
+  vint##DTYPE##m##LMUL##_t                                  \
+  shortcut_for_riscv_vrsub_case_##LMUL##_##DTYPE            \
+  (vint##DTYPE##m##LMUL##_t v1,                             \
+   size_t vl)                                               \
+  {                                                         \
+    return __riscv_vrsub_vx_i##DTYPE##m##LMUL (v1, -1, vl); \
+  }
+
+VRSUB_WITH_LMUL (1, 16)
+VRSUB_WITH_LMUL (1, 32)
+
+/* { dg-final { scan-assembler-times {vnot\.v} 2 } } */
-- 
2.41.0

Re: [PATCH] RISC-V: Support simplify (-1-x) for vector.

[Jeff Law]

On 8/16/23 02:40, yanzhang.wang--- via Gcc-patches wrote:
> From: Yanzhang Wang <yanzhang.wang@intel.com>
> 
> The pattern is enabled for scalar but not for vector. The patch try to
> make it consistent and will convert below code,
> 
> shortcut_for_riscv_vrsub_case_1_32:
>          vl1re32.v       v1,0(a1)
>          vsetvli zero,a2,e32,m1,ta,ma
>          vrsub.vi        v1,v1,-1
>          vs1r.v  v1,0(a0)
>          ret
> 
> to,
> 
> shortcut_for_riscv_vrsub_case_1_32:
>          vl1re32.v       v1,0(a1)
>          vsetvli zero,a2,e32,m1,ta,ma
>          vnot.v  v1,v1
>          vs1r.v  v1,0(a0)
>          ret
> 
> gcc/ChangeLog:
> 
> 	* simplify-rtx.cc (simplify_context::simplify_binary_operation_1):
>      Get -1 with mode.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.target/riscv/rvv/base/simplify-vrsub.c: New test.
Just a note.  It is customary to indicate what testing you did for each 
patch.  A patch which changes target independent code should be 
bootstrapped and regression tested on at least one major target (most 
folks use x86_64 or aarch64).

If you change target code it is customary to run the testsuite on that 
target.  Ideally that would include a bootstrap and regression test, but 
that's not always possible (cross compilers) in which case you just 
build the toolchain and run the cross tests.

I went ahead and bootstrapped & regression tested this on 
x86_64-linux-gnu where it passed without regressions.

I'll push this to the trunk.

Thanks,
jeff

RE: [PATCH] RISC-V: Support simplify (-1-x) for vector.

[Wang, Yanzhang]

Hi Jeff,

Thank you so much for the note and testing :D.
I'll attach the test result next time.

Thanks,
Yanzhang

> -----Original Message-----
> From: Jeff Law <jeffreyalaw@gmail.com>
> Sent: Thursday, August 17, 2023 12:33 PM
> To: Wang, Yanzhang <yanzhang.wang@intel.com>; gcc-patches@gcc.gnu.org
> Cc: juzhe.zhong@rivai.ai; kito.cheng@sifive.com; Li, Pan2
> <pan2.li@intel.com>
> Subject: Re: [PATCH] RISC-V: Support simplify (-1-x) for vector.
> 
> 
> 
> On 8/16/23 02:40, yanzhang.wang--- via Gcc-patches wrote:
> > From: Yanzhang Wang <yanzhang.wang@intel.com>
> >
> > The pattern is enabled for scalar but not for vector. The patch try to
> > make it consistent and will convert below code,
> >
> > shortcut_for_riscv_vrsub_case_1_32:
> >          vl1re32.v       v1,0(a1)
> >          vsetvli zero,a2,e32,m1,ta,ma
> >          vrsub.vi        v1,v1,-1
> >          vs1r.v  v1,0(a0)
> >          ret
> >
> > to,
> >
> > shortcut_for_riscv_vrsub_case_1_32:
> >          vl1re32.v       v1,0(a1)
> >          vsetvli zero,a2,e32,m1,ta,ma
> >          vnot.v  v1,v1
> >          vs1r.v  v1,0(a0)
> >          ret
> >
> > gcc/ChangeLog:
> >
> > 	* simplify-rtx.cc (simplify_context::simplify_binary_operation_1):
> >      Get -1 with mode.
> >
> > gcc/testsuite/ChangeLog:
> >
> > 	* gcc.target/riscv/rvv/base/simplify-vrsub.c: New test.
> Just a note.  It is customary to indicate what testing you did for each
> patch.  A patch which changes target independent code should be
> bootstrapped and regression tested on at least one major target (most folks
> use x86_64 or aarch64).
> 
> If you change target code it is customary to run the testsuite on that
> target.  Ideally that would include a bootstrap and regression test, but
> that's not always possible (cross compilers) in which case you just build
> the toolchain and run the cross tests.
> 
> I went ahead and bootstrapped & regression tested this on x86_64-linux-gnu
> where it passed without regressions.
> 
> I'll push this to the trunk.
> 
> Thanks,
> jeff

Re: [PATCH] RISC-V: Support simplify (-1-x) for vector.

[Prathamesh Kulkarni]

On Wed, 16 Aug 2023 at 14:12, yanzhang.wang--- via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> From: Yanzhang Wang <yanzhang.wang@intel.com>
>
> The pattern is enabled for scalar but not for vector. The patch try to
> make it consistent and will convert below code,
(CCing Richard S.)
Hi,
Sorry if this comment is not relevant to the patch but I was wondering if it
should also fold -1 - x --> ~x for the following test or is the test
written incorrectly ?

svint32_t f(svint32_t x)
{
  return svsub_s32_x (svptrue_b8 (), svdup_s32 (-1), x);
}

expand dump shows:
(insn 2 4 3 2 (set (reg/v:VNx4SI 93 [ x ])
        (reg:VNx4SI 32 v0 [ x ])) "foo.c":9:1 -1
     (nil))
(note 3 2 6 2 NOTE_INSN_FUNCTION_BEG)
(insn 6 3 7 2 (set (reg:VNx4SI 94)
        (const_vector:VNx4SI repeat [
                (const_int -1 [0xffffffffffffffff])
            ])) "foo.c":10:10 -1
     (nil))
(insn 7 6 11 2 (set (reg:VNx4SI 92 [ <retval> ])
        (minus:VNx4SI (reg:VNx4SI 94)
            (reg/v:VNx4SI 93 [ x ]))) "foo.c":10:10 -1
     (nil))
(insn 11 7 12 2 (set (reg/i:VNx4SI 32 v0)
        (reg:VNx4SI 92 [ <retval> ])) "foo.c":11:1 -1
     (nil))
(insn 12 11 0 2 (use (reg/i:VNx4SI 32 v0)) "foo.c":11:1 -1
     (nil))

and results in following code-gen:
f:
        mov     z31.b, #-1
        sub     z0.s, z31.s, z0.s
        ret

Altho I suppose at TREE level the above call to svsub_s32_x could be folded by
implementing the same transform (-1 - x -> ~x) in svsub_impl::fold ?

Thanks,
Prathamesh




>
> shortcut_for_riscv_vrsub_case_1_32:
>         vl1re32.v       v1,0(a1)
>         vsetvli zero,a2,e32,m1,ta,ma
>         vrsub.vi        v1,v1,-1
>         vs1r.v  v1,0(a0)
>         ret
>
> to,
>
> shortcut_for_riscv_vrsub_case_1_32:
>         vl1re32.v       v1,0(a1)
>         vsetvli zero,a2,e32,m1,ta,ma
>         vnot.v  v1,v1
>         vs1r.v  v1,0(a0)
>         ret
>
> gcc/ChangeLog:
>
>         * simplify-rtx.cc (simplify_context::simplify_binary_operation_1):
>     Get -1 with mode.
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.target/riscv/rvv/base/simplify-vrsub.c: New test.
>
> Signed-off-by: Yanzhang Wang <yanzhang.wang@intel.com>
> ---
>  gcc/simplify-rtx.cc                            |  2 +-
>  .../gcc.target/riscv/rvv/base/simplify-vrsub.c | 18 ++++++++++++++++++
>  2 files changed, 19 insertions(+), 1 deletion(-)
>  create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/simplify-vrsub.c
>
> diff --git a/gcc/simplify-rtx.cc b/gcc/simplify-rtx.cc
> index d7315d82aa3..eb1ac120832 100644
> --- a/gcc/simplify-rtx.cc
> +++ b/gcc/simplify-rtx.cc
> @@ -3071,7 +3071,7 @@ simplify_context::simplify_binary_operation_1 (rtx_code code,
>        /* (-1 - a) is ~a, unless the expression contains symbolic
>          constants, in which case not retaining additions and
>          subtractions could cause invalid assembly to be produced.  */
> -      if (trueop0 == constm1_rtx
> +      if (trueop0 == CONSTM1_RTX (mode)
>           && !contains_symbolic_reference_p (op1))
>         return simplify_gen_unary (NOT, mode, op1, mode);
>
> diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/simplify-vrsub.c b/gcc/testsuite/gcc.target/riscv/rvv/base/simplify-vrsub.c
> new file mode 100644
> index 00000000000..df87ed94ea4
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/rvv/base/simplify-vrsub.c
> @@ -0,0 +1,18 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gcv -mabi=lp64 -O3 -Wno-psabi" } */
> +
> +#include "riscv_vector.h"
> +
> +#define VRSUB_WITH_LMUL(LMUL, DTYPE)                        \
> +  vint##DTYPE##m##LMUL##_t                                  \
> +  shortcut_for_riscv_vrsub_case_##LMUL##_##DTYPE            \
> +  (vint##DTYPE##m##LMUL##_t v1,                             \
> +   size_t vl)                                               \
> +  {                                                         \
> +    return __riscv_vrsub_vx_i##DTYPE##m##LMUL (v1, -1, vl); \
> +  }
> +
> +VRSUB_WITH_LMUL (1, 16)
> +VRSUB_WITH_LMUL (1, 32)
> +
> +/* { dg-final { scan-assembler-times {vnot\.v} 2 } } */
> --
> 2.41.0
>