Jeff Law <jeffreya...@gmail.com> writes:
> On 8/12/24 1:49 PM, Richard Sandiford wrote:
>
>>>
>>> - regno = subreg_regno (x);
>>> + /* A paradoxical should always be REGNO (y) + 0. Using subreg_regno
>>> + for something like (subreg:DI (reg:SI N) 0) on a WORDS_BIG_ENDIAN
>>> + target will return N-1 which is catastrophic for N == 0 and just
>>> + wrong for other cases.
>>> +
>>> + Fixing subreg_regno would be a better option, except that reload
>>> + depends on its current behavior. */
>>> + if (paradoxical_subreg_p (x))
>>> + regno = REGNO (y);
>>> + else
>>> + regno = subreg_regno (x);
>>
>> Are you sure that's right? For a 32-bit big-endian target,
>> (subreg:DI (reg:SI 1) 0) really should simplify to (reg:DI 0) rather
>> than (reg:DI 1).
> Correct, we want to get (reg:DI 0). We get "0" back from REGNO (y).
> And we get 0 back from byte_lowpart_offset (remember, it's paradoxical).
> The sum is 0 resulting in (reg:DI 0).
But in my example, REGNO (y) is 1 (it's a different example from the
one that prompted the patch). The simplified subreg should be the
(reg:DI 0) given by subreg_regno, rather than the (reg:DI 1) given
by using REGNO (y).
E.g.:
(set (reg:SI 1) (mem:SI ADDR))
(set (reg:DI 2) (and:DI (subreg:DI (reg:SI 1) 0)
(const_int 127)))
should (on big-endian targets) be equivalent to:
(set (reg:SI 1) (mem:SI ADDR))
(set (reg:DI 2) (and:DI (reg:DI 0)
(const_int 127)))
so that r2 is set to 0 and r3 is set to (mem:SI ADDR) & 127.
If instead we simplified it to:
(set (reg:SI 1) (mem:SI ADDR))
(set (reg:DI 2) (and:DI (reg:DI 1)
(const_int 127)))
then r2 would be set to 0 and r3 would be set to an indeterminate
value & 127.
Thanks,
Richard
>
>
> > Like you say, this leads to an invalid result for (reg:SI 0) in place
>> of (reg:SI 1). But AIUI, it's reload's/LRA's job to ensure that that
>> never happens. This is part of the reason why LRA/IRA need to track
>> the widest referenced mode.
>>
>> So it sounds like something might have gone wrong earlier/elsewhere.
> Not that I could find, but maybe I missed something. It's been a long
> time since I wandered through reload.
>
> jeff
