diff mbox series

[v3] RISC-V: Bugfix for doesn't honor no-signed-zeros option

Message ID 20240102115538.1471137-1-pan2.li@intel.com
State New
Headers show
Series [v3] RISC-V: Bugfix for doesn't honor no-signed-zeros option | expand

Commit Message

Li, Pan2 Jan. 2, 2024, 11:55 a.m. UTC 
From: Pan Li <pan2.li@intel.com>

According to the sematics of no-signed-zeros option, the backend
like RISC-V should treat the minus zero -0.0f as plus zero 0.0f.

Consider below example with option -fno-signed-zeros.

void
test (float *a)
{
  *a = -0.0;
}

We will generate code as below, which doesn't treat the minus zero
as plus zero.

test:
  lui  a5,%hi(.LC0)
  flw  fa5,%lo(.LC0)(a5)
  fsw  fa5,0(a0)
  ret

.LC0:
  .word -2147483648 // aka -0.0 (0x80000000 in hex)

This patch would like to fix the bug and treat the minus zero -0.0
as plus zero, aka +0.0. Thus after this patch we will have asm code
as below for the above sampe code.

test:
  sw zero,0(a0)
  ret

This patch also fix the run failure of the test case pr30957-1.c. The
below tests are passed for this patch.

* The riscv regression tests.
* The pr30957-1.c run tests.

gcc/ChangeLog:

	* config/riscv/constraints.md: Leverage func riscv_float_const_zero_rtx_p
	for predicating the rtx is const zero float or not.
	* config/riscv/predicates.md: Ditto.
	* config/riscv/riscv.cc (riscv_const_insns): Ditto.
	(riscv_float_const_zero_rtx_p): New func impl for predicating the rtx is
	const zero float or not.
	(riscv_const_zero_rtx_p): New func impl for predicating the rtx
	is const zero (both int and fp) or not.
	* config/riscv/riscv-protos.h (riscv_float_const_zero_rtx_p):
	New func decl.
	(riscv_const_zero_rtx_p): Ditto.
	* config/riscv/riscv.md: Making sure the operand[1] of movfp is
	CONST0_RTX when the operand[1] is const zero float.

gcc/testsuite/ChangeLog:

	* gcc.target/riscv/no-signed-zeros-0.c: New test.
	* gcc.target/riscv/no-signed-zeros-1.c: New test.
	* gcc.target/riscv/no-signed-zeros-2.c: New test.
	* gcc.target/riscv/no-signed-zeros-3.c: New test.
	* gcc.target/riscv/no-signed-zeros-4.c: New test.
	* gcc.target/riscv/no-signed-zeros-5.c: New test.
	* gcc.target/riscv/no-signed-zeros-run-0.c: New test.
	* gcc.target/riscv/no-signed-zeros-run-1.c: New test.

Signed-off-by: Pan Li <pan2.li@intel.com>
---
 gcc/config/riscv/constraints.md               |  2 +-
 gcc/config/riscv/predicates.md                |  2 +-
 gcc/config/riscv/riscv-protos.h               |  2 +
 gcc/config/riscv/riscv.cc                     | 35 ++++++++++++-
 gcc/config/riscv/riscv.md                     | 49 ++++++++++++++++---
 .../gcc.target/riscv/no-signed-zeros-0.c      | 26 ++++++++++
 .../gcc.target/riscv/no-signed-zeros-1.c      | 28 +++++++++++
 .../gcc.target/riscv/no-signed-zeros-2.c      | 26 ++++++++++
 .../gcc.target/riscv/no-signed-zeros-3.c      | 28 +++++++++++
 .../gcc.target/riscv/no-signed-zeros-4.c      | 26 ++++++++++
 .../gcc.target/riscv/no-signed-zeros-5.c      | 28 +++++++++++
 .../gcc.target/riscv/no-signed-zeros-run-0.c  | 36 ++++++++++++++
 .../gcc.target/riscv/no-signed-zeros-run-1.c  | 36 ++++++++++++++
 13 files changed, 314 insertions(+), 10 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/riscv/no-signed-zeros-0.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/no-signed-zeros-1.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/no-signed-zeros-2.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/no-signed-zeros-3.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/no-signed-zeros-4.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/no-signed-zeros-5.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/no-signed-zeros-run-0.c
 create mode 100644 gcc/testsuite/gcc.target/riscv/no-signed-zeros-run-1.c

Comments

Richard Biener Jan. 8, 2024, 10:45 a.m. UTC | #1 
On Tue, Jan 2, 2024 at 2:37 PM <pan2.li@intel.com> wrote:
>
> From: Pan Li <pan2.li@intel.com>
>
> According to the sematics of no-signed-zeros option, the backend
> like RISC-V should treat the minus zero -0.0f as plus zero 0.0f.
>
> Consider below example with option -fno-signed-zeros.
>
> void
> test (float *a)
> {
>   *a = -0.0;
> }
>
> We will generate code as below, which doesn't treat the minus zero
> as plus zero.
>
> test:
>   lui  a5,%hi(.LC0)
>   flw  fa5,%lo(.LC0)(a5)
>   fsw  fa5,0(a0)
>   ret
>
> .LC0:
>   .word -2147483648 // aka -0.0 (0x80000000 in hex)
>
> This patch would like to fix the bug and treat the minus zero -0.0
> as plus zero, aka +0.0. Thus after this patch we will have asm code
> as below for the above sampe code.
>
> test:
>   sw zero,0(a0)
>   ret
>
> This patch also fix the run failure of the test case pr30957-1.c. The
> below tests are passed for this patch.

We don't really expect targets to do this.  The small testcase above
is somewhat ill-formed with -fno-signed-zeros.  Note there's no
-0.0 in pr30957-1.c so why does that one fail for you?  Does
the -fvariable-expansion-in-unroller code maybe not trigger for
riscv?

I think we should go to PR30957 and see what that was filed originally
for, the testcase doesn't make much sense to me.

