This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[4.1] Fix PR rtl-optimization/29329


This is yet another ICE related to REG_NOTES handling in the combiner, a 
regression present at -O2 on the 4.1 branch for the ARM architecture.

The combiner is passed

(insn 55 54 102 6 (set (reg/v:SI 107 [ j ])
        (plus:SI (reg/v:SI 107 [ j ])
            (const_int 1 [0x1]))) 4 {*arm_addsi3} (nil)
    (expr_list:REG_EQUAL (const_int 3 [0x3])
        (nil)))

(insn 102 55 115 6 (set (reg:SI 124)
        (ashift:SI (reg/v:SI 107 [ j ])
            (const_int 2 [0x2]))) 98 {*arm_shiftsi3} (insn_list:REG_DEP_TRUE 
55 (nil))
    (expr_list:REG_DEAD (reg/v:SI 107 [ j ])
        (expr_list:REG_EQUAL (const_int 12 [0xc])
            (nil))))

It will first try to directly replace the source of insn 102 by the 
destination of insn 55, but the result fails to validate.  Then it will
try again, after having replaced the source of insn 55 by the contents of the 
REG_EQUAL note, and the result this time validates.

Things go awry when distribute_notes is asked to find a new position for the 
REG_DEAD note because (reg/v:SI 107) is considered the eliminated register so 
the note is ditched without further ado.  Now (reg/v:SI 107) was formally 
live on entry of the basic block and life2 expects it to be still live after 
combine, so it stops the compiler.

This REG_EQUAL + REG_DEAD game has been already "studied" by Richard S. and me 
and we agreed that the best solution is implemented by

2007-01-06  Richard Sandiford  <richard@codesourcery.com>

	Backport from mainline:
	2006-05-23  Richard Sandiford  <richard@codesourcery.com>

	PR rtl-optimization/27736
	* combine.c (replaced_rhs_value): New variable.
	(combine_instructions): Set it.
	(distribute_notes): When distributing a note in replaced_rhs_insn,
	check whether the value was used in replaced_rhs_value.

	2006-05-22  Richard Sandiford  <richard@codesourcery.com>

	PR rtl-optimization/25514
	* combine.c (replaced_rhs_insn): New variable.
	(combine_instructions): Set replaced_rhs_insn when trying to replace
	a SET_SRC with a REG_EQUAL note.
	(distribute_notes): Use replaced_rhs_insn when determining the live
	range of a REG_DEAD register.

that Richard recently backported to the 4.1 branch.  Therefore the fix builds 
on Richard's work to avoid ditching REG_DEAD notes when they pertain to the
source of the insn to which the REG_EQUAL note is attached.  Since this stuff 
is strongly tied to the 2nd insn of the 3-insn window used by the combiner,
I took the liberty to rename the objects to emphasize it.

The patch also reverts

2006-09-12  Eric Botcazou  <ebotcazou@libertysurf.fr>

	PR rtl-optimization/28243
	* combine.c (distribute_notes) <REG_DEAD>: Do not consider SETs past
	the insn to which the note was originally attached.

which was a fix for another variant of the problem and has been superseded by 
Richard's backport (no compilers released by the FSF contain it).

Bootstrapped/regtested on x86_64-suse-linux, applied to mainline, 4.2 branch 
and 4.1 branch.


2007-01-21  Eric Botcazou  <ebotcazou@libertysurf.fr>

	PR rtl-optimization/29329
	* combine.c (replaced_rhs_insn): Rename to i2mod.
	(replaced_rhs_value): Rename to i2mod_new_rhs.
	(i2mod_old_rhs): New global variable.
	(combine_instructions): Adjust for above change.  Save a copy of
	the old RHS into i2mod_old_rhs when the contents of a REG_EQUAL
	note are substituted in the second instruction.
	(distribute_notes) <REG_DEAD>: Adjust for above change.  Do not
	ditch the note if it pertains to the second eliminated register
	and this register is mentioned in i2mod_old_rhs.

	Revert:
	2006-09-12  Eric Botcazou  <ebotcazou@libertysurf.fr>

	* combine.c (distribute_notes) <REG_DEAD>: Do not consider SETs past
	the insn to which the note was originally attached.


2007-01-21  Eric Botcazou  <ebotcazou@libertysurf.fr>

	* gcc.c-torture/compile/20070121.c: New test.


-- 
Eric Botcazou
Index: combine.c
===================================================================
--- combine.c	(revision 120999)
+++ combine.c	(working copy)
@@ -123,16 +123,22 @@ static int combine_successes;
 
 static int total_attempts, total_merges, total_extras, total_successes;
 
