Richard Biener <richard.guent...@gmail.com> writes: > On Thu, Aug 15, 2024 at 4:57 PM Jeff Law <jeffreya...@gmail.com> wrote: >> >> >> >> On 8/15/24 2:50 AM, Richard Sandiford wrote: >> > The PR points out that, for an address like: >> > >> > (plus (zero_extend X) Y) >> > >> > decompose_normal_address doesn't establish a strong preference >> > between treating X as the base or Y as the base. As the comment >> > in the patch says, zero_extend isn't enough on its own to assume >> > an index, at least not on POINTERS_EXTEND_UNSIGNED targets. >> > But in a construct like the one above, X and Y have different modes, >> > and it seems reasonable to assume that the one with the expected >> > address mode is the base. >> > >> > This matters on targets like m68k that support index extension >> > and that require different classes for bases and indices. >> > >> > Tested on aarch64-linux-gnu & x86_64-linux-gnu. Andreas also confirms >> > that it fixes the m68k LRA problem. OK to install? >> > >> > Richard >> > >> > >> > gcc/ >> > PR middle-end/116236 >> > * rtlanal.cc (decompose_normal_address): Try to distinguish >> > bases and indices based on mode, before resorting to "baseness". >> OK. Thanks to everyone for chasing this down. No idea where we sit >> with the conversion of m68k to LRA but this looks like it'd be helpful >> irrespective of that effort. > > I'll point out that this change merely adjusts heuristics and whether there's > an underlying issue in the target or LRA remains to be seen?
The PR has a lot more discussion around this. :) The historical interface is that the target can request different register classes for base registers and index registers, but the target doesn't get to choose what it considers to be a base and what it considers to be an index. This is instead determined by target-independent code (like it is for tree-ssa-address.cc, for example). decompose_normal_address wasn't making the same choice between base and index that reload made (or that tree-ssa-address.c would make). Some inconsistencies like that are ok, if both interpretations are valid. But IMO the old decompose_normal_address behaviour was clearly wrong for this case. So I think the patch is fixing a genuine bug, rather than papering over a bug elsewhere. I agree that in some ways it's not a very satisfactory situation, since there's a fair bit of guesswork and inference going on (and is in reload too). But I don't think we can avoid that without changing the interface. Changing the interface would be great, but it's a daunting amount of work, especially given that we have so many inactive ports in-tree that would each need to be updated individually. Thanks, Richard