Re: [PATCH] Always enable LRA

[Segher Boessenkool <segher@kernel.crashing.org>]

Hi!

On Thu, Oct 13, 2022 at 10:47:20PM -0600, Jeff Law wrote:
> On 10/13/22 17:56, Segher Boessenkool wrote:
> >h8300 fails during GCC build:
> >/home/segher/src/gcc/libgcc/unwind.inc: In function 
> >'_Unwind_SjLj_RaiseException':
> >/home/segher/src/gcc/libgcc/unwind.inc:141:1: error: could not split insn
> >   141 | }
> >       | ^
> >(insn 69 256 327 (set (mem/f:SI (pre_dec:SI (reg/f:SI 7 sp)) [12  S4 A32])
> >         (reg/f:SI 7 sp)) "/home/segher/src/gcc/libgcc/unwind.inc":118:12 
> >         19 {*movsi}
> >      (expr_list:REG_ARGS_SIZE (const_int 4 [0x4])
> >         (nil)))
> >during RTL pass: final
> >which looks like a backend bug, I don't see a pattern that could split
> >this (without needing an extra clobber)?
> 
> Really smells like an LRA bug to me.
> 
> 
> We have this as we leave IRA:
> 
> 
> (insn 10 9 11 2 (set (reg/f:SI 30)
>        (plus:SI (reg/f:SI 11 fp)
>            (const_int -4 [0xfffffffffffffffc]))) "j.c":31:7 264 
> {*addsi}
>     (expr_list:REG_EQUIV (plus:SI (reg/f:SI 11 fp)
>            (const_int -4 [0xfffffffffffffffc]))
> (nil)))
> (insn 11 10 12 2 (set (mem/f:SI (pre_dec:SI (reg/f:SI 7 sp)) [3  S4 A32])
>        (reg/f:SI 30)) "j.c":31:7 19 {*movsi}
>     (expr_list:REG_DEAD (reg/f:SI 30)
>        (expr_list:REG_ARGS_SIZE (const_int 4 [0x4])
>            (nil))))
> 
> 
> And as we leave LRA:
> 
> (note 10 9 11 2 NOTE_INSN_DELETED)
> (insn 11 10 13 2 (set (mem/f:SI (pre_dec:SI (reg/f:SI 7 sp)) [3 S4 A32])
>         (reg/f:SI 7 sp)) "j.c":31:7 19 {*movsi}
>      (expr_list:REG_ARGS_SIZE (const_int 4 [0x4])
>         (nil)))

LRA only ever generates insns that pass recog.  The backend allows this
define_insn, requiring it to be split (it returns template "#"), but
then somehow it doesn't match in any split pass?

> Register elimination ultimately discovered that (reg 30) was the same as 
> the stack pointer and did the natural substitution.    The natural 
> substitution results in invalid RTL and there's really no way to back 
> out and do something different AFAICT in lra-eliminations.cc.
> 
> The only reason we fault is because the H8 backend knows this is invalid 
> RTL and refuses to split it.  If we were to try and re-recognize the 
> insn in question it would fail to recognize after the substitution 
> because the auto-inc'd operand overlaps with the other operand.

But it *did* recog?  Or does LRA somehow not always recog() everything?
I always thought that was one of the huge improvements over old reload
(it does everything very locally instead of very globally)!


Segher