On Sun, Nov 12, 2023, 23:10 Tamar Christina <tamar.christ...@arm.com> wrote:
> > -----Original Message----- > > From: Richard Biener <richard.guent...@gmail.com> > > Sent: Monday, November 13, 2023 6:55 AM > > To: Xi Ruoyao <xry...@xry111.site> > > Cc: gcc-patches@gcc.gnu.org; chenglulu <chengl...@loongson.cn>; > > i...@xen0n.name; xucheng...@loongson.cn; Tamar Christina > > <tamar.christ...@arm.com>; tschwi...@gcc.gnu.org; Roger Sayle > > <ro...@nextmovesoftware.com> > > Subject: Re: [PATCH] Fix (fcopysign x, NEGATIVE_CONST) -> (fneg (fabs x)) > > simplification [PR112483] > > > > On Sun, Nov 12, 2023 at 9:27 PM Xi Ruoyao <xry...@xry111.site> wrote: > > > > > > (fcopysign x, NEGATIVE_CONST) can be simplified to (fneg (fabs x)), > > > but a logic error in the code caused it mistakenly simplified to (fneg > > > x) instead. > > The fix aside, I actually wonder if simplify-rtx.cc should be doing this > at all. > The mid-end didn't do it because the target said it had an optab for the > copysign operation. Otherwise during expand_COPYSIGN it would have been > expanded as FNEG (FABS (..)) already. > > In the case of e.g. longaarch64 It looks like the target actually has an > fcopysign > Instruction, so wouldn't this rewriting by simplify-rtx be a > de-optimization? > Maybe the simplify_gen_unary under the if statement should really be simplify_unary_operation. This allows for constant folding but not generating a non canonical form here. Also note Canonical RTL forms have a section in the internals document too. The gimple level Canonical forms are not documented yet; I started writing some of it on the wiki though: https://gcc.gnu.org/wiki/GimpleCanonical . Thanks, Andrew Pinski > Thanks, > Tamar > > > > OK. > > > > > gcc/ChangeLog: > > > > > > PR rtl-optimization/112483 > > > * simplify-rtx.cc (simplify_binary_operation_1) <case > COPYSIGN>: > > > Fix the simplification of (fcopysign x, NEGATIVE_CONST). > > > --- > > > > > > Bootstrapped and regtested on loongarch64-linux-gnu and > > > x86_64-linux-gnu. Ok for trunk? > > > > > > gcc/simplify-rtx.cc | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/gcc/simplify-rtx.cc b/gcc/simplify-rtx.cc index > > > 69d87579d9c..2d2e5a3c1ca 100644 > > > --- a/gcc/simplify-rtx.cc > > > +++ b/gcc/simplify-rtx.cc > > > @@ -4392,7 +4392,7 @@ simplify_ashift: > > > real_convert (&f1, mode, CONST_DOUBLE_REAL_VALUE (trueop1)); > > > rtx tmp = simplify_gen_unary (ABS, mode, op0, mode); > > > if (REAL_VALUE_NEGATIVE (f1)) > > > - tmp = simplify_gen_unary (NEG, mode, op0, mode); > > > + tmp = simplify_gen_unary (NEG, mode, tmp, mode); > > > return tmp; > > > } > > > if (GET_CODE (op0) == NEG || GET_CODE (op0) == ABS) > > > -- > > > 2.42.1 > > > >
