Re: [PATCH] aarch64: Do not alter force_reg returned rtx expanding pauth builtins

[Andrea Corallo <andrea.corallo@arm.com>]

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