Re: [Patch] pr65779 - [5/6 Regression] undefined local symbol on powerpc

[Alan Modra <amodra at gmail dot com>]

On Tue, Apr 21, 2015 at 02:30:15PM +0200, Jakub Jelinek wrote:
> This isn't safe.  There could be a debug_insn for the same decl anywhere in
> between the dinsn and bb_note (bb) on the chosen live path, if there is,
> this change will break stuff.

Yeah, I see now that what I was trying to do can't be done so easily.
In fact, the more I look at debug_insns the less sure I am of any
solution other than simply zapping them with UNKNOWN_VAR_LOC.
However, I'm not going to write such a patch.  Simply because I don't
want to be responsible for even more cases where "print var" in gdb
gets "<optimized out>" and upon examing the disassembly you can easily
determine that "var" is live in some register.

One of the potential problems I've discovered is that debug_insns do
not satisfy an assumption made by propagate_for_debug, that they are
always located immediately after their associated insn.  For example,
gcc-5 branch on x86_64 varasm.c:hashtab_entry_note_pointers has this
in vregs pass:

(code_label 18 17 19 5 57 "" [1 uses])
(note 19 18 7 5 [bb 5] NOTE_INSN_BASIC_BLOCK)
(insn 7 19 40 5 (set (reg/v:DI 91 [ i ])
        (const_int 0 [0])) /src/gcc-5-cur/gcc/hash-table.h:1723 89 {*movdi_internal}
     (nil))
;;  succ:       6 [100.0%]  (FALLTHRU)

;; basic block 6, loop depth 1, count 0, freq 9100, maybe hot
;;  prev block 5, next block 7, flags: (NEW, REACHABLE, RTL)
;;  pred:       5 [100.0%]  (FALLTHRU)
;;              8 [100.0%] 
(code_label 40 7 20 6 60 "" [1 uses])
(note 20 40 21 6 [bb 6] NOTE_INSN_BASIC_BLOCK)
(debug_insn 21 20 22 6 (var_location:DI i (reg/v:DI 91 [ i ])) -1
     (nil))

OK, so that particular example can't cause a problem with combine's
use of propagate_for_debug but something like the above would break
what I'm trying to do in shrink_wrap.


Here's another problem, x86_64 varasm.c again, last occurrence of
valid0 in debug, cprop1:

(debug_insn 858 629 630 104 (var_location:DI D#293 (reg:DI 0 ax)) -1
     (nil))
(insn 630 858 631 104 (set (reg/v/f:DI 195 [ ret ])
        (reg:DI 0 ax)) /src/gcc-5-cur/gcc/varasm.c:4541 89 {*movdi_internal}
     (expr_list:REG_DEAD (reg:DI 0 ax)
        (nil)))
(debug_insn 631 630 633 104 (var_location:DI valid0 (debug_expr:DI D#293)) /src/gcc-5-cur/gcc/varasm.c:4541 -1
     (nil))

Now that seems just plain wrong to me.  This is just after a call.
After insn 630 valid0 lives in reg 195, not reg 0 as the debug_insn
says.  What I'm concerned about as far as fixing shrink_wrap is
whether this sort of thing can occur in correct debug expressions,
ie. if debug_isns 858 said D#293 is equal to reg 195.  We'd have part
of the debug info about valid0 *before* the insn setting reg 195.
As far as I can tell, sched doesn't create this sort of problem.
Note that prior to cprop1, we just had
(insn 630 629 631 93 (set (reg/v/f:DI 195 [ ret ])
        (reg:DI 0 ax)) /src/gcc-5-cur/gcc/varasm.c:4541 89 {*movdi_internal}
     (expr_list:REG_DEAD (reg:DI 0 ax)
        (nil)))
(debug_insn 631 630 633 93 (var_location:DI valid0 (reg/v/f:DI 195 [ ret ])) /src/gcc-5-cur/gcc/varasm.c:4541 -1
     (nil))


Anyway, here's my best attempt at fixing PR65779.  If it isn't
acceptable I'm going to have to leave this problem to someone else,
not because I'm uninterested in fixing it properly, but because I'm
being asked to work on other things by my employer..

Bootstrapped and regression tested powerpc64-linux and x86_64-linux.

gcc/
	PR debug/65779
	* shrink-wrap.c (insn_uses_reg): New function.
	(move_insn_for_shrink_wrap): Try to fix up debug insns related
	to the moved insn.
gcc/testsuite/
	* gcc.dg/pr65779.c: New.

Index: gcc/shrink-wrap.c
===================================================================
--- gcc/shrink-wrap.c	(revision 222708)
+++ gcc/shrink-wrap.c	(working copy)
@@ -79,6 +79,7 @@
 #include "shrink-wrap.h"
 #include "regcprop.h"
 #include "rtl-iter.h"
+#include "valtrack.h"
 
 #ifdef HAVE_simple_return
 
@@ -182,6 +183,24 @@
   return live_edge;
 }
 
+/* Return true if INSN df shows a use of a reg in the range
+   [REGNO,END_REGNO).  */
+
+static bool
+insn_uses_reg (rtx_insn *insn, unsigned int regno, unsigned int end_regno)
+{
+  df_ref use;
+
+  FOR_EACH_INSN_USE (use, insn)
+    {
+      rtx reg = DF_REF_REG (use);
+
+      if (REG_P (reg) && REGNO (reg) >= regno && REGNO (reg) < end_regno)
+	return true;
+    }
+  return false;
+}
+
 /* Try to move INSN from BB to a successor.  Return true on success.
    USES and DEFS are the set of registers that are used and defined
    after INSN in BB.  SPLIT_P indicates whether a live edge from BB
@@ -193,7 +212,7 @@
 			   const HARD_REG_SET defs,
 			   bool *split_p)
 {
-  rtx set, src, dest;
+  rtx set, src, orig_src, dest;
   bitmap live_out, live_in, bb_uses, bb_defs;
   unsigned int i, dregno, end_dregno;
   unsigned int sregno = FIRST_PSEUDO_REGISTER;
@@ -209,7 +228,7 @@
   set = PATTERN (insn);
   if (GET_CODE (set) != SET)
     return false;
-  src = SET_SRC (set);
+  orig_src = src = SET_SRC (set);
   dest = SET_DEST (set);
 
   /* For the destination, we want only a register.  Also disallow STACK
@@ -342,8 +361,11 @@
 
   /* At this point we are committed to moving INSN, but let's try to
      move it as far as we can.  */
+  auto_vec<basic_block, 5> live_bbs;
   do
     {
+      if (MAY_HAVE_DEBUG_INSNS)
+	live_bbs.safe_push (bb);
       live_out = df_get_live_out (bb);
       live_in = df_get_live_in (next_block);
       bb = next_block;
@@ -426,6 +448,71 @@
 	SET_REGNO_REG_SET (bb_uses, i);
     }
 
+  /* Try to fix up debug insns in the tail of the entry block and any
+     intervening blocks that use regs set by the insn we are moving.  */
+  if (MAY_HAVE_DEBUG_INSNS)
+    {
+      basic_block *elt;
+      unsigned int indx;
+      bool do_prop = false;
+
+      FOR_EACH_VEC_ELT (live_bbs, indx, elt)
+	{
+	  rtx_insn *dinsn;
+	  basic_block tmp_bb = *elt;
+
+	  if (indx == 0)
+	    {
+	      /* First check that we have some debug info relevant to
+		 the insn being moved.  It ought to be just after
+		 insn.  */
+	      dinsn = insn;
+	      while (dinsn != BB_END (tmp_bb))
+		{
+		  dinsn = NEXT_INSN (dinsn);
+		  if (!DEBUG_INSN_P (dinsn))
+		    break;
+		  if (insn_uses_reg (dinsn, dregno, end_dregno))
+		    {
+		      do_prop = true;
+		      break;
+		    }
+		}
+
+	      if (do_prop)
+		{
+		  /* Put debug info for the insn we'll be moving
+		     into the destination block.  */
+		  rtx_insn *newdinsn = bb_note (bb);
+
+		  dinsn = insn;
+		  while (dinsn != BB_END (tmp_bb))
+		    {
+		      dinsn = NEXT_INSN (dinsn);
+		      if (!DEBUG_INSN_P (dinsn))
+			break;
+		      newdinsn
+			= emit_debug_insn_after (copy_rtx (PATTERN (dinsn)),
+						 newdinsn);
+		      df_insn_rescan (newdinsn);
+		    }
+		}
+	      if (dump_file)
+		fprintf (dump_file,
+			 "Propagating %sdebug for insn %d.\n",
+			 do_prop ? "" : "no ", INSN_UID (insn));
+	    }
+	  dinsn = insn;
+	  if (indx != 0)
+	    dinsn = BB_HEAD (tmp_bb);
+
+	  if (do_prop)
+	    propagate_for_debug (dinsn, BB_END (tmp_bb),
+				 dest, orig_src,
+				 tmp_bb);
+	}
+    }
+
   emit_insn_after (PATTERN (insn), bb_note (bb));
   delete_insn (insn);
   return true;
Index: gcc/testsuite/gcc.dg/pr65779.c
===================================================================
--- gcc/testsuite/gcc.dg/pr65779.c	(revision 0)
+++ gcc/testsuite/gcc.dg/pr65779.c	(working copy)
@@ -0,0 +1,64 @@
+/* { dg-do run } */
+/* { dg-options "-O2 -g" } */
+/* { dg-additional-options "-mrelocatable" { target powerpc-*-rtems* } } */
+
+unsigned long __attribute__ ((noinline))
+adler32 (unsigned long adler, unsigned char *buf, unsigned int len)
+{
+  unsigned long s1 = adler & 0xffff;
+  unsigned long s2 = (adler >> 16) & 0xffff;
+  int k;
+
+  if (buf == 0)
+    return 1L;
+
+  while (len > 0)
+    {
+      k = len < 5552 ? len : 5552;
+      len -= k;
+      while (k >= 16)
+	{
+	  s1 += *buf++; s2 += s1;
+	  s1 += *buf++; s2 += s1;
+	  s1 += *buf++; s2 += s1;
+	  s1 += *buf++; s2 += s1;
+	  s1 += *buf++; s2 += s1;
+	  s1 += *buf++; s2 += s1;
+	  s1 += *buf++; s2 += s1;
+	  s1 += *buf++; s2 += s1;
+	  s1 += *buf++; s2 += s1;
+	  s1 += *buf++; s2 += s1;
+	  s1 += *buf++; s2 += s1;
+	  s1 += *buf++; s2 += s1;
+	  s1 += *buf++; s2 += s1;
+	  s1 += *buf++; s2 += s1;
+	  s1 += *buf++; s2 += s1;
+	  s1 += *buf++; s2 += s1;
+	  k -= 16;
+	}
+      if (k != 0)
+	do
+	  {
+	    s1 += *buf++; s2 += s1;
+	  } while (--k);
+      s1 &= 0xffffffffUL;
+      s2 &= 0xffffffffUL;
+      s1 %= 65521L;
+      s2 %= 65521L;
+    }
+  return (s2 << 16) | s1;
+}
+
+unsigned char buf[] = { 0, 1, 2, 3, 4, 5, 6, 7,
+			8, 9, 10, 11, 12, 13, 14, 15,
+			0x80, 0x81, 0x82, 0x83, 0x84, 0x85, 0x86, 0x87,
+			0x88, 0x89, 0x8a, 0x8b, 0x8c, 0x8d, 0x8e, 0x8f,
+			0x55, 0xaa };
+int
+main ()
+{
+  unsigned long x = adler32 (0, buf, sizeof buf);
+  if (x != 0x640409efUL)
+    __builtin_abort ();
+  return 0;
+}


-- 
Alan Modra
Australia Development Lab, IBM