Eric Botcazou <ebotca...@adacore.com> writes:
> > I'll run testing for at least x86_64, MIPS and another
> > WORD_REGISTER_OPERATIONS target and try to get this committed in the
> > next couple of days so it can get into everyone's testing well before
> release.
>
> No issues found on SPARC.
Thanks Eric.
I'm still bootstrap testing this on MIPS. It's taking longer as although
the fix allows the bootstrap to continue there are now stage2/stage3
differences. The bug appears to be essentially the same issue as I just
fixed but in reverse i.e. a paradoxical sub-reg loading too much data
from its stack slot. Extracts from dumps are below; first of all the
input rtl to the combine pass:
(insn 248 246 249 38 (set (reg:QI 282)
(subreg:QI (reg:SI 300) 0)) "/home/mfortune/gcc/gcc/predict.c":2904 362
{*movqi_internal}
(nil))
(insn 249 248 250 38 (set (reg:DI 284)
(zero_extend:DI (reg:QI 282))) "/home/mfortune/gcc/gcc/predict.c":2904
216 {*zero_extendqidi2}
(expr_list:REG_DEAD (reg:QI 282)
(nil)))
(insn 250 249 251 38 (set (reg:DI 6 $6)
(reg:DI 284)) "/home/mfortune/gcc/gcc/predict.c":2904 310 {*movdi_64bit}
(expr_list:REG_DEAD (reg:DI 284)
(nil)))
Trying 248 -> 249:
Successfully matched this instruction:
(set (reg:DI 284)
(subreg:DI (reg:SI 300) 0))
allowing combination of insns 248 and 249
original costs 4 + 4 = 8
replacement cost 4
deferring deletion of insn with uid = 248.
modifying insn i3 249: r284:DI=r300:SI#0
deferring rescan insn with uid = 249.
Trying 249 -> 250:
Successfully matched this instruction:
(set (reg:DI 6 $6)
(subreg:DI (reg:SI 300) 0))
allowing combination of insns 249 and 250
original costs 4 + 4 = 8
replacement cost 4
deferring deletion of insn with uid = 249.
modifying insn i3 250: $6:DI=r300:SI#0
deferring rescan insn with uid = 250.
Which results in the following coming out of combine:
(note 248 246 249 38 NOTE_INSN_DELETED)
(note 249 248 250 38 NOTE_INSN_DELETED)
(insn 250 249 251 38 (set (reg:DI 6 $6)
(subreg:DI (reg:SI 300) 0)) "/home/mfortune/gcc/gcc/predict.c":2904 310
{*movdi_64bit}
(nil))
Pseudo 300 is assigned to memory and then LRA produces a simple DImode load from
the assigned stack slot. The only instruction to set pseudo 300 is:
(insn 247 212 389 3 (set (reg:SI 300)
(ne:SI (subreg/s/u:SI (reg/v:DI 231 [ taken ]) 0)
(const_int 0 [0]))) "/home/mfortune/gcc/gcc/predict.c":2904 504
{*sne_zero_sisi}
(nil))
Which leads to an SImode store to the stack slot:
(insn 247 392 393 3 (set (reg:SI 4 $4 [300])
(ne:SI (reg:SI 20 $20 [orig:231 taken ] [231])
(const_int 0 [0]))) "/home/mfortune/gcc/gcc/predict.c":2904 504
{*sne_zero_sisi}
(nil))
(insn 393 247 389 3 (set (mem/c:SI (plus:DI (reg/f:DI 29 $sp)
(const_int 16 [0x10])) [403 %sfp+16 S4 A64])
(reg:SI 4 $4 [300])) "/home/mfortune/gcc/gcc/predict.c":2904 312
{*movsi_internal}
(nil))
...
(note 248 246 249 40 NOTE_INSN_DELETED)
(note 249 248 256 40 NOTE_INSN_DELETED)
(note 256 249 250 40 NOTE_INSN_DELETED)
(insn 250 256 251 40 (set (reg:DI 6 $6)
(mem/c:DI (plus:DI (reg/f:DI 29 $sp)
(const_int 16 [0x10])) [403 %sfp+16 S8 A64]))
"/home/mfortune/gcc/gcc/predict.c":2904 310 {*movdi_64bit}
(nil))
My assumption is that LRA is again expected to deal with this case and for insn
250 should be recognising that it must load 32-bits and rely on implicit
LOAD_EXTEND_OP behaviour producing an acceptable 64-bit value. In this case it
does not matter whether it is sign or zero extension and my assumption is that
this construct would never appear if a specific sign or zero extension was
required.
I haven't got to looking at where the issue is this time but it seems different
as
this is a subreg in a simple move instruction where we already support the load/
store directly so no new reload instruction is required. I don't know if this
implies that simple move patterns should reject subregs but that doesn't sound
right either.
Resolving this fixes at least one bug and potentially all bugs in the MIPS
bootstrap
as manually modified the generated assembly code to use LW instead of LD for
insn
250 and one of the buggy stage 3 objects is fixed.
I'll keep thinking, any advice in the meantime is appreciated.
Thanks,
Matthew
