On Sun, Nov 12, 2023, 23:10 Tamar Christina <Tamar.Christina@arm.com> wrote:

> > -----Original Message-----
> > From: Richard Biener <richard.guenther@gmail.com>
> > Sent: Monday, November 13, 2023 6:55 AM
> > To: Xi Ruoyao <xry111@xry111.site>
> > Cc: gcc-patches@gcc.gnu.org; chenglulu <chenglulu@loongson.cn>;
> > i@xen0n.name; xuchenghua@loongson.cn; Tamar Christina
> > <Tamar.Christina@arm.com>; tschwinge@gcc.gnu.org; Roger Sayle
> > <roger@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 <xry111@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
> > >
>