RISC-V: Allow zero operand for DI variants of vssubu.vx

The RISC-V vector machine description relies on the helper function
`sew64_scalar_helper` to emit actual insns for the DI variants of
vssub.vx and vssubu.vx.  This works with vssub.vx, but can cause
problems with vssubu.vx with the scalar operand being constant zero,
because `has_vi_variant_p` returns false, and the operand will be taken
without being loaded into a reg.  The attached testcases can cause an
internal compiler error as a result.

Allowing a constant zero operand in those insns seems to be a simple
solution that only affects minimum existing code.

gcc/ChangeLog:

	* config/riscv/vector.md: Allow zero operand for DI variants of
	vssubu.vx

gcc/testsuite/ChangeLog:

	* gcc.target/riscv/rvv/base/vssubu-1.c: New test.
	* gcc.target/riscv/rvv/base/vssubu-2.c: New test.
---
 gcc/config/riscv/vector.md                         |  8 ++++----
 gcc/testsuite/gcc.target/riscv/rvv/base/vssubu-1.c | 11 +++++++++++
 gcc/testsuite/gcc.target/riscv/rvv/base/vssubu-2.c | 11 +++++++++++
 3 files changed, 26 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/vssubu-1.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/vssubu-2.c

LGTM, thanks :)

Bohan Lei <garthlei@linux.alibaba.com> 於 2024年9月18日 週三 05:28 寫道：

> The RISC-V vector machine description relies on the helper function
> `sew64_scalar_helper` to emit actual insns for the DI variants of
> vssub.vx and vssubu.vx.  This works with vssub.vx, but can cause
> problems with vssubu.vx with the scalar operand being constant zero,
> because `has_vi_variant_p` returns false, and the operand will be taken
> without being loaded into a reg.  The attached testcases can cause an
> internal compiler error as a result.
>
> Allowing a constant zero operand in those insns seems to be a simple
> solution that only affects minimum existing code.
>
> gcc/ChangeLog:
>
>         * config/riscv/vector.md: Allow zero operand for DI variants of
>         vssubu.vx
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.target/riscv/rvv/base/vssubu-1.c: New test.
>         * gcc.target/riscv/rvv/base/vssubu-2.c: New test.
> ---
>  gcc/config/riscv/vector.md                         |  8 ++++----
>  gcc/testsuite/gcc.target/riscv/rvv/base/vssubu-1.c | 11 +++++++++++
>  gcc/testsuite/gcc.target/riscv/rvv/base/vssubu-2.c | 11 +++++++++++
>  3 files changed, 26 insertions(+), 4 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/vssubu-1.c
>  create mode 100644 gcc/testsuite/gcc.target/riscv/rvv/base/vssubu-2.c
>
> diff --git a/gcc/config/riscv/vector.md b/gcc/config/riscv/vector.md
> index d0677325ba1..66a2a477faf 100644
> --- a/gcc/config/riscv/vector.md
> +++ b/gcc/config/riscv/vector.md
> @@ -4400,10 +4400,10 @@ (define_insn "*pred_<optab><mode>_scalar"
>           (sat_int_minus_binop:VI_D
>             (match_operand:VI_D 3 "register_operand"     " vr, vr, vr, vr")
>             (vec_duplicate:VI_D
> -             (match_operand:<VEL> 4 "register_operand"  "  r,  r,  r,
> r")))
> +             (match_operand:<VEL> 4 "reg_or_0_operand"  "  rJ, rJ, rJ,
> rJ")))
>           (match_operand:VI_D 2 "vector_merge_operand"   " vu,  0, vu,
> 0")))]
>    "TARGET_VECTOR"
> -  "v<insn>.vx\t%0,%3,%4%p1"
> +  "v<insn>.vx\t%0,%3,%z4%p1"
>    [(set_attr "type" "<int_binop_insn_type>")
>     (set_attr "mode" "<MODE>")])
>
> @@ -4422,10 +4422,10 @@ (define_insn "*pred_<optab><mode>_extended_scalar"
>             (match_operand:VI_D 3 "register_operand"         " vr, vr, vr,
> vr")
>             (vec_duplicate:VI_D
>               (sign_extend:<VEL>
> -               (match_operand:<VSUBEL> 4 "register_operand" "  r,  r,
> r,  r"))))
> +               (match_operand:<VSUBEL> 4 "reg_or_0_operand" "  rJ, rJ,
> rJ, rJ"))))
>           (match_operand:VI_D 2 "vector_merge_operand"       " vu,  0,
> vu,  0")))]
>    "TARGET_VECTOR && !TARGET_64BIT"
> -  "v<insn>.vx\t%0,%3,%4%p1"
> +  "v<insn>.vx\t%0,%3,%z4%p1"
>    [(set_attr "type" "<int_binop_insn_type>")
>     (set_attr "mode" "<MODE>")])
>
> diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/vssubu-1.c
> b/gcc/testsuite/gcc.target/riscv/rvv/base/vssubu-1.c
> new file mode 100644
> index 00000000000..f19b42aed04
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/rvv/base/vssubu-1.c
> @@ -0,0 +1,11 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -march=rv64gcv -mabi=lp64d" } */
> +
> +#include <riscv_vector.h>
> +
> +vuint64m1_t test_vssubu_vx_u64m1(vuint64m1_t op1)
> +{
> +  return __riscv_vssubu_vx_u64m1(op1,0,0);
> +}
> +
> +/* { dg-final { scan-assembler-not {\tvssubu} } } */
> \ No newline at end of file
> diff --git a/gcc/testsuite/gcc.target/riscv/rvv/base/vssubu-2.c
> b/gcc/testsuite/gcc.target/riscv/rvv/base/vssubu-2.c
> new file mode 100644
> index 00000000000..cb4e4f48a9b
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/rvv/base/vssubu-2.c
> @@ -0,0 +1,11 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -march=rv32gcv -mabi=ilp32d" } */
> +
> +#include <riscv_vector.h>
> +
> +vuint64m1_t test_vssubu_vx_u64m1(vuint64m1_t op1)
> +{
> +  return __riscv_vssubu_vx_u64m1(op1,0,0);
> +}
> +
> +/* { dg-final { scan-assembler-not {\tvssubu} } } */
> \ No newline at end of file
> --
> 2.46.0
>
>

On 9/18/24 1:41 AM, Kito Cheng wrote:
> LGTM, thanks :)
Agreed and pushed.

> 
> 
> Bohan Lei <garthlei@linux.alibaba.com 
> <mailto:garthlei@linux.alibaba.com>> 於 2024年9月18日 週三 05:28 寫道：
> 
>     The RISC-V vector machine description relies on the helper function
>     `sew64_scalar_helper` to emit actual insns for the DI variants of
>     vssub.vx and vssubu.vx.  This works with vssub.vx, but can cause
>     problems with vssubu.vx with the scalar operand being constant zero,
>     because `has_vi_variant_p` returns false, and the operand will be taken
>     without being loaded into a reg.  The attached testcases can cause an
>     internal compiler error as a result.
> 
>     Allowing a constant zero operand in those insns seems to be a simple
>     solution that only affects minimum existing code.
> 
>     gcc/ChangeLog:
> 
>              * config/riscv/vector.md: Allow zero operand for DI variants of
>              vssubu.vx
> 
>     gcc/testsuite/ChangeLog:
> 
>              * gcc.target/riscv/rvv/base/vssubu-1.c: New test.
>              * gcc.target/riscv/rvv/base/vssubu-2.c: New test.
>     ---