Andrew Stubbs <[email protected]> writes: > On 12/10/2026 13:15, Richard Sandiford wrote: >> Andrew Stubbs <[email protected]> writes: >>> On 09/10/2026 16:22, Richard Sandiford wrote: >>>> Andrew Stubbs <[email protected]> writes: >>>>> On 09/10/2026 12:15, Richard Sandiford wrote: >>>>>> Sorry, a couple of other things: >>>>>> >>>>>>>> + >>>>>>>> + rtx new_addr >>>>>>>> + = (REG_P (addr) >>>>>>>> + ? simplify_gen_subreg (addr_elt_mode, addr, addrmode, >>>>>>>> + addr_byte) >>>>>>>> + : simplify_subreg (addr_elt_mode, addr, addrmode, addr_byte)); >>>>>> >>>>>> I think we should use simplify_gen_subreg unconditionally unless there's >>>>>> a specific reason not to. This is a simplification regardless of what >>>>>> the address is. >>>>> >>>>> I tried this and got an ICE. AFAICT, simplify_gen_subreg does the wrong >>>>> thing (in this situation) when the address has an offset: >>>>> >>>>> (mem/i:V8QI (plus:V8DI (reg:V8DI 720) >>>>> (vec_duplicate:V8DI (const_int 0)))) >>>>> >>>>> It tries to wrap the plus in a subreg, which is not valid. So, I'm going >>>>> to leave this bit as-is, unless you have a better suggestion? >>>> >>>> Why's it not valid though? >>> gcn_addr_space_legitimate_address_p rejects it because there's no >>> instruction that can do that in a load/store instruction. >> >> Yeah, I agree that it should do that. >> >>> Is that not how that's supposed to work? >> >> Yeah, gcn_addr_space_legitimate_address_p should describe the >> target restrictions. >> >> But simplification routines aren't required or expected to restrict >> themselves to things that the target supports directly. It's up to the >> caller of the simplification routines to test whether the target supports >> a particular operation. E.g. elsewhere we have: >> >> /* If we have a SUBREG of a register that we are replacing and we are >> replacing it with a MEM, make a new MEM and try replacing the >> SUBREG with it. Don't do this if the MEM has a mode-dependent address >> or if we would be widening it. */ >> >> if (MEM_P (op) >> && ! mode_dependent_address_p (XEXP (op, 0), MEM_ADDR_SPACE (op)) >> /* Allow splitting of volatile memory references in case we don't >> have instruction to move the whole thing. */ >> && (! MEM_VOLATILE_P (op) >> || ! have_insn_for (SET, innermode)) >> && !(STRICT_ALIGNMENT && MEM_ALIGN (op) < GET_MODE_ALIGNMENT >> (outermode)) >> && known_le (outersize, innersize)) >> return adjust_address_nv (op, outermode, byte); >> >> The mode_dependent_address_p check admittedly muddies the waters, but the >> use of adjust_address_nv means that the new MEM isn't necessarily valid. >> >>> There's also a chance to fix it in gcn_addr_space_legitimize_address, so >>> I'm trying to figure out if that's something it should be doing. >>> >>> The testcase is trying to convert >>> >>> (mem/i:V8QI (plus:V8DI (reg:V8DI 720) >>> (vec_duplicate:V8DI (const_int 0)))) >>> >>> into >>> >>> (mem:QI (subreg:DI (plus:V8DI (reg:V8DI 720) >>> (vec_duplicate:V8DI (const_int 0))) >>> 0)) >>> >>> Which is because CSE is trying to split this >>> >>> (set (mem/i:V8QI (plus:V8DI (reg:V8DI 720) >>> (vec_duplicate:V8DI (const_int 0)))) >>> (const_vector:V8QI [(const_int 0 [0]) repeated x8])) >>> >>> into 8 separate operations. The comment says "These are templates and >>> don't actually get emitted but are used to tell CSE how to get to a >>> particular constant." >> >> That too sounds ok. But CSE should only use the registered MEMs in an >> insn if the insn passes a recog test. >> >> Do you have a backtrace for the ICE? > Here's what I have, slightly cleaned up for email: > > internal_error(char const*, ...) > .../gcc/diagnostic-global-context.cc:787 > fancy_abort(char const*, int, char const*) > .../gcc/diagnostics/context.cc:1813 > df_insn_delete(rtx_insn*) > .../gcc/df-scan.cc:948 > delete_insn(rtx_insn*) > .../gcc/cfgrtl.cc:179 > delete_insn_and_edges(rtx_insn*) > .../gcc/cfgrtl.cc:251 > delete_trivially_dead_insns(rtx_insn*, int) > .../gcc/cse.cc:7450 > rest_of_handle_cse_after_global_opts > .../gcc/cse.cc:8036 > execute > ...gcc/cse.cc:8085 > > However, I think the delete functions are fallout from change_address et > al not managing to legitimize the subreg address.
Hmm, yeah, that's not the kind of backtrace I expected. Is there a testcase I can use that would show the problem, after applying the patch series? Thanks, Richard
