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

Reply via email to