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? Thanks! Andrea
