diff mbox series

Fix combiner move_deaths DEBUG_INSN handling (PR target/84614)

Message ID 20180301231351.GV5867@tucnak
State New
Headers show
Series Fix combiner move_deaths DEBUG_INSN handling (PR target/84614) | expand

Commit Message

Jakub Jelinek March 1, 2018, 11:13 p.m. UTC 
Hi!

prev_real_insn doesn't skip over DEBUG_INSNs, on aarch64 on this
testcase we get both -fcompare-debug failure and wrong-code with -g.

Instead of adding yet another two:
      do
        insn = prev_real_insn (insn);
      while (DEBUG_INSN_P (insn));
loops, this patch introduces new functions that stop only at
NONDEBUG_INSN_P.

Bootstrapped/regtested on x86_64-linux and i686-linux, tested with
cross to aarch64-linux on the testcase, ok for trunk?

2018-03-02  Jakub Jelinek  <jakub@redhat.com>

	PR target/84614
	* rtl.h (prev_real_nondebug_insn, next_real_nondebug_insn): New
	prototypes.
	* emit-rtl.c (next_real_insn, prev_real_insn): Fix up function
	comments.
	(next_real_nondebug_insn, prev_real_nondebug_insn): New functions.
	* cfgcleanup.c (try_head_merge_bb): Use prev_real_nondebug_insn
	instead of a loop around prev_real_insn.
	* combine.c (move_deaths): Use prev_real_nondebug_insn instead of
	prev_real_insn.

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


	Jakub

Comments

Segher Boessenkool March 1, 2018, 11:47 p.m. UTC | #1 
Hi Jakub,

On Fri, Mar 02, 2018 at 12:13:51AM +0100, Jakub Jelinek wrote:
> prev_real_insn doesn't skip over DEBUG_INSNs, on aarch64 on this
> testcase we get both -fcompare-debug failure and wrong-code with -g.
> 
> Instead of adding yet another two:
>       do
>         insn = prev_real_insn (insn);
>       while (DEBUG_INSN_P (insn));
> loops, this patch introduces new functions that stop only at
> NONDEBUG_INSN_P.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, tested with
> cross to aarch64-linux on the testcase, ok for trunk?

Looks good to me, thanks!  (The non-combine parts are arguably obvious
and trivial cleanup).


Segher


> 2018-03-02  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR target/84614
> 	* rtl.h (prev_real_nondebug_insn, next_real_nondebug_insn): New
> 	prototypes.
> 	* emit-rtl.c (next_real_insn, prev_real_insn): Fix up function
> 	comments.
> 	(next_real_nondebug_insn, prev_real_nondebug_insn): New functions.
> 	* cfgcleanup.c (try_head_merge_bb): Use prev_real_nondebug_insn
> 	instead of a loop around prev_real_insn.
> 	* combine.c (move_deaths): Use prev_real_nondebug_insn instead of
> 	prev_real_insn.
> 
> 	* gcc.dg/pr84614.c: New test.
Jeff Law March 2, 2018, 4:13 p.m. UTC | #2 
On 03/01/2018 04:13 PM, Jakub Jelinek wrote:
> Hi!
> 
> prev_real_insn doesn't skip over DEBUG_INSNs, on aarch64 on this
> testcase we get both -fcompare-debug failure and wrong-code with -g.
> 
> Instead of adding yet another two:
>       do
>         insn = prev_real_insn (insn);
>       while (DEBUG_INSN_P (insn));
> loops, this patch introduces new functions that stop only at
> NONDEBUG_INSN_P.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, tested with
> cross to aarch64-linux on the testcase, ok for trunk?
> 
> 2018-03-02  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR target/84614
> 	* rtl.h (prev_real_nondebug_insn, next_real_nondebug_insn): New
> 	prototypes.
> 	* emit-rtl.c (next_real_insn, prev_real_insn): Fix up function
> 	comments.
> 	(next_real_nondebug_insn, prev_real_nondebug_insn): New functions.
> 	* cfgcleanup.c (try_head_merge_bb): Use prev_real_nondebug_insn
> 	instead of a loop around prev_real_insn.
> 	* combine.c (move_deaths): Use prev_real_nondebug_insn instead of
> 	prev_real_insn.
> 
> 	* gcc.dg/pr84614.c: New test.
THanks.  We're going to need the {next,prev}_real_nondebug_insn routines
in reorg/resource as well.  MIPS can't currently build a linux kernel
because we fail all over the place with DEBUG_INSNs within reorg.

I've got a patch that's enough to get mips to bootstrap and build
kernels, but haven't posted it yet.

jeff
diff mbox series

Patch

--- gcc/rtl.h.jj	2018-02-09 06:44:38.366804455 +0100
+++ gcc/rtl.h	2018-03-01 13:38:05.445681834 +0100
@@ -3277,6 +3277,8 @@  extern rtx_insn *next_nonnote_nondebug_i
 extern rtx_insn *next_nonnote_nondebug_insn_bb (rtx_insn *);
 extern rtx_insn *prev_real_insn (rtx_insn *);
 extern rtx_insn *next_real_insn (rtx);
+extern rtx_insn *prev_real_nondebug_insn (rtx_insn *);
+extern rtx_insn *next_real_nondebug_insn (rtx);
 extern rtx_insn *prev_active_insn (rtx_insn *);
 extern rtx_insn *next_active_insn (rtx_insn *);
 extern int active_insn_p (const rtx_insn *);
--- gcc/emit-rtl.c.jj	2018-02-19 22:57:06.508917827 +0100
+++ gcc/emit-rtl.c	2018-03-01 13:37:19.846688578 +0100
@@ -3597,7 +3597,7 @@  prev_nonnote_nondebug_insn_bb (rtx_insn
   return insn;
 }
 
-/* Return the next INSN, CALL_INSN or JUMP_INSN after INSN;
+/* Return the next INSN, CALL_INSN, JUMP_INSN or DEBUG_INSN after INSN;
    or 0, if there is none.  This routine does not look inside
    SEQUENCEs.  */
 
@@ -3616,7 +3616,7 @@  next_real_insn (rtx uncast_insn)
   return insn;
 }
 
-/* Return the last INSN, CALL_INSN or JUMP_INSN before INSN;
+/* 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.  */
 
@@ -3630,6 +3630,42 @@  prev_real_insn (rtx_insn *insn)
 	break;
     }
 
+  return insn;
+}
+
+/* Return the next INSN, CALL_INSN or JUMP_INSN after INSN;
+   or 0, if there is none.  This routine does not look inside
+   SEQUENCEs.  */
+
+rtx_insn *
+next_real_nondebug_insn (rtx uncast_insn)
+{
+  rtx_insn *insn = safe_as_a <rtx_insn *> (uncast_insn);
+
+  while (insn)
+    {
+      insn = NEXT_INSN (insn);
+      if (insn == 0 || NONDEBUG_INSN_P (insn))
+	break;
+    }
+
+  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_insn *
+prev_real_nondebug_insn (rtx_insn *insn)
+{
+  while (insn)
+    {
+      insn = PREV_INSN (insn);
+      if (insn == 0 || NONDEBUG_INSN_P (insn))
+	break;
+    }
+
   return insn;
 }
 
--- gcc/cfgcleanup.c.jj	2018-01-24 17:18:43.779392151 +0100
+++ gcc/cfgcleanup.c	2018-03-01 13:39:13.972671695 +0100
@@ -2448,9 +2448,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/combine.c.jj	2018-02-16 19:39:09.485062861 +0100
+++ gcc/combine.c	2018-03-01 13:46:45.292531080 +0100
@@ -13923,7 +13923,7 @@  move_deaths (rtx x, rtx maybe_kill_insn,
 	 FROM_LUID and TO_INSN.  If so, find it.  This is PR83304.  */
       if (!where_dead || DF_INSN_LUID (where_dead) >= DF_INSN_LUID (to_insn))
 	{
-	  rtx_insn *insn = prev_real_insn (to_insn);
+	  rtx_insn *insn = prev_real_nondebug_insn (to_insn);
 	  while (insn
 		 && BLOCK_FOR_INSN (insn) == BLOCK_FOR_INSN (to_insn)
 		 && DF_INSN_LUID (insn) >= from_luid)
@@ -13935,7 +13935,7 @@  move_deaths (rtx x, rtx maybe_kill_insn,
 		  break;
 		}
 
-	      insn = prev_real_insn (insn);
+	      insn = prev_real_nondebug_insn (insn);
 	    }
 	}
 
--- gcc/testsuite/gcc.dg/pr84614.c.jj	2018-03-01 14:01:25.256546456 +0100
+++ gcc/testsuite/gcc.dg/pr84614.c	2018-03-01 13:59:30.657611490 +0100
@@ -0,0 +1,28 @@ 
+/* PR target/84614 */
+/* { dg-do run { target int128 } } */
+/* { dg-options "-Og -fno-split-wide-types -fno-tree-coalesce-vars -g --param=max-combine-insns=3 -fcompare-debug" } */
+
+unsigned __int128 a;
+
+unsigned __int128
+b (unsigned short c, unsigned int d)
+{
+  unsigned long long e;
+  __builtin_sub_overflow (0, d, &e);
+  e >>= c;
+  c ^= 65535;
+  d ^= 824;
+  return c + a + d + e;
+}
+
+int
+main ()
+{
+  unsigned __int128 x = b (0, 9);
+  if (__SIZEOF_INT__ * __CHAR_BIT__ == 32
+      && __SIZEOF_LONG_LONG__ * __CHAR_BIT__ == 64
+      && __SIZEOF_INT128__ * __CHAR_BIT__ == 128
+      && x != (((unsigned __int128) 1 << 64) | 0x10327))
+    __builtin_abort ();
+  return 0;
+}