Thanks for the review. Jeff Law <jeffreya...@gmail.com> writes: > On 7/10/24 9:44 AM, Richard Sandiford wrote: >> expand_fn_using_insn has code to handle SUBREG_PROMOTED_VAR_P >> destinations. Specifically, for: >> >> (subreg/v:M1 (reg:M2 R) ...) >> >> it creates a new temporary register T, uses it for the output >> operand, then sign- or zero-extends the M1 lowpart of T to M2, >> storing the result in R. >> >> This patch splits this handling out into helper routines and >> uses them for other instances of: >> >> if (!rtx_equal_p (target, ops[0].value)) >> emit_move_insn (target, ops[0].value); >> >> It's quite probable that this doesn't help any of the other cases; >> in particular, it shouldn't affect vectors. But I think it could >> be useful for the CRC work. >> >> Bootstrapped & regression-tested on aarch64-linux-gnu. OK to install? >> >> Richard >> >> >> gcc/ >> * internal-fn.cc (create_call_lhs_operand, assign_call_lhs): New >> functions, split out from... >> (expand_fn_using_insn): ...here. >> (expand_load_lanes_optab_fn): Use them. >> (expand_GOMP_SIMT_ENTER_ALLOC): Likewise. >> (expand_GOMP_SIMT_LAST_LANE): Likewise. >> (expand_GOMP_SIMT_ORDERED_PRED): Likewise. >> (expand_GOMP_SIMT_VOTE_ANY): Likewise. >> (expand_GOMP_SIMT_XCHG_BFLY): Likewise. >> (expand_GOMP_SIMT_XCHG_IDX): Likewise. >> (expand_partial_load_optab_fn): Likewise. >> (expand_vec_cond_optab_fn): Likewise. >> (expand_vec_cond_mask_optab_fn): Likewise. >> (expand_RAWMEMCHR): Likewise. >> (expand_gather_load_optab_fn): Likewise. >> (expand_while_optab_fn): Likewise. >> (expand_SPACESHIP): Likewise. > OK. > > FWIW, we did some testing for cases where we didn't utilize the > SUBREG_PROMOTED* bits to eliminate extensions coming out of the > expansion phase. For the most part we're doing a good job. IIRC our > instrumentation showed they tended to sneak in mostly through expansion > of builtins (particularly the arithemtic overflow builtins).
That sounds great! I can see why SUBREG_PROMOTED* was a nice hack, but it's also a source of subtle bugs, and can sometimes mean that we generate extensions that aren't really needed. It would be good if the optimisers are getting to the state where we could remove it and express things in "natural rtl". (Admittedly I'm using "natural rtl" to mean "rtl that seems sensible to me". It's not very objective.) On a similar theme, have you ever tried getting rid of WORD_REGISTER_OPERATIONS for riscv? Kyrill did that in 2016 for aarch64 (56c9ef5f2fa5787ddd7b2c83804a46554fa1ffc9) and I've never seen it cause a missed optimisation. There too, we seem to get good results using natural rtl. Richard
