diff mbox

Fix PR rtl-optimization/58662

Message ID 8480829.mQp5mcqABq@polaris
State New
Headers show

Commit Message

Eric Botcazou Oct. 13, 2013, 1:29 p.m. UTC 
This is a problem with note distribution in the combiner present on the 
mainline.  From:

(insn 19 18 21 2 (parallel [
            (set (reg:SI 83 [ D.1762 ])
                (neg:SI (reg:SI 90 [ D.1762 ])))
            (clobber (reg:CC 17 flags))
        ]) ../pr58662.c:17 464 {*negsi2_1}
     (expr_list:REG_DEAD (reg:SI 90 [ D.1762 ])
        (expr_list:REG_UNUSED (reg:CC 17 flags)
            (nil))))

(insn 21 19 23 2 (parallel [
            (set (reg:SI 104)
                (ashift:SI (reg:SI 83 [ D.1762 ])
                    (const_int 1 [0x1])))
            (clobber (reg:CC 17 flags))
        ]) ../pr58662.c:17 516 {*ashlsi3_1}
     (expr_list:REG_UNUSED (reg:CC 17 flags)
        (nil)))

(insn 23 21 24 2 (parallel [
            (set (reg:SI 105)
                (ashift:SI (reg:SI 83 [ D.1762 ])
                    (const_int 4 [0x4])))
            (clobber (reg:CC 17 flags))
        ]) ../pr58662.c:17 516 {*ashlsi3_1}
     (expr_list:REG_DEAD (reg:SI 83 [ D.1762 ])
        (expr_list:REG_UNUSED (reg:CC 17 flags)
            (nil))))

(insn 26 25 27 2 (parallel [
            (set (reg:CCZ 17 flags)
                (compare:CCZ (minus:SI (reg:SI 104)
                        (reg:SI 105))
                    (const_int 0 [0])))
            (set (reg:SI 83 [ D.1762 ])
                (minus:SI (reg:SI 104)
                    (reg:SI 105)))
        ]) ../pr58662.c:18 302 {*subsi_2}
     (expr_list:REG_DEAD (reg:SI 105)
        (expr_list:REG_DEAD (reg:SI 104)
            (nil))))

we go through the intermediate (and correct) stage:

(parallel [
        (set (pc)
            (pc))
        (set (reg:SI 83 [ D.1762 ])
            (const_int 0 [0]))
    ])

which is split into:

(gdb) p debug_rtx(newpat)
(set (pc)
    (pc))

(gdb) p debug_rtx(newi2pat)
(set (reg:SI 83 [ D.1762 ])
    (const_int 0 [0]))

The problem is that the REG_DEAD note for (reg:SI 83) on insn 23 is put back
onto the new i2 insn by distribute_notes, thus making it wrongly dead code.

Fixed by explicitly checking for this case in the code that decides how to 
split the PARALLEL.  I didn't find anything less ad-hoc and there is a similar 
check for REG_UNUSED notes in the first part of the code.

Tested on x86_64-suse-linux, applied on the mainline.


2013-10-13  Eric Botcazou  <ebotcazou@adacore.com>

	PR rtl-optimization/58662
	* combine.c (try_combine): Take into account death nodes on I2 when
	splitting a PARALLEL of two independent SETs.  Fix dump message.
	

2013-10-13  Richard Biener  <rguenther@suse.de>

	* gcc.c-torture/execute/pr58662.c: New test.
diff mbox

Patch

