diff options
author | Jeff Law <jlaw@ventanamicro.com> | 2024-08-04 13:09:02 -0600 |
---|---|---|
committer | Jeff Law <jlaw@ventanamicro.com> | 2024-08-04 13:11:59 -0600 |
commit | cfeb994d64743691d4a787f8eef7ce52d8711756 (patch) | |
tree | a9c425ea6ea43e30f9cc8b682a019a2ee14abac7 /gcc/ChangeLog | |
parent | 7cd71c88637e133cf983c235a29f65fdcc1e28ca (diff) | |
download | gcc-cfeb994d64743691d4a787f8eef7ce52d8711756.zip gcc-cfeb994d64743691d4a787f8eef7ce52d8711756.tar.gz gcc-cfeb994d64743691d4a787f8eef7ce52d8711756.tar.bz2 |
[committed][PR rtl-optimization/116199] Fix latent bug in reload's SUBREG handling
Building glibc on the m68k has exposed a long standing latent bug in reload.
Basically ext-dce replaced an extension with a subreg expression (good)
resulting in this pair of insns:
> (insn 7 4 8 2 (set (reg:DI 31 [ _1 ])
> (subreg:DI (reg/v:SI 37 [ __major ]) 0)) "j.c":7:32 75 {*m68k.md:1568}
> (nil))
> (insn 8 7 10 2 (set (reg:DI 39 [ _2 ])
> (ashift:DI (reg:DI 31 [ _1 ])
> (const_int 8 [0x8]))) "j.c":7:48 322 {*ashldi3}
> (expr_list:REG_DEAD (reg:DI 31 [ _1 ])
> (nil)))
insn 7 was optimized to the simple copy by ext-dce. That looks fine. Combine
comes along and squashes them together resulting in:
> (insn 8 7 10 2 (set (reg:DI 39 [ _2 ])
> (ashift:DI (subreg:DI (reg/v:SI 37 [ __major ]) 0)
> (const_int 8 [0x8]))) "j.c":7:48 322 {*ashldi3}
> (nil))
Which also looks good.
After IRA's allocation, in the middle of reload we have:
> (insn 8 7 10 2 (set (reg:DI 8 %a0 [orig:39 _2 ] [39])
> (ashift:DI (subreg:DI (reg/v:SI 0 %d0 [orig:37 __major ] [37]) 0)
> (const_int 8 [0x8]))) "j.c":7:48 322 {*ashldi3}
> (nil))
Again, sensible. The pattern requires op0 and op1 to match, so we try to
figure out if d0 & a0 are the same underlying register. So we get into this
code in operands_match_p:
> if (code == SUBREG)
> {
> i = REGNO (SUBREG_REG (x));
> if (i >= FIRST_PSEUDO_REGISTER)
> goto slow;
> i += subreg_regno_offset (REGNO (SUBREG_REG (x)),
> GET_MODE (SUBREG_REG (x)),
> SUBREG_BYTE (x),
> GET_MODE (x));
> }
> else
> i = REGNO (x);
There's a similar fragment for the other operand. The key is that
subreg_regno_offset call. That call assumes the subreg is representable. But
in the case of (subreg:DI (reg:SI d0)) we're going to get -1 (remember, m68k is
a big endian target). That -1 gets passed to hard_regno_regs via this code
(again, just showing one of the two copies of this fragment):
> if (REG_WORDS_BIG_ENDIAN
> && is_a <scalar_int_mode> (GET_MODE (x), &xmode)
> && GET_MODE_SIZE (xmode) > UNITS_PER_WORD
> && i < FIRST_PSEUDO_REGISTER)
> i += hard_regno_nregs (i, xmode) - 1;
That triggers the reported ICE. It appears this has been broken since the
conversion to SUBREG_BYTE way back in 2001, though possibly it could have been
some minor changes around this code circa 2005 as well, it didn't seem worth
putting under the debugger to be sure. Certainly the code from 2001 looks
suspicious to me.
Anyway, the fix here is pretty simple. The routine "simplify_subreg_regno" is
meant to be used to determine if we can simplify the subreg expression and will
explicitly return -1 if it can't be represented for one reason or another. It
checks a variety of conditions that aren't worth listing here.
Bootstrapped and regression tested on x86 (after reverting an unrelated patch
from Richard S that's causing multiple unrelated failures), which of course
doesn't really test the code as x86 is an LRA target. Also built & tested the
crosses, none of which show issues (and some of which are reload targets).
m68k will bootstrap & regression test tomorrow, but I don't think there's any
point in waiting for that.
Pushing to the trunk.
PR rtl-optimization/116199
gcc/
* reload.cc (operands_match_p): Verify subreg is expressable before
trying to simplify and match it to another operand.
gcc/testsuite/
* gcc.dg/torture/pr116199.c: New test.
Diffstat (limited to 'gcc/ChangeLog')
0 files changed, 0 insertions, 0 deletions