* [PATCH] aarch64: Do not alter value on a force_reg returned rtx expanding __jcvt
@ 2020-09-21 10:31 Andrea Corallo
2020-09-21 10:58 ` Richard Sandiford
0 siblings, 1 reply; 5+ messages in thread
From: Andrea Corallo @ 2020-09-21 10:31 UTC (permalink / raw)
To: gcc-patches; +Cc: kyrylo.tkachov, richard.earnshaw, nd
[-- Attachment #1: Type: text/plain, Size: 395 bytes --]
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.
Regtested and bootsraped on aarch64-linux-gnu.
Andrea
[-- Attachment #2: 0001-aarch64-Do-not-alter-value-on-a-force_reg-returned-r.patch --]
[-- Type: text/plain, Size: 1595 bytes --]
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.
---
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:
--
2.17.1
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] aarch64: Do not alter value on a force_reg returned rtx expanding __jcvt
2020-09-21 10:31 [PATCH] aarch64: Do not alter value on a force_reg returned rtx expanding __jcvt Andrea Corallo
@ 2020-09-21 10:58 ` Richard Sandiford
2020-09-21 11:00 ` Kyrylo Tkachov
0 siblings, 1 reply; 5+ messages in thread
From: Richard Sandiford @ 2020-09-21 10:58 UTC (permalink / raw)
To: Andrea Corallo; +Cc: gcc-patches, nd, richard.earnshaw
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.
> 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:
^ permalink raw reply [flat|nested] 5+ messages in thread* RE: [PATCH] aarch64: Do not alter value on a force_reg returned rtx expanding __jcvt
2020-09-21 10:58 ` Richard Sandiford
@ 2020-09-21 11:00 ` Kyrylo Tkachov
2020-09-21 12:13 ` Andrea Corallo
0 siblings, 1 reply; 5+ messages in thread
From: Kyrylo Tkachov @ 2020-09-21 11:00 UTC (permalink / raw)
To: Richard Sandiford, Andrea Corallo; +Cc: Richard Earnshaw, nd, gcc-patches
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:
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] aarch64: Do not alter value on a force_reg returned rtx expanding __jcvt
2020-09-21 11:00 ` Kyrylo Tkachov
@ 2020-09-21 12:13 ` Andrea Corallo
2020-09-24 9:50 ` Andrea Corallo
0 siblings, 1 reply; 5+ messages in thread
From: Andrea Corallo @ 2020-09-21 12:13 UTC (permalink / raw)
To: Kyrylo Tkachov; +Cc: Richard Sandiford, Richard Earnshaw, nd, gcc-patches
Kyrylo Tkachov <Kyrylo.Tkachov@arm.com> writes:
> 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
Ciao Kyrill,
Sure happy to do that. For now into trunk as 2c62952f816.
Thanks
Andrea
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] aarch64: Do not alter value on a force_reg returned rtx expanding __jcvt
2020-09-21 12:13 ` Andrea Corallo
@ 2020-09-24 9:50 ` Andrea Corallo
0 siblings, 0 replies; 5+ messages in thread
From: Andrea Corallo @ 2020-09-24 9:50 UTC (permalink / raw)
To: Kyrylo Tkachov; +Cc: nd, gcc-patches, Richard Earnshaw
Andrea Corallo <andrea.corallo@arm.com> writes:
> Kyrylo Tkachov <Kyrylo.Tkachov@arm.com> writes:
[...]
>>
>> Can you please also backport it to the appropriate branches as well after some time on trunk.
>> Thanks,
>> Kyrill
>
> Ciao Kyrill,
>
> Sure happy to do that. For now into trunk as 2c62952f816.
Backported into gcc-10 as aa47c987340.
Thanks
Andrea
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-09-24 9:51 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-21 10:31 [PATCH] aarch64: Do not alter value on a force_reg returned rtx expanding __jcvt Andrea Corallo
2020-09-21 10:58 ` Richard Sandiford
2020-09-21 11:00 ` Kyrylo Tkachov
2020-09-21 12:13 ` Andrea Corallo
2020-09-24 9:50 ` Andrea Corallo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).