Jeff Law <j...@ventanamicro.com> writes: > On 8/12/24 4:02 PM, Richard Sandiford wrote: >> 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. > I've gone back and forth over this before I sent that patch. I can > certainly see your logic. I'd convinced myself that the subreg should > have simplified to (reg:DI 1). > > And the inconsistency was driving me bananas as my mental model is that > (reg:DI N) covers N and N+1 and all that changes in the order based on > endianness. ie, if we have (set (reg:DI 0) (...)) that changes d0/d1. > But maybe that's just 20 years of little endian thinking creeping in. > > In which case (subreg:DI (reg:SI d0) 0) is actually meaningless.
Yeah. The same problem can happen for little-endian too, with e.g. a doubleword paradoxical subreg of the final hard register leaking into the virtual registers. But the failure mode tends to be less brutal then. That case is probably more like big-endian (subreg:DI (reg:SI a0) 0), where the combination d7+a0 would be rejected for register class reasons. Richard