-/* Sometimes combine tries to replace the right hand side of an insn
-   with the value of a REG_EQUAL note.  This is the insn that has been
-   so modified, or null if none.  */
+/* combine_instructions may try to replace the right hand side of the
+   second instruction with the value of an associated REG_EQUAL note
+   before throwing it at try_combine.  That is problematic when there
+   is a REG_DEAD note for a register used in the old right hand side
+   and can cause distribute_notes to do wrong things.  This is the
+   second instruction if it has been so modified, null otherwise.  */
 
-static rtx replaced_rhs_insn;
+static rtx i2mod;
 
-/* When REPLACED_RHS_INSN is nonnull, this is a copy of the new right
-   hand side.  */
+/* When I2MOD is nonnull, this is a copy of the old right hand side.  */
 
-static rtx replaced_rhs_value;
+static rtx i2mod_old_rhs;
+
+/* When I2MOD is nonnull, this is a copy of the new right hand side.  */
+
+static rtx i2mod_new_rhs;
 
 /* Vector mapping INSN_UIDs to cuids.
    The cuids are like uids but increase monotonically always.
@@ -887,11 +893,12 @@ combine_instructions (rtx f, unsigned in
 			 be deleted or recognized by try_combine.  */
 		      rtx orig = SET_SRC (set);
 		      SET_SRC (set) = note;
-		      replaced_rhs_insn = temp;
-		      replaced_rhs_value = copy_rtx (note);
-		      next = try_combine (insn, temp, NULL_RTX,
+		      i2mod = temp;
+		      i2mod_old_rhs = copy_rtx (orig);
+		      i2mod_new_rhs = copy_rtx (note);
+		      next = try_combine (insn, i2mod, NULL_RTX,
 					  &new_direct_jump_p);
-		      replaced_rhs_insn = NULL;
+		      i2mod = NULL_RTX;
 		      if (next)
 			goto retry;
 		      SET_SRC (set) = orig;
@@ -12260,8 +12267,8 @@ distribute_notes (rtx notes, rtx from_in
 	     use of A and put the death note there.  */
 
 	  if (from_insn
-	      && from_insn == replaced_rhs_insn
-	      && !reg_overlap_mentioned_p (XEXP (note, 0), replaced_rhs_value))
+	      && from_insn == i2mod
+	      && !reg_overlap_mentioned_p (XEXP (note, 0), i2mod_new_rhs))
 	    tem = from_insn;
 	  else
 	    {
@@ -12274,7 +12281,10 @@ distribute_notes (rtx notes, rtx from_in
 	      else if (i2 != 0 && next_nonnote_insn (i2) == i3
 		       && reg_referenced_p (XEXP (note, 0), PATTERN (i2)))
 		place = i2;
-	      else if (rtx_equal_p (XEXP (note, 0), elim_i2)
+	      else if ((rtx_equal_p (XEXP (note, 0), elim_i2)
+			&& !(i2mod
+			     && reg_overlap_mentioned_p (XEXP (note, 0),
+							 i2mod_old_rhs)))
 		       || rtx_equal_p (XEXP (note, 0), elim_i1))
 		break;
 	      tem = i3;
@@ -12293,14 +12303,12 @@ distribute_notes (rtx notes, rtx from_in
 		      continue;
 		    }
 
-		  /* If TEM is a (reaching) definition of the use to which the
-		     note was attached, see if that is all TEM is doing.  If so,
-		     delete TEM.  Otherwise, make this into a REG_UNUSED note
-		     instead.  Don't delete sets to global register vars.  */
-		  if ((!from_insn
-		       || INSN_CUID (tem) < INSN_CUID (from_insn))
-		      && (REGNO (XEXP (note, 0)) >= FIRST_PSEUDO_REGISTER
-			  || !global_regs[REGNO (XEXP (note, 0))])
+		  /* If the register is being set at TEM, see if that is all
+		     TEM is doing.  If so, delete TEM.  Otherwise, make this
+		     into a REG_UNUSED note instead. Don't delete sets to
+		     global register vars.  */
+		  if ((REGNO (XEXP (note, 0)) >= FIRST_PSEUDO_REGISTER
+		       || !global_regs[REGNO (XEXP (note, 0))])
 		      && reg_set_p (XEXP (note, 0), PATTERN (tem)))
 		    {
 		      rtx set = single_set (tem);
/* PR rtl-optimization/29329 */
/* Origin: Debian GCC Maintainers <debian-gcc@lists.debian.org> */
/* Testcase by: Andrew Pinski <pinskia@gmail.com> */

struct node234_Tag
{
  int t1;
  int kids[4];
  void *elems[3];
};

void *add234_internal(struct node234_Tag *n, int ei)
{
  int j;
  for (j = ei; j < 2 && n->elems[j+1];)
    j++;
  n->kids[j+1] = 0;
}

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]