Index: combine.c
===================================================================
--- combine.c	(revision 203486)
+++ combine.c	(working copy)
@@ -3693,29 +3693,42 @@  try_combine (rtx i3, rtx i2, rtx i1, rtx
 	   && ! (contains_muldiv (SET_SRC (XVECEXP (newpat, 0, 0)))
 		 && contains_muldiv (SET_SRC (XVECEXP (newpat, 0, 1)))))
     {
+      rtx set0 = XVECEXP (newpat, 0, 0);
+      rtx set1 = XVECEXP (newpat, 0, 1);
+
       /* Normally, it doesn't matter which of the two is done first,
 	 but the one that references cc0 can't be the second, and
 	 one which uses any regs/memory set in between i2 and i3 can't
-	 be first.  */
-      if (!use_crosses_set_p (SET_SRC (XVECEXP (newpat, 0, 1)),
-			      DF_INSN_LUID (i2))
+	 be first.  The PARALLEL might also have been pre-existing in i3,
+	 so we need to make sure that we won't wrongly hoist a SET to i2
+	 that would conflict with a death note present in there.  */
+      if (!use_crosses_set_p (SET_SRC (set1), DF_INSN_LUID (i2))
+	  && !(REG_P (SET_DEST (set1))
+	       && find_reg_note (i2, REG_DEAD, SET_DEST (set1)))
+	  && !(GET_CODE (SET_DEST (set1)) == SUBREG
+	       && find_reg_note (i2, REG_DEAD,
+				 SUBREG_REG (SET_DEST (set1))))
 #ifdef HAVE_cc0
-	  && !reg_referenced_p (cc0_rtx, XVECEXP (newpat, 0, 0))
+	  && !reg_referenced_p (cc0_rtx, set0)
 #endif
 	 )
 	{
-	  newi2pat = XVECEXP (newpat, 0, 1);
-	  newpat = XVECEXP (newpat, 0, 0);
+	  newi2pat = set1;
+	  newpat = set0;
 	}
-      else if (!use_crosses_set_p (SET_SRC (XVECEXP (newpat, 0, 0)),
-				   DF_INSN_LUID (i2))
+      else if (!use_crosses_set_p (SET_SRC (set0), DF_INSN_LUID (i2))
+	       && !(REG_P (SET_DEST (set0))
+		    && find_reg_note (i2, REG_DEAD, SET_DEST (set0)))
+	       && !(GET_CODE (SET_DEST (set0)) == SUBREG
+		    && find_reg_note (i2, REG_DEAD,
+				      SUBREG_REG (SET_DEST (set0))))
 #ifdef HAVE_cc0
-	       && !reg_referenced_p (cc0_rtx, XVECEXP (newpat, 0, 1))
+	       && !reg_referenced_p (cc0_rtx, set1)
 #endif
 	      )
 	{
-	  newi2pat = XVECEXP (newpat, 0, 0);
-	  newpat = XVECEXP (newpat, 0, 1);
+	  newi2pat = set0;
+	  newpat = set1;
 	}
       else
 	{
@@ -4261,9 +4274,8 @@  try_combine (rtx i3, rtx i2, rtx i1, rtx
       }
 
     /* Update reg_stat[].nonzero_bits et al for any changes that may have
-       been made to this insn.  The order of
-       set_nonzero_bits_and_sign_copies() is important.  Because newi2pat
-       can affect nonzero_bits of newpat */
+       been made to this insn.  The order is important, because newi2pat
+       can affect nonzero_bits of newpat.  */
     if (newi2pat)
       note_stores (newi2pat, set_nonzero_bits_and_sign_copies, NULL);
     note_stores (newpat, set_nonzero_bits_and_sign_copies, NULL);
@@ -4283,7 +4295,7 @@  try_combine (rtx i3, rtx i2, rtx i1, rtx
     {
       if (dump_file)
 	{
-	  fprintf (dump_file, "modifying insn i1 ");
+	  fprintf (dump_file, "modifying insn i0 ");
 	  dump_insn_slim (dump_file, i0);
 	}
       df_insn_rescan (i0);
@@ -4321,7 +4333,6 @@  try_combine (rtx i3, rtx i2, rtx i1, rtx
 
   /* Set new_direct_jump_p if a new return or simple jump instruction
      has been created.  Adjust the CFG accordingly.  */
-
   if (returnjump_p (i3) || any_uncondjump_p (i3))
     {
       *new_direct_jump_p = 1;