Hi!
I'd like to ping this patch.
Thanks
On Sat, Nov 25, 2023 at 11:17:48AM +0100, Jakub Jelinek wrote:
> The middle-end has been changed quite recently to canonicalize
> -abs (x) to copysign (x, -1) rather than the other way around.
> While I agree with that at GIMPLE level, since it matches the GIMPLE
> goal of as few operations as possible for a canonical form (-abs (x)
> is 2 GIMPLE statements, copysign (x, -1) is just one), I must say
> I don't really like that being done on RTL as well (or at least
> not canonicalizing (COPYSIGN x, negative) back to (NEG (ABS x))),
> because on most targets most of floating point constants need to be loaded
> from memory, there are a few exceptions but -1 is often not one of them.
>
> Anyway, the following patch fixes the rs6000 regression caused by the
> change in GIMPLE canonicalization (i.e. the desirable one). As rs6000
> clearly prefers -abs (x) form because it has a single instruction to do
> that while it also has copysign instruction, but that requires loading the
> -1 from memory, the following patch just ensures the copysign expander
> can actually see the floating point constant and in that case emits the
> -abs (x) code (or in the hypothetical case of copysign with non-negative
> constant abs (x) - but there copysign (x, 1) in GIMPLE is canonicalized
> to abs (x)), otherwise forces the operand to be the expected gpc_reg_operand
> and does what it did before.
>
> Bootstrapped/regtested on powerpc64le-linux, ok for trunk?
>
> 2023-11-25 Jakub Jelinek <ja...@redhat.com>
>
> PR target/112606
> * config/rs6000/rs6000.md (copysign<mode>3): Change predicate
> of the last argument from gpc_reg_operand to any_operand. If
> operands[2] is CONST_DOUBLE, emit abs or neg abs depending on
> its sign, otherwise if it doesn't satisfy gpc_reg_operand,
> force it to REG using copy_to_mode_reg.
>
> --- gcc/config/rs6000/rs6000.md.jj 2023-10-13 19:34:43.927834877 +0200
> +++ gcc/config/rs6000/rs6000.md 2023-11-24 18:54:13.587876170 +0100
> @@ -5358,7 +5358,7 @@ (define_expand "copysign<mode>3"
> (set (match_dup 4)
> (neg:SFDF (abs:SFDF (match_dup 1))))
> (set (match_operand:SFDF 0 "gpc_reg_operand")
> - (if_then_else:SFDF (ge (match_operand:SFDF 2 "gpc_reg_operand")
> + (if_then_else:SFDF (ge (match_operand:SFDF 2 "any_operand")
> (match_dup 5))
> (match_dup 3)
> (match_dup 4)))]
> @@ -5369,6 +5369,24 @@ (define_expand "copysign<mode>3"
> || TARGET_CMPB
> || VECTOR_UNIT_VSX_P (<MODE>mode))"
> {
> + /* Middle-end canonicalizes -fabs (x) to copysign (x, -1),
> + but PowerPC prefers -fabs (x). */
> + if (CONST_DOUBLE_AS_FLOAT_P (operands[2]))
> + {
> + if (real_isneg (CONST_DOUBLE_REAL_VALUE (operands[2])))
> + {
> + operands[3] = gen_reg_rtx (<MODE>mode);
> + emit_insn (gen_abs<mode>2 (operands[3], operands[1]));
> + emit_insn (gen_neg<mode>2 (operands[0], operands[3]));
> + }
> + else
> + emit_insn (gen_abs<mode>2 (operands[0], operands[1]));
> + DONE;
> + }
> +
> + if (!gpc_reg_operand (operands[2], <MODE>mode))
> + operands[2] = copy_to_mode_reg (<MODE>mode, operands[2]);
> +
> if (TARGET_CMPB || VECTOR_UNIT_VSX_P (<MODE>mode))
> {
> emit_insn (gen_copysign<mode>3_fcpsgn (operands[0], operands[1],
Jakub
