[PATCH] Add gen_lowpart_for_debug (PR debug/55730)

[Jakub Jelinek]

Hi!

On Tue, Dec 18, 2012 at 09:25:14AM +0100, Paolo Bonzini wrote:
> Il 17/12/2012 22:33, Jakub Jelinek ha scritto:
> > If gen_lowpart_if_possible returns NULL, the default
> > rtl_hooks.gen_lowpart_no_emit hook returns the original value, which is not
> > of the desired mode.  I bet in most passes for real insns such rtx is then
> > meant to fail recog and thrown away, but for DEBUG_INSN modification that
> > doesn't work, since there is no verification (but also e.g. any kind of
> > SUBREG is fine).  So we can e.g. end up with a (plus:SI (mem:DI ...) (mem:SI ...))
> > or similar and then various passes (in this testcase on s390x reload) can be
> > very upset about that.
> 
> Makes sense, and it could even be a wrong-code bug for this simplification:

Richi reported another related failure today.  During combine,
rtl_hooks.gen_lowpart_no_emit is the combine version, which instead of
giving up creates (clobber:MODE (const_int 0)).  This is slightly less wrong
than what the general hook did, but still dwarf2out would ICE when seeing
that (with checking, without it just not provide location info).
We can easily emit the SUBREG though (e.g. var-tracking itself also calls
gen_rtx_raw_SUBREG as last resort) in the DEBUG_INSN operands.

Bootstrapped/regtested on x86_64-linux and i686-linux (and on the testcase
using -> powerpc64-linux cross), ok for trunk?

2012-12-18  Jakub Jelinek  <jakub@redhat.com>

	PR debug/55730
	* dwarf2out.c (mem_loc_descriptor): Ignore CLOBBER.
	* valtrack.c (gen_lowpart_for_debug): New function.
	(propagate_for_debug): Temporarily set rtl_hooks.gen_lowpart_no_emit
	to gen_lowpart_for_debug.

	* gcc.dg/debug/pr55730.c: New test.

--- gcc/dwarf2out.c.jj	2012-12-18 11:41:30.000000000 +0100
+++ gcc/dwarf2out.c	2012-12-18 16:38:26.925380294 +0100
@@ -12714,6 +12714,7 @@ mem_loc_descriptor (rtx rtl, enum machin
     case CONST_VECTOR:
     case CONST_FIXED:
     case CLRSB:
+    case CLOBBER:
       /* If delegitimize_address couldn't do anything with the UNSPEC, we
 	 can't express it in the debug info.  This can happen e.g. with some
 	 TLS UNSPECs.  */
--- gcc/valtrack.c.jj	2012-11-05 15:02:17.000000000 +0100
+++ gcc/valtrack.c	2012-12-18 17:15:18.499375776 +0100
@@ -29,6 +29,24 @@ along with GCC; see the file COPYING3.
 #include "regs.h"
 #include "emit-rtl.h"
 
+/* gen_lowpart_no_emit hook implementation for DEBUG_INSNs.  In DEBUG_INSNs,
+   all lowpart SUBREGs are valid, despite what the machine requires for
+   instructions.  */
+
+static rtx
+gen_lowpart_for_debug (enum machine_mode mode, rtx x)
+{
+  rtx result = gen_lowpart_if_possible (mode, x);
+  if (result)
+    return result;
+
+  if (GET_MODE (x) != VOIDmode)
+    return gen_rtx_raw_SUBREG (mode, x,
+			       subreg_lowpart_offset (mode, GET_MODE (x)));
+
+  return NULL_RTX;
+}
+
 /* Replace auto-increment addressing modes with explicit operations to access
    the same addresses without modifying the corresponding registers.  */
 
@@ -158,6 +176,7 @@ propagate_for_debug (rtx insn, rtx last,
 		     basic_block this_basic_block)
 {
   rtx next, loc, end = NEXT_INSN (BB_END (this_basic_block));
+  rtx (*saved_rtl_hook_no_emit) (enum machine_mode, rtx);
 
   struct rtx_subst_pair p;
   p.to = src;
@@ -165,6 +184,8 @@ propagate_for_debug (rtx insn, rtx last,
 
   next = NEXT_INSN (insn);
   last = NEXT_INSN (last);
+  saved_rtl_hook_no_emit = rtl_hooks.gen_lowpart_no_emit;
+  rtl_hooks.gen_lowpart_no_emit = gen_lowpart_for_debug;
   while (next != last && next != end)
     {
       insn = next;
@@ -179,6 +200,7 @@ propagate_for_debug (rtx insn, rtx last,
 	  df_insn_rescan (insn);
 	}
     }
+  rtl_hooks.gen_lowpart_no_emit = saved_rtl_hook_no_emit;
 }
 
 /* Initialize DEBUG to an empty list, and clear USED, if given.  */
--- gcc/testsuite/gcc.dg/debug/pr55730.c.jj	2012-12-18 17:08:29.649351676 +0100
+++ gcc/testsuite/gcc.dg/debug/pr55730.c	2012-12-18 17:08:04.000000000 +0100
@@ -0,0 +1,24 @@
+/* PR debug/55730 */
+/* { dg-do compile } */
+/* { dg-options "-w" } */
+
+union U
+{
+  float f;
+  int i;
+};
+
+void
+foo (unsigned short *x, unsigned char y)
+{
+  unsigned char g;
+  union U u;
+  if (u.i < 0)
+    g = 0;
+  else
+    {
+      u.f = u.f * (255.0F / 256.0F) + 32768.0F;
+      g = (unsigned char) u.i;
+    }
+  *x = (g << 8) | y;
+}

	Jakub