[Bug target/89984] Extra register move

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89984

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org
             Status|RESOLVED                    |REOPENED
         Resolution|FIXED                       |---

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
This patch is incorrect, it breaks e.g.
float bar (float x, float y) { return x * __builtin_copysignf (1.0f, y); }

On top of the PR102224 patch, I've tried:
--- gcc/config/i386/i386.md.jj  2021-09-08 09:55:40.791551976 +0200
+++ gcc/config/i386/i386.md     2021-09-08 10:15:10.241011077 +0200
@@ -10919,18 +10919,19 @@ (define_expand "xorsign<mode>3"
 })

 (define_insn_and_split "@xorsign<mode>3_1"
-  [(set (match_operand:MODEF 0 "register_operand" "=&Yv,&Yv,&Yv")
+  [(set (match_operand:MODEF 0 "register_operand" "=&Yv,Yv")
        (unspec:MODEF
-         [(match_operand:MODEF 1 "register_operand" "Yv,0,Yv")
-          (match_operand:MODEF 2 "register_operand" "0,Yv,Yv")
-          (match_operand:<ssevecmode> 3 "nonimmediate_operand" "Yvm,Yvm,Yvm")]
-         UNSPEC_XORSIGN))]
+         [(match_operand:MODEF 1 "register_operand" "Yv,Yv")
+          (match_operand:MODEF 2 "register_operand" "0,Yv")
+          (match_operand:<ssevecmode> 3 "nonimmediate_operand" "Yvm,Yvm")]
+         UNSPEC_XORSIGN))
+   (clobber (match_scratch:MODEF 4 "=X,Yv"))]
   "SSE_FLOAT_MODE_P (<MODE>mode) && TARGET_SSE_MATH"
   "#"
   "&& reload_completed"
   [(const_int 0)]
   "ix86_split_xorsign (operands); DONE;"
-  [(set_attr "isa" "*,avx,avx")])
+  [(set_attr "isa" "*,avx")])


 ;; One complement instructions

--- gcc/config/i386/i386-expand.c.jj    2021-09-08 09:55:40.000000000 +0200
+++ gcc/config/i386/i386-expand.c       2021-09-08 10:04:15.470268062 +0200
@@ -2296,23 +2296,25 @@ void
 ix86_split_xorsign (rtx operands[])
 {
   machine_mode mode, vmode;
-  rtx dest, op0, op1, mask, x;
+  rtx dest, op0, op1, mask, x, temp;

   dest = operands[0];
   op0 = operands[1];
   op1 = operands[2];
   mask = operands[3];
+  temp = operands[4];

   mode = GET_MODE (dest);
   vmode = GET_MODE (mask);

-  /* The constraints ensure that for non-AVX dest == op1 is
-     different from op0, and for AVX that at most two of
-     dest, op0 and op1 are the same register but the third one
-     is different.  */
+  if (!reg_overlap_mentioned_p (dest, op0))
+    temp = dest;
+  else
+    gcc_assert (TARGET_AVX);
+
   if (rtx_equal_p (op0, op1))
     {
-      gcc_assert (TARGET_AVX && !rtx_equal_p (op0, dest));
+      gcc_assert (TARGET_AVX);
       if (vmode == V4SFmode)
        vmode = V4SImode;
       else
@@ -2323,9 +2325,9 @@ ix86_split_xorsign (rtx operands[])
       mask = lowpart_subreg (vmode, mask, GET_MODE (mask));
       if (MEM_P (mask))
        {
-         rtx msk = lowpart_subreg (vmode, dest, mode);
-         emit_insn (gen_rtx_SET (msk, mask));
-         mask = msk;
+         temp = lowpart_subreg (vmode, temp, mode);
+         emit_insn (gen_rtx_SET (temp, mask));
+         mask = temp;
        }
       op0 = lowpart_subreg (vmode, op0, mode);
       x = gen_rtx_AND (vmode, gen_rtx_NOT (vmode, mask), op0);
@@ -2334,10 +2336,11 @@ ix86_split_xorsign (rtx operands[])
     {
       op1 = lowpart_subreg (vmode, op1, mode);
       x = gen_rtx_AND (vmode, op1, mask);
-      emit_insn (gen_rtx_SET (op1, x));
+      temp = lowpart_subreg (vmode, temp, mode);
+      emit_insn (gen_rtx_SET (temp, x));

       op0 = lowpart_subreg (vmode, op0, mode);
-      x = gen_rtx_XOR (vmode, op1, op0);
+      x = gen_rtx_XOR (vmode, temp, op0);
     }

   dest = lowpart_subreg (vmode, dest, mode);
and also with additional &Yv <- Yv, Yv, Yvm, X alternative with various ways to
disparage it slightly (^s or $s), but never convinced RA that it should use the
alternative with the scratch.
--- gcc/config/i386/i386.md.jj  2021-09-08 09:55:40.791551976 +0200
+++ gcc/config/i386/i386.md     2021-09-08 10:20:29.633498636 +0200
@@ -10919,18 +10919,19 @@
 })

 (define_insn_and_split "@xorsign<mode>3_1"
-  [(set (match_operand:MODEF 0 "register_operand" "=&Yv,&Yv,&Yv")
+  [(set (match_operand:MODEF 0 "register_operand" "=&Yv,Yv")
        (unspec:MODEF
-         [(match_operand:MODEF 1 "register_operand" "Yv,0,Yv")
-          (match_operand:MODEF 2 "register_operand" "0,Yv,Yv")
-          (match_operand:<ssevecmode> 3 "nonimmediate_operand" "Yvm,Yvm,Yvm")]
-         UNSPEC_XORSIGN))]
+         [(match_operand:MODEF 1 "register_operand" "Yv,Yv")
+          (match_operand:MODEF 2 "register_operand" "0,Yv")
+          (match_operand:<ssevecmode> 3 "nonimmediate_operand" "Yvm,Yvm")]
+         UNSPEC_XORSIGN))
+   (clobber (match_scratch:MODEF 4 "=X,&Yv"))]
   "SSE_FLOAT_MODE_P (<MODE>mode) && TARGET_SSE_MATH"
   "#"
   "&& reload_completed"
   [(const_int 0)]
   "ix86_split_xorsign (operands); DONE;"
-  [(set_attr "isa" "*,avx,avx")])
+  [(set_attr "isa" "noavx,avx")])


 ;; One complement instructions
with the same i386-expand.c changes does, but then it forces a scratch even for
the cases where it is not needed.