* [PATCH] aarch64: Do not alter force_reg returned rtx expanding pauth builtins
@ 2020-09-22 10:02 Andrea Corallo
2020-09-23 10:33 ` Richard Sandiford
0 siblings, 1 reply; 7+ messages in thread
From: Andrea Corallo @ 2020-09-22 10:02 UTC (permalink / raw)
To: gcc-patches; +Cc: kyrylo.tkachov, richard.earnshaw, nd
[-- Attachment #1: Type: text/plain, Size: 257 bytes --]
Hi all,
having a look for force_reg returned rtx later on modified I've found
this other case in `aarch64_general_expand_builtin` while expanding
pointer authentication builtins.
Regtested and bootsraped on aarch64-linux-gnu.
Okay for trunk?
Andrea
[-- Attachment #2: 0001-aarch64-Do-not-alter-force_reg-returned-rtx-expandin.patch --]
[-- Type: text/plain, Size: 1169 bytes --]
From 8869ee04e3788fdec86aa7e5a13e2eb477091d0e Mon Sep 17 00:00:00 2001
From: Andrea Corallo <andrea.corallo@arm.com>
Date: Mon, 21 Sep 2020 13:52:45 +0100
Subject: [PATCH] aarch64: Do not alter force_reg returned rtx expanding pauth
builtins
2020-09-21 Andrea Corallo <andrea.corallo@arm.com>
* config/aarch64/aarch64-builtins.c
(aarch64_general_expand_builtin): Do not alter value on a
force_reg returned rtx.
---
gcc/config/aarch64/aarch64-builtins.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/gcc/config/aarch64/aarch64-builtins.c b/gcc/config/aarch64/aarch64-builtins.c
index b787719cf5e..a77718ccfac 100644
--- a/gcc/config/aarch64/aarch64-builtins.c
+++ b/gcc/config/aarch64/aarch64-builtins.c
@@ -2079,10 +2079,10 @@ aarch64_general_expand_builtin (unsigned int fcode, tree exp, rtx target,
arg0 = CALL_EXPR_ARG (exp, 0);
op0 = force_reg (Pmode, expand_normal (arg0));
- if (!target)
+ if (!(target
+ && REG_P (target)
+ && GET_MODE (target) == Pmode))
target = gen_reg_rtx (Pmode);
- else
- target = force_reg (Pmode, target);
emit_move_insn (target, op0);
--
2.17.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] aarch64: Do not alter force_reg returned rtx expanding pauth builtins
2020-09-22 10:02 [PATCH] aarch64: Do not alter force_reg returned rtx expanding pauth builtins Andrea Corallo
@ 2020-09-23 10:33 ` Richard Sandiford
2020-09-24 15:20 ` Andrea Corallo
0 siblings, 1 reply; 7+ messages in thread
From: Richard Sandiford @ 2020-09-23 10:33 UTC (permalink / raw)
To: Andrea Corallo; +Cc: gcc-patches, nd, richard.earnshaw
Andrea Corallo <andrea.corallo@arm.com> writes:
> Hi all,
>
> having a look for force_reg returned rtx later on modified I've found
> this other case in `aarch64_general_expand_builtin` while expanding
> pointer authentication builtins.
>
> Regtested and bootsraped on aarch64-linux-gnu.
>
> Okay for trunk?
>
> Andrea
>
> From 8869ee04e3788fdec86aa7e5a13e2eb477091d0e Mon Sep 17 00:00:00 2001
> From: Andrea Corallo <andrea.corallo@arm.com>
> Date: Mon, 21 Sep 2020 13:52:45 +0100
> Subject: [PATCH] aarch64: Do not alter force_reg returned rtx expanding pauth
> builtins
>
> 2020-09-21 Andrea Corallo <andrea.corallo@arm.com>
>
> * config/aarch64/aarch64-builtins.c
> (aarch64_general_expand_builtin): Do not alter value on a
> force_reg returned rtx.
> ---
> gcc/config/aarch64/aarch64-builtins.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/gcc/config/aarch64/aarch64-builtins.c b/gcc/config/aarch64/aarch64-builtins.c
> index b787719cf5e..a77718ccfac 100644
> --- a/gcc/config/aarch64/aarch64-builtins.c
> +++ b/gcc/config/aarch64/aarch64-builtins.c
> @@ -2079,10 +2079,10 @@ aarch64_general_expand_builtin (unsigned int fcode, tree exp, rtx target,
> arg0 = CALL_EXPR_ARG (exp, 0);
> op0 = force_reg (Pmode, expand_normal (arg0));
>
> - if (!target)
> + if (!(target
> + && REG_P (target)
> + && GET_MODE (target) == Pmode))
> target = gen_reg_rtx (Pmode);
> - else
> - target = force_reg (Pmode, target);
>
> emit_move_insn (target, op0);
Do we actually use the result of this move? It looked like we always
use op0 rather than target (good) and overwrite target with a later move.
If so, I think we should delete the move and convert the later code
to use expand_insn.
Thanks,
Richard
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] aarch64: Do not alter force_reg returned rtx expanding pauth builtins
2020-09-23 10:33 ` Richard Sandiford
@ 2020-09-24 15:20 ` Andrea Corallo
2020-09-25 12:27 ` Richard Sandiford
0 siblings, 1 reply; 7+ messages in thread
From: Andrea Corallo @ 2020-09-24 15:20 UTC (permalink / raw)
To: gcc-patches; +Cc: nd, richard.earnshaw, richard.sandiford
Hi Richard,
thanks for reviewing
Richard Sandiford <richard.sandiford@arm.com> writes:
> Andrea Corallo <andrea.corallo@arm.com> writes:
>> Hi all,
>>
>> having a look for force_reg returned rtx later on modified I've found
>> this other case in `aarch64_general_expand_builtin` while expanding
>> pointer authentication builtins.
>>
>> Regtested and bootsraped on aarch64-linux-gnu.
>>
>> Okay for trunk?
>>
>> Andrea
>>
>> From 8869ee04e3788fdec86aa7e5a13e2eb477091d0e Mon Sep 17 00:00:00 2001
>> From: Andrea Corallo <andrea.corallo@arm.com>
>> Date: Mon, 21 Sep 2020 13:52:45 +0100
>> Subject: [PATCH] aarch64: Do not alter force_reg returned rtx expanding pauth
>> builtins
>>
>> 2020-09-21 Andrea Corallo <andrea.corallo@arm.com>
>>
>> * config/aarch64/aarch64-builtins.c
>> (aarch64_general_expand_builtin): Do not alter value on a
>> force_reg returned rtx.
>> ---
>> gcc/config/aarch64/aarch64-builtins.c | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/gcc/config/aarch64/aarch64-builtins.c b/gcc/config/aarch64/aarch64-builtins.c
>> index b787719cf5e..a77718ccfac 100644
>> --- a/gcc/config/aarch64/aarch64-builtins.c
>> +++ b/gcc/config/aarch64/aarch64-builtins.c
>> @@ -2079,10 +2079,10 @@ aarch64_general_expand_builtin (unsigned int fcode, tree exp, rtx target,
>> arg0 = CALL_EXPR_ARG (exp, 0);
>> op0 = force_reg (Pmode, expand_normal (arg0));
>>
>> - if (!target)
>> + if (!(target
>> + && REG_P (target)
>> + && GET_MODE (target) == Pmode))
>> target = gen_reg_rtx (Pmode);
>> - else
>> - target = force_reg (Pmode, target);
>>
>> emit_move_insn (target, op0);
>
> Do we actually use the result of this move? It looked like we always
> use op0 rather than target (good) and overwrite target with a later move.
>
> If so, I think we should delete the move
Good point agree.
> and convert the later code to use expand_insn.
I'm not sure I understand the suggestion right, xpaclri&friends patterns
are written with hardcoded in/out regs, is the suggestion to just use like
'expand_insn (CODE_FOR_xpaclri, 0, NULL)' in place of GEN_FCN+emit_insn?
Thanks!
Andrea
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] aarch64: Do not alter force_reg returned rtx expanding pauth builtins
2020-09-24 15:20 ` Andrea Corallo
@ 2020-09-25 12:27 ` Richard Sandiford
2020-09-28 8:59 ` [PATCH V2] " Andrea Corallo
0 siblings, 1 reply; 7+ messages in thread
From: Richard Sandiford @ 2020-09-25 12:27 UTC (permalink / raw)
To: Andrea Corallo; +Cc: gcc-patches, nd, richard.earnshaw
Andrea Corallo <andrea.corallo@arm.com> writes:
> Hi Richard,
>
> thanks for reviewing
>
> Richard Sandiford <richard.sandiford@arm.com> writes:
>
>> Andrea Corallo <andrea.corallo@arm.com> writes:
>>> Hi all,
>>>
>>> having a look for force_reg returned rtx later on modified I've found
>>> this other case in `aarch64_general_expand_builtin` while expanding
>>> pointer authentication builtins.
>>>
>>> Regtested and bootsraped on aarch64-linux-gnu.
>>>
>>> Okay for trunk?
>>>
>>> Andrea
>>>
>>> From 8869ee04e3788fdec86aa7e5a13e2eb477091d0e Mon Sep 17 00:00:00 2001
>>> From: Andrea Corallo <andrea.corallo@arm.com>
>>> Date: Mon, 21 Sep 2020 13:52:45 +0100
>>> Subject: [PATCH] aarch64: Do not alter force_reg returned rtx expanding pauth
>>> builtins
>>>
>>> 2020-09-21 Andrea Corallo <andrea.corallo@arm.com>
>>>
>>> * config/aarch64/aarch64-builtins.c
>>> (aarch64_general_expand_builtin): Do not alter value on a
>>> force_reg returned rtx.
>>> ---
>>> gcc/config/aarch64/aarch64-builtins.c | 6 +++---
>>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/gcc/config/aarch64/aarch64-builtins.c b/gcc/config/aarch64/aarch64-builtins.c
>>> index b787719cf5e..a77718ccfac 100644
>>> --- a/gcc/config/aarch64/aarch64-builtins.c
>>> +++ b/gcc/config/aarch64/aarch64-builtins.c
>>> @@ -2079,10 +2079,10 @@ aarch64_general_expand_builtin (unsigned int fcode, tree exp, rtx target,
>>> arg0 = CALL_EXPR_ARG (exp, 0);
>>> op0 = force_reg (Pmode, expand_normal (arg0));
>>>
>>> - if (!target)
>>> + if (!(target
>>> + && REG_P (target)
>>> + && GET_MODE (target) == Pmode))
>>> target = gen_reg_rtx (Pmode);
>>> - else
>>> - target = force_reg (Pmode, target);
>>>
>>> emit_move_insn (target, op0);
>>
>> Do we actually use the result of this move? It looked like we always
>> use op0 rather than target (good) and overwrite target with a later move.
>>
>> If so, I think we should delete the move
>
> Good point agree.
>
>> and convert the later code to use expand_insn.
>
> I'm not sure I understand the suggestion right, xpaclri&friends patterns
> are written with hardcoded in/out regs, is the suggestion to just use like
> 'expand_insn (CODE_FOR_xpaclri, 0, NULL)' in place of GEN_FCN+emit_insn?
Oops, sorry for the bogus comment, didn't look closely enough.
So yeah, no need to use expand_insn. Rather than generate a new target,
it should be OK to return lr and x17_reg directly. (Hope I'm right
this time. ;-))
Thanks,
Richard
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH V2] aarch64: Do not alter force_reg returned rtx expanding pauth builtins
2020-09-25 12:27 ` Richard Sandiford
@ 2020-09-28 8:59 ` Andrea Corallo
2020-09-28 9:48 ` Richard Sandiford
0 siblings, 1 reply; 7+ messages in thread
From: Andrea Corallo @ 2020-09-28 8:59 UTC (permalink / raw)
To: gcc-patches; +Cc: nd, richard.earnshaw, richard.sandiford, Kyrylo Tkachov
[-- Attachment #1: Type: text/plain, Size: 152 bytes --]
Hi all,
here the reworked patch addressing Richard's suggestions.
Regtested and bootsraped on aarch64-linux-gnu.
Okay for trunk?
Thanks!
Andrea
[-- Attachment #2: 0001-aarch64-Do-not-alter-force_reg-returned-rtx-expandin.patch --]
[-- Type: text/plain, Size: 1711 bytes --]
From 946d22aa247f2d1bb0c6b10a6e6db415b34feff2 Mon Sep 17 00:00:00 2001
From: Andrea Corallo <andrea.corallo@arm.com>
Date: Mon, 21 Sep 2020 13:52:45 +0100
Subject: [PATCH] aarch64: Do not alter force_reg returned rtx expanding pauth
builtins
2020-09-21 Andrea Corallo <andrea.corallo@arm.com>
* config/aarch64/aarch64-builtins.c
(aarch64_general_expand_builtin): Do not alter value on a
force_reg returned rtx.
---
gcc/config/aarch64/aarch64-builtins.c | 13 ++-----------
1 file changed, 2 insertions(+), 11 deletions(-)
diff --git a/gcc/config/aarch64/aarch64-builtins.c b/gcc/config/aarch64/aarch64-builtins.c
index b787719cf5e..d7eb8772b14 100644
--- a/gcc/config/aarch64/aarch64-builtins.c
+++ b/gcc/config/aarch64/aarch64-builtins.c
@@ -2079,20 +2079,13 @@ aarch64_general_expand_builtin (unsigned int fcode, tree exp, rtx target,
arg0 = CALL_EXPR_ARG (exp, 0);
op0 = force_reg (Pmode, expand_normal (arg0));
- if (!target)
- target = gen_reg_rtx (Pmode);
- else
- target = force_reg (Pmode, target);
-
- emit_move_insn (target, op0);
-
if (fcode == AARCH64_PAUTH_BUILTIN_XPACLRI)
{
rtx lr = gen_rtx_REG (Pmode, R30_REGNUM);
icode = CODE_FOR_xpaclri;
emit_move_insn (lr, op0);
emit_insn (GEN_FCN (icode) ());
- emit_move_insn (target, lr);
+ return lr;
}
else
{
@@ -2122,11 +2115,9 @@ aarch64_general_expand_builtin (unsigned int fcode, tree exp, rtx target,
emit_move_insn (x17_reg, op0);
emit_move_insn (x16_reg, op1);
emit_insn (GEN_FCN (icode) ());
- emit_move_insn (target, x17_reg);
+ return x17_reg;
}
- return target;
-
case AARCH64_JSCVT:
{
expand_operand ops[2];
--
2.17.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH V2] aarch64: Do not alter force_reg returned rtx expanding pauth builtins
2020-09-28 8:59 ` [PATCH V2] " Andrea Corallo
@ 2020-09-28 9:48 ` Richard Sandiford
2020-09-28 10:33 ` Andrea Corallo
0 siblings, 1 reply; 7+ messages in thread
From: Richard Sandiford @ 2020-09-28 9:48 UTC (permalink / raw)
To: Andrea Corallo; +Cc: gcc-patches, nd, richard.earnshaw, Kyrylo Tkachov
Andrea Corallo <andrea.corallo@arm.com> writes:
> Hi all,
>
> here the reworked patch addressing Richard's suggestions.
>
> Regtested and bootsraped on aarch64-linux-gnu.
>
> Okay for trunk?
OK, thanks.
Richard
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH V2] aarch64: Do not alter force_reg returned rtx expanding pauth builtins
2020-09-28 9:48 ` Richard Sandiford
@ 2020-09-28 10:33 ` Andrea Corallo
0 siblings, 0 replies; 7+ messages in thread
From: Andrea Corallo @ 2020-09-28 10:33 UTC (permalink / raw)
To: gcc-patches; +Cc: nd, richard.earnshaw, Kyrylo Tkachov, richard.sandiford
Richard Sandiford <richard.sandiford@arm.com> writes:
> Andrea Corallo <andrea.corallo@arm.com> writes:
>> Hi all,
>>
>> here the reworked patch addressing Richard's suggestions.
>>
>> Regtested and bootsraped on aarch64-linux-gnu.
>>
>> Okay for trunk?
>
> OK, thanks.
>
> Richard
Into trunk as 92f0d3d03a7.
Thanks!
Andrea
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-09-28 10:34 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-22 10:02 [PATCH] aarch64: Do not alter force_reg returned rtx expanding pauth builtins Andrea Corallo
2020-09-23 10:33 ` Richard Sandiford
2020-09-24 15:20 ` Andrea Corallo
2020-09-25 12:27 ` Richard Sandiford
2020-09-28 8:59 ` [PATCH V2] " Andrea Corallo
2020-09-28 9:48 ` Richard Sandiford
2020-09-28 10:33 ` 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).