On 12/10/2026 17:46, Richard Sandiford wrote:
Andrew Stubbs <[email protected]> writes:
On 12/10/2026 13:37, Richard Sandiford wrote:
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?
I've attached my WIP patchset. I think your first comments were folded
into the first 3, and the "and another thing" comments are in the
"fixup" patches.
To reproduce the issue don't apply the subreg fixup in the final patch.
You should see that lib2-divmod-di.c in libgcc fails to build.
Thanks. I've not got to the stage of being able to build libgcc yet,
but looking at the series: I forgot that change_address is inherently
a validating function, which means that it can emit fix-up instructions
(or ICE if called after RA). Here we want to suppress that and use a
non-validating form.
Does the attached patch help? I still think it makes sense to use
change_address* directly in this case, since the more specific adjustment
functions kind-of assume a contiguous access.
The patch fixes the problems. I've cleaned everything up and rebased. If
it tests cleanly I'll push to git and post the final versions here.
Thanks for the help
Andrew
