RE: [PATCH] aarch64: Do not alter value on a force_reg returned rtx expanding __jcvt

[Kyrylo Tkachov <Kyrylo.Tkachov@arm.com>]

Hi Andrea,

> -----Original Message-----
> From: Gcc-patches <gcc-patches-bounces@gcc.gnu.org> On Behalf Of
> Richard Sandiford
> Sent: 21 September 2020 11:58
> To: Andrea Corallo <Andrea.Corallo@arm.com>
> Cc: Richard Earnshaw <Richard.Earnshaw@arm.com>; nd <nd@arm.com>;
> gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH] aarch64: Do not alter value on a force_reg returned rtx
> expanding __jcvt
> 
> Andrea Corallo <andrea.corallo@arm.com> writes:
> > Hi all,
> >
> > From the `force_reg` description comment I see the returned register
> > should not be modified, thus IIUC should not be used as a GEN_FCN
> > target.
> >
> > Assuming my interpretation is correct this fix this case inside
> > `aarch64_general_expand_builtin` while expanding expanding the
> > `__jcvt` intrinsic.  If is not the case please discard.
> 
> Good catch.

Can you please also backport it to the appropriate branches as well after some time on trunk.
Thanks,
Kyrill

> 
> > Regtested and bootsraped on aarch64-linux-gnu.
> >
> >   Andrea
> >
> > From 403ad66b8f9c108d7f38b406ed1afcb603b7e25f Mon Sep 17 00:00:00
> 2001
> > From: Andrea Corallo <andrea.corallo@arm.com>
> > Date: Thu, 17 Sep 2020 17:17:52 +0100
> > Subject: [PATCH] aarch64: Do not alter value on a force_reg returned rtx
> >  expanding __jcvt
> >
> > 2020-09-17  Andrea Corallo  <andrea.corallo@arm.com>
> >
> > 	* config/aarch64/aarch64-builtins.c
> > 	(aarch64_general_expand_builtin): Use expand machinery not to
> > 	alter the value of an rtx returned by force_reg.
> 
> OK, thanks.
> 
> Richard
> 
> > ---
> >  gcc/config/aarch64/aarch64-builtins.c | 16 ++++++++--------
> >  1 file changed, 8 insertions(+), 8 deletions(-)
> >
> > diff --git a/gcc/config/aarch64/aarch64-builtins.c
> b/gcc/config/aarch64/aarch64-builtins.c
> > index 4f33dd936c7..b787719cf5e 100644
> > --- a/gcc/config/aarch64/aarch64-builtins.c
> > +++ b/gcc/config/aarch64/aarch64-builtins.c
> > @@ -2128,14 +2128,14 @@ aarch64_general_expand_builtin (unsigned int
> fcode, tree exp, rtx target,
> >        return target;
> >
> >      case AARCH64_JSCVT:
> > -      arg0 = CALL_EXPR_ARG (exp, 0);
> > -      op0 = force_reg (DFmode, expand_normal (arg0));
> > -      if (!target)
> > -	target = gen_reg_rtx (SImode);
> > -      else
> > -	target = force_reg (SImode, target);
> > -      emit_insn (GEN_FCN (CODE_FOR_aarch64_fjcvtzs) (target, op0));
> > -      return target;
> > +      {
> > +	expand_operand ops[2];
> > +	create_output_operand (&ops[0], target, SImode);
> > +	op0 = expand_normal (CALL_EXPR_ARG (exp, 0));
> > +	create_input_operand (&ops[1], op0, DFmode);
> > +	expand_insn (CODE_FOR_aarch64_fjcvtzs, 2, ops);
> > +	return ops[0].value;
> > +      }
> >
> >      case AARCH64_SIMD_BUILTIN_FCMLA_LANEQ0_V2SF:
> >      case AARCH64_SIMD_BUILTIN_FCMLA_LANEQ90_V2SF: