[PATCH] Fix combiner ICEs after my recent patch (PR rtl-optimization/48549)

[PATCH] Fix combiner ICEs after my recent patch (PR rtl-optimization/48549)

[Jakub Jelinek]

[-- Attachment #1: Type: text/plain, Size: 5189 bytes --]

Hi!

On the testcase below we ICE since my PR48343 patch.
The problem is that update_cfg_for_uncondjump delete_insns the noop move,
if the insn to be deleted is last_combined_insn (which can be both
if it is i3 or when retrying and undobuf.other_insn happens to be
last_combined_insn), next propagate_for_debug won't find the deleted
insn in the stream and thus will search through the whole rest of the
function and crash on dereferencing NULL NEXT_INSN.

The following patch fixes it, bootstrapped/regtested on x86_64-linux
and i686-linux (also with the patch attached after it), but it is
mainly for discussion what to do.

The propagate_for_debug change alone could fix it, we should never
fall through into next basic block.  We are unforuntately not deleting
just jumps (which ought to appear at the end of bbs), but also
any other noop moves, which I think is unintentional, we have
delete_noop_moves that should clean that up instead (see the second patch).
With the second patch, bootstrap succeeded even with gcc_assert (at_end);
in update_cfg_for_uncondjump, so either it is always at the end, or at
least very often, thus perhaps with the second patch being applied
in combine_instructions we could just for INSN_DELETED_P clear
last_combined_insn right away, meaning we want to propagate till end of
current bb.

2011-04-11  Jakub Jelinek  <jakub@redhat.com>

	PR rtl-optimization/48549
	* combine.c (propagate_for_debug): Also stop after BB_END of
	this_basic_block.
	(combine_instructions): If last_combined_insn has been deleted,
	set last_combined_insn to the next nondebug insn, or NULL
	if after the end of BB.
	(try_combine): Handle last_combined_insn being NULL.

	* g++.dg/opt/pr48549.C: New test.

--- gcc/combine.c.jj	2011-04-10 19:05:25.000000000 +0200
+++ gcc/combine.c	2011-04-11 09:13:04.000000000 +0200
@@ -1179,7 +1179,7 @@ combine_instructions (rtx f, unsigned in
 
   FOR_EACH_BB (this_basic_block)
     {
-      rtx last_combined_insn = NULL_RTX;
+      rtx last_combined_insn = pc_rtx;
       optimize_this_for_speed_p = optimize_bb_for_speed_p (this_basic_block);
       last_call_luid = 0;
       mem_last_set = -1;
@@ -1198,9 +1198,26 @@ combine_instructions (rtx f, unsigned in
 	  next = 0;
 	  if (NONDEBUG_INSN_P (insn))
 	    {
-	      if (last_combined_insn == NULL_RTX
-		  || DF_INSN_LUID (last_combined_insn) < DF_INSN_LUID (insn))
+	      if (last_combined_insn == pc_rtx)
 		last_combined_insn = insn;
+	      else if (last_combined_insn)
+		{
+		  if (INSN_DELETED_P (last_combined_insn))
+		    {
+		      while (last_combined_insn
+			     != NEXT_INSN (BB_END (this_basic_block))
+			     && (!NONDEBUG_INSN_P (last_combined_insn)
+				 || INSN_DELETED_P (last_combined_insn)))
+			last_combined_insn = NEXT_INSN (last_combined_insn);
+		      if (last_combined_insn
+			  == NEXT_INSN (BB_END (this_basic_block)))
+			last_combined_insn = NULL_RTX;
+		    }
+		  if (last_combined_insn
+		      && DF_INSN_LUID (last_combined_insn)
+			 <= DF_INSN_LUID (insn))
+		    last_combined_insn = insn;
+		}
 
 	      /* See if we know about function return values before this
 		 insn based upon SUBREG flags.  */
@@ -2451,14 +2468,14 @@ propagate_for_debug_subst (rtx from, con
 static void
 propagate_for_debug (rtx insn, rtx last, rtx dest, rtx src)
 {
-  rtx next, loc;
+  rtx next, loc, end = NEXT_INSN (BB_END (this_basic_block));
 
   struct rtx_subst_pair p;
   p.to = src;
   p.adjusted = false;
 
   next = NEXT_INSN (insn);
-  while (next != last)
+  while (next != last && next != end)
     {
       insn = next;
       next = NEXT_INSN (insn);
@@ -3904,8 +3921,9 @@ try_combine (rtx i3, rtx i2, rtx i1, rtx
 		    first = i3;
 		    last = undobuf.other_insn;
 		    gcc_assert (last);
-		    if (DF_INSN_LUID (last)
-			< DF_INSN_LUID (last_combined_insn))
+		    if (last_combined_insn == NULL_RTX
+			|| (DF_INSN_LUID (last)
+			    < DF_INSN_LUID (last_combined_insn)))
 		      last = last_combined_insn;
 		  }
 
--- gcc/testsuite/g++.dg/opt/pr48549.C.jj	2011-04-11 08:35:03.000000000 +0200
+++ gcc/testsuite/g++.dg/opt/pr48549.C	2011-04-11 08:34:19.000000000 +0200
@@ -0,0 +1,63 @@
+// PR rtl-optimization/48549
+// { dg-do compile }
+// { dg-options "-fcompare-debug -O2" }
+
+void
+foo (void *from, void *to)
+{
+  long offset = reinterpret_cast <long>(to) - reinterpret_cast <long>(from);
+  if (offset != static_cast <int>(offset))
+    *(int *) 0xC0DE = 0;
+  reinterpret_cast <int *>(from)[1] = offset;
+}
+struct A
+{
+  A () : a () {}
+  A (void *x) : a (x) {}
+  void *bar () { return a; }
+  void *a;
+};
+struct C;
+struct D;
+struct E : public A
+{
+  C m1 (int);
+  D m2 ();
+  E () {}
+  E (A x) : A (x) {}
+};
+struct C : public E
+{
+  C () {}
+  C (void *x) : E (x) {}
+};
+struct D : public E
+{
+  D (void *x) : E (x) {}
+};
+C
+E::m1 (int x)
+{
+  return (reinterpret_cast <char *>(bar ()) + x);
+}
+D
+E::m2 ()
+{
+  return reinterpret_cast <char *>(bar ());
+}
+struct B
+{
+  E a;
+  unsigned b : 16;
+  unsigned c : 1;
+};
+void
+baz (B *x)
+{
+  for (unsigned i = 0; i < 64; i++)
+    {
+      D d = x[i].a.m2 ();
+      C c = x[i].a.m1 (x[i].c);
+      foo (d.bar (), c.bar ());
+    }
+}

	Jakub

[-- Attachment #2: 1 --]
[-- Type: text/plain, Size: 915 bytes --]

2011-04-11  Jakub Jelinek  <jakub@redhat.com>

	* combine.c (try_combine): Don't call update_cfg_for_uncondjump
	for noop non-jump moves.

--- gcc/combine.c.jj	2011-04-09 19:29:19.083421147 +0200
+++ gcc/combine.c	2011-04-11 10:27:41.769671151 +0200
@@ -4402,7 +4421,8 @@ try_combine (rtx i3, rtx i2, rtx i1, rtx
 
   /* A noop might also need cleaning up of CFG, if it comes from the
      simplification of a jump.  */
-  if (GET_CODE (newpat) == SET
+  if (JUMP_P (i3)
+      && GET_CODE (newpat) == SET
       && SET_SRC (newpat) == pc_rtx
       && SET_DEST (newpat) == pc_rtx)
     {
@@ -4411,6 +4431,7 @@ try_combine (rtx i3, rtx i2, rtx i1, rtx
     }
 
   if (undobuf.other_insn != NULL_RTX
+      && JUMP_P (undobuf.other_insn)
       && GET_CODE (PATTERN (undobuf.other_insn)) == SET
       && SET_SRC (PATTERN (undobuf.other_insn)) == pc_rtx
       && SET_DEST (PATTERN (undobuf.other_insn)) == pc_rtx)

Re: [PATCH] Fix combiner ICEs after my recent patch (PR rtl-optimization/48549)

[Eric Botcazou]

> The propagate_for_debug change alone could fix it, we should never
> fall through into next basic block.  We are unforuntately not deleting
> just jumps (which ought to appear at the end of bbs), but also
> any other noop moves, which I think is unintentional, we have
> delete_noop_moves that should clean that up instead (see the second patch).

Yes, I wondered several times about that.  This clearly looks unintentional and 
I agree that it is worth fixing on the mainline, adding gcc_assert (at_end); to 
update_cfg_for_uncondjump in the process.

> With the second patch, bootstrap succeeded even with gcc_assert (at_end);
> in update_cfg_for_uncondjump, so either it is always at the end, or at
> least very often, thus perhaps with the second patch being applied
> in combine_instructions we could just for INSN_DELETED_P clear
> last_combined_insn right away, meaning we want to propagate till end of
> current bb.

Something I don't understand at the moment: why moving last_combined_insn down 
to the next insn instead of up to the previous one when it has been deleted?
Wouldn't the latter simplify things, in particular avoid setting it to NULL_RTX 
which looks rather awkward to me?

-- 
Eric Botcazou

Re: [PATCH] Fix combiner ICEs after my recent patch (PR rtl-optimization/48549)

[Jakub Jelinek]

On Tue, Apr 12, 2011 at 09:18:01AM +0200, Eric Botcazou wrote:
> > With the second patch, bootstrap succeeded even with gcc_assert (at_end);
> > in update_cfg_for_uncondjump, so either it is always at the end, or at
> > least very often, thus perhaps with the second patch being applied
> > in combine_instructions we could just for INSN_DELETED_P clear
> > last_combined_insn right away, meaning we want to propagate till end of
> > current bb.
> 
> Something I don't understand at the moment: why moving last_combined_insn down 
> to the next insn instead of up to the previous one when it has been deleted?

That is because propagate_for_debug stops at the insn before last, doesn't
process last any longer.  So, if there is a DEBUG_INSN right before the jump
being deleted, and it has been propagated into, after the jump is deleted
and retry is done at i2 earlier, following propagate_for_debug won't see
that DEBUG_INSN any longer.

We could certainly change propagate_for_debug, so that it would stop either
before end (NEXT_INSN (BB_END (this_basic_block)) added in this patch), or
after processing last (which doesn't change anything in the non-deleted insn
case, because i3 from any try_combine surely isn't a DEBUG_INSN and
propagate_for_debug only propagates into those), then on deletion set it to
PREV_INSN instead of NEXT_INSN.  I wonder whether we couldn't be deleting
the first insn in a basic block, but probably there should be at least a
note before it.

> Wouldn't the latter simplify things, in particular avoid setting it to NULL_RTX 
> which looks rather awkward to me?

Below is an untested patch (well, just tested on the testcase in question).
If that's what you prefer, I'll bootstrap/regtest it.

Perhaps instead of the assert we could in that case just set
last_combined_insn to insn.

2011-04-12  Jakub Jelinek  <jakub@redhat.com>

	PR rtl-optimization/48549
	* combine.c (propagate_for_debug): Also stop after BB_END of
	this_basic_block.  Process LAST and just stop processing after it.
	(combine_instructions): If last_combined_insn has been deleted,
	set last_combined_insn to its PREV_INSN.

	* g++.dg/opt/pr48549.C: New test.

--- gcc/combine.c.jj	2011-04-12 09:37:36.000000000 +0200
+++ gcc/combine.c	2011-04-12 09:50:44.000000000 +0200
@@ -1198,8 +1198,18 @@ combine_instructions (rtx f, unsigned in
 	  next = 0;
 	  if (NONDEBUG_INSN_P (insn))
 	    {
-	      if (last_combined_insn == NULL_RTX
-		  || DF_INSN_LUID (last_combined_insn) < DF_INSN_LUID (insn))
+	      if (last_combined_insn)
+		{
+		  while (INSN_DELETED_P (last_combined_insn))
+		    last_combined_insn = PREV_INSN (last_combined_insn);
+		  gcc_assert (!BARRIER_P (last_combined_insn)
+			      && BLOCK_FOR_INSN (last_combined_insn)
+				 == this_basic_block);
+		  if (DF_INSN_LUID (last_combined_insn)
+		      <= DF_INSN_LUID (insn))
+		    last_combined_insn = insn;
+		}
+	      else
 		last_combined_insn = insn;
 
 	      /* See if we know about function return values before this
@@ -2446,19 +2456,21 @@ propagate_for_debug_subst (rtx from, con
 }
 
 /* Replace all the occurrences of DEST with SRC in DEBUG_INSNs between INSN
-   and LAST.  */
+   and LAST, not including INSN, but including LAST.  Also stop at the end
+   of THIS_BASIC_BLOCK.  */
 
 static void
 propagate_for_debug (rtx insn, rtx last, rtx dest, rtx src)
 {
-  rtx next, loc;
+  rtx next, loc, end = NEXT_INSN (BB_END (this_basic_block));
 
   struct rtx_subst_pair p;
   p.to = src;
   p.adjusted = false;
 
   next = NEXT_INSN (insn);
-  while (next != last)
+  last = NEXT_INSN (last);
+  while (next != last && next != end)
     {
       insn = next;
       next = NEXT_INSN (insn);
--- gcc/testsuite/g++.dg/opt/pr48549.C.jj	2011-04-12 09:41:52.000000000 +0200
+++ gcc/testsuite/g++.dg/opt/pr48549.C	2011-04-12 09:41:52.000000000 +0200
@@ -0,0 +1,63 @@
+// PR rtl-optimization/48549
+// { dg-do compile }
+// { dg-options "-fcompare-debug -O2" }
+
+void
+foo (void *from, void *to)
+{
+  long offset = reinterpret_cast <long>(to) - reinterpret_cast <long>(from);
+  if (offset != static_cast <int>(offset))
+    *(int *) 0xC0DE = 0;
+  reinterpret_cast <int *>(from)[1] = offset;
+}
+struct A
+{
+  A () : a () {}
+  A (void *x) : a (x) {}
+  void *bar () { return a; }
+  void *a;
+};
+struct C;
+struct D;
+struct E : public A
+{
+  C m1 (int);
+  D m2 ();
+  E () {}
+  E (A x) : A (x) {}
+};
+struct C : public E
+{
+  C () {}
+  C (void *x) : E (x) {}
+};
+struct D : public E
+{
+  D (void *x) : E (x) {}
+};
+C
+E::m1 (int x)
+{
+  return (reinterpret_cast <char *>(bar ()) + x);
+}
+D
+E::m2 ()
+{
+  return reinterpret_cast <char *>(bar ());
+}
+struct B
+{
+  E a;
+  unsigned b : 16;
+  unsigned c : 1;
+};
+void
+baz (B *x)
+{
+  for (unsigned i = 0; i < 64; i++)
+    {
+      D d = x[i].a.m2 ();
+      C c = x[i].a.m1 (x[i].c);
+      foo (d.bar (), c.bar ());
+    }
+}


	Jakub

Re: [PATCH] Fix combiner ICEs after my recent patch (PR rtl-optimization/48549)

[Eric Botcazou]

> That is because propagate_for_debug stops at the insn before last, doesn't
> process last any longer.  So, if there is a DEBUG_INSN right before the
> jump being deleted, and it has been propagated into, after the jump is
> deleted and retry is done at i2 earlier, following propagate_for_debug
> won't see that DEBUG_INSN any longer.

Ah, yes, thanks.

> We could certainly change propagate_for_debug, so that it would stop either
> before end (NEXT_INSN (BB_END (this_basic_block)) added in this patch), or
> after processing last (which doesn't change anything in the non-deleted
> insn case, because i3 from any try_combine surely isn't a DEBUG_INSN and
> propagate_for_debug only propagates into those), then on deletion set it to
> PREV_INSN instead of NEXT_INSN.  I wonder whether we couldn't be deleting
> the first insn in a basic block, but probably there should be at least a
> note before it.

This looks fine to me.

> Perhaps instead of the assert we could in that case just set
> last_combined_insn to insn.

Yes, this looks preferable to me, IMO we don't need to stop the compiler when 
something goes wrong with debug insns.

> 2011-04-12  Jakub Jelinek  <jakub@redhat.com>
>
> 	PR rtl-optimization/48549
> 	* combine.c (propagate_for_debug): Also stop after BB_END of
> 	this_basic_block.  Process LAST and just stop processing after it.
> 	(combine_instructions): If last_combined_insn has been deleted,
> 	set last_combined_insn to its PREV_INSN.
>
> 	* g++.dg/opt/pr48549.C: New test.

So OK modulo the last point, thanks.

-- 
Eric Botcazou

Re: [PATCH] Fix combiner ICEs after my recent patch (PR rtl-optimization/48549)

[Jakub Jelinek]

On Tue, Apr 12, 2011 at 10:29:57AM +0200, Eric Botcazou wrote:
> > 2011-04-12  Jakub Jelinek  <jakub@redhat.com>
> >
> > 	PR rtl-optimization/48549
> > 	* combine.c (propagate_for_debug): Also stop after BB_END of
> > 	this_basic_block.  Process LAST and just stop processing after it.
> > 	(combine_instructions): If last_combined_insn has been deleted,
> > 	set last_combined_insn to its PREV_INSN.
> >
> > 	* g++.dg/opt/pr48549.C: New test.
> 
> So OK modulo the last point, thanks.

Thanks, here is what I've committed after bootstrap/regtest on x86_64-linux
and i686-linux.  Will now regtest it on 4.6 branch too, then test the extra
JUMP_P patch + gcc_assert (at_end); just for trunk.

2011-04-12  Jakub Jelinek  <jakub@redhat.com>

	PR rtl-optimization/48549
	* combine.c (propagate_for_debug): Also stop after BB_END of
	this_basic_block.  Process LAST and just stop processing after it.
	(combine_instructions): If last_combined_insn has been deleted,
	set last_combined_insn to its PREV_INSN.

	* g++.dg/opt/pr48549.C: New test.

--- gcc/combine.c.jj	2011-04-11 19:26:52.067577164 +0200
+++ gcc/combine.c	2011-04-12 10:40:12.592795949 +0200
@@ -1198,8 +1198,13 @@ combine_instructions (rtx f, unsigned in
 	  next = 0;
 	  if (NONDEBUG_INSN_P (insn))
 	    {
+	      while (last_combined_insn
+		     && INSN_DELETED_P (last_combined_insn))
+		last_combined_insn = PREV_INSN (last_combined_insn);
 	      if (last_combined_insn == NULL_RTX
-		  || DF_INSN_LUID (last_combined_insn) < DF_INSN_LUID (insn))
+		  || BARRIER_P (last_combined_insn)
+		  || BLOCK_FOR_INSN (last_combined_insn) != this_basic_block
+		  || DF_INSN_LUID (last_combined_insn) <= DF_INSN_LUID (insn))
 		last_combined_insn = insn;
 
 	      /* See if we know about function return values before this
@@ -2446,19 +2451,21 @@ propagate_for_debug_subst (rtx from, con
 }
 
 /* Replace all the occurrences of DEST with SRC in DEBUG_INSNs between INSN
-   and LAST.  */
+   and LAST, not including INSN, but including LAST.  Also stop at the end
+   of THIS_BASIC_BLOCK.  */
 
 static void
 propagate_for_debug (rtx insn, rtx last, rtx dest, rtx src)
 {
-  rtx next, loc;
+  rtx next, loc, end = NEXT_INSN (BB_END (this_basic_block));
 
   struct rtx_subst_pair p;
   p.to = src;
   p.adjusted = false;
 
   next = NEXT_INSN (insn);
-  while (next != last)
+  last = NEXT_INSN (last);
+  while (next != last && next != end)
     {
       insn = next;
       next = NEXT_INSN (insn);
--- gcc/testsuite/g++.dg/opt/pr48549.C.jj	2011-04-12 10:36:23.720513425 +0200
+++ gcc/testsuite/g++.dg/opt/pr48549.C	2011-04-12 10:36:23.720513425 +0200
@@ -0,0 +1,63 @@
+// PR rtl-optimization/48549
+// { dg-do compile }
+// { dg-options "-fcompare-debug -O2" }
+
+void
+foo (void *from, void *to)
+{
+  long offset = reinterpret_cast <long>(to) - reinterpret_cast <long>(from);
+  if (offset != static_cast <int>(offset))
+    *(int *) 0xC0DE = 0;
+  reinterpret_cast <int *>(from)[1] = offset;
+}
+struct A
+{
+  A () : a () {}
+  A (void *x) : a (x) {}
+  void *bar () { return a; }
+  void *a;
+};
+struct C;
+struct D;
+struct E : public A
+{
+  C m1 (int);
+  D m2 ();
+  E () {}
+  E (A x) : A (x) {}
+};
+struct C : public E
+{
+  C () {}
+  C (void *x) : E (x) {}
+};
+struct D : public E
+{
+  D (void *x) : E (x) {}
+};
+C
+E::m1 (int x)
+{
+  return (reinterpret_cast <char *>(bar ()) + x);
+}
+D
+E::m2 ()
+{
+  return reinterpret_cast <char *>(bar ());
+}
+struct B
+{
+  E a;
+  unsigned b : 16;
+  unsigned c : 1;
+};
+void
+baz (B *x)
+{
+  for (unsigned i = 0; i < 64; i++)
+    {
+      D d = x[i].a.m2 ();
+      C c = x[i].a.m1 (x[i].c);
+      foo (d.bar (), c.bar ());
+    }
+}


	Jakub

[PATCH] Don't update_cfg_for_uncondjump for noop non-jump moves

[Jakub Jelinek]

On Tue, Apr 12, 2011 at 09:18:01AM +0200, Eric Botcazou wrote:
> > The propagate_for_debug change alone could fix it, we should never
> > fall through into next basic block.  We are unforuntately not deleting
> > just jumps (which ought to appear at the end of bbs), but also
> > any other noop moves, which I think is unintentional, we have
> > delete_noop_moves that should clean that up instead (see the second patch).
> 
> Yes, I wondered several times about that.  This clearly looks unintentional and 
> I agree that it is worth fixing on the mainline, adding gcc_assert (at_end); to 
> update_cfg_for_uncondjump in the process.

Here is a patch, bootstrapped/regtested on x86_64-linux and i686-linux.
Ok for trunk?

2011-04-12  Jakub Jelinek  <jakub@redhat.com>

	* combine.c (update_cfg_for_uncondjump): Instead of testing at_end
	assert it is always true.
	(try_combine): Don't call update_cfg_for_uncondjump for noop non-jump
	moves.

--- gcc/combine.c.jj	2011-04-12 12:52:11.963669819 +0200
+++ gcc/combine.c	2011-04-12 16:31:57.059671430 +0200
@@ -2490,13 +2490,12 @@ static void
 update_cfg_for_uncondjump (rtx insn)
 {
   basic_block bb = BLOCK_FOR_INSN (insn);
-  bool at_end = (BB_END (bb) == insn);
+  gcc_assert (BB_END (bb) == insn);
 
-  if (at_end)
-    purge_dead_edges (bb);
+  purge_dead_edges (bb);
 
   delete_insn (insn);
-  if (at_end && EDGE_COUNT (bb->succs) == 1)
+  if (EDGE_COUNT (bb->succs) == 1)
     {
       rtx insn;
 
@@ -4409,7 +4408,8 @@ try_combine (rtx i3, rtx i2, rtx i1, rtx
 
   /* A noop might also need cleaning up of CFG, if it comes from the
      simplification of a jump.  */
-  if (GET_CODE (newpat) == SET
+  if (JUMP_P (i3)
+      && GET_CODE (newpat) == SET
       && SET_SRC (newpat) == pc_rtx
       && SET_DEST (newpat) == pc_rtx)
     {
@@ -4418,6 +4418,7 @@ try_combine (rtx i3, rtx i2, rtx i1, rtx
     }
 
   if (undobuf.other_insn != NULL_RTX
+      && JUMP_P (undobuf.other_insn)
       && GET_CODE (PATTERN (undobuf.other_insn)) == SET
       && SET_SRC (PATTERN (undobuf.other_insn)) == pc_rtx
       && SET_DEST (PATTERN (undobuf.other_insn)) == pc_rtx)


	Jakub

Re: [PATCH] Don't update_cfg_for_uncondjump for noop non-jump moves

[Jeff Law]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 04/12/11 11:08, Jakub Jelinek wrote:
> On Tue, Apr 12, 2011 at 09:18:01AM +0200, Eric Botcazou wrote:
>>> The propagate_for_debug change alone could fix it, we should never
>>> fall through into next basic block.  We are unforuntately not deleting
>>> just jumps (which ought to appear at the end of bbs), but also
>>> any other noop moves, which I think is unintentional, we have
>>> delete_noop_moves that should clean that up instead (see the second patch).
>>
>> Yes, I wondered several times about that.  This clearly looks unintentional and 
>> I agree that it is worth fixing on the mainline, adding gcc_assert (at_end); to 
>> update_cfg_for_uncondjump in the process.
> 
> Here is a patch, bootstrapped/regtested on x86_64-linux and i686-linux.
> Ok for trunk?
> 
> 2011-04-12  Jakub Jelinek  <jakub@redhat.com>
> 
> 	* combine.c (update_cfg_for_uncondjump): Instead of testing at_end
> 	assert it is always true.
> 	(try_combine): Don't call update_cfg_for_uncondjump for noop non-jump
> 	moves.
Looks good to me, but please wait for Eric to chime in since you've been
discussing it with him already.

Thanks,
jeff
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/

iQEcBAEBAgAGBQJNpIfYAAoJEBRtltQi2kC7yS4IAJZKDfF2fSkrEV1bWlz2p6Gm
P9w4mby36+tqY6AJBLXlaSo97DCGfhgJHuYNJdtC8a4ySz8EpVmEFdjGAfydU6oc
eIhaKfffm/dC8ciQfIQjBGh3mCe85LAOva9hY/3swyDJ2vFVZud/xY8QcC8RZIOk
b6qZ/whcbTDOWSbII81auzhI9fIfESx5Rk73nD2hUqoi8e5uCMPRlnwsLTDwiJfR
Cv8IJZ1kkIIc0i++wlZ9uuha+oRADcsj6awOKR9F07Qfa/ePU3xWw4e+auA6pfLL
eui0+Xp5BmkwOBc4vF7dl/lxugwRvKXdEB2OdiB96d7dwDBnC6B0a/7zqXZ9bsc=
=bwhq
-----END PGP SIGNATURE-----

Re: [PATCH] Don't update_cfg_for_uncondjump for noop non-jump moves

[Eric Botcazou]

> Looks good to me, but please wait for Eric to chime in since you've been
> discussing it with him already.

Fine with me as well.

-- 
Eric Botcazou