Fun bug in dead store elimination

Jeffrey A Law law@upchuck.cygnus.com
Tue May 25 13:13:00 GMT 1999


Whee.

In .lreg we had something like this:

(insn 34 28 44 (parallel[
            (set (mem/s:BLK (reg/v:SI 84) 0)
                (mem/s:BLK (reg:SI 1 at) 0))
            (clobber (scratch:SI))
            (clobber (scratch:SI))
            (clobber (scratch:SI))
            (clobber (scratch:SI))
            (use (const_int 16))
            (use (const_int 4))
            (use (const_int 0))
        ] ) 290 {movstrsi_internal} (insn_list 13 (insn_list 18 (insn_list 23 (insn_list 28 (insn_list 4 (nil))))))
    (expr_list:REG_DEAD (reg/v:SI 84)
        (expr_list:REG_UNUSED (scratch:SI)
            (expr_list:REG_UNUSED (scratch:SI)
                (expr_list:REG_UNUSED (scratch:SI)
                    (expr_list:REG_UNUSED (scratch:SI)
                        (nil)))))))


Which is a normal block move for a mips processor.  After reload it looked
like:

(insn 49 28 50 (parallel[
            (set (mem:BLK (reg/v:SI 4 a0) 0)
                (mem:BLK (reg:SI 29 sp) 0))
            (clobber (reg:SI 3 v1))
            (clobber (reg:SI 5 a1))
            (clobber (reg:SI 6 a2))
            (clobber (reg:SI 7 a3))
            (use (const_int 16))
            (use (const_int 4))
            (use (const_int 1))
        ] ) -1 (nil)
    (nil))

(insn 50 49 44 (parallel[
            (set (mem:BLK (reg/v:SI 4 a0) 0)
                (mem:BLK (reg:SI 29 sp) 0))
            (clobber (reg:SI 3 v1))
            (clobber (reg:SI 5 a1))
            (clobber (reg:SI 6 a2))
            (clobber (reg:SI 7 a3))
            (use (const_int 16))
            (use (const_int 4))
            (use (const_int 2))
        ] ) -1 (nil)
    (nil))

Which is the same thing, but split into two insns.  The first insn handles all
of the block copy except the final store.  The second insn handles the final
store.

Can you guess what happened?  dse found two identical looking stores and
eliminated the first one.  Opps.

In general it is not safe to delete a BLKmode store since we do not know its
size.  This patch fixes the problem.  I'll also submit a testcase shortly for
this bug.


        * flow.c (mark_set_1): Do not record BLKmode stores as dead
        store elimination candidates.

Index: flow.c
===================================================================
RCS file: /cvs/cvsfiles/devo/gcc/flow.c,v
retrieving revision 1.168
diff -c -3 -p -r1.168 flow.c
*** flow.c	1999/03/09 23:50:50	1.168
--- flow.c	1999/05/25 20:09:45
*************** mark_set_1 (needed, dead, x, insn, signi
*** 3225,3230 ****
--- 3225,3233 ----
      invalidate_mems_from_autoinc (insn);
  
    if (GET_CODE (reg) == MEM && ! side_effects_p (reg)
+       /* We do not know the size of a BLKmode store, so we do not track
+ 	 them for redundant store elimination.  */
+       && GET_MODE (reg) != BLKmode
        /* There are no REG_INC notes for SP, so we can't assume we'll see 
  	 everything that invalidates it.  To be safe, don't eliminate any
  	 stores though SP; none of them should be redundant anyway.  */












More information about the Gcc-patches mailing list