[PATCH] aarch64: Do not alter force_reg returned rtx expanding pauth builtins
Richard Sandiford
richard.sandiford@arm.com
Fri Sep 25 12:27:51 GMT 2020
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
More information about the Gcc-patches
mailing list