[PATCH] MIPS: Fix wrong MSA FP vector negation

[PATCH] MIPS: Fix wrong MSA FP vector negation

[Xi Ruoyao]

We expanded (neg x) to (minus const0 x) for MSA FP vectors, this is
wrong because -0.0 is not 0 - 0.0.  This causes some Python tests to
fail when Python is built with MSA enabled.

Use the bnegi.df instructions to simply reverse the sign bit instead.

gcc/ChangeLog:

	* config/mips/mips-msa.md (elmsgnbit): New define_mode_attr.
	(neg<mode>2): Change the mode iterator from MSA to IMSA because
	in FP arithmetic we cannot use (0 - x) for -x.
	(neg<mode>2): New define_insn to implement FP vector negation,
	using a bnegi instruction to negate the sign bit.
---

Bootstrapped and regtested on mips64el-linux-gnuabi64.  Ok for trunk
and/or release branches?

 gcc/config/mips/mips-msa.md | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/gcc/config/mips/mips-msa.md b/gcc/config/mips/mips-msa.md
index 83d9a08e360..920161ed1d8 100644
--- a/gcc/config/mips/mips-msa.md
+++ b/gcc/config/mips/mips-msa.md
@@ -231,6 +231,10 @@ (define_mode_attr bitimm
    (V4SI  "uimm5")
    (V2DI  "uimm6")])
 
