Re: PATCH: PR 16590

[Jeffrey A Law <law at redhat dot com>]

On Tue, 2004-08-31 at 00:31, Mark Mitchell wrote:
> Jeffrey A Law wrote:
> 
> >On Sun, 2004-08-29 at 13:41, Mark Mitchell wrote:
> >  
> >
> >>This PR is a case where GCSE creates an invalid REG_EQUAL note.  (More
> >>details are in the audit trail.)  I asked Jeff Law for his thoughts on
> >>this patch about ten days ago, but haven't heard back, so I'm going
> >>ahead and checking this in.
> >>
> >>Tested on i686-pc-linux-gnu, applied on the 3.4 branch and on the
> >>mainline.
> >>    
> >>
> >Sorry, I didn't expect the cross-town move to leave me without an
> >internet connection for so long.
> >
> >I haven't followed all the changes to gcse which arrange for it
> >to create REG_EQUAL notes.  However, I do know most of those changes
> >were done in an attempt to improve later optimizations, so it's
> >possible your change is pessimizing code in some ways.
> >
> >But more importantly, I don't see how that REG_EQUAL note is
> >incorrect.
> >
> OK, I have reverted the patch on the 3.4 branch and on the mainline.
Thanks.  What I think needs to happen is that the store motion
code needs to invalidate REG_EQUAL notes referring to any memory
locations which were subject to load/store optimization.

ie, after GCSE/PRE, but before store motion the REG_EQUAL note is
correct as every path to the offending insn stores the correct value
back into (mem (plus (frame (const_int -24))

(insn 37 110 38 0 (set (mem/s:SI (plus:SI (reg/f:SI 20 frame)
                (const_int -24 [0xffffffe8])) [4 p+0 S4 A32])
        (reg/v/f:SI 71 [ start ])) 36 {*movsi_1} (nil)
    (nil))

[ ... ]

(jump_insn 45 44 98 0 (set (pc)
        (if_then_else (ne (reg:CCZ 17 flags)
                (const_int 0 [0x0]))
            (label_ref 52)
            (pc))) 342 {*jcc_1} (nil)
    (nil))
 
[ ... ]
 
(insn 51 108 52 1 (set (mem/s:SI (plus:SI (reg/f:SI 20 frame)
                (const_int -24 [0xffffffe8])) [4 p.p+0 S4 A32])
        (reg/f:SI 76)) 36 {*movsi_1} (nil)
    (nil))
 
(code_label 52 51 99 2 3 "" [1 uses])
 
(note 99 52 106 2 [bb 2] NOTE_INSN_BASIC_BLOCK)
 
(insn 106 99 57 2 (set (reg:SI 77 [ p.p ])
        (reg:SI 85)) -1 (nil)
    (expr_list:REG_EQUAL (mem/s:SI (plus:SI (reg/f:SI 20 frame)
                (const_int -24 [0xffffffe8])) [4 p.p+0 S4 A32])
        (nil)))

Store motion eliminates the store at 51 and thus makes the REG_EQUAL
note at insn 106 invalid.

So the question now is how ugly is it going to be to invalidate 
those notes in store motion...

Jeff