> * The riscv regression tests.
> * The pr30957-1.c run tests.
>
> gcc/ChangeLog:
>
>         * config/riscv/constraints.md: Leverage func riscv_float_const_zero_rtx_p
>         for predicating the rtx is const zero float or not.
>         * config/riscv/predicates.md: Ditto.
>         * config/riscv/riscv.cc (riscv_const_insns): Ditto.
>         (riscv_float_const_zero_rtx_p): New func impl for predicating the rtx is
>         const zero float or not.
>         (riscv_const_zero_rtx_p): New func impl for predicating the rtx
>         is const zero (both int and fp) or not.
>         * config/riscv/riscv-protos.h (riscv_float_const_zero_rtx_p):
>         New func decl.
>         (riscv_const_zero_rtx_p): Ditto.
>         * config/riscv/riscv.md: Making sure the operand[1] of movfp is
>         CONST0_RTX when the operand[1] is const zero float.
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.target/riscv/no-signed-zeros-0.c: New test.
>         * gcc.target/riscv/no-signed-zeros-1.c: New test.
>         * gcc.target/riscv/no-signed-zeros-2.c: New test.
>         * gcc.target/riscv/no-signed-zeros-3.c: New test.
>         * gcc.target/riscv/no-signed-zeros-4.c: New test.
>         * gcc.target/riscv/no-signed-zeros-5.c: New test.
>         * gcc.target/riscv/no-signed-zeros-run-0.c: New test.
>         * gcc.target/riscv/no-signed-zeros-run-1.c: New test.
>
> Signed-off-by: Pan Li <pan2.li@intel.com>
> ---
>  gcc/config/riscv/constraints.md               |  2 +-
>  gcc/config/riscv/predicates.md                |  2 +-
>  gcc/config/riscv/riscv-protos.h               |  2 +
>  gcc/config/riscv/riscv.cc                     | 35 ++++++++++++-
>  gcc/config/riscv/riscv.md                     | 49 ++++++++++++++++---
>  .../gcc.target/riscv/no-signed-zeros-0.c      | 26 ++++++++++
>  .../gcc.target/riscv/no-signed-zeros-1.c      | 28 +++++++++++
>  .../gcc.target/riscv/no-signed-zeros-2.c      | 26 ++++++++++
>  .../gcc.target/riscv/no-signed-zeros-3.c      | 28 +++++++++++
>  .../gcc.target/riscv/no-signed-zeros-4.c      | 26 ++++++++++
>  .../gcc.target/riscv/no-signed-zeros-5.c      | 28 +++++++++++
>  .../gcc.target/riscv/no-signed-zeros-run-0.c  | 36 ++++++++++++++
>  .../gcc.target/riscv/no-signed-zeros-run-1.c  | 36 ++++++++++++++
>  13 files changed, 314 insertions(+), 10 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/riscv/no-signed-zeros-0.c
>  create mode 100644 gcc/testsuite/gcc.target/riscv/no-signed-zeros-1.c
>  create mode 100644 gcc/testsuite/gcc.target/riscv/no-signed-zeros-2.c
>  create mode 100644 gcc/testsuite/gcc.target/riscv/no-signed-zeros-3.c
>  create mode 100644 gcc/testsuite/gcc.target/riscv/no-signed-zeros-4.c
>  create mode 100644 gcc/testsuite/gcc.target/riscv/no-signed-zeros-5.c
>  create mode 100644 gcc/testsuite/gcc.target/riscv/no-signed-zeros-run-0.c
>  create mode 100644 gcc/testsuite/gcc.target/riscv/no-signed-zeros-run-1.c
>
> diff --git a/gcc/config/riscv/constraints.md b/gcc/config/riscv/constraints.md
> index de4359af00d..db1d5e1385f 100644
> --- a/gcc/config/riscv/constraints.md
> +++ b/gcc/config/riscv/constraints.md
> @@ -108,7 +108,7 @@ (define_constraint "DnS"
>  (define_constraint "G"
>    "@internal"
>    (and (match_code "const_double")
> -       (match_test "op == CONST0_RTX (mode)")))
> +       (match_test "riscv_float_const_zero_rtx_p (op)")))
>
>  (define_memory_constraint "A"
>    "An address that is held in a general-purpose register."
> diff --git a/gcc/config/riscv/predicates.md b/gcc/config/riscv/predicates.md
> index b87a6900841..b428d842101 100644
> --- a/gcc/config/riscv/predicates.md
> +++ b/gcc/config/riscv/predicates.md
> @@ -78,7 +78,7 @@ (define_predicate "sleu_operand"
>
>  (define_predicate "const_0_operand"
>    (and (match_code "const_int,const_wide_int,const_double,const_vector")
> -       (match_test "op == CONST0_RTX (GET_MODE (op))")))
> +       (match_test "riscv_const_zero_rtx_p (op)")))
>
>  (define_predicate "const_1_operand"
>    (and (match_code "const_int,const_wide_int,const_vector")
> diff --git a/gcc/config/riscv/riscv-protos.h b/gcc/config/riscv/riscv-protos.h
> index 31049ef7523..fcf30e084a3 100644
> --- a/gcc/config/riscv/riscv-protos.h
> +++ b/gcc/config/riscv/riscv-protos.h
> @@ -131,6 +131,8 @@ extern void riscv_asm_output_external (FILE *, const tree, const char *);
>  extern bool
>  riscv_zcmp_valid_stack_adj_bytes_p (HOST_WIDE_INT, int);
>  extern void riscv_legitimize_poly_move (machine_mode, rtx, rtx, rtx);
> +extern bool riscv_float_const_zero_rtx_p (rtx);
> +extern bool riscv_const_zero_rtx_p (rtx);
>
>  #ifdef RTX_CODE
>  extern void riscv_expand_int_scc (rtx, enum rtx_code, rtx, rtx, bool *invert_ptr = 0);
> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> index 0d1cbc5cb5f..a8ad86b7068 100644
> --- a/gcc/config/riscv/riscv.cc
> +++ b/gcc/config/riscv/riscv.cc
> @@ -1635,7 +1635,7 @@ riscv_const_insns (rtx x)
>         return 1;
>
>        /* We can use x0 to load floating-point zero.  */
> -      return x == CONST0_RTX (GET_MODE (x)) ? 1 : 0;
> +      return riscv_float_const_zero_rtx_p (x) ? 1 : 0;
>      case CONST_VECTOR:
>        {
>         /* TODO: This is not accurate, we will need to
> @@ -9481,6 +9481,39 @@ riscv_zcmp_valid_stack_adj_bytes_p (HOST_WIDE_INT total, int regs_num)
>          || additioanl_bytes == ZCMP_MAX_SPIMM * ZCMP_SP_INC_STEP;
>  }
>
> +/* Return true if rtx op is const zero of the floating-point.  */
> +bool
> +riscv_float_const_zero_rtx_p (rtx op)
> +{
> +  machine_mode mode = GET_MODE (op);
> +
> +  if (GET_MODE_CLASS (mode) != MODE_FLOAT)
> +    return false;
> +
> +  if (GET_CODE (op) != CONST_DOUBLE)
> +    return false;
> +
> +  const struct real_value *op_rvalue = CONST_DOUBLE_REAL_VALUE (op);
> +
> +  if (REAL_VALUE_MINUS_ZERO (*op_rvalue))
> +    return !HONOR_SIGNED_ZEROS (mode);
> +
> +  return real_equal (op_rvalue, &dconst0);
> +}
> +
> +/* Return true if rtx op is const zero, include both the integer
> +   and floating-point.  */
> +bool
> +riscv_const_zero_rtx_p (rtx op)
> +{
> +  machine_mode mode = GET_MODE (op);
> +
> +  if (GET_MODE_CLASS (mode) == MODE_FLOAT)
> +    return riscv_float_const_zero_rtx_p (op);
> +
> +  return op == CONST0_RTX (mode);
> +}
> +
>  /* Return true if it's valid gpr_save pattern.  */
>
>  bool
> diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
> index 68f7203b676..cd429f6dcdd 100644
> --- a/gcc/config/riscv/riscv.md
> +++ b/gcc/config/riscv/riscv.md
> @@ -1878,7 +1878,12 @@ (define_insn "*movhf_hardfloat"
>    "TARGET_ZFHMIN
>     && (register_operand (operands[0], HFmode)
>         || reg_or_0_operand (operands[1], HFmode))"
> -  { return riscv_output_move (operands[0], operands[1]); }
> +  {
> +    if (riscv_float_const_zero_rtx_p (operands[1]))
> +      operands[1] = CONST0_RTX (GET_MODE (operands[1]));
> +
> +    return riscv_output_move (operands[0], operands[1]);
> +  }
>    [(set_attr "move_type" "fmove,fmove,mtc,fpload,fpstore,store,mtc,mfc,move,load,store")
>     (set_attr "type" "fmove")
>     (set_attr "mode" "HF")])
> @@ -1889,7 +1894,12 @@ (define_insn "*movhf_softfloat"
>    "!TARGET_ZFHMIN
>     && (register_operand (operands[0], HFmode)
>         || reg_or_0_operand (operands[1], HFmode))"
> -  { return riscv_output_move (operands[0], operands[1]); }
> +  {
> +    if (riscv_float_const_zero_rtx_p (operands[1]))
> +      operands[1] = CONST0_RTX (GET_MODE (operands[1]));
> +
> +    return riscv_output_move (operands[0], operands[1]);
> +  }
>    [(set_attr "move_type" "fmove,move,load,store,mtc,mfc")
>     (set_attr "type" "fmove")
>     (set_attr "mode" "HF")])
> @@ -2243,7 +2253,12 @@ (define_insn "*movsf_hardfloat"
>    "TARGET_HARD_FLOAT
>     && (register_operand (operands[0], SFmode)
>         || reg_or_0_operand (operands[1], SFmode))"
> -  { return riscv_output_move (operands[0], operands[1]); }
> +  {
> +    if (riscv_float_const_zero_rtx_p (operands[1]))
> +      operands[1] = CONST0_RTX (GET_MODE (operands[1]));
> +
> +    return riscv_output_move (operands[0], operands[1]);
> +  }
>    [(set_attr "move_type" "fmove,fmove,mtc,fpload,fpstore,store,mtc,mfc,move,load,store")
>     (set_attr "type" "fmove")
>     (set_attr "mode" "SF")])
> @@ -2254,7 +2269,12 @@ (define_insn "*movsf_softfloat"
>    "!TARGET_HARD_FLOAT
>     && (register_operand (operands[0], SFmode)
>         || reg_or_0_operand (operands[1], SFmode))"
> -  { return riscv_output_move (operands[0], operands[1]); }
> +  {
> +    if (riscv_float_const_zero_rtx_p (operands[1]))
> +      operands[1] = CONST0_RTX (GET_MODE (operands[1]));
> +
> +    return riscv_output_move (operands[0], operands[1]);
> +  }
>    [(set_attr "move_type" "move,load,store")
>     (set_attr "type" "fmove")
>     (set_attr "mode" "SF")])
> @@ -2279,7 +2299,12 @@ (define_insn "*movdf_hardfloat_rv32"
>    "!TARGET_64BIT && TARGET_DOUBLE_FLOAT
>     && (register_operand (operands[0], DFmode)
>         || reg_or_0_operand (operands[1], DFmode))"
> -  { return riscv_output_move (operands[0], operands[1]); }
> +  {
> +    if (riscv_float_const_zero_rtx_p (operands[1]))
> +      operands[1] = CONST0_RTX (GET_MODE (operands[1]));
> +
> +    return riscv_output_move (operands[0], operands[1]);
> +  }
>    [(set_attr "move_type" "fmove,fmove,mtc,fpload,fpstore,store,mtc,mfc,move,load,store")
>     (set_attr "type" "fmove")
>     (set_attr "mode" "DF")])
> @@ -2290,7 +2315,12 @@ (define_insn "*movdf_hardfloat_rv64"
>    "TARGET_64BIT && TARGET_DOUBLE_FLOAT
>     && (register_operand (operands[0], DFmode)
>         || reg_or_0_operand (operands[1], DFmode))"
> -  { return riscv_output_move (operands[0], operands[1]); }
> +  {
> +    if (riscv_float_const_zero_rtx_p (operands[1]))
> +      operands[1] = CONST0_RTX (GET_MODE (operands[1]));
> +
> +    return riscv_output_move (operands[0], operands[1]);
> +  }
>    [(set_attr "move_type" "fmove,fmove,mtc,fpload,fpstore,store,mtc,mfc,move,load,store")
>     (set_attr "type" "fmove")
>     (set_attr "mode" "DF")])
> @@ -2301,7 +2331,12 @@ (define_insn "*movdf_softfloat"
>    "!TARGET_DOUBLE_FLOAT
>     && (register_operand (operands[0], DFmode)
>         || reg_or_0_operand (operands[1], DFmode))"
> -  { return riscv_output_move (operands[0], operands[1]); }
> +  {
> +    if (riscv_float_const_zero_rtx_p (operands[1]))
> +      operands[1] = CONST0_RTX (GET_MODE (operands[1]));
> +
> +    return riscv_output_move (operands[0], operands[1]);
> +  }
>    [(set_attr "move_type" "move,load,store")
>     (set_attr "type" "fmove")
>     (set_attr "mode" "DF")])
> diff --git a/gcc/testsuite/gcc.target/riscv/no-signed-zeros-0.c b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-0.c
> new file mode 100644
> index 00000000000..1eda13a3406
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-0.c
> @@ -0,0 +1,26 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gcv -mabi=lp64d -O3 -fno-signed-zeros -fno-schedule-insns -fno-schedule-insns2" } */
> +/* { dg-skip-if "" { *-*-* } { "-flto" } } */
> +/* { dg-final { check-function-bodies "**" "" } } */
> +
> +/*
> +** test_float_plus_zero_assign:
> +** sw\s+zero,0\([atx][0-9]+\)
> +** ret
> +*/
> +void
> +test_float_plus_zero_assign (float *a)
> +{
> +  *a = +0.0;
> +}
> +
> +/*
> +** test_float_plus_zero_assign:
> +** sw\s+zero,0\([atx][0-9]+\)
> +** ret
> +*/
> +void
> +test_float_minus_zero_assign (float *a)
> +{
> +  *a = -0.0;
> +}
> diff --git a/gcc/testsuite/gcc.target/riscv/no-signed-zeros-1.c b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-1.c
> new file mode 100644
> index 00000000000..8041ec3ea95
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-1.c
> @@ -0,0 +1,28 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gcv -mabi=lp64d -O3 -fno-signed-zeros -fno-schedule-insns -fno-schedule-insns2" } */
> +/* { dg-skip-if "" { *-*-* } { "-flto" } } */
> +/* { dg-final { check-function-bodies "**" "" } } */
> +
> +extern void float_assign (float *a, float b);
> +
> +/*
> +** test_float_plus_zero_assign:
> +** fmv\.s\.x\s+fa[0-9]+,\s*zero
> +** tail\s+float_assign
> +*/
> +void
> +test_float_plus_zero_assign (float *a)
> +{
> +  float_assign (a, +0.0);
> +}
> +
> +/*
> +** test_float_minus_zero_assign:
> +** fmv\.s\.x\s+fa[0-9]+,\s*zero
> +** tail\s+float_assign
> +*/
> +void
> +test_float_minus_zero_assign (float *a)
> +{
> +  float_assign (a, -0.0);
> +}
> diff --git a/gcc/testsuite/gcc.target/riscv/no-signed-zeros-2.c b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-2.c
> new file mode 100644
> index 00000000000..a6996dae4de
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-2.c
> @@ -0,0 +1,26 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gcv -mabi=lp64d -O3 -fno-signed-zeros -fno-schedule-insns -fno-schedule-insns2" } */
> +/* { dg-skip-if "" { *-*-* } { "-flto" } } */
> +/* { dg-final { check-function-bodies "**" "" } } */
> +
> +/*
> +** test_float_plus_zero_assign:
> +** sd\s+zero,0\([atx][0-9]+\)
> +** ret
> +*/
> +void
> +test_float_plus_zero_assign (double *a)
> +{
> +  *a = +0.0;
> +}
> +
> +/*
> +** test_float_plus_zero_assign:
> +** sd\s+zero,0\([atx][0-9]+\)
> +** ret
> +*/
> +void
> +test_float_minus_zero_assign (double *a)
> +{
> +  *a = -0.0;
> +}
> diff --git a/gcc/testsuite/gcc.target/riscv/no-signed-zeros-3.c b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-3.c
> new file mode 100644
> index 00000000000..b4ba8a247df
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-3.c
> @@ -0,0 +1,28 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gcv -mabi=lp64d -O3 -fno-signed-zeros -fno-schedule-insns -fno-schedule-insns2" } */
> +/* { dg-skip-if "" { *-*-* } { "-flto" } } */
> +/* { dg-final { check-function-bodies "**" "" } } */
> +
> +extern void float_assign (double *a, double b);
> +
> +/*
> +** test_float_plus_zero_assign:
> +** fmv\.d\.x\s+fa[0-9]+,\s*zero
> +** tail\s+float_assign
> +*/
> +void
> +test_float_plus_zero_assign (double *a)
> +{
> +  float_assign (a, +0.0);
> +}
> +
> +/*
> +** test_float_minus_zero_assign:
> +** fmv\.d\.x\s+fa[0-9]+,\s*zero
> +** tail\s+float_assign
> +*/
> +void
> +test_float_minus_zero_assign (double *a)
> +{
> +  float_assign (a, -0.0);
> +}
> diff --git a/gcc/testsuite/gcc.target/riscv/no-signed-zeros-4.c b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-4.c
> new file mode 100644
> index 00000000000..60acf7155d3
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-4.c
> @@ -0,0 +1,26 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gcv_zfh -mabi=lp64d -O3 -fno-signed-zeros -fno-schedule-insns -fno-schedule-insns2" } */
> +/* { dg-skip-if "" { *-*-* } { "-flto" } } */
> +/* { dg-final { check-function-bodies "**" "" } } */
> +
> +/*
> +** test_float_plus_zero_assign:
> +** sh\s+zero,0\([atx][0-9]+\)
> +** ret
> +*/
> +void
> +test_float_plus_zero_assign (_Float16 *a)
> +{
> +  *a = +0.0;
> +}
> +
> +/*
> +** test_float_plus_zero_assign:
> +** sh\s+zero,0\([atx][0-9]+\)
> +** ret
> +*/
> +void
> +test_float_minus_zero_assign (_Float16 *a)
> +{
> +  *a = -0.0;
> +}
> diff --git a/gcc/testsuite/gcc.target/riscv/no-signed-zeros-5.c b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-5.c
> new file mode 100644
> index 00000000000..d10efbeb37b
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-5.c
> @@ -0,0 +1,28 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gcv_zfh -mabi=lp64d -O3 -fno-signed-zeros -fno-schedule-insns -fno-schedule-insns2" } */
> +/* { dg-skip-if "" { *-*-* } { "-flto" } } */
> +/* { dg-final { check-function-bodies "**" "" } } */
> +
> +extern void float_assign (_Float16 *a, _Float16 b);
> +
> +/*
> +** test_float_plus_zero_assign:
> +** fmv\.h\.x\s+fa[0-9]+,\s*zero
> +** tail\s+float_assign
> +*/
> +void
> +test_float_plus_zero_assign (_Float16 *a)
> +{
> +  float_assign (a, +0.0);
> +}
> +
> +/*
> +** test_float_minus_zero_assign:
> +** fmv\.h\.x\s+fa[0-9]+,\s*zero
> +** tail\s+float_assign
> +*/
> +void
> +test_float_minus_zero_assign (_Float16 *a)
> +{
> +  float_assign (a, -0.0);
> +}
> diff --git a/gcc/testsuite/gcc.target/riscv/no-signed-zeros-run-0.c b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-run-0.c
> new file mode 100644
> index 00000000000..347e4b0ff74
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-run-0.c
> @@ -0,0 +1,36 @@
> +/* { dg-do run { target { riscv_v } } } */
> +/* { dg-additional-options "-std=c99 -O3 -fno-signed-zeros" } */
> +
> +float f_val;
> +
> +void
> +__attribute__ ((noinline))
> +test_float_plus_zero_assign (float *a)
> +{
> +  *a = +0.0;
> +}
> +
> +void
> +__attribute__ ((noinline))
> +test_float_minus_zero_assign (float *a)
> +{
> +  *a = -0.0;
> +}
> +
> +int
> +main ()
> +{
> +  f_val = -1.0;
> +  test_float_plus_zero_assign (&f_val);
> +
> +  if (__builtin_copysignf (1.0, f_val) != 1.0)
> +    __builtin_abort ();
> +
> +  f_val = -1.0;
> +  test_float_minus_zero_assign (&f_val);
> +
> +  if (__builtin_copysignf (1.0, f_val) != 1.0)
> +    __builtin_abort ();
> +
> +  return 0;
> +}
> diff --git a/gcc/testsuite/gcc.target/riscv/no-signed-zeros-run-1.c b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-run-1.c
> new file mode 100644
> index 00000000000..1eb1edba457
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-run-1.c
> @@ -0,0 +1,36 @@
> +/* { dg-do run { target { riscv_v } } } */
> +/* { dg-additional-options "-std=c99 -O3 -fno-signed-zeros" } */
> +
> +double f_val;
> +
> +void
> +__attribute__ ((noinline))
> +test_float_plus_zero_assign (double *a)
> +{
> +  *a = +0.0;
> +}
> +
> +void
> +__attribute__ ((noinline))
> +test_float_minus_zero_assign (double *a)
> +{
> +  *a = -0.0;
> +}
> +
> +int
> +main ()
> +{
> +  f_val = -1.0;
> +  test_float_plus_zero_assign (&f_val);
> +
> +  if (__builtin_copysignf (1.0, f_val) != 1.0)
> +    __builtin_abort ();
> +
> +  f_val = -1.0;
> +  test_float_minus_zero_assign (&f_val);
> +
> +  if (__builtin_copysignf (1.0, f_val) != 1.0)
> +    __builtin_abort ();
> +
> +  return 0;
> +}
> --
> 2.34.1
>
Li, Pan2 Jan. 9, 2024, 1:22 a.m. UTC | #2 
Thanks Richard B for comments.

> We don't really expect targets to do this.  The small testcase above
> is somewhat ill-formed with -fno-signed-zeros.  Note there's no
> -0.0 in pr30957-1.c so why does that one fail for you?  Does
> the -fvariable-expansion-in-unroller code maybe not trigger for
> riscv?

Sorry this confused me a little about the sematics of the option -fno-signed-zeros.
I wonder what the target/backend need to do for this option.

About the failure, it comes from below code in pr30957-1.c. The 0.0 / -5.0 is initialized to -0.0 in riscv but +0.0 in aarch64.

  if (__builtin_copysignf (1.0, foo (0.0 / -5.0, 10)) != 1.0)
    abort ();

If my understanding is correct, the loop will be vectorized during vect_transform_loop with a variable factor.
It won't benefit from unrolling/peeling and mark the loop->unroll as 1, and then we have tree-vect log similar to below:

Disabling unrolling due to variable-length vectorization factor.

> I think we should go to PR30957 and see what that was filed originally
> for, the testcase doesn't make much sense to me.

Sure thing, will take a look and back to you later.

Pan

-----Original Message-----
From: Richard Biener <richard.guenther@gmail.com> 
Sent: Monday, January 8, 2024 6:45 PM
To: Li, Pan2 <pan2.li@intel.com>
Cc: gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai; Wang, Yanzhang <yanzhang.wang@intel.com>; kito.cheng@gmail.com; jeffreyalaw@gmail.com
Subject: Re: [PATCH v3] RISC-V: Bugfix for doesn't honor no-signed-zeros option

On Tue, Jan 2, 2024 at 2:37 PM <pan2.li@intel.com> wrote:
>
> From: Pan Li <pan2.li@intel.com>
>
> According to the sematics of no-signed-zeros option, the backend
> like RISC-V should treat the minus zero -0.0f as plus zero 0.0f.
>
> Consider below example with option -fno-signed-zeros.
>
> void
> test (float *a)
> {
>   *a = -0.0;
> }
>
> We will generate code as below, which doesn't treat the minus zero
> as plus zero.
>
> test:
>   lui  a5,%hi(.LC0)
>   flw  fa5,%lo(.LC0)(a5)
>   fsw  fa5,0(a0)
>   ret
>
> .LC0:
>   .word -2147483648 // aka -0.0 (0x80000000 in hex)
>
> This patch would like to fix the bug and treat the minus zero -0.0
> as plus zero, aka +0.0. Thus after this patch we will have asm code
> as below for the above sampe code.
>
> test:
>   sw zero,0(a0)
>   ret
>
> This patch also fix the run failure of the test case pr30957-1.c. The
> below tests are passed for this patch.

We don't really expect targets to do this.  The small testcase above
is somewhat ill-formed with -fno-signed-zeros.  Note there's no
-0.0 in pr30957-1.c so why does that one fail for you?  Does
the -fvariable-expansion-in-unroller code maybe not trigger for
riscv?

I think we should go to PR30957 and see what that was filed originally
for, the testcase doesn't make much sense to me.

> * The riscv regression tests.
> * The pr30957-1.c run tests.
>
> gcc/ChangeLog:
>
>         * config/riscv/constraints.md: Leverage func riscv_float_const_zero_rtx_p
>         for predicating the rtx is const zero float or not.
>         * config/riscv/predicates.md: Ditto.
>         * config/riscv/riscv.cc (riscv_const_insns): Ditto.
>         (riscv_float_const_zero_rtx_p): New func impl for predicating the rtx is
>         const zero float or not.
>         (riscv_const_zero_rtx_p): New func impl for predicating the rtx
>         is const zero (both int and fp) or not.
>         * config/riscv/riscv-protos.h (riscv_float_const_zero_rtx_p):
>         New func decl.
>         (riscv_const_zero_rtx_p): Ditto.
>         * config/riscv/riscv.md: Making sure the operand[1] of movfp is
>         CONST0_RTX when the operand[1] is const zero float.
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.target/riscv/no-signed-zeros-0.c: New test.
>         * gcc.target/riscv/no-signed-zeros-1.c: New test.
>         * gcc.target/riscv/no-signed-zeros-2.c: New test.
>         * gcc.target/riscv/no-signed-zeros-3.c: New test.
>         * gcc.target/riscv/no-signed-zeros-4.c: New test.
>         * gcc.target/riscv/no-signed-zeros-5.c: New test.
>         * gcc.target/riscv/no-signed-zeros-run-0.c: New test.
>         * gcc.target/riscv/no-signed-zeros-run-1.c: New test.
>
> Signed-off-by: Pan Li <pan2.li@intel.com>
> ---
>  gcc/config/riscv/constraints.md               |  2 +-
>  gcc/config/riscv/predicates.md                |  2 +-
>  gcc/config/riscv/riscv-protos.h               |  2 +
>  gcc/config/riscv/riscv.cc                     | 35 ++++++++++++-
>  gcc/config/riscv/riscv.md                     | 49 ++++++++++++++++---
>  .../gcc.target/riscv/no-signed-zeros-0.c      | 26 ++++++++++
>  .../gcc.target/riscv/no-signed-zeros-1.c      | 28 +++++++++++
>  .../gcc.target/riscv/no-signed-zeros-2.c      | 26 ++++++++++
>  .../gcc.target/riscv/no-signed-zeros-3.c      | 28 +++++++++++
>  .../gcc.target/riscv/no-signed-zeros-4.c      | 26 ++++++++++
>  .../gcc.target/riscv/no-signed-zeros-5.c      | 28 +++++++++++
>  .../gcc.target/riscv/no-signed-zeros-run-0.c  | 36 ++++++++++++++
>  .../gcc.target/riscv/no-signed-zeros-run-1.c  | 36 ++++++++++++++
>  13 files changed, 314 insertions(+), 10 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/riscv/no-signed-zeros-0.c
>  create mode 100644 gcc/testsuite/gcc.target/riscv/no-signed-zeros-1.c
>  create mode 100644 gcc/testsuite/gcc.target/riscv/no-signed-zeros-2.c
>  create mode 100644 gcc/testsuite/gcc.target/riscv/no-signed-zeros-3.c
>  create mode 100644 gcc/testsuite/gcc.target/riscv/no-signed-zeros-4.c
>  create mode 100644 gcc/testsuite/gcc.target/riscv/no-signed-zeros-5.c
>  create mode 100644 gcc/testsuite/gcc.target/riscv/no-signed-zeros-run-0.c
>  create mode 100644 gcc/testsuite/gcc.target/riscv/no-signed-zeros-run-1.c
>
> diff --git a/gcc/config/riscv/constraints.md b/gcc/config/riscv/constraints.md
> index de4359af00d..db1d5e1385f 100644
> --- a/gcc/config/riscv/constraints.md
> +++ b/gcc/config/riscv/constraints.md
> @@ -108,7 +108,7 @@ (define_constraint "DnS"
>  (define_constraint "G"
>    "@internal"
>    (and (match_code "const_double")
> -       (match_test "op == CONST0_RTX (mode)")))
> +       (match_test "riscv_float_const_zero_rtx_p (op)")))
>
>  (define_memory_constraint "A"
>    "An address that is held in a general-purpose register."
> diff --git a/gcc/config/riscv/predicates.md b/gcc/config/riscv/predicates.md
> index b87a6900841..b428d842101 100644
> --- a/gcc/config/riscv/predicates.md
> +++ b/gcc/config/riscv/predicates.md
> @@ -78,7 +78,7 @@ (define_predicate "sleu_operand"
>
>  (define_predicate "const_0_operand"
>    (and (match_code "const_int,const_wide_int,const_double,const_vector")
> -       (match_test "op == CONST0_RTX (GET_MODE (op))")))
> +       (match_test "riscv_const_zero_rtx_p (op)")))
>
>  (define_predicate "const_1_operand"
>    (and (match_code "const_int,const_wide_int,const_vector")
> diff --git a/gcc/config/riscv/riscv-protos.h b/gcc/config/riscv/riscv-protos.h
> index 31049ef7523..fcf30e084a3 100644
> --- a/gcc/config/riscv/riscv-protos.h
> +++ b/gcc/config/riscv/riscv-protos.h
> @@ -131,6 +131,8 @@ extern void riscv_asm_output_external (FILE *, const tree, const char *);
>  extern bool
>  riscv_zcmp_valid_stack_adj_bytes_p (HOST_WIDE_INT, int);
>  extern void riscv_legitimize_poly_move (machine_mode, rtx, rtx, rtx);
> +extern bool riscv_float_const_zero_rtx_p (rtx);
> +extern bool riscv_const_zero_rtx_p (rtx);
>
>  #ifdef RTX_CODE
>  extern void riscv_expand_int_scc (rtx, enum rtx_code, rtx, rtx, bool *invert_ptr = 0);
> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> index 0d1cbc5cb5f..a8ad86b7068 100644
> --- a/gcc/config/riscv/riscv.cc
> +++ b/gcc/config/riscv/riscv.cc
> @@ -1635,7 +1635,7 @@ riscv_const_insns (rtx x)
>         return 1;
>
>        /* We can use x0 to load floating-point zero.  */
> -      return x == CONST0_RTX (GET_MODE (x)) ? 1 : 0;
> +      return riscv_float_const_zero_rtx_p (x) ? 1 : 0;
>      case CONST_VECTOR:
>        {
>         /* TODO: This is not accurate, we will need to
> @@ -9481,6 +9481,39 @@ riscv_zcmp_valid_stack_adj_bytes_p (HOST_WIDE_INT total, int regs_num)
>          || additioanl_bytes == ZCMP_MAX_SPIMM * ZCMP_SP_INC_STEP;
>  }
>
> +/* Return true if rtx op is const zero of the floating-point.  */
> +bool
> +riscv_float_const_zero_rtx_p (rtx op)
> +{
> +  machine_mode mode = GET_MODE (op);
> +
> +  if (GET_MODE_CLASS (mode) != MODE_FLOAT)
> +    return false;
> +
> +  if (GET_CODE (op) != CONST_DOUBLE)
> +    return false;
> +
> +  const struct real_value *op_rvalue = CONST_DOUBLE_REAL_VALUE (op);
> +
> +  if (REAL_VALUE_MINUS_ZERO (*op_rvalue))
> +    return !HONOR_SIGNED_ZEROS (mode);
> +
> +  return real_equal (op_rvalue, &dconst0);
> +}
> +
> +/* Return true if rtx op is const zero, include both the integer
> +   and floating-point.  */
> +bool
> +riscv_const_zero_rtx_p (rtx op)
> +{
> +  machine_mode mode = GET_MODE (op);
> +
> +  if (GET_MODE_CLASS (mode) == MODE_FLOAT)
> +    return riscv_float_const_zero_rtx_p (op);
> +
> +  return op == CONST0_RTX (mode);
> +}
> +
>  /* Return true if it's valid gpr_save pattern.  */
>
>  bool
> diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
> index 68f7203b676..cd429f6dcdd 100644
> --- a/gcc/config/riscv/riscv.md
> +++ b/gcc/config/riscv/riscv.md
> @@ -1878,7 +1878,12 @@ (define_insn "*movhf_hardfloat"
>    "TARGET_ZFHMIN
>     && (register_operand (operands[0], HFmode)
>         || reg_or_0_operand (operands[1], HFmode))"
> -  { return riscv_output_move (operands[0], operands[1]); }
> +  {
> +    if (riscv_float_const_zero_rtx_p (operands[1]))
> +      operands[1] = CONST0_RTX (GET_MODE (operands[1]));
> +
> +    return riscv_output_move (operands[0], operands[1]);
> +  }
>    [(set_attr "move_type" "fmove,fmove,mtc,fpload,fpstore,store,mtc,mfc,move,load,store")
>     (set_attr "type" "fmove")
>     (set_attr "mode" "HF")])
> @@ -1889,7 +1894,12 @@ (define_insn "*movhf_softfloat"
>    "!TARGET_ZFHMIN
>     && (register_operand (operands[0], HFmode)
>         || reg_or_0_operand (operands[1], HFmode))"
> -  { return riscv_output_move (operands[0], operands[1]); }
> +  {
> +    if (riscv_float_const_zero_rtx_p (operands[1]))
> +      operands[1] = CONST0_RTX (GET_MODE (operands[1]));
> +
> +    return riscv_output_move (operands[0], operands[1]);
> +  }
>    [(set_attr "move_type" "fmove,move,load,store,mtc,mfc")
>     (set_attr "type" "fmove")
>     (set_attr "mode" "HF")])
> @@ -2243,7 +2253,12 @@ (define_insn "*movsf_hardfloat"
>    "TARGET_HARD_FLOAT
>     && (register_operand (operands[0], SFmode)
>         || reg_or_0_operand (operands[1], SFmode))"
> -  { return riscv_output_move (operands[0], operands[1]); }
> +  {
> +    if (riscv_float_const_zero_rtx_p (operands[1]))
> +      operands[1] = CONST0_RTX (GET_MODE (operands[1]));
> +
> +    return riscv_output_move (operands[0], operands[1]);
> +  }
>    [(set_attr "move_type" "fmove,fmove,mtc,fpload,fpstore,store,mtc,mfc,move,load,store")
>     (set_attr "type" "fmove")
>     (set_attr "mode" "SF")])
> @@ -2254,7 +2269,12 @@ (define_insn "*movsf_softfloat"
>    "!TARGET_HARD_FLOAT
>     && (register_operand (operands[0], SFmode)
>         || reg_or_0_operand (operands[1], SFmode))"
> -  { return riscv_output_move (operands[0], operands[1]); }
> +  {
> +    if (riscv_float_const_zero_rtx_p (operands[1]))
> +      operands[1] = CONST0_RTX (GET_MODE (operands[1]));
> +
> +    return riscv_output_move (operands[0], operands[1]);
> +  }
>    [(set_attr "move_type" "move,load,store")
>     (set_attr "type" "fmove")
>     (set_attr "mode" "SF")])
> @@ -2279,7 +2299,12 @@ (define_insn "*movdf_hardfloat_rv32"
>    "!TARGET_64BIT && TARGET_DOUBLE_FLOAT
>     && (register_operand (operands[0], DFmode)
>         || reg_or_0_operand (operands[1], DFmode))"
> -  { return riscv_output_move (operands[0], operands[1]); }
> +  {
> +    if (riscv_float_const_zero_rtx_p (operands[1]))
> +      operands[1] = CONST0_RTX (GET_MODE (operands[1]));
> +
> +    return riscv_output_move (operands[0], operands[1]);
> +  }
>    [(set_attr "move_type" "fmove,fmove,mtc,fpload,fpstore,store,mtc,mfc,move,load,store")
>     (set_attr "type" "fmove")
>     (set_attr "mode" "DF")])
> @@ -2290,7 +2315,12 @@ (define_insn "*movdf_hardfloat_rv64"
>    "TARGET_64BIT && TARGET_DOUBLE_FLOAT
>     && (register_operand (operands[0], DFmode)
>         || reg_or_0_operand (operands[1], DFmode))"
> -  { return riscv_output_move (operands[0], operands[1]); }
> +  {
> +    if (riscv_float_const_zero_rtx_p (operands[1]))
> +      operands[1] = CONST0_RTX (GET_MODE (operands[1]));
> +
> +    return riscv_output_move (operands[0], operands[1]);
> +  }
>    [(set_attr "move_type" "fmove,fmove,mtc,fpload,fpstore,store,mtc,mfc,move,load,store")
>     (set_attr "type" "fmove")
>     (set_attr "mode" "DF")])
> @@ -2301,7 +2331,12 @@ (define_insn "*movdf_softfloat"
>    "!TARGET_DOUBLE_FLOAT
>     && (register_operand (operands[0], DFmode)
>         || reg_or_0_operand (operands[1], DFmode))"
> -  { return riscv_output_move (operands[0], operands[1]); }
> +  {
> +    if (riscv_float_const_zero_rtx_p (operands[1]))
> +      operands[1] = CONST0_RTX (GET_MODE (operands[1]));
> +
> +    return riscv_output_move (operands[0], operands[1]);
> +  }
>    [(set_attr "move_type" "move,load,store")
>     (set_attr "type" "fmove")
>     (set_attr "mode" "DF")])
> diff --git a/gcc/testsuite/gcc.target/riscv/no-signed-zeros-0.c b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-0.c
> new file mode 100644
> index 00000000000..1eda13a3406
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-0.c
> @@ -0,0 +1,26 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gcv -mabi=lp64d -O3 -fno-signed-zeros -fno-schedule-insns -fno-schedule-insns2" } */
> +/* { dg-skip-if "" { *-*-* } { "-flto" } } */
> +/* { dg-final { check-function-bodies "**" "" } } */
> +
> +/*
> +** test_float_plus_zero_assign:
> +** sw\s+zero,0\([atx][0-9]+\)
> +** ret
> +*/
> +void
> +test_float_plus_zero_assign (float *a)
> +{
> +  *a = +0.0;
> +}
> +
> +/*
> +** test_float_plus_zero_assign:
> +** sw\s+zero,0\([atx][0-9]+\)
> +** ret
> +*/
> +void
> +test_float_minus_zero_assign (float *a)
> +{
> +  *a = -0.0;
> +}
> diff --git a/gcc/testsuite/gcc.target/riscv/no-signed-zeros-1.c b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-1.c
> new file mode 100644
> index 00000000000..8041ec3ea95
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-1.c
> @@ -0,0 +1,28 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gcv -mabi=lp64d -O3 -fno-signed-zeros -fno-schedule-insns -fno-schedule-insns2" } */
> +/* { dg-skip-if "" { *-*-* } { "-flto" } } */
> +/* { dg-final { check-function-bodies "**" "" } } */
> +
> +extern void float_assign (float *a, float b);
> +
> +/*
> +** test_float_plus_zero_assign:
> +** fmv\.s\.x\s+fa[0-9]+,\s*zero
> +** tail\s+float_assign
> +*/
> +void
> +test_float_plus_zero_assign (float *a)
> +{
> +  float_assign (a, +0.0);
> +}
> +
> +/*
> +** test_float_minus_zero_assign:
> +** fmv\.s\.x\s+fa[0-9]+,\s*zero
> +** tail\s+float_assign
> +*/
> +void
> +test_float_minus_zero_assign (float *a)
> +{
> +  float_assign (a, -0.0);
> +}
> diff --git a/gcc/testsuite/gcc.target/riscv/no-signed-zeros-2.c b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-2.c
> new file mode 100644
> index 00000000000..a6996dae4de
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-2.c
> @@ -0,0 +1,26 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gcv -mabi=lp64d -O3 -fno-signed-zeros -fno-schedule-insns -fno-schedule-insns2" } */
> +/* { dg-skip-if "" { *-*-* } { "-flto" } } */
> +/* { dg-final { check-function-bodies "**" "" } } */
> +
> +/*
> +** test_float_plus_zero_assign:
> +** sd\s+zero,0\([atx][0-9]+\)
> +** ret
> +*/
> +void
> +test_float_plus_zero_assign (double *a)
> +{
> +  *a = +0.0;
> +}
> +
> +/*
> +** test_float_plus_zero_assign:
> +** sd\s+zero,0\([atx][0-9]+\)
> +** ret
> +*/
> +void
> +test_float_minus_zero_assign (double *a)
> +{
> +  *a = -0.0;
> +}
> diff --git a/gcc/testsuite/gcc.target/riscv/no-signed-zeros-3.c b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-3.c
> new file mode 100644
> index 00000000000..b4ba8a247df
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-3.c
> @@ -0,0 +1,28 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gcv -mabi=lp64d -O3 -fno-signed-zeros -fno-schedule-insns -fno-schedule-insns2" } */
> +/* { dg-skip-if "" { *-*-* } { "-flto" } } */
> +/* { dg-final { check-function-bodies "**" "" } } */
> +
> +extern void float_assign (double *a, double b);
> +
> +/*
> +** test_float_plus_zero_assign:
> +** fmv\.d\.x\s+fa[0-9]+,\s*zero
> +** tail\s+float_assign
> +*/
> +void
> +test_float_plus_zero_assign (double *a)
> +{
> +  float_assign (a, +0.0);
> +}
> +
> +/*
> +** test_float_minus_zero_assign:
> +** fmv\.d\.x\s+fa[0-9]+,\s*zero
> +** tail\s+float_assign
> +*/
> +void
> +test_float_minus_zero_assign (double *a)
> +{
> +  float_assign (a, -0.0);
> +}
> diff --git a/gcc/testsuite/gcc.target/riscv/no-signed-zeros-4.c b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-4.c
> new file mode 100644
> index 00000000000..60acf7155d3
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-4.c
> @@ -0,0 +1,26 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gcv_zfh -mabi=lp64d -O3 -fno-signed-zeros -fno-schedule-insns -fno-schedule-insns2" } */
> +/* { dg-skip-if "" { *-*-* } { "-flto" } } */
> +/* { dg-final { check-function-bodies "**" "" } } */
> +
> +/*
> +** test_float_plus_zero_assign:
> +** sh\s+zero,0\([atx][0-9]+\)
> +** ret
> +*/
> +void
> +test_float_plus_zero_assign (_Float16 *a)
> +{
> +  *a = +0.0;
> +}
> +
> +/*
> +** test_float_plus_zero_assign:
> +** sh\s+zero,0\([atx][0-9]+\)
> +** ret
> +*/
> +void
> +test_float_minus_zero_assign (_Float16 *a)
> +{
> +  *a = -0.0;
> +}
> diff --git a/gcc/testsuite/gcc.target/riscv/no-signed-zeros-5.c b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-5.c
> new file mode 100644
> index 00000000000..d10efbeb37b
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-5.c
> @@ -0,0 +1,28 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gcv_zfh -mabi=lp64d -O3 -fno-signed-zeros -fno-schedule-insns -fno-schedule-insns2" } */
> +/* { dg-skip-if "" { *-*-* } { "-flto" } } */
> +/* { dg-final { check-function-bodies "**" "" } } */
> +
> +extern void float_assign (_Float16 *a, _Float16 b);
> +
> +/*
> +** test_float_plus_zero_assign:
> +** fmv\.h\.x\s+fa[0-9]+,\s*zero
> +** tail\s+float_assign
> +*/
> +void
> +test_float_plus_zero_assign (_Float16 *a)
> +{
> +  float_assign (a, +0.0);
> +}
> +
> +/*
> +** test_float_minus_zero_assign:
> +** fmv\.h\.x\s+fa[0-9]+,\s*zero
> +** tail\s+float_assign
> +*/
> +void
> +test_float_minus_zero_assign (_Float16 *a)
> +{
> +  float_assign (a, -0.0);
> +}
> diff --git a/gcc/testsuite/gcc.target/riscv/no-signed-zeros-run-0.c b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-run-0.c
> new file mode 100644
> index 00000000000..347e4b0ff74
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-run-0.c
> @@ -0,0 +1,36 @@
> +/* { dg-do run { target { riscv_v } } } */
> +/* { dg-additional-options "-std=c99 -O3 -fno-signed-zeros" } */
> +
> +float f_val;
> +
> +void
> +__attribute__ ((noinline))
> +test_float_plus_zero_assign (float *a)
> +{
> +  *a = +0.0;
> +}
> +
> +void
> +__attribute__ ((noinline))
> +test_float_minus_zero_assign (float *a)
> +{
> +  *a = -0.0;
> +}
> +
> +int
> +main ()
> +{
> +  f_val = -1.0;
> +  test_float_plus_zero_assign (&f_val);
> +
> +  if (__builtin_copysignf (1.0, f_val) != 1.0)
> +    __builtin_abort ();
> +
> +  f_val = -1.0;
> +  test_float_minus_zero_assign (&f_val);
> +
> +  if (__builtin_copysignf (1.0, f_val) != 1.0)
> +    __builtin_abort ();
> +
> +  return 0;
> +}
> diff --git a/gcc/testsuite/gcc.target/riscv/no-signed-zeros-run-1.c b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-run-1.c
> new file mode 100644
> index 00000000000..1eb1edba457
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-run-1.c
> @@ -0,0 +1,36 @@
> +/* { dg-do run { target { riscv_v } } } */
> +/* { dg-additional-options "-std=c99 -O3 -fno-signed-zeros" } */
> +
> +double f_val;
> +
> +void
> +__attribute__ ((noinline))
> +test_float_plus_zero_assign (double *a)
> +{
> +  *a = +0.0;
> +}
> +
> +void
> +__attribute__ ((noinline))
> +test_float_minus_zero_assign (double *a)
> +{
> +  *a = -0.0;
> +}
> +
> +int
> +main ()
> +{
> +  f_val = -1.0;
> +  test_float_plus_zero_assign (&f_val);
> +
> +  if (__builtin_copysignf (1.0, f_val) != 1.0)
> +    __builtin_abort ();
> +
> +  f_val = -1.0;
> +  test_float_minus_zero_assign (&f_val);
> +
> +  if (__builtin_copysignf (1.0, f_val) != 1.0)
> +    __builtin_abort ();
> +
> +  return 0;
> +}
> --
> 2.34.1
>
Li, Pan2 Jan. 9, 2024, 7:17 a.m. UTC | #3 
The test case pr30957-1.c first comes from this commit about 19 years ago which expect the -1.0 for testing.

https://github.com/gcc-mirror/gcc/commit/290358f770d21d9204ea621f839ee8fba606a275

Then the below commit changes from -1.0 to +1.0 for this test file only, because of the instantiates copy(s) of the
accumulator which it initializes with +0.0.

https://github.com/gcc-mirror/gcc/commit/ffefa9288ab95b06b1dfed95e7235f4c09619a91

According to the implementation details of insert_var_expansion_initialization. The zero_init will be
the CONST0_RTX (mode) if HONOR_SIGNED_ZEROS is false. If my understanding is correct, maybe the test
case is not well designed for the variable expanding in unrolling? At least it is not good idea to mix/rely on
the HONOR_SIGNED_ZEROS when testing variable-expansion-in-unroller.

CC the original author and please feel free to correct me if any misunderstanding.

Pan

-----Original Message-----
From: Li, Pan2 
Sent: Tuesday, January 9, 2024 9:22 AM
To: Richard Biener <richard.guenther@gmail.com>
Cc: gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai; Wang, Yanzhang <yanzhang.wang@intel.com>; kito.cheng@gmail.com; jeffreyalaw@gmail.com
Subject: RE: [PATCH v3] RISC-V: Bugfix for doesn't honor no-signed-zeros option

Thanks Richard B for comments.

> We don't really expect targets to do this.  The small testcase above
> is somewhat ill-formed with -fno-signed-zeros.  Note there's no
> -0.0 in pr30957-1.c so why does that one fail for you?  Does
> the -fvariable-expansion-in-unroller code maybe not trigger for
> riscv?

Sorry this confused me a little about the sematics of the option -fno-signed-zeros.
I wonder what the target/backend need to do for this option.

About the failure, it comes from below code in pr30957-1.c. The 0.0 / -5.0 is initialized to -0.0 in riscv but +0.0 in aarch64.

  if (__builtin_copysignf (1.0, foo (0.0 / -5.0, 10)) != 1.0)
    abort ();

If my understanding is correct, the loop will be vectorized during vect_transform_loop with a variable factor.
It won't benefit from unrolling/peeling and mark the loop->unroll as 1, and then we have tree-vect log similar to below:

Disabling unrolling due to variable-length vectorization factor.

> I think we should go to PR30957 and see what that was filed originally
> for, the testcase doesn't make much sense to me.

Sure thing, will take a look and back to you later.

Pan

-----Original Message-----
From: Richard Biener <richard.guenther@gmail.com> 
Sent: Monday, January 8, 2024 6:45 PM
To: Li, Pan2 <pan2.li@intel.com>
Cc: gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai; Wang, Yanzhang <yanzhang.wang@intel.com>; kito.cheng@gmail.com; jeffreyalaw@gmail.com
Subject: Re: [PATCH v3] RISC-V: Bugfix for doesn't honor no-signed-zeros option

On Tue, Jan 2, 2024 at 2:37 PM <pan2.li@intel.com> wrote:
>
> From: Pan Li <pan2.li@intel.com>
>
> According to the sematics of no-signed-zeros option, the backend
> like RISC-V should treat the minus zero -0.0f as plus zero 0.0f.
>
> Consider below example with option -fno-signed-zeros.
>
> void
> test (float *a)
> {
>   *a = -0.0;
> }
>
> We will generate code as below, which doesn't treat the minus zero
> as plus zero.
>
> test:
>   lui  a5,%hi(.LC0)
>   flw  fa5,%lo(.LC0)(a5)
>   fsw  fa5,0(a0)
>   ret
>
> .LC0:
>   .word -2147483648 // aka -0.0 (0x80000000 in hex)
>
> This patch would like to fix the bug and treat the minus zero -0.0
> as plus zero, aka +0.0. Thus after this patch we will have asm code
> as below for the above sampe code.
>
> test:
>   sw zero,0(a0)
>   ret
>
> This patch also fix the run failure of the test case pr30957-1.c. The
> below tests are passed for this patch.

We don't really expect targets to do this.  The small testcase above
is somewhat ill-formed with -fno-signed-zeros.  Note there's no
-0.0 in pr30957-1.c so why does that one fail for you?  Does
the -fvariable-expansion-in-unroller code maybe not trigger for
riscv?

I think we should go to PR30957 and see what that was filed originally
for, the testcase doesn't make much sense to me.

> * The riscv regression tests.
> * The pr30957-1.c run tests.
>
> gcc/ChangeLog:
>
>         * config/riscv/constraints.md: Leverage func riscv_float_const_zero_rtx_p
>         for predicating the rtx is const zero float or not.
>         * config/riscv/predicates.md: Ditto.
>         * config/riscv/riscv.cc (riscv_const_insns): Ditto.
>         (riscv_float_const_zero_rtx_p): New func impl for predicating the rtx is
>         const zero float or not.
>         (riscv_const_zero_rtx_p): New func impl for predicating the rtx
>         is const zero (both int and fp) or not.
>         * config/riscv/riscv-protos.h (riscv_float_const_zero_rtx_p):
>         New func decl.
>         (riscv_const_zero_rtx_p): Ditto.
>         * config/riscv/riscv.md: Making sure the operand[1] of movfp is
>         CONST0_RTX when the operand[1] is const zero float.
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.target/riscv/no-signed-zeros-0.c: New test.
>         * gcc.target/riscv/no-signed-zeros-1.c: New test.
>         * gcc.target/riscv/no-signed-zeros-2.c: New test.
>         * gcc.target/riscv/no-signed-zeros-3.c: New test.
>         * gcc.target/riscv/no-signed-zeros-4.c: New test.
>         * gcc.target/riscv/no-signed-zeros-5.c: New test.
>         * gcc.target/riscv/no-signed-zeros-run-0.c: New test.
>         * gcc.target/riscv/no-signed-zeros-run-1.c: New test.
>
> Signed-off-by: Pan Li <pan2.li@intel.com>
> ---
>  gcc/config/riscv/constraints.md               |  2 +-
>  gcc/config/riscv/predicates.md                |  2 +-
>  gcc/config/riscv/riscv-protos.h               |  2 +
>  gcc/config/riscv/riscv.cc                     | 35 ++++++++++++-
>  gcc/config/riscv/riscv.md                     | 49 ++++++++++++++++---
>  .../gcc.target/riscv/no-signed-zeros-0.c      | 26 ++++++++++
>  .../gcc.target/riscv/no-signed-zeros-1.c      | 28 +++++++++++
>  .../gcc.target/riscv/no-signed-zeros-2.c      | 26 ++++++++++
>  .../gcc.target/riscv/no-signed-zeros-3.c      | 28 +++++++++++
>  .../gcc.target/riscv/no-signed-zeros-4.c      | 26 ++++++++++
>  .../gcc.target/riscv/no-signed-zeros-5.c      | 28 +++++++++++
>  .../gcc.target/riscv/no-signed-zeros-run-0.c  | 36 ++++++++++++++
>  .../gcc.target/riscv/no-signed-zeros-run-1.c  | 36 ++++++++++++++
>  13 files changed, 314 insertions(+), 10 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.target/riscv/no-signed-zeros-0.c
>  create mode 100644 gcc/testsuite/gcc.target/riscv/no-signed-zeros-1.c
>  create mode 100644 gcc/testsuite/gcc.target/riscv/no-signed-zeros-2.c
>  create mode 100644 gcc/testsuite/gcc.target/riscv/no-signed-zeros-3.c
>  create mode 100644 gcc/testsuite/gcc.target/riscv/no-signed-zeros-4.c
>  create mode 100644 gcc/testsuite/gcc.target/riscv/no-signed-zeros-5.c
>  create mode 100644 gcc/testsuite/gcc.target/riscv/no-signed-zeros-run-0.c
>  create mode 100644 gcc/testsuite/gcc.target/riscv/no-signed-zeros-run-1.c
>
> diff --git a/gcc/config/riscv/constraints.md b/gcc/config/riscv/constraints.md
> index de4359af00d..db1d5e1385f 100644
> --- a/gcc/config/riscv/constraints.md
> +++ b/gcc/config/riscv/constraints.md
> @@ -108,7 +108,7 @@ (define_constraint "DnS"
>  (define_constraint "G"
>    "@internal"
>    (and (match_code "const_double")
> -       (match_test "op == CONST0_RTX (mode)")))
> +       (match_test "riscv_float_const_zero_rtx_p (op)")))
>
>  (define_memory_constraint "A"
>    "An address that is held in a general-purpose register."
> diff --git a/gcc/config/riscv/predicates.md b/gcc/config/riscv/predicates.md
> index b87a6900841..b428d842101 100644
> --- a/gcc/config/riscv/predicates.md
> +++ b/gcc/config/riscv/predicates.md
> @@ -78,7 +78,7 @@ (define_predicate "sleu_operand"
>
>  (define_predicate "const_0_operand"
>    (and (match_code "const_int,const_wide_int,const_double,const_vector")
> -       (match_test "op == CONST0_RTX (GET_MODE (op))")))
> +       (match_test "riscv_const_zero_rtx_p (op)")))
>
>  (define_predicate "const_1_operand"
>    (and (match_code "const_int,const_wide_int,const_vector")
> diff --git a/gcc/config/riscv/riscv-protos.h b/gcc/config/riscv/riscv-protos.h
> index 31049ef7523..fcf30e084a3 100644
> --- a/gcc/config/riscv/riscv-protos.h
> +++ b/gcc/config/riscv/riscv-protos.h
> @@ -131,6 +131,8 @@ extern void riscv_asm_output_external (FILE *, const tree, const char *);
>  extern bool
>  riscv_zcmp_valid_stack_adj_bytes_p (HOST_WIDE_INT, int);
>  extern void riscv_legitimize_poly_move (machine_mode, rtx, rtx, rtx);
> +extern bool riscv_float_const_zero_rtx_p (rtx);
> +extern bool riscv_const_zero_rtx_p (rtx);
>
>  #ifdef RTX_CODE
>  extern void riscv_expand_int_scc (rtx, enum rtx_code, rtx, rtx, bool *invert_ptr = 0);
> diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> index 0d1cbc5cb5f..a8ad86b7068 100644
> --- a/gcc/config/riscv/riscv.cc
> +++ b/gcc/config/riscv/riscv.cc
> @@ -1635,7 +1635,7 @@ riscv_const_insns (rtx x)
>         return 1;
>
>        /* We can use x0 to load floating-point zero.  */
> -      return x == CONST0_RTX (GET_MODE (x)) ? 1 : 0;
> +      return riscv_float_const_zero_rtx_p (x) ? 1 : 0;
>      case CONST_VECTOR:
>        {
>         /* TODO: This is not accurate, we will need to
> @@ -9481,6 +9481,39 @@ riscv_zcmp_valid_stack_adj_bytes_p (HOST_WIDE_INT total, int regs_num)
>          || additioanl_bytes == ZCMP_MAX_SPIMM * ZCMP_SP_INC_STEP;
>  }
>
> +/* Return true if rtx op is const zero of the floating-point.  */
> +bool
> +riscv_float_const_zero_rtx_p (rtx op)
> +{
> +  machine_mode mode = GET_MODE (op);
> +
> +  if (GET_MODE_CLASS (mode) != MODE_FLOAT)
> +    return false;
> +
> +  if (GET_CODE (op) != CONST_DOUBLE)
> +    return false;
> +
> +  const struct real_value *op_rvalue = CONST_DOUBLE_REAL_VALUE (op);
> +
> +  if (REAL_VALUE_MINUS_ZERO (*op_rvalue))
> +    return !HONOR_SIGNED_ZEROS (mode);
> +
> +  return real_equal (op_rvalue, &dconst0);
> +}
> +
> +/* Return true if rtx op is const zero, include both the integer
> +   and floating-point.  */
> +bool
> +riscv_const_zero_rtx_p (rtx op)
> +{
> +  machine_mode mode = GET_MODE (op);
> +
> +  if (GET_MODE_CLASS (mode) == MODE_FLOAT)
> +    return riscv_float_const_zero_rtx_p (op);
> +
> +  return op == CONST0_RTX (mode);
> +}
> +
>  /* Return true if it's valid gpr_save pattern.  */
>
>  bool
> diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
> index 68f7203b676..cd429f6dcdd 100644
> --- a/gcc/config/riscv/riscv.md
> +++ b/gcc/config/riscv/riscv.md
> @@ -1878,7 +1878,12 @@ (define_insn "*movhf_hardfloat"
>    "TARGET_ZFHMIN
>     && (register_operand (operands[0], HFmode)
>         || reg_or_0_operand (operands[1], HFmode))"
> -  { return riscv_output_move (operands[0], operands[1]); }
> +  {
> +    if (riscv_float_const_zero_rtx_p (operands[1]))
> +      operands[1] = CONST0_RTX (GET_MODE (operands[1]));
> +
> +    return riscv_output_move (operands[0], operands[1]);
> +  }
>    [(set_attr "move_type" "fmove,fmove,mtc,fpload,fpstore,store,mtc,mfc,move,load,store")
>     (set_attr "type" "fmove")
>     (set_attr "mode" "HF")])
> @@ -1889,7 +1894,12 @@ (define_insn "*movhf_softfloat"
>    "!TARGET_ZFHMIN
>     && (register_operand (operands[0], HFmode)
>         || reg_or_0_operand (operands[1], HFmode))"
> -  { return riscv_output_move (operands[0], operands[1]); }
> +  {
> +    if (riscv_float_const_zero_rtx_p (operands[1]))
> +      operands[1] = CONST0_RTX (GET_MODE (operands[1]));
> +
> +    return riscv_output_move (operands[0], operands[1]);
> +  }
>    [(set_attr "move_type" "fmove,move,load,store,mtc,mfc")
>     (set_attr "type" "fmove")
>     (set_attr "mode" "HF")])
> @@ -2243,7 +2253,12 @@ (define_insn "*movsf_hardfloat"
>    "TARGET_HARD_FLOAT
>     && (register_operand (operands[0], SFmode)
>         || reg_or_0_operand (operands[1], SFmode))"
> -  { return riscv_output_move (operands[0], operands[1]); }
> +  {
> +    if (riscv_float_const_zero_rtx_p (operands[1]))
> +      operands[1] = CONST0_RTX (GET_MODE (operands[1]));
> +
> +    return riscv_output_move (operands[0], operands[1]);
> +  }
>    [(set_attr "move_type" "fmove,fmove,mtc,fpload,fpstore,store,mtc,mfc,move,load,store")
>     (set_attr "type" "fmove")
>     (set_attr "mode" "SF")])
> @@ -2254,7 +2269,12 @@ (define_insn "*movsf_softfloat"
>    "!TARGET_HARD_FLOAT
>     && (register_operand (operands[0], SFmode)
>         || reg_or_0_operand (operands[1], SFmode))"
> -  { return riscv_output_move (operands[0], operands[1]); }
> +  {
> +    if (riscv_float_const_zero_rtx_p (operands[1]))
> +      operands[1] = CONST0_RTX (GET_MODE (operands[1]));
> +
> +    return riscv_output_move (operands[0], operands[1]);
> +  }
>    [(set_attr "move_type" "move,load,store")
>     (set_attr "type" "fmove")
>     (set_attr "mode" "SF")])
> @@ -2279,7 +2299,12 @@ (define_insn "*movdf_hardfloat_rv32"
>    "!TARGET_64BIT && TARGET_DOUBLE_FLOAT
>     && (register_operand (operands[0], DFmode)
>         || reg_or_0_operand (operands[1], DFmode))"
> -  { return riscv_output_move (operands[0], operands[1]); }
> +  {
> +    if (riscv_float_const_zero_rtx_p (operands[1]))
> +      operands[1] = CONST0_RTX (GET_MODE (operands[1]));
> +
> +    return riscv_output_move (operands[0], operands[1]);
> +  }
>    [(set_attr "move_type" "fmove,fmove,mtc,fpload,fpstore,store,mtc,mfc,move,load,store")
>     (set_attr "type" "fmove")
>     (set_attr "mode" "DF")])
> @@ -2290,7 +2315,12 @@ (define_insn "*movdf_hardfloat_rv64"
>    "TARGET_64BIT && TARGET_DOUBLE_FLOAT
>     && (register_operand (operands[0], DFmode)
>         || reg_or_0_operand (operands[1], DFmode))"
> -  { return riscv_output_move (operands[0], operands[1]); }
> +  {
> +    if (riscv_float_const_zero_rtx_p (operands[1]))
> +      operands[1] = CONST0_RTX (GET_MODE (operands[1]));
> +
> +    return riscv_output_move (operands[0], operands[1]);
> +  }
>    [(set_attr "move_type" "fmove,fmove,mtc,fpload,fpstore,store,mtc,mfc,move,load,store")
>     (set_attr "type" "fmove")
>     (set_attr "mode" "DF")])
> @@ -2301,7 +2331,12 @@ (define_insn "*movdf_softfloat"
>    "!TARGET_DOUBLE_FLOAT
>     && (register_operand (operands[0], DFmode)
>         || reg_or_0_operand (operands[1], DFmode))"
> -  { return riscv_output_move (operands[0], operands[1]); }
> +  {
> +    if (riscv_float_const_zero_rtx_p (operands[1]))
> +      operands[1] = CONST0_RTX (GET_MODE (operands[1]));
> +
> +    return riscv_output_move (operands[0], operands[1]);
> +  }
>    [(set_attr "move_type" "move,load,store")
>     (set_attr "type" "fmove")
>     (set_attr "mode" "DF")])
> diff --git a/gcc/testsuite/gcc.target/riscv/no-signed-zeros-0.c b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-0.c
> new file mode 100644
> index 00000000000..1eda13a3406
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-0.c
> @@ -0,0 +1,26 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gcv -mabi=lp64d -O3 -fno-signed-zeros -fno-schedule-insns -fno-schedule-insns2" } */
> +/* { dg-skip-if "" { *-*-* } { "-flto" } } */
> +/* { dg-final { check-function-bodies "**" "" } } */
> +
> +/*
> +** test_float_plus_zero_assign:
> +** sw\s+zero,0\([atx][0-9]+\)
> +** ret
> +*/
> +void
> +test_float_plus_zero_assign (float *a)
> +{
> +  *a = +0.0;
> +}
> +
> +/*
> +** test_float_plus_zero_assign:
> +** sw\s+zero,0\([atx][0-9]+\)
> +** ret
> +*/
> +void
> +test_float_minus_zero_assign (float *a)
> +{
> +  *a = -0.0;
> +}
> diff --git a/gcc/testsuite/gcc.target/riscv/no-signed-zeros-1.c b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-1.c
> new file mode 100644
> index 00000000000..8041ec3ea95
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-1.c
> @@ -0,0 +1,28 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gcv -mabi=lp64d -O3 -fno-signed-zeros -fno-schedule-insns -fno-schedule-insns2" } */
> +/* { dg-skip-if "" { *-*-* } { "-flto" } } */
> +/* { dg-final { check-function-bodies "**" "" } } */
> +
> +extern void float_assign (float *a, float b);
> +
> +/*
> +** test_float_plus_zero_assign:
> +** fmv\.s\.x\s+fa[0-9]+,\s*zero
> +** tail\s+float_assign
> +*/
> +void
> +test_float_plus_zero_assign (float *a)
> +{
> +  float_assign (a, +0.0);
> +}
> +
> +/*
> +** test_float_minus_zero_assign:
> +** fmv\.s\.x\s+fa[0-9]+,\s*zero
> +** tail\s+float_assign
> +*/
> +void
> +test_float_minus_zero_assign (float *a)
> +{
> +  float_assign (a, -0.0);
> +}
> diff --git a/gcc/testsuite/gcc.target/riscv/no-signed-zeros-2.c b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-2.c
> new file mode 100644
> index 00000000000..a6996dae4de
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-2.c
> @@ -0,0 +1,26 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gcv -mabi=lp64d -O3 -fno-signed-zeros -fno-schedule-insns -fno-schedule-insns2" } */
> +/* { dg-skip-if "" { *-*-* } { "-flto" } } */
> +/* { dg-final { check-function-bodies "**" "" } } */
> +
> +/*
> +** test_float_plus_zero_assign:
> +** sd\s+zero,0\([atx][0-9]+\)
> +** ret
> +*/
> +void
> +test_float_plus_zero_assign (double *a)
> +{
> +  *a = +0.0;
> +}
> +
> +/*
> +** test_float_plus_zero_assign:
> +** sd\s+zero,0\([atx][0-9]+\)
> +** ret
> +*/
> +void
> +test_float_minus_zero_assign (double *a)
> +{
> +  *a = -0.0;
> +}
> diff --git a/gcc/testsuite/gcc.target/riscv/no-signed-zeros-3.c b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-3.c
> new file mode 100644
> index 00000000000..b4ba8a247df
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-3.c
> @@ -0,0 +1,28 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gcv -mabi=lp64d -O3 -fno-signed-zeros -fno-schedule-insns -fno-schedule-insns2" } */
> +/* { dg-skip-if "" { *-*-* } { "-flto" } } */
> +/* { dg-final { check-function-bodies "**" "" } } */
> +
> +extern void float_assign (double *a, double b);
> +
> +/*
> +** test_float_plus_zero_assign:
> +** fmv\.d\.x\s+fa[0-9]+,\s*zero
> +** tail\s+float_assign
> +*/
> +void
> +test_float_plus_zero_assign (double *a)
> +{
> +  float_assign (a, +0.0);
> +}
> +
> +/*
> +** test_float_minus_zero_assign:
> +** fmv\.d\.x\s+fa[0-9]+,\s*zero
> +** tail\s+float_assign
> +*/
> +void
> +test_float_minus_zero_assign (double *a)
> +{
> +  float_assign (a, -0.0);
> +}
> diff --git a/gcc/testsuite/gcc.target/riscv/no-signed-zeros-4.c b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-4.c
> new file mode 100644
> index 00000000000..60acf7155d3
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-4.c
> @@ -0,0 +1,26 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gcv_zfh -mabi=lp64d -O3 -fno-signed-zeros -fno-schedule-insns -fno-schedule-insns2" } */
> +/* { dg-skip-if "" { *-*-* } { "-flto" } } */
> +/* { dg-final { check-function-bodies "**" "" } } */
> +
> +/*
> +** test_float_plus_zero_assign:
> +** sh\s+zero,0\([atx][0-9]+\)
> +** ret
> +*/
> +void
> +test_float_plus_zero_assign (_Float16 *a)
> +{
> +  *a = +0.0;
> +}
> +
> +/*
> +** test_float_plus_zero_assign:
> +** sh\s+zero,0\([atx][0-9]+\)
> +** ret
> +*/
> +void
> +test_float_minus_zero_assign (_Float16 *a)
> +{
> +  *a = -0.0;
> +}
> diff --git a/gcc/testsuite/gcc.target/riscv/no-signed-zeros-5.c b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-5.c
> new file mode 100644
> index 00000000000..d10efbeb37b
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-5.c
> @@ -0,0 +1,28 @@
> +/* { dg-do compile } */
> +/* { dg-options "-march=rv64gcv_zfh -mabi=lp64d -O3 -fno-signed-zeros -fno-schedule-insns -fno-schedule-insns2" } */
> +/* { dg-skip-if "" { *-*-* } { "-flto" } } */
> +/* { dg-final { check-function-bodies "**" "" } } */
> +
> +extern void float_assign (_Float16 *a, _Float16 b);
> +
> +/*
> +** test_float_plus_zero_assign:
> +** fmv\.h\.x\s+fa[0-9]+,\s*zero
> +** tail\s+float_assign
> +*/
> +void
> +test_float_plus_zero_assign (_Float16 *a)
> +{
> +  float_assign (a, +0.0);
> +}
> +
> +/*
> +** test_float_minus_zero_assign:
> +** fmv\.h\.x\s+fa[0-9]+,\s*zero
> +** tail\s+float_assign
> +*/
> +void
> +test_float_minus_zero_assign (_Float16 *a)
> +{
> +  float_assign (a, -0.0);
> +}
> diff --git a/gcc/testsuite/gcc.target/riscv/no-signed-zeros-run-0.c b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-run-0.c
> new file mode 100644
> index 00000000000..347e4b0ff74
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-run-0.c
> @@ -0,0 +1,36 @@
> +/* { dg-do run { target { riscv_v } } } */
> +/* { dg-additional-options "-std=c99 -O3 -fno-signed-zeros" } */
> +
> +float f_val;
> +
> +void
> +__attribute__ ((noinline))
> +test_float_plus_zero_assign (float *a)
> +{
> +  *a = +0.0;
> +}
> +
> +void
> +__attribute__ ((noinline))
> +test_float_minus_zero_assign (float *a)
> +{
> +  *a = -0.0;
> +}
> +
> +int
> +main ()
> +{
> +  f_val = -1.0;
> +  test_float_plus_zero_assign (&f_val);
> +
> +  if (__builtin_copysignf (1.0, f_val) != 1.0)
> +    __builtin_abort ();
> +
> +  f_val = -1.0;
> +  test_float_minus_zero_assign (&f_val);
> +
> +  if (__builtin_copysignf (1.0, f_val) != 1.0)
> +    __builtin_abort ();
> +
> +  return 0;
> +}
> diff --git a/gcc/testsuite/gcc.target/riscv/no-signed-zeros-run-1.c b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-run-1.c
> new file mode 100644
> index 00000000000..1eb1edba457
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-run-1.c
> @@ -0,0 +1,36 @@
> +/* { dg-do run { target { riscv_v } } } */
> +/* { dg-additional-options "-std=c99 -O3 -fno-signed-zeros" } */
> +
> +double f_val;
> +
> +void
> +__attribute__ ((noinline))
> +test_float_plus_zero_assign (double *a)
> +{
> +  *a = +0.0;
> +}
> +
> +void
> +__attribute__ ((noinline))
> +test_float_minus_zero_assign (double *a)
> +{
> +  *a = -0.0;
> +}
> +
> +int
> +main ()
> +{
> +  f_val = -1.0;
> +  test_float_plus_zero_assign (&f_val);
> +
> +  if (__builtin_copysignf (1.0, f_val) != 1.0)
> +    __builtin_abort ();
> +
> +  f_val = -1.0;
> +  test_float_minus_zero_assign (&f_val);
> +
> +  if (__builtin_copysignf (1.0, f_val) != 1.0)
> +    __builtin_abort ();
> +
> +  return 0;
> +}
> --
> 2.34.1
>
Richard Biener Jan. 9, 2024, 1:08 p.m. UTC | #4 
On Tue, Jan 9, 2024 at 8:17 AM Li, Pan2 <pan2.li@intel.com> wrote:
>
> The test case pr30957-1.c first comes from this commit about 19 years ago which expect the -1.0 for testing.
>
> https://github.com/gcc-mirror/gcc/commit/290358f770d21d9204ea621f839ee8fba606a275
>
> Then the below commit changes from -1.0 to +1.0 for this test file only, because of the instantiates copy(s) of the
> accumulator which it initializes with +0.0.
>
> https://github.com/gcc-mirror/gcc/commit/ffefa9288ab95b06b1dfed95e7235f4c09619a91

This second change looks bogus - it makes the testcase not
well-defined by specifying
-fno-signed-zeros but expecting defined behavior when signed zeros are
involved.  It might be
that it's currently not possible to trigger the "bad" behavior but
then the testcase should still
not runfail.  I suggest to instead remove -funsafe-math-optimizations
from the original option set.

> According to the implementation details of insert_var_expansion_initialization. The zero_init will be
> the CONST0_RTX (mode) if HONOR_SIGNED_ZEROS is false. If my understanding is correct, maybe the test
> case is not well designed for the variable expanding in unrolling? At least it is not good idea to mix/rely on
> the HONOR_SIGNED_ZEROS when testing variable-expansion-in-unroller.

For the first change I think insert_var_expansion_initialization
behavior shouldn't depend on
HONOR_SIGNED_ZEROS but instead check MODE_HAS_SINGED_ZEROS as -0.0 is
always the correct initial value to use.  Note that this will likely
break the testcase expectation
when it's not changed, and the testcase would need to be guarded for
targets not supporting
signed zeros.

Richard.

> CC the original author and please feel free to correct me if any misunderstanding.
>
> Pan
>
> -----Original Message-----
> From: Li, Pan2
> Sent: Tuesday, January 9, 2024 9:22 AM
> To: Richard Biener <richard.guenther@gmail.com>
> Cc: gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai; Wang, Yanzhang <yanzhang.wang@intel.com>; kito.cheng@gmail.com; jeffreyalaw@gmail.com
> Subject: RE: [PATCH v3] RISC-V: Bugfix for doesn't honor no-signed-zeros option
>
> Thanks Richard B for comments.
>
> > We don't really expect targets to do this.  The small testcase above
> > is somewhat ill-formed with -fno-signed-zeros.  Note there's no
> > -0.0 in pr30957-1.c so why does that one fail for you?  Does
> > the -fvariable-expansion-in-unroller code maybe not trigger for
> > riscv?
>
> Sorry this confused me a little about the sematics of the option -fno-signed-zeros.
> I wonder what the target/backend need to do for this option.
>
> About the failure, it comes from below code in pr30957-1.c. The 0.0 / -5.0 is initialized to -0.0 in riscv but +0.0 in aarch64.
>
>   if (__builtin_copysignf (1.0, foo (0.0 / -5.0, 10)) != 1.0)
>     abort ();
>
> If my understanding is correct, the loop will be vectorized during vect_transform_loop with a variable factor.
> It won't benefit from unrolling/peeling and mark the loop->unroll as 1, and then we have tree-vect log similar to below:
>
> Disabling unrolling due to variable-length vectorization factor.
>
> > I think we should go to PR30957 and see what that was filed originally
> > for, the testcase doesn't make much sense to me.
>
> Sure thing, will take a look and back to you later.
>
> Pan
>
> -----Original Message-----
> From: Richard Biener <richard.guenther@gmail.com>
> Sent: Monday, January 8, 2024 6:45 PM
> To: Li, Pan2 <pan2.li@intel.com>
> Cc: gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai; Wang, Yanzhang <yanzhang.wang@intel.com>; kito.cheng@gmail.com; jeffreyalaw@gmail.com
> Subject: Re: [PATCH v3] RISC-V: Bugfix for doesn't honor no-signed-zeros option
>
> On Tue, Jan 2, 2024 at 2:37 PM <pan2.li@intel.com> wrote:
> >
> > From: Pan Li <pan2.li@intel.com>
> >
> > According to the sematics of no-signed-zeros option, the backend
> > like RISC-V should treat the minus zero -0.0f as plus zero 0.0f.
> >
> > Consider below example with option -fno-signed-zeros.
> >
> > void
> > test (float *a)
> > {
> >   *a = -0.0;
> > }
> >
> > We will generate code as below, which doesn't treat the minus zero
> > as plus zero.
> >
> > test:
> >   lui  a5,%hi(.LC0)
> >   flw  fa5,%lo(.LC0)(a5)
> >   fsw  fa5,0(a0)
> >   ret
> >
> > .LC0:
> >   .word -2147483648 // aka -0.0 (0x80000000 in hex)
> >
> > This patch would like to fix the bug and treat the minus zero -0.0
> > as plus zero, aka +0.0. Thus after this patch we will have asm code
> > as below for the above sampe code.
> >
> > test:
> >   sw zero,0(a0)
> >   ret
> >
> > This patch also fix the run failure of the test case pr30957-1.c. The
> > below tests are passed for this patch.
>
> We don't really expect targets to do this.  The small testcase above
> is somewhat ill-formed with -fno-signed-zeros.  Note there's no
> -0.0 in pr30957-1.c so why does that one fail for you?  Does
> the -fvariable-expansion-in-unroller code maybe not trigger for
> riscv?
>
> I think we should go to PR30957 and see what that was filed originally
> for, the testcase doesn't make much sense to me.
>
> > * The riscv regression tests.
> > * The pr30957-1.c run tests.
> >
> > gcc/ChangeLog:
> >
> >         * config/riscv/constraints.md: Leverage func riscv_float_const_zero_rtx_p
> >         for predicating the rtx is const zero float or not.
> >         * config/riscv/predicates.md: Ditto.
> >         * config/riscv/riscv.cc (riscv_const_insns): Ditto.
> >         (riscv_float_const_zero_rtx_p): New func impl for predicating the rtx is
> >         const zero float or not.
> >         (riscv_const_zero_rtx_p): New func impl for predicating the rtx
> >         is const zero (both int and fp) or not.
> >         * config/riscv/riscv-protos.h (riscv_float_const_zero_rtx_p):
> >         New func decl.
> >         (riscv_const_zero_rtx_p): Ditto.
> >         * config/riscv/riscv.md: Making sure the operand[1] of movfp is
> >         CONST0_RTX when the operand[1] is const zero float.
> >
> > gcc/testsuite/ChangeLog:
> >
> >         * gcc.target/riscv/no-signed-zeros-0.c: New test.
> >         * gcc.target/riscv/no-signed-zeros-1.c: New test.
> >         * gcc.target/riscv/no-signed-zeros-2.c: New test.
> >         * gcc.target/riscv/no-signed-zeros-3.c: New test.
> >         * gcc.target/riscv/no-signed-zeros-4.c: New test.
> >         * gcc.target/riscv/no-signed-zeros-5.c: New test.
> >         * gcc.target/riscv/no-signed-zeros-run-0.c: New test.
> >         * gcc.target/riscv/no-signed-zeros-run-1.c: New test.
> >
> > Signed-off-by: Pan Li <pan2.li@intel.com>
> > ---
> >  gcc/config/riscv/constraints.md               |  2 +-
> >  gcc/config/riscv/predicates.md                |  2 +-
> >  gcc/config/riscv/riscv-protos.h               |  2 +
> >  gcc/config/riscv/riscv.cc                     | 35 ++++++++++++-
> >  gcc/config/riscv/riscv.md                     | 49 ++++++++++++++++---
> >  .../gcc.target/riscv/no-signed-zeros-0.c      | 26 ++++++++++
> >  .../gcc.target/riscv/no-signed-zeros-1.c      | 28 +++++++++++
> >  .../gcc.target/riscv/no-signed-zeros-2.c      | 26 ++++++++++
> >  .../gcc.target/riscv/no-signed-zeros-3.c      | 28 +++++++++++
> >  .../gcc.target/riscv/no-signed-zeros-4.c      | 26 ++++++++++
> >  .../gcc.target/riscv/no-signed-zeros-5.c      | 28 +++++++++++
> >  .../gcc.target/riscv/no-signed-zeros-run-0.c  | 36 ++++++++++++++
> >  .../gcc.target/riscv/no-signed-zeros-run-1.c  | 36 ++++++++++++++
> >  13 files changed, 314 insertions(+), 10 deletions(-)
> >  create mode 100644 gcc/testsuite/gcc.target/riscv/no-signed-zeros-0.c
> >  create mode 100644 gcc/testsuite/gcc.target/riscv/no-signed-zeros-1.c
> >  create mode 100644 gcc/testsuite/gcc.target/riscv/no-signed-zeros-2.c
> >  create mode 100644 gcc/testsuite/gcc.target/riscv/no-signed-zeros-3.c
> >  create mode 100644 gcc/testsuite/gcc.target/riscv/no-signed-zeros-4.c
> >  create mode 100644 gcc/testsuite/gcc.target/riscv/no-signed-zeros-5.c
> >  create mode 100644 gcc/testsuite/gcc.target/riscv/no-signed-zeros-run-0.c
> >  create mode 100644 gcc/testsuite/gcc.target/riscv/no-signed-zeros-run-1.c
> >
> > diff --git a/gcc/config/riscv/constraints.md b/gcc/config/riscv/constraints.md
> > index de4359af00d..db1d5e1385f 100644
> > --- a/gcc/config/riscv/constraints.md
> > +++ b/gcc/config/riscv/constraints.md
> > @@ -108,7 +108,7 @@ (define_constraint "DnS"
> >  (define_constraint "G"
> >    "@internal"
> >    (and (match_code "const_double")
> > -       (match_test "op == CONST0_RTX (mode)")))
> > +       (match_test "riscv_float_const_zero_rtx_p (op)")))
> >
> >  (define_memory_constraint "A"
> >    "An address that is held in a general-purpose register."
> > diff --git a/gcc/config/riscv/predicates.md b/gcc/config/riscv/predicates.md
> > index b87a6900841..b428d842101 100644
> > --- a/gcc/config/riscv/predicates.md
> > +++ b/gcc/config/riscv/predicates.md
> > @@ -78,7 +78,7 @@ (define_predicate "sleu_operand"
> >
> >  (define_predicate "const_0_operand"
> >    (and (match_code "const_int,const_wide_int,const_double,const_vector")
> > -       (match_test "op == CONST0_RTX (GET_MODE (op))")))
> > +       (match_test "riscv_const_zero_rtx_p (op)")))
> >
> >  (define_predicate "const_1_operand"
> >    (and (match_code "const_int,const_wide_int,const_vector")
> > diff --git a/gcc/config/riscv/riscv-protos.h b/gcc/config/riscv/riscv-protos.h
> > index 31049ef7523..fcf30e084a3 100644
> > --- a/gcc/config/riscv/riscv-protos.h
> > +++ b/gcc/config/riscv/riscv-protos.h
> > @@ -131,6 +131,8 @@ extern void riscv_asm_output_external (FILE *, const tree, const char *);
> >  extern bool
> >  riscv_zcmp_valid_stack_adj_bytes_p (HOST_WIDE_INT, int);
> >  extern void riscv_legitimize_poly_move (machine_mode, rtx, rtx, rtx);
> > +extern bool riscv_float_const_zero_rtx_p (rtx);
> > +extern bool riscv_const_zero_rtx_p (rtx);
> >
> >  #ifdef RTX_CODE
> >  extern void riscv_expand_int_scc (rtx, enum rtx_code, rtx, rtx, bool *invert_ptr = 0);
> > diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
> > index 0d1cbc5cb5f..a8ad86b7068 100644
> > --- a/gcc/config/riscv/riscv.cc
> > +++ b/gcc/config/riscv/riscv.cc
> > @@ -1635,7 +1635,7 @@ riscv_const_insns (rtx x)
> >         return 1;
> >
> >        /* We can use x0 to load floating-point zero.  */
> > -      return x == CONST0_RTX (GET_MODE (x)) ? 1 : 0;
> > +      return riscv_float_const_zero_rtx_p (x) ? 1 : 0;
> >      case CONST_VECTOR:
> >        {
> >         /* TODO: This is not accurate, we will need to
> > @@ -9481,6 +9481,39 @@ riscv_zcmp_valid_stack_adj_bytes_p (HOST_WIDE_INT total, int regs_num)
> >          || additioanl_bytes == ZCMP_MAX_SPIMM * ZCMP_SP_INC_STEP;
> >  }
> >
> > +/* Return true if rtx op is const zero of the floating-point.  */
> > +bool
> > +riscv_float_const_zero_rtx_p (rtx op)
> > +{
> > +  machine_mode mode = GET_MODE (op);
> > +
> > +  if (GET_MODE_CLASS (mode) != MODE_FLOAT)
> > +    return false;
> > +
> > +  if (GET_CODE (op) != CONST_DOUBLE)
> > +    return false;
> > +
> > +  const struct real_value *op_rvalue = CONST_DOUBLE_REAL_VALUE (op);
> > +
> > +  if (REAL_VALUE_MINUS_ZERO (*op_rvalue))
> > +    return !HONOR_SIGNED_ZEROS (mode);
> > +
> > +  return real_equal (op_rvalue, &dconst0);
> > +}
> > +
> > +/* Return true if rtx op is const zero, include both the integer
> > +   and floating-point.  */
> > +bool
> > +riscv_const_zero_rtx_p (rtx op)
> > +{
> > +  machine_mode mode = GET_MODE (op);
> > +
> > +  if (GET_MODE_CLASS (mode) == MODE_FLOAT)
> > +    return riscv_float_const_zero_rtx_p (op);
> > +
> > +  return op == CONST0_RTX (mode);
> > +}
> > +
> >  /* Return true if it's valid gpr_save pattern.  */
> >
> >  bool
> > diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
> > index 68f7203b676..cd429f6dcdd 100644
> > --- a/gcc/config/riscv/riscv.md
> > +++ b/gcc/config/riscv/riscv.md
> > @@ -1878,7 +1878,12 @@ (define_insn "*movhf_hardfloat"
> >    "TARGET_ZFHMIN
> >     && (register_operand (operands[0], HFmode)
> >         || reg_or_0_operand (operands[1], HFmode))"
> > -  { return riscv_output_move (operands[0], operands[1]); }
> > +  {
> > +    if (riscv_float_const_zero_rtx_p (operands[1]))
> > +      operands[1] = CONST0_RTX (GET_MODE (operands[1]));
> > +
> > +    return riscv_output_move (operands[0], operands[1]);
> > +  }
> >    [(set_attr "move_type" "fmove,fmove,mtc,fpload,fpstore,store,mtc,mfc,move,load,store")
> >     (set_attr "type" "fmove")
> >     (set_attr "mode" "HF")])
> > @@ -1889,7 +1894,12 @@ (define_insn "*movhf_softfloat"
> >    "!TARGET_ZFHMIN
> >     && (register_operand (operands[0], HFmode)
> >         || reg_or_0_operand (operands[1], HFmode))"
> > -  { return riscv_output_move (operands[0], operands[1]); }
> > +  {
> > +    if (riscv_float_const_zero_rtx_p (operands[1]))
> > +      operands[1] = CONST0_RTX (GET_MODE (operands[1]));
> > +
> > +    return riscv_output_move (operands[0], operands[1]);
> > +  }
> >    [(set_attr "move_type" "fmove,move,load,store,mtc,mfc")
> >     (set_attr "type" "fmove")
> >     (set_attr "mode" "HF")])
> > @@ -2243,7 +2253,12 @@ (define_insn "*movsf_hardfloat"
> >    "TARGET_HARD_FLOAT
> >     && (register_operand (operands[0], SFmode)
> >         || reg_or_0_operand (operands[1], SFmode))"
> > -  { return riscv_output_move (operands[0], operands[1]); }
> > +  {
> > +    if (riscv_float_const_zero_rtx_p (operands[1]))
> > +      operands[1] = CONST0_RTX (GET_MODE (operands[1]));
> > +
> > +    return riscv_output_move (operands[0], operands[1]);
> > +  }
> >    [(set_attr "move_type" "fmove,fmove,mtc,fpload,fpstore,store,mtc,mfc,move,load,store")
> >     (set_attr "type" "fmove")
> >     (set_attr "mode" "SF")])
> > @@ -2254,7 +2269,12 @@ (define_insn "*movsf_softfloat"
> >    "!TARGET_HARD_FLOAT
> >     && (register_operand (operands[0], SFmode)
> >         || reg_or_0_operand (operands[1], SFmode))"
> > -  { return riscv_output_move (operands[0], operands[1]); }
> > +  {
> > +    if (riscv_float_const_zero_rtx_p (operands[1]))
> > +      operands[1] = CONST0_RTX (GET_MODE (operands[1]));
> > +
> > +    return riscv_output_move (operands[0], operands[1]);
> > +  }
> >    [(set_attr "move_type" "move,load,store")
> >     (set_attr "type" "fmove")
> >     (set_attr "mode" "SF")])
> > @@ -2279,7 +2299,12 @@ (define_insn "*movdf_hardfloat_rv32"
> >    "!TARGET_64BIT && TARGET_DOUBLE_FLOAT
> >     && (register_operand (operands[0], DFmode)
> >         || reg_or_0_operand (operands[1], DFmode))"
> > -  { return riscv_output_move (operands[0], operands[1]); }
> > +  {
> > +    if (riscv_float_const_zero_rtx_p (operands[1]))
> > +      operands[1] = CONST0_RTX (GET_MODE (operands[1]));
> > +
> > +    return riscv_output_move (operands[0], operands[1]);
> > +  }
> >    [(set_attr "move_type" "fmove,fmove,mtc,fpload,fpstore,store,mtc,mfc,move,load,store")
> >     (set_attr "type" "fmove")
> >     (set_attr "mode" "DF")])
> > @@ -2290,7 +2315,12 @@ (define_insn "*movdf_hardfloat_rv64"
> >    "TARGET_64BIT && TARGET_DOUBLE_FLOAT
> >     && (register_operand (operands[0], DFmode)
> >         || reg_or_0_operand (operands[1], DFmode))"
> > -  { return riscv_output_move (operands[0], operands[1]); }
> > +  {
> > +    if (riscv_float_const_zero_rtx_p (operands[1]))
> > +      operands[1] = CONST0_RTX (GET_MODE (operands[1]));
> > +
> > +    return riscv_output_move (operands[0], operands[1]);
> > +  }
> >    [(set_attr "move_type" "fmove,fmove,mtc,fpload,fpstore,store,mtc,mfc,move,load,store")
> >     (set_attr "type" "fmove")
> >     (set_attr "mode" "DF")])
> > @@ -2301,7 +2331,12 @@ (define_insn "*movdf_softfloat"
> >    "!TARGET_DOUBLE_FLOAT
> >     && (register_operand (operands[0], DFmode)
> >         || reg_or_0_operand (operands[1], DFmode))"
> > -  { return riscv_output_move (operands[0], operands[1]); }
> > +  {
> > +    if (riscv_float_const_zero_rtx_p (operands[1]))
> > +      operands[1] = CONST0_RTX (GET_MODE (operands[1]));
> > +
> > +    return riscv_output_move (operands[0], operands[1]);
> > +  }
> >    [(set_attr "move_type" "move,load,store")
> >     (set_attr "type" "fmove")
> >     (set_attr "mode" "DF")])
> > diff --git a/gcc/testsuite/gcc.target/riscv/no-signed-zeros-0.c b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-0.c
> > new file mode 100644
> > index 00000000000..1eda13a3406
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-0.c
> > @@ -0,0 +1,26 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-march=rv64gcv -mabi=lp64d -O3 -fno-signed-zeros -fno-schedule-insns -fno-schedule-insns2" } */
> > +/* { dg-skip-if "" { *-*-* } { "-flto" } } */
> > +/* { dg-final { check-function-bodies "**" "" } } */
> > +
> > +/*
> > +** test_float_plus_zero_assign:
> > +** sw\s+zero,0\([atx][0-9]+\)
> > +** ret
> > +*/
> > +void
> > +test_float_plus_zero_assign (float *a)
> > +{
> > +  *a = +0.0;
> > +}
> > +
> > +/*
> > +** test_float_plus_zero_assign:
> > +** sw\s+zero,0\([atx][0-9]+\)
> > +** ret
> > +*/
> > +void
> > +test_float_minus_zero_assign (float *a)
> > +{
> > +  *a = -0.0;
> > +}
> > diff --git a/gcc/testsuite/gcc.target/riscv/no-signed-zeros-1.c b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-1.c
> > new file mode 100644
> > index 00000000000..8041ec3ea95
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-1.c
> > @@ -0,0 +1,28 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-march=rv64gcv -mabi=lp64d -O3 -fno-signed-zeros -fno-schedule-insns -fno-schedule-insns2" } */
> > +/* { dg-skip-if "" { *-*-* } { "-flto" } } */
> > +/* { dg-final { check-function-bodies "**" "" } } */
> > +
> > +extern void float_assign (float *a, float b);
> > +
> > +/*
> > +** test_float_plus_zero_assign:
> > +** fmv\.s\.x\s+fa[0-9]+,\s*zero
> > +** tail\s+float_assign
> > +*/
> > +void
> > +test_float_plus_zero_assign (float *a)
> > +{
> > +  float_assign (a, +0.0);
> > +}
> > +
> > +/*
> > +** test_float_minus_zero_assign:
> > +** fmv\.s\.x\s+fa[0-9]+,\s*zero
> > +** tail\s+float_assign
> > +*/
> > +void
> > +test_float_minus_zero_assign (float *a)
> > +{
> > +  float_assign (a, -0.0);
> > +}
> > diff --git a/gcc/testsuite/gcc.target/riscv/no-signed-zeros-2.c b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-2.c
> > new file mode 100644
> > index 00000000000..a6996dae4de
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-2.c
> > @@ -0,0 +1,26 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-march=rv64gcv -mabi=lp64d -O3 -fno-signed-zeros -fno-schedule-insns -fno-schedule-insns2" } */
> > +/* { dg-skip-if "" { *-*-* } { "-flto" } } */
> > +/* { dg-final { check-function-bodies "**" "" } } */
> > +
> > +/*
> > +** test_float_plus_zero_assign:
> > +** sd\s+zero,0\([atx][0-9]+\)
> > +** ret
> > +*/
> > +void
> > +test_float_plus_zero_assign (double *a)
> > +{
> > +  *a = +0.0;
> > +}
> > +
> > +/*
> > +** test_float_plus_zero_assign:
> > +** sd\s+zero,0\([atx][0-9]+\)
> > +** ret
> > +*/
> > +void
> > +test_float_minus_zero_assign (double *a)
> > +{
> > +  *a = -0.0;
> > +}
> > diff --git a/gcc/testsuite/gcc.target/riscv/no-signed-zeros-3.c b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-3.c
> > new file mode 100644
> > index 00000000000..b4ba8a247df
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-3.c
> > @@ -0,0 +1,28 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-march=rv64gcv -mabi=lp64d -O3 -fno-signed-zeros -fno-schedule-insns -fno-schedule-insns2" } */
> > +/* { dg-skip-if "" { *-*-* } { "-flto" } } */
> > +/* { dg-final { check-function-bodies "**" "" } } */
> > +
> > +extern void float_assign (double *a, double b);
> > +
> > +/*
> > +** test_float_plus_zero_assign:
> > +** fmv\.d\.x\s+fa[0-9]+,\s*zero
> > +** tail\s+float_assign
> > +*/
> > +void
> > +test_float_plus_zero_assign (double *a)
> > +{
> > +  float_assign (a, +0.0);
> > +}
> > +
> > +/*
> > +** test_float_minus_zero_assign:
> > +** fmv\.d\.x\s+fa[0-9]+,\s*zero
> > +** tail\s+float_assign
> > +*/
> > +void
> > +test_float_minus_zero_assign (double *a)
> > +{
> > +  float_assign (a, -0.0);
> > +}
> > diff --git a/gcc/testsuite/gcc.target/riscv/no-signed-zeros-4.c b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-4.c
> > new file mode 100644
> > index 00000000000..60acf7155d3
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-4.c
> > @@ -0,0 +1,26 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-march=rv64gcv_zfh -mabi=lp64d -O3 -fno-signed-zeros -fno-schedule-insns -fno-schedule-insns2" } */
> > +/* { dg-skip-if "" { *-*-* } { "-flto" } } */
> > +/* { dg-final { check-function-bodies "**" "" } } */
> > +
> > +/*
> > +** test_float_plus_zero_assign:
> > +** sh\s+zero,0\([atx][0-9]+\)
> > +** ret
> > +*/
> > +void
> > +test_float_plus_zero_assign (_Float16 *a)
> > +{
> > +  *a = +0.0;
> > +}
> > +
> > +/*
> > +** test_float_plus_zero_assign:
> > +** sh\s+zero,0\([atx][0-9]+\)
> > +** ret
> > +*/
> > +void
> > +test_float_minus_zero_assign (_Float16 *a)
> > +{
> > +  *a = -0.0;
> > +}
> > diff --git a/gcc/testsuite/gcc.target/riscv/no-signed-zeros-5.c b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-5.c
> > new file mode 100644
> > index 00000000000..d10efbeb37b
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-5.c
> > @@ -0,0 +1,28 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-march=rv64gcv_zfh -mabi=lp64d -O3 -fno-signed-zeros -fno-schedule-insns -fno-schedule-insns2" } */
> > +/* { dg-skip-if "" { *-*-* } { "-flto" } } */
> > +/* { dg-final { check-function-bodies "**" "" } } */
> > +
> > +extern void float_assign (_Float16 *a, _Float16 b);
> > +
> > +/*
> > +** test_float_plus_zero_assign:
> > +** fmv\.h\.x\s+fa[0-9]+,\s*zero
> > +** tail\s+float_assign
> > +*/
> > +void
> > +test_float_plus_zero_assign (_Float16 *a)
> > +{
> > +  float_assign (a, +0.0);
> > +}
> > +
> > +/*
> > +** test_float_minus_zero_assign:
> > +** fmv\.h\.x\s+fa[0-9]+,\s*zero
> > +** tail\s+float_assign
> > +*/
> > +void
> > +test_float_minus_zero_assign (_Float16 *a)
> > +{
> > +  float_assign (a, -0.0);
> > +}
> > diff --git a/gcc/testsuite/gcc.target/riscv/no-signed-zeros-run-0.c b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-run-0.c
> > new file mode 100644
> > index 00000000000..347e4b0ff74
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-run-0.c
> > @@ -0,0 +1,36 @@
> > +/* { dg-do run { target { riscv_v } } } */
> > +/* { dg-additional-options "-std=c99 -O3 -fno-signed-zeros" } */
> > +
> > +float f_val;
> > +
> > +void
> > +__attribute__ ((noinline))
> > +test_float_plus_zero_assign (float *a)
> > +{
> > +  *a = +0.0;
> > +}
> > +
> > +void
> > +__attribute__ ((noinline))
> > +test_float_minus_zero_assign (float *a)
> > +{
> > +  *a = -0.0;
> > +}
> > +
> > +int
> > +main ()
> > +{
> > +  f_val = -1.0;
> > +  test_float_plus_zero_assign (&f_val);
> > +
> > +  if (__builtin_copysignf (1.0, f_val) != 1.0)
> > +    __builtin_abort ();
> > +
> > +  f_val = -1.0;
> > +  test_float_minus_zero_assign (&f_val);
> > +
> > +  if (__builtin_copysignf (1.0, f_val) != 1.0)
> > +    __builtin_abort ();
> > +
> > +  return 0;
> > +}
> > diff --git a/gcc/testsuite/gcc.target/riscv/no-signed-zeros-run-1.c b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-run-1.c
> > new file mode 100644
> > index 00000000000..1eb1edba457
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-run-1.c
> > @@ -0,0 +1,36 @@
> > +/* { dg-do run { target { riscv_v } } } */
> > +/* { dg-additional-options "-std=c99 -O3 -fno-signed-zeros" } */
> > +
> > +double f_val;
> > +
> > +void
> > +__attribute__ ((noinline))
> > +test_float_plus_zero_assign (double *a)
> > +{
> > +  *a = +0.0;
> > +}
> > +
> > +void
> > +__attribute__ ((noinline))
> > +test_float_minus_zero_assign (double *a)
> > +{
> > +  *a = -0.0;
> > +}
> > +
> > +int
> > +main ()
> > +{
> > +  f_val = -1.0;
> > +  test_float_plus_zero_assign (&f_val);
> > +
> > +  if (__builtin_copysignf (1.0, f_val) != 1.0)
> > +    __builtin_abort ();
> > +
> > +  f_val = -1.0;
> > +  test_float_minus_zero_assign (&f_val);
> > +
> > +  if (__builtin_copysignf (1.0, f_val) != 1.0)
> > +    __builtin_abort ();
> > +
> > +  return 0;
> > +}
> > --
> > 2.34.1
> >
Jeff Law Jan. 9, 2024, 5:46 p.m. UTC | #5 
On 1/8/24 03:45, Richard Biener wrote:
> On Tue, Jan 2, 2024 at 2:37 PM <pan2.li@intel.com> wrote:
>>
>> From: Pan Li <pan2.li@intel.com>
>>
>> According to the sematics of no-signed-zeros option, the backend
>> like RISC-V should treat the minus zero -0.0f as plus zero 0.0f.
>>
>> Consider below example with option -fno-signed-zeros.
>>
>> void
>> test (float *a)
>> {
>>    *a = -0.0;
>> }
>>
>> We will generate code as below, which doesn't treat the minus zero
>> as plus zero.
>>
>> test:
>>    lui  a5,%hi(.LC0)
>>    flw  fa5,%lo(.LC0)(a5)
>>    fsw  fa5,0(a0)
>>    ret
>>
>> .LC0:
>>    .word -2147483648 // aka -0.0 (0x80000000 in hex)
>>
>> This patch would like to fix the bug and treat the minus zero -0.0
>> as plus zero, aka +0.0. Thus after this patch we will have asm code
>> as below for the above sampe code.
>>
>> test:
>>    sw zero,0(a0)
>>    ret
>>
>> This patch also fix the run failure of the test case pr30957-1.c. The
>> below tests are passed for this patch.
> 
> We don't really expect targets to do this.  The small testcase above
> is somewhat ill-formed with -fno-signed-zeros.  Note there's no
> -0.0 in pr30957-1.c so why does that one fail for you?  Does
> the -fvariable-expansion-in-unroller code maybe not trigger for
> riscv?
Loop unrolling (and thus variable expansion) doesn't trigger on the VLA 
style architectures.  aarch64 passes becuase its backend knows it can 
translate -0.0 into 0.0.

While we don't require that from ports, I'd just assume do the 
optimization similar to aarch64 rather than xfail or skip the test on 
RISC-V.  We can load 0.0 more efficiently than -0.0.


> 
> I think we should go to PR30957 and see what that was filed originally
> for, the testcase doesn't make much sense to me.
It's got more history than I'd like :(


jeff
Li, Pan2 Jan. 10, 2024, 4:28 a.m. UTC | #6 
Thanks Jeff and Richard for confirmation and comments.

It looks like firstly we should address the issue of the original commits in v4 and then
back to if there is something we need to deal with option no-signed-zero for the riscv.

Pan

-----Original Message-----
From: Jeff Law <jeffreyalaw@gmail.com> 
Sent: Wednesday, January 10, 2024 1:46 AM
To: Richard Biener <richard.guenther@gmail.com>; Li, Pan2 <pan2.li@intel.com>
Cc: gcc-patches@gcc.gnu.org; juzhe.zhong@rivai.ai; Wang, Yanzhang <yanzhang.wang@intel.com>; kito.cheng@gmail.com
Subject: Re: [PATCH v3] RISC-V: Bugfix for doesn't honor no-signed-zeros option



On 1/8/24 03:45, Richard Biener wrote:
> On Tue, Jan 2, 2024 at 2:37 PM <pan2.li@intel.com> wrote:
>>
>> From: Pan Li <pan2.li@intel.com>
>>
>> According to the sematics of no-signed-zeros option, the backend
>> like RISC-V should treat the minus zero -0.0f as plus zero 0.0f.
>>
>> Consider below example with option -fno-signed-zeros.
>>
>> void
>> test (float *a)
>> {
>>    *a = -0.0;
>> }
>>
>> We will generate code as below, which doesn't treat the minus zero
>> as plus zero.
>>
>> test:
>>    lui  a5,%hi(.LC0)
>>    flw  fa5,%lo(.LC0)(a5)
>>    fsw  fa5,0(a0)
>>    ret
>>
>> .LC0:
>>    .word -2147483648 // aka -0.0 (0x80000000 in hex)
>>
>> This patch would like to fix the bug and treat the minus zero -0.0
>> as plus zero, aka +0.0. Thus after this patch we will have asm code
>> as below for the above sampe code.
>>
>> test:
>>    sw zero,0(a0)
>>    ret
>>
>> This patch also fix the run failure of the test case pr30957-1.c. The
>> below tests are passed for this patch.
> 
> We don't really expect targets to do this.  The small testcase above
> is somewhat ill-formed with -fno-signed-zeros.  Note there's no
> -0.0 in pr30957-1.c so why does that one fail for you?  Does
> the -fvariable-expansion-in-unroller code maybe not trigger for
> riscv?
Loop unrolling (and thus variable expansion) doesn't trigger on the VLA 
style architectures.  aarch64 passes becuase its backend knows it can 
translate -0.0 into 0.0.

While we don't require that from ports, I'd just assume do the 
optimization similar to aarch64 rather than xfail or skip the test on 
RISC-V.  We can load 0.0 more efficiently than -0.0.


> 
> I think we should go to PR30957 and see what that was filed originally
> for, the testcase doesn't make much sense to me.
It's got more history than I'd like :(


jeff
diff mbox series

Patch

diff --git a/gcc/config/riscv/constraints.md b/gcc/config/riscv/constraints.md
index de4359af00d..db1d5e1385f 100644
--- a/gcc/config/riscv/constraints.md
+++ b/gcc/config/riscv/constraints.md
@@ -108,7 +108,7 @@  (define_constraint "DnS"
 (define_constraint "G"
   "@internal"
   (and (match_code "const_double")
-       (match_test "op == CONST0_RTX (mode)")))
+       (match_test "riscv_float_const_zero_rtx_p (op)")))
 
 (define_memory_constraint "A"
   "An address that is held in a general-purpose register."
diff --git a/gcc/config/riscv/predicates.md b/gcc/config/riscv/predicates.md
index b87a6900841..b428d842101 100644
--- a/gcc/config/riscv/predicates.md
+++ b/gcc/config/riscv/predicates.md
@@ -78,7 +78,7 @@  (define_predicate "sleu_operand"
 
 (define_predicate "const_0_operand"
   (and (match_code "const_int,const_wide_int,const_double,const_vector")
-       (match_test "op == CONST0_RTX (GET_MODE (op))")))
+       (match_test "riscv_const_zero_rtx_p (op)")))
 
 (define_predicate "const_1_operand"
   (and (match_code "const_int,const_wide_int,const_vector")
diff --git a/gcc/config/riscv/riscv-protos.h b/gcc/config/riscv/riscv-protos.h
index 31049ef7523..fcf30e084a3 100644
--- a/gcc/config/riscv/riscv-protos.h
+++ b/gcc/config/riscv/riscv-protos.h
@@ -131,6 +131,8 @@  extern void riscv_asm_output_external (FILE *, const tree, const char *);
 extern bool
 riscv_zcmp_valid_stack_adj_bytes_p (HOST_WIDE_INT, int);
 extern void riscv_legitimize_poly_move (machine_mode, rtx, rtx, rtx);
+extern bool riscv_float_const_zero_rtx_p (rtx);
+extern bool riscv_const_zero_rtx_p (rtx);
 
 #ifdef RTX_CODE
 extern void riscv_expand_int_scc (rtx, enum rtx_code, rtx, rtx, bool *invert_ptr = 0);
diff --git a/gcc/config/riscv/riscv.cc b/gcc/config/riscv/riscv.cc
index 0d1cbc5cb5f..a8ad86b7068 100644
--- a/gcc/config/riscv/riscv.cc
+++ b/gcc/config/riscv/riscv.cc
@@ -1635,7 +1635,7 @@  riscv_const_insns (rtx x)
 	return 1;
 
       /* We can use x0 to load floating-point zero.  */
-      return x == CONST0_RTX (GET_MODE (x)) ? 1 : 0;
+      return riscv_float_const_zero_rtx_p (x) ? 1 : 0;
     case CONST_VECTOR:
       {
 	/* TODO: This is not accurate, we will need to
@@ -9481,6 +9481,39 @@  riscv_zcmp_valid_stack_adj_bytes_p (HOST_WIDE_INT total, int regs_num)
 	 || additioanl_bytes == ZCMP_MAX_SPIMM * ZCMP_SP_INC_STEP;
 }
 
+/* Return true if rtx op is const zero of the floating-point.  */
+bool
+riscv_float_const_zero_rtx_p (rtx op)
+{
+  machine_mode mode = GET_MODE (op);
+
+  if (GET_MODE_CLASS (mode) != MODE_FLOAT)
+    return false;
+
+  if (GET_CODE (op) != CONST_DOUBLE)
+    return false;
+
+  const struct real_value *op_rvalue = CONST_DOUBLE_REAL_VALUE (op);
+
+  if (REAL_VALUE_MINUS_ZERO (*op_rvalue))
+    return !HONOR_SIGNED_ZEROS (mode);
+
+  return real_equal (op_rvalue, &dconst0);
+}
+
+/* Return true if rtx op is const zero, include both the integer
+   and floating-point.  */
+bool
+riscv_const_zero_rtx_p (rtx op)
+{
+  machine_mode mode = GET_MODE (op);
+
+  if (GET_MODE_CLASS (mode) == MODE_FLOAT)
+    return riscv_float_const_zero_rtx_p (op);
+
+  return op == CONST0_RTX (mode);
+}
+
 /* Return true if it's valid gpr_save pattern.  */
 
 bool
diff --git a/gcc/config/riscv/riscv.md b/gcc/config/riscv/riscv.md
index 68f7203b676..cd429f6dcdd 100644
--- a/gcc/config/riscv/riscv.md
+++ b/gcc/config/riscv/riscv.md
@@ -1878,7 +1878,12 @@  (define_insn "*movhf_hardfloat"
   "TARGET_ZFHMIN
    && (register_operand (operands[0], HFmode)
        || reg_or_0_operand (operands[1], HFmode))"
-  { return riscv_output_move (operands[0], operands[1]); }
+  {
+    if (riscv_float_const_zero_rtx_p (operands[1]))
+      operands[1] = CONST0_RTX (GET_MODE (operands[1]));
+
+    return riscv_output_move (operands[0], operands[1]);
+  }
   [(set_attr "move_type" "fmove,fmove,mtc,fpload,fpstore,store,mtc,mfc,move,load,store")
    (set_attr "type" "fmove")
    (set_attr "mode" "HF")])
@@ -1889,7 +1894,12 @@  (define_insn "*movhf_softfloat"
   "!TARGET_ZFHMIN
    && (register_operand (operands[0], HFmode)
        || reg_or_0_operand (operands[1], HFmode))"
-  { return riscv_output_move (operands[0], operands[1]); }
+  {
+    if (riscv_float_const_zero_rtx_p (operands[1]))
+      operands[1] = CONST0_RTX (GET_MODE (operands[1]));
+
+    return riscv_output_move (operands[0], operands[1]);
+  }
   [(set_attr "move_type" "fmove,move,load,store,mtc,mfc")
    (set_attr "type" "fmove")
    (set_attr "mode" "HF")])
@@ -2243,7 +2253,12 @@  (define_insn "*movsf_hardfloat"
   "TARGET_HARD_FLOAT
    && (register_operand (operands[0], SFmode)
        || reg_or_0_operand (operands[1], SFmode))"
-  { return riscv_output_move (operands[0], operands[1]); }
+  {
+    if (riscv_float_const_zero_rtx_p (operands[1]))
+      operands[1] = CONST0_RTX (GET_MODE (operands[1]));
+
+    return riscv_output_move (operands[0], operands[1]);
+  }
   [(set_attr "move_type" "fmove,fmove,mtc,fpload,fpstore,store,mtc,mfc,move,load,store")
    (set_attr "type" "fmove")
    (set_attr "mode" "SF")])
@@ -2254,7 +2269,12 @@  (define_insn "*movsf_softfloat"
   "!TARGET_HARD_FLOAT
    && (register_operand (operands[0], SFmode)
        || reg_or_0_operand (operands[1], SFmode))"
-  { return riscv_output_move (operands[0], operands[1]); }
+  {
+    if (riscv_float_const_zero_rtx_p (operands[1]))
+      operands[1] = CONST0_RTX (GET_MODE (operands[1]));
+
+    return riscv_output_move (operands[0], operands[1]);
+  }
   [(set_attr "move_type" "move,load,store")
    (set_attr "type" "fmove")
    (set_attr "mode" "SF")])
@@ -2279,7 +2299,12 @@  (define_insn "*movdf_hardfloat_rv32"
   "!TARGET_64BIT && TARGET_DOUBLE_FLOAT
    && (register_operand (operands[0], DFmode)
        || reg_or_0_operand (operands[1], DFmode))"
-  { return riscv_output_move (operands[0], operands[1]); }
+  {
+    if (riscv_float_const_zero_rtx_p (operands[1]))
+      operands[1] = CONST0_RTX (GET_MODE (operands[1]));
+
+    return riscv_output_move (operands[0], operands[1]);
+  }
   [(set_attr "move_type" "fmove,fmove,mtc,fpload,fpstore,store,mtc,mfc,move,load,store")
    (set_attr "type" "fmove")
    (set_attr "mode" "DF")])
@@ -2290,7 +2315,12 @@  (define_insn "*movdf_hardfloat_rv64"
   "TARGET_64BIT && TARGET_DOUBLE_FLOAT
    && (register_operand (operands[0], DFmode)
        || reg_or_0_operand (operands[1], DFmode))"
-  { return riscv_output_move (operands[0], operands[1]); }
+  {
+    if (riscv_float_const_zero_rtx_p (operands[1]))
+      operands[1] = CONST0_RTX (GET_MODE (operands[1]));
+
+    return riscv_output_move (operands[0], operands[1]);
+  }
   [(set_attr "move_type" "fmove,fmove,mtc,fpload,fpstore,store,mtc,mfc,move,load,store")
    (set_attr "type" "fmove")
    (set_attr "mode" "DF")])
@@ -2301,7 +2331,12 @@  (define_insn "*movdf_softfloat"
   "!TARGET_DOUBLE_FLOAT
    && (register_operand (operands[0], DFmode)
        || reg_or_0_operand (operands[1], DFmode))"
-  { return riscv_output_move (operands[0], operands[1]); }
+  {
+    if (riscv_float_const_zero_rtx_p (operands[1]))
+      operands[1] = CONST0_RTX (GET_MODE (operands[1]));
+
+    return riscv_output_move (operands[0], operands[1]);
+  }
   [(set_attr "move_type" "move,load,store")
    (set_attr "type" "fmove")
    (set_attr "mode" "DF")])
diff --git a/gcc/testsuite/gcc.target/riscv/no-signed-zeros-0.c b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-0.c
new file mode 100644
index 00000000000..1eda13a3406
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-0.c
@@ -0,0 +1,26 @@ 
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gcv -mabi=lp64d -O3 -fno-signed-zeros -fno-schedule-insns -fno-schedule-insns2" } */
+/* { dg-skip-if "" { *-*-* } { "-flto" } } */
+/* { dg-final { check-function-bodies "**" "" } } */
+
+/*
+** test_float_plus_zero_assign:
+** sw\s+zero,0\([atx][0-9]+\)
+** ret
+*/
+void
+test_float_plus_zero_assign (float *a)
+{
+  *a = +0.0;
+}
+
+/*
+** test_float_plus_zero_assign:
+** sw\s+zero,0\([atx][0-9]+\)
+** ret
+*/
+void
+test_float_minus_zero_assign (float *a)
+{
+  *a = -0.0;
+}
diff --git a/gcc/testsuite/gcc.target/riscv/no-signed-zeros-1.c b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-1.c
new file mode 100644
index 00000000000..8041ec3ea95
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-1.c
@@ -0,0 +1,28 @@ 
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gcv -mabi=lp64d -O3 -fno-signed-zeros -fno-schedule-insns -fno-schedule-insns2" } */
+/* { dg-skip-if "" { *-*-* } { "-flto" } } */
+/* { dg-final { check-function-bodies "**" "" } } */
+
+extern void float_assign (float *a, float b);
+
+/*
+** test_float_plus_zero_assign:
+** fmv\.s\.x\s+fa[0-9]+,\s*zero
+** tail\s+float_assign
+*/
+void
+test_float_plus_zero_assign (float *a)
+{
+  float_assign (a, +0.0);
+}
+
+/*
+** test_float_minus_zero_assign:
+** fmv\.s\.x\s+fa[0-9]+,\s*zero
+** tail\s+float_assign
+*/
+void
+test_float_minus_zero_assign (float *a)
+{
+  float_assign (a, -0.0);
+}
diff --git a/gcc/testsuite/gcc.target/riscv/no-signed-zeros-2.c b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-2.c
new file mode 100644
index 00000000000..a6996dae4de
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-2.c
@@ -0,0 +1,26 @@ 
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gcv -mabi=lp64d -O3 -fno-signed-zeros -fno-schedule-insns -fno-schedule-insns2" } */
+/* { dg-skip-if "" { *-*-* } { "-flto" } } */
+/* { dg-final { check-function-bodies "**" "" } } */
+
+/*
+** test_float_plus_zero_assign:
+** sd\s+zero,0\([atx][0-9]+\)
+** ret
+*/
+void
+test_float_plus_zero_assign (double *a)
+{
+  *a = +0.0;
+}
+
+/*
+** test_float_plus_zero_assign:
+** sd\s+zero,0\([atx][0-9]+\)
+** ret
+*/
+void
+test_float_minus_zero_assign (double *a)
+{
+  *a = -0.0;
+}
diff --git a/gcc/testsuite/gcc.target/riscv/no-signed-zeros-3.c b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-3.c
new file mode 100644
index 00000000000..b4ba8a247df
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-3.c
@@ -0,0 +1,28 @@ 
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gcv -mabi=lp64d -O3 -fno-signed-zeros -fno-schedule-insns -fno-schedule-insns2" } */
+/* { dg-skip-if "" { *-*-* } { "-flto" } } */
+/* { dg-final { check-function-bodies "**" "" } } */
+
+extern void float_assign (double *a, double b);
+
+/*
+** test_float_plus_zero_assign:
+** fmv\.d\.x\s+fa[0-9]+,\s*zero
+** tail\s+float_assign
+*/
+void
+test_float_plus_zero_assign (double *a)
+{
+  float_assign (a, +0.0);
+}
+
+/*
+** test_float_minus_zero_assign:
+** fmv\.d\.x\s+fa[0-9]+,\s*zero
+** tail\s+float_assign
+*/
+void
+test_float_minus_zero_assign (double *a)
+{
+  float_assign (a, -0.0);
+}
diff --git a/gcc/testsuite/gcc.target/riscv/no-signed-zeros-4.c b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-4.c
new file mode 100644
index 00000000000..60acf7155d3
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-4.c
@@ -0,0 +1,26 @@ 
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gcv_zfh -mabi=lp64d -O3 -fno-signed-zeros -fno-schedule-insns -fno-schedule-insns2" } */
+/* { dg-skip-if "" { *-*-* } { "-flto" } } */
+/* { dg-final { check-function-bodies "**" "" } } */
+
+/*
+** test_float_plus_zero_assign:
+** sh\s+zero,0\([atx][0-9]+\)
+** ret
+*/
+void
+test_float_plus_zero_assign (_Float16 *a)
+{
+  *a = +0.0;
+}
+
+/*
+** test_float_plus_zero_assign:
+** sh\s+zero,0\([atx][0-9]+\)
+** ret
+*/
+void
+test_float_minus_zero_assign (_Float16 *a)
+{
+  *a = -0.0;
+}
diff --git a/gcc/testsuite/gcc.target/riscv/no-signed-zeros-5.c b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-5.c
new file mode 100644
index 00000000000..d10efbeb37b
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-5.c
@@ -0,0 +1,28 @@ 
+/* { dg-do compile } */
+/* { dg-options "-march=rv64gcv_zfh -mabi=lp64d -O3 -fno-signed-zeros -fno-schedule-insns -fno-schedule-insns2" } */
+/* { dg-skip-if "" { *-*-* } { "-flto" } } */
+/* { dg-final { check-function-bodies "**" "" } } */
+
+extern void float_assign (_Float16 *a, _Float16 b);
+
+/*
+** test_float_plus_zero_assign:
+** fmv\.h\.x\s+fa[0-9]+,\s*zero
+** tail\s+float_assign
+*/
+void
+test_float_plus_zero_assign (_Float16 *a)
+{
+  float_assign (a, +0.0);
+}
+
+/*
+** test_float_minus_zero_assign:
+** fmv\.h\.x\s+fa[0-9]+,\s*zero
+** tail\s+float_assign
+*/
+void
+test_float_minus_zero_assign (_Float16 *a)
+{
+  float_assign (a, -0.0);
+}
diff --git a/gcc/testsuite/gcc.target/riscv/no-signed-zeros-run-0.c b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-run-0.c
new file mode 100644
index 00000000000..347e4b0ff74
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-run-0.c
@@ -0,0 +1,36 @@ 
+/* { dg-do run { target { riscv_v } } } */
+/* { dg-additional-options "-std=c99 -O3 -fno-signed-zeros" } */
+
+float f_val;
+
+void
+__attribute__ ((noinline))
+test_float_plus_zero_assign (float *a)
+{
+  *a = +0.0;
+}
+
+void
+__attribute__ ((noinline))
+test_float_minus_zero_assign (float *a)
+{
+  *a = -0.0;
+}
+
+int
+main ()
+{
+  f_val = -1.0;
+  test_float_plus_zero_assign (&f_val);
+
+  if (__builtin_copysignf (1.0, f_val) != 1.0)
+    __builtin_abort ();
+
+  f_val = -1.0;
+  test_float_minus_zero_assign (&f_val);
+
+  if (__builtin_copysignf (1.0, f_val) != 1.0)
+    __builtin_abort ();
+
+  return 0;
+}
diff --git a/gcc/testsuite/gcc.target/riscv/no-signed-zeros-run-1.c b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-run-1.c
new file mode 100644
index 00000000000..1eb1edba457
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/no-signed-zeros-run-1.c
@@ -0,0 +1,36 @@ 
+/* { dg-do run { target { riscv_v } } } */
+/* { dg-additional-options "-std=c99 -O3 -fno-signed-zeros" } */
+
+double f_val;
+
+void
+__attribute__ ((noinline))
+test_float_plus_zero_assign (double *a)
+{
+  *a = +0.0;
+}
+
+void
+__attribute__ ((noinline))
+test_float_minus_zero_assign (double *a)
+{
+  *a = -0.0;
+}
+
+int
+main ()
+{
+  f_val = -1.0;
+  test_float_plus_zero_assign (&f_val);
+
+  if (__builtin_copysignf (1.0, f_val) != 1.0)
+    __builtin_abort ();
+
+  f_val = -1.0;
+  test_float_minus_zero_assign (&f_val);
+
+  if (__builtin_copysignf (1.0, f_val) != 1.0)
+    __builtin_abort ();
+
+  return 0;
+}