RISC-V: Support simplify (-1-x) for vector.
Checks
Commit Message
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
Comments
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
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
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
>
@@ -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);
new file mode 100644
@@ -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 } } */