+;; The index of sign bit in FP vector elements.
+(define_mode_attr elmsgnbit [(V2DF "63") (V4DF "63")
+			     (V4SF "31") (V8SF "31")])
+
 (define_expand "vec_init<mode><unitmode>"
   [(match_operand:MSA 0 "register_operand")
    (match_operand:MSA 1 "")]
@@ -597,9 +601,9 @@ (define_expand "abs<mode>2"
 })
 
 (define_expand "neg<mode>2"
-  [(set (match_operand:MSA 0 "register_operand")
-	(minus:MSA (match_dup 2)
-		   (match_operand:MSA 1 "register_operand")))]
+  [(set (match_operand:IMSA 0 "register_operand")
+	(minus:IMSA (match_dup 2)
+		   (match_operand:IMSA 1 "register_operand")))]
   "ISA_HAS_MSA"
 {
   rtx reg = gen_reg_rtx (<MODE>mode);
@@ -607,6 +611,14 @@ (define_expand "neg<mode>2"
   operands[2] = reg;
 })
 
+(define_insn "neg<mode>2"
+  [(set (match_operand:FMSA 0 "register_operand" "=f")
+	(neg (match_operand:FMSA 1 "register_operand" "f")))]
+  "ISA_HAS_MSA"
+  "bnegi.<msafmt>\t%w0,%w1,<elmsgnbit>"
+  [(set_attr "type" "simd_bit")
+   (set_attr "mode" "<MODE>")])
+
 (define_expand "msa_ldi<mode>"
   [(match_operand:IMSA 0 "register_operand")
    (match_operand 1 "const_imm10_operand")]
-- 
2.43.0

Re: [PATCH] MIPS: Fix wrong MSA FP vector negation

[YunQiang Su]

Xi Ruoyao <xry111@xry111.site> 于2024年2月5日周一 02:01写道：
>
> We expanded (neg x) to (minus const0 x) for MSA FP vectors, this is
> wrong because -0.0 is not 0 - 0.0.  This causes some Python tests to
> fail when Python is built with MSA enabled.
>
> Use the bnegi.df instructions to simply reverse the sign bit instead.
>
> gcc/ChangeLog:
>
>         * config/mips/mips-msa.md (elmsgnbit): New define_mode_attr.
>         (neg<mode>2): Change the mode iterator from MSA to IMSA because
>         in FP arithmetic we cannot use (0 - x) for -x.
>         (neg<mode>2): New define_insn to implement FP vector negation,
>         using a bnegi instruction to negate the sign bit.
> ---
>
> Bootstrapped and regtested on mips64el-linux-gnuabi64.  Ok for trunk
> and/or release branches?
>
>  gcc/config/mips/mips-msa.md | 18 +++++++++++++++---
>  1 file changed, 15 insertions(+), 3 deletions(-)
>

LGTM, while I guess that we also need a test case.

> diff --git a/gcc/config/mips/mips-msa.md b/gcc/config/mips/mips-msa.md
> index 83d9a08e360..920161ed1d8 100644
> --- a/gcc/config/mips/mips-msa.md
> +++ b/gcc/config/mips/mips-msa.md
> @@ -231,6 +231,10 @@ (define_mode_attr bitimm
>     (V4SI  "uimm5")
>     (V2DI  "uimm6")])
>
> +;; The index of sign bit in FP vector elements.
> +(define_mode_attr elmsgnbit [(V2DF "63") (V4DF "63")
> +                            (V4SF "31") (V8SF "31")])
> +
>  (define_expand "vec_init<mode><unitmode>"
>    [(match_operand:MSA 0 "register_operand")
>     (match_operand:MSA 1 "")]
> @@ -597,9 +601,9 @@ (define_expand "abs<mode>2"
>  })
>
>  (define_expand "neg<mode>2"
> -  [(set (match_operand:MSA 0 "register_operand")
> -       (minus:MSA (match_dup 2)
> -                  (match_operand:MSA 1 "register_operand")))]
> +  [(set (match_operand:IMSA 0 "register_operand")
> +       (minus:IMSA (match_dup 2)
> +                  (match_operand:IMSA 1 "register_operand")))]
>    "ISA_HAS_MSA"
>  {
>    rtx reg = gen_reg_rtx (<MODE>mode);
> @@ -607,6 +611,14 @@ (define_expand "neg<mode>2"
>    operands[2] = reg;
>  })
>
> +(define_insn "neg<mode>2"
> +  [(set (match_operand:FMSA 0 "register_operand" "=f")
> +       (neg (match_operand:FMSA 1 "register_operand" "f")))]
> +  "ISA_HAS_MSA"
> +  "bnegi.<msafmt>\t%w0,%w1,<elmsgnbit>"
> +  [(set_attr "type" "simd_bit")
> +   (set_attr "mode" "<MODE>")])
> +
>  (define_expand "msa_ldi<mode>"
>    [(match_operand:IMSA 0 "register_operand")
>     (match_operand 1 "const_imm10_operand")]
> --
> 2.43.0
>

Pushed: [PATCH] MIPS: Fix wrong MSA FP vector negation

[Xi Ruoyao]

On Mon, 2024-02-05 at 09:56 +0800, YunQiang Su wrote:
> Xi Ruoyao <xry111@xry111.site> 于2024年2月5日周一 02:01写道：
> > 
> > We expanded (neg x) to (minus const0 x) for MSA FP vectors, this is
> > wrong because -0.0 is not 0 - 0.0.  This causes some Python tests to
> > fail when Python is built with MSA enabled.
> > 
> > Use the bnegi.df instructions to simply reverse the sign bit instead.
> > 
> > gcc/ChangeLog:
> > 
> >          * config/mips/mips-msa.md (elmsgnbit): New define_mode_attr.
> >          (neg<mode>2): Change the mode iterator from MSA to IMSA because
> >          in FP arithmetic we cannot use (0 - x) for -x.
> >          (neg<mode>2): New define_insn to implement FP vector negation,
> >          using a bnegi instruction to negate the sign bit.
> > ---
> > 
> > Bootstrapped and regtested on mips64el-linux-gnuabi64.  Ok for trunk
> > and/or release branches?
> > 
> >   gcc/config/mips/mips-msa.md | 18 +++++++++++++++---
> >   1 file changed, 15 insertions(+), 3 deletions(-)
> > 
> 
> LGTM, while I guess that we also need a test case.

Pushed to trunk and release branches, with a following obvious fix:

diff --git a/gcc/config/mips/mips-msa.md b/gcc/config/mips/mips-msa.md
index 920161ed1d8..779157f2a0c 100644
--- a/gcc/config/mips/mips-msa.md
+++ b/gcc/config/mips/mips-msa.md
@@ -613,7 +613,7 @@ (define_expand "neg<mode>2"
 
 (define_insn "neg<mode>2"
   [(set (match_operand:FMSA 0 "register_operand" "=f")
-       (neg (match_operand:FMSA 1 "register_operand" "f")))]
+       (neg:FMSA (match_operand:FMSA 1 "register_operand" "f")))]
   "ISA_HAS_MSA"
   "bnegi.<msafmt>\t%w0,%w1,<elmsgnbit>"
   [(set_attr "type" "simd_bit")

I'll write a test case for gcc.dg/vect later (now I have to do
$SOME_REAL_LIFE_THING...)

-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University