Andrea Corallo <andrea.cora...@arm.com> writes: > Hi Richard, > > thanks for reviewing > > Richard Sandiford <richard.sandif...@arm.com> writes: > >> Andrea Corallo <andrea.cora...@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.cora...@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.cora...@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
