diff mbox

Fix -fcompare-debug issue in reload1 (PR debug/46252)

Message ID 20101101192348.GB29412@tyan-ft48-01.lab.bos.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek Nov. 1, 2010, 7:23 p.m. UTC 
Hi!

delete_dead_insn removes unused preceeding insn, but will not do that
if there are debug insns in between.
Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
trunk?

2010-11-01  Jakub Jelinek  <jakub@redhat.com>

	PR debug/46252
	* rtl.h (prev_real_nondebug_insn): New prototype.
	* emit-rtl.c (prev_real_nondebug_insn): New function.
	* reload1.c (delete_dead_insn): Use it instead of
	prev_real_insn.
	* cfgcleanup.c (try_head_merge_bb): Likewise.

	* gcc.dg/pr46252.c: New test.


	Jakub

Comments

Eric Botcazou Nov. 2, 2010, 6:42 p.m. UTC | #1 
> delete_dead_insn removes unused preceeding insn, but will not do that
> if there are debug insns in between.
> Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> trunk?
>
> 2010-11-01  Jakub Jelinek  <jakub@redhat.com>
>
> 	PR debug/46252
> 	* rtl.h (prev_real_nondebug_insn): New prototype.
> 	* emit-rtl.c (prev_real_nondebug_insn): New function.
> 	* reload1.c (delete_dead_insn): Use it instead of
> 	prev_real_insn.
> 	* cfgcleanup.c (try_head_merge_bb): Likewise.

The question is, shouldn't prev_real_insn (and next_real_insn) avoid returning 
a DEBUG_INSN in the first place?  The remaining use of prev_real_insn seems 
to want prev_real_nondebug_insn as well.
Jakub Jelinek Nov. 2, 2010, 6:53 p.m. UTC | #2 
On Tue, Nov 02, 2010 at 07:42:43PM +0100, Eric Botcazou wrote:
> > delete_dead_insn removes unused preceeding insn, but will not do that
> > if there are debug insns in between.
> > Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, ok for
> > trunk?
> >
> > 2010-11-01  Jakub Jelinek  <jakub@redhat.com>
> >
> > 	PR debug/46252
> > 	* rtl.h (prev_real_nondebug_insn): New prototype.
> > 	* emit-rtl.c (prev_real_nondebug_insn): New function.
> > 	* reload1.c (delete_dead_insn): Use it instead of
> > 	prev_real_insn.
> > 	* cfgcleanup.c (try_head_merge_bb): Likewise.
> 
> The question is, shouldn't prev_real_insn (and next_real_insn) avoid returning 
> a DEBUG_INSN in the first place?  The remaining use of prev_real_insn seems 
> to want prev_real_nondebug_insn as well.

For prev_real_insn I think you're right, the remaining two uses also do want
to skip debug insns.  For next_real_insn I'm not that sure, it is used e.g.
in the scheduler where we want to see debug insns.
Having prev_real_insn skip debug insns and next_real_insn not skip them
would be inconsistent though.

	Jakub
diff mbox

Patch

--- gcc/rtl.h.jj	2010-11-01 09:07:23.000000000 +0100
+++ gcc/rtl.h	2010-11-01 15:41:31.000000000 +0100
@@ -1751,6 +1751,7 @@  extern rtx prev_nonnote_nondebug_insn (r
 extern rtx next_nonnote_nondebug_insn (rtx);
 extern rtx prev_real_insn (rtx);
 extern rtx next_real_insn (rtx);
+extern rtx prev_real_nondebug_insn (rtx);
 extern rtx prev_active_insn (rtx);
 extern rtx next_active_insn (rtx);
 extern int active_insn_p (const_rtx);
--- gcc/emit-rtl.c.jj	2010-11-01 09:07:23.000000000 +0100
+++ gcc/emit-rtl.c	2010-11-01 15:41:01.000000000 +0100
@@ -3133,8 +3133,8 @@  next_real_insn (rtx insn)
   return insn;
 }
 
-/* Return the last INSN, CALL_INSN or JUMP_INSN before INSN;
-   or 0, if there is none.  This routine does not look inside
+/* Return the last INSN, CALL_INSN, JUMP_INSN or DEBUG_INSN
+   before INSN; or 0, if there is none.  This routine does not look inside
    SEQUENCEs.  */
 
 rtx
@@ -3150,6 +3150,23 @@  prev_real_insn (rtx insn)
   return insn;
 }
 
+/* Return the last INSN, CALL_INSN or JUMP_INSN before INSN;
+   or 0, if there is none.  This routine does not look inside
+   SEQUENCEs.  */
+
+rtx
+prev_real_nondebug_insn (rtx insn)
+{
+  while (insn)
+    {
+      insn = PREV_INSN (insn);
+      if (insn == 0 || NONDEBUG_INSN_P (insn))
+	break;
+    }
+
+  return insn;
+}
+
 /* Return the last CALL_INSN in the current list, or 0 if there is none.
    This routine does not look inside SEQUENCEs.  */
 
--- gcc/reload1.c.jj	2010-11-01 09:07:24.000000000 +0100
+++ gcc/reload1.c	2010-11-01 15:39:21.000000000 +0100
@@ -2112,7 +2112,7 @@  spill_failure (rtx insn, enum reg_class 
 static void
 delete_dead_insn (rtx insn)
 {
-  rtx prev = prev_real_insn (insn);
+  rtx prev = prev_real_nondebug_insn (insn);
   rtx prev_dest;
 
   /* If the previous insn sets a register that dies in our insn, delete it
--- gcc/cfgcleanup.c.jj	2010-11-01 09:07:23.000000000 +0100
+++ gcc/cfgcleanup.c	2010-11-01 15:42:08.000000000 +0100
@@ -2054,9 +2054,7 @@  try_head_merge_bb (basic_block bb)
       max_match--;
       if (max_match == 0)
 	return false;
-      do
-	e0_last_head = prev_real_insn (e0_last_head);
-      while (DEBUG_INSN_P (e0_last_head));
+      e0_last_head = prev_real_nondebug_insn (e0_last_head);
     }
 
   if (max_match == 0)
--- gcc/testsuite/gcc.dg/pr46252.c.jj	2010-11-01 16:12:22.000000000 +0100
+++ gcc/testsuite/gcc.dg/pr46252.c	2010-11-01 16:12:07.000000000 +0100
@@ -0,0 +1,15 @@ 
+/* PR debug/46252 */
+/* { dg-do compile } */
+/* { dg-options "-O -frerun-cse-after-loop -fno-tree-loop-optimize -funroll-loops -fcompare-debug" } */
+
+void
+foo (float *f)
+{
+  int i;
+  for (i = 0; i < 4; i++)
+    f[i] = i;
+  bar ();
+  for (i = 0; i < 4; i++)
+    if (f[i] != i)
+      __builtin_abort ();
+}