Hi Jeff, > On 1 Oct 2026, at 11:54, Jeff Law <[email protected]> wrote: > > > > On 9/28/26 8:15 AM, [email protected] wrote: >> From: Kyrylo Tkachov <[email protected]> >> >> A spilled vector can be stored in one mode and immediately reloaded in >> another. CSELIB's memory lookup requires an exact mode match, so postreload >> misses an available register value. For example: >> >> mem:VNx4SI (addr) = v7:VNx4SI; >> v23:VNx2DI = mem:VNx2DI (addr); >> >> can use a register copy for the second set. The store must remain when >> other paths still load its value. >> >> Search the existing address-value list for an equal-sized vector value in >> a register. Require the target's permission for a bit-preserving mode >> change and a >> valid subreg, then use the existing cost and instruction validation rules. >> Retain volatile loads and honor float-store. Expose elt_list alongside >> the other CSELIB list structures. >> >> Bootstrapped and tested on aarch64-none-linux-gnu. >> Ok for trunk? >> Thanks, >> Kyrill >> >> gcc/ChangeLog: >> >> * cselib.cc (elt_list): Move to cselib.h. >> * cselib.h (elt_list): Move from cselib.cc. >> * postreload.cc (reload_cse_simplify_mem): New. >> (reload_cse_simplify_set): Try reload_cse_simplify_mem. >> >> gcc/testsuite/ChangeLog: >> >> * gcc.dg/rtl/aarch64/postreload-vector-forward.c: New test. >> * gcc.dg/rtl/aarch64/postreload-vector-forward-be.c: Likewise. >> * gcc.target/aarch64/sve/spill-mode-change.c: New test. >> >> Signed-off-by: Kyrylo Tkachov <[email protected]> >> --- >> >> +/* Try to replace the memory source of SET in INSN with a register that >> + holds the same bytes in another mode. Return nonzero if a replacement >> + is added to the current change group. */ >> + >> +static int >> +reload_cse_simplify_mem (rtx set, rtx_insn *insn) >> +{ >> + rtx mem = SET_SRC (set); >> + if (!MEM_P (mem) || MEM_VOLATILE_P (mem)) >> + return 0; >> + >> + machine_mode mode = GET_MODE (mem); >> + if (!VECTOR_MODE_P (mode) >> + || (FLOAT_MODE_P (mode) && flag_float_store)) >> + return 0; > Unclear why you're rejecting integer modes here. In many ways those are the > easiest cases to handle. So if they're already being handled somewhere else, > then perhaps we should be looking at generalizing that code rather than > adding special case code here? > >> + >> + rtx address = XEXP (mem, 0); >> + machine_mode address_mode = GET_MODE (address); >> + if (address_mode == VOIDmode) >> + address_mode = Pmode; > Does VOIDmode ever actually happen here? That would seem to indicate a > problem elsewhere if it does. Guessing this is just being overly safe? > > >> + >> + reg_class_t dclass = REGNO_REG_CLASS (true_regnum (SET_DEST (set))); >> + int best_cost = memory_move_cost (mode, dclass, true); >> + rtx best = NULL_RTX; >> + for (elt_list *l = addr->addr_list; l; l = l->next) >> + { >> + machine_mode from = GET_MODE (l->elt->val_rtx); >> + if (from == mode || !VECTOR_MODE_P (from) >> + || !known_eq (GET_MODE_SIZE (from), GET_MODE_SIZE (mode))) >> + continue; > Again, not sure why this is restricted to vector modes. > >
The patch is just conservative as I was tackling this particular vector intrinsics test case that was reported to me and I’m trying to fix. But it can generalize to the other modes without too much trouble. I’ll do a respin. Thanks, Kyrill > So not accepting or rejecting at this time. Really want to understand why > we're restricting to vectors. If we're going to reject floats, then > shouldn't we reject vectors of floats? And finally a bit surprised this > isn't already handled somewhere else that we might be better off generalizing. > > Jeff
