On 2/26/19 4:19 PM, Segher Boessenkool wrote:
> On Tue, Feb 26, 2019 at 10:07:54AM -0700, Jeff Law wrote:
>> As we enter regcprop we have the following horrific RTL:
>>>> (insn 35 7 36 2 (set (reg:DI 2 $2 [orig:200 x ] [200])
>>>> (const_int 0 [0])) "j.c":8:49 313 {*movdi_64bit}
>>>> (nil))
>>>> (insn 36 35 10 2 (set (reg:DI 3 $3 [ x+8 ])
>>>> (const_int 0 [0])) "j.c":8:49 313 {*movdi_64bit}
>>>> (nil))
>>>> (insn 10 36 11 2 (set (reg:DI 2 $2 [orig:200 x ] [200])
>>>> (reg:DI 4 $4 [206])) "j.c":8:49 313 {*movdi_64bit}
>>>> (nil))
>>>> (insn 11 10 37 2 (set (reg:DI 3 $3 [ x+8 ])
>>>> (const_int 0 [0])) "j.c":8:49 313 {*movdi_64bit}
>>>> (nil))
>
> where 35+36 used to be a single set in TImode:
> insn_cost 4 for 29: r200:TI=0
> insn_cost 4 for 10: r200:TI#0=r197:DI
> REG_DEAD r197:DI
> insn_cost 4 for 11: r200:TI#8=0
>
> I would hope combine could clean this up, and it tried, but
That was one of my early thoughts... But....
>
> Trying 29, 10 -> 11:
> 29: r200:TI=0
> 10: r200:TI#0=r206:DI
> REG_DEAD r206:DI
> 11: r200:TI#8=0
> Can't combine i2 into i3
>
> This is because i2 (that's insn 10) has a subreg on the LHS.
Right. Already went down that rathole and several others :-)
>
>> You'll note there's a fair amount of obviously dead code. The dead code
>> really hampers regcprop's ability to propagate values.
>
> And prevents all of the RTL optimisers before it from doing any useful
> work. Ideally this should be fixed much earlier. Not that your patch
> isn't pretty obviously correct :-)
For gcc-10 we should:
1. Avoid really stupid stuff in init-regs.
2. Avoid really stupid stuff in the splitters.
3. Make REE SUBREG-aware
4. Seriously consider a RTL DCE pass after post-reload splitting
But none of those would actually fix this BZ :-)
>
> [ I don't see any problems with -O2 btw, only with -O1, so should we care
> at all anyway? ]
You can get this stuff at -O2 on other platforms. It's far more common
than I ever imagined.
Jeff
ps. THe patch isn't actually correct. I committed the wrong version.
We should be looking at notrap_p on SRC, not INSN. WHen you call it on
the insn, the EXPR_LIST for the notes is considered trapping. Ugh. I
didn't notice it until I started trying to clean up the 2nd patch and
was seeing dead code much later than I expected :( I'll fix that goof
tomorrow.
