[PATCH] Fix up creation of lowpart subregs in valtrack (PR debug/65678)

[PATCH] Fix up creation of lowpart subregs in valtrack (PR debug/65678)

[Jakub Jelinek]

Hi!

gen_rtx_SUBREG/gen_lowpart_SUBREG ICE when trying to create a subreg
the backend doesn't like, lowpart_subreg returns NULL.  But, for debug info
purposes, all lowpart subregs are valid, a subreg in the debug insns or
var-tracking notes is simply a mode punning construct, and even when the
backend doesn't have any instruction to perform such operation, the debugger
can always do the punning.

var-tracking.c already has code to emit gen_rtx_raw_SUBREG if everything
else fails, the following patch adjusts valtrack.c to do the same thing.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2015-04-07  Jakub Jelinek  <jakub@redhat.com>

	PR debug/65678
	* valtrack.c (debug_lowpart_subreg): New function.
	(dead_debug_insert_temp): Use it.

	* g++.dg/debug/pr65678.C: New test.

--- gcc/valtrack.c.jj	2015-01-05 13:07:14.000000000 +0100
+++ gcc/valtrack.c	2015-04-07 11:10:36.045912355 +0200
@@ -534,6 +534,22 @@ dead_debug_add (struct dead_debug_local
   bitmap_set_bit (debug->used, uregno);
 }
 
+/* Like lowpart_subreg, but if a subreg is not valid for machine, force
+   it anyway - for use in debug insns.  */
+
+static rtx
+debug_lowpart_subreg (machine_mode outer_mode, rtx expr,
+		      machine_mode inner_mode)
+{
+  if (inner_mode == VOIDmode)
+    inner_mode = GET_MODE (expr);
+  int offset = subreg_lowpart_offset (outer_mode, inner_mode);
+  rtx ret = simplify_gen_subreg (outer_mode, expr, inner_mode, offset);
+  if (ret)
+    return ret;
+  return gen_rtx_raw_SUBREG (outer_mode, expr, offset);
+}
+
 /* If UREGNO is referenced by any entry in DEBUG, emit a debug insn
    before or after INSN (depending on WHERE), that binds a (possibly
    global) debug temp to the widest-mode use of UREGNO, if WHERE is
@@ -662,9 +678,9 @@ dead_debug_insert_temp (struct dead_debu
 	  /* Ok, it's the same (hardware) REG, but with a different
 	     mode, so SUBREG it.  */
 	  else
-	    breg = lowpart_subreg (GET_MODE (reg),
-				   cleanup_auto_inc_dec (src, VOIDmode),
-				   GET_MODE (dest));
+	    breg = debug_lowpart_subreg (GET_MODE (reg),
+					 cleanup_auto_inc_dec (src, VOIDmode),
+					 GET_MODE (dest));
 	}
       else if (GET_CODE (dest) == SUBREG)
 	{
@@ -684,9 +700,9 @@ dead_debug_insert_temp (struct dead_debu
 	    breg = NULL;
 	  /* Yay, we can use SRC, just adjust its mode.  */
 	  else
-	    breg = lowpart_subreg (GET_MODE (reg),
-				   cleanup_auto_inc_dec (src, VOIDmode),
-				   GET_MODE (dest));
+	    breg = debug_lowpart_subreg (GET_MODE (reg),
+					 cleanup_auto_inc_dec (src, VOIDmode),
+					 GET_MODE (dest));
 	}
       /* Oh well, we're out of luck.  */
       else
@@ -740,7 +756,8 @@ dead_debug_insert_temp (struct dead_debu
 	*DF_REF_REAL_LOC (cur->use) = dval;
       else
 	*DF_REF_REAL_LOC (cur->use)
-	  = gen_lowpart_SUBREG (GET_MODE (*DF_REF_REAL_LOC (cur->use)), dval);
+	  = debug_lowpart_subreg (GET_MODE (*DF_REF_REAL_LOC (cur->use)), dval,
+				  GET_MODE (dval));
       /* ??? Should we simplify subreg of subreg?  */
       bitmap_set_bit (debug->to_rescan, INSN_UID (DF_REF_INSN (cur->use)));
       uses = cur->next;
--- gcc/testsuite/g++.dg/debug/pr65678.C.jj	2015-04-07 11:18:51.701891845 +0200
+++ gcc/testsuite/g++.dg/debug/pr65678.C	2015-04-07 11:19:50.285943861 +0200
@@ -0,0 +1,35 @@
+// PR debug/65678
+// { dg-do compile }
+
+long long v;
+
+static int
+bar (double x)
+{
+#if __SIZEOF_DOUBLE__ == __SIZEOF_LONG_LONG__
+  __builtin_memmove (&v, &x, sizeof v);
+#else
+  (void) x;
+  v = 0;
+#endif
+  return v;
+}
+
+struct A
+{
+  A (double x) : a (bar (x)) {}
+  int m1 ();
+  int m2 () { int b = a; return b; }
+  int a;
+};
+
+void foo ();
+
+void
+baz (double x)
+{
+  int c = A (x).m2 ();
+  int d = A (x).m1 ();
+  if (d)
+    foo ();
+}

	Jakub

Re: [PATCH] Fix up creation of lowpart subregs in valtrack (PR debug/65678)

[Richard Biener]

On Tue, 7 Apr 2015, Jakub Jelinek wrote:

> Hi!
> 
> gen_rtx_SUBREG/gen_lowpart_SUBREG ICE when trying to create a subreg
> the backend doesn't like, lowpart_subreg returns NULL.  But, for debug info
> purposes, all lowpart subregs are valid, a subreg in the debug insns or
> var-tracking notes is simply a mode punning construct, and even when the
> backend doesn't have any instruction to perform such operation, the debugger
> can always do the punning.
> 
> var-tracking.c already has code to emit gen_rtx_raw_SUBREG if everything
> else fails, the following patch adjusts valtrack.c to do the same thing.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Ok.

Thanks,
Richard.

> 2015-04-07  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR debug/65678
> 	* valtrack.c (debug_lowpart_subreg): New function.
> 	(dead_debug_insert_temp): Use it.
> 
> 	* g++.dg/debug/pr65678.C: New test.
> 
> --- gcc/valtrack.c.jj	2015-01-05 13:07:14.000000000 +0100
> +++ gcc/valtrack.c	2015-04-07 11:10:36.045912355 +0200
> @@ -534,6 +534,22 @@ dead_debug_add (struct dead_debug_local
>    bitmap_set_bit (debug->used, uregno);
>  }
>  
> +/* Like lowpart_subreg, but if a subreg is not valid for machine, force
> +   it anyway - for use in debug insns.  */
> +
> +static rtx
> +debug_lowpart_subreg (machine_mode outer_mode, rtx expr,
> +		      machine_mode inner_mode)
> +{
> +  if (inner_mode == VOIDmode)
> +    inner_mode = GET_MODE (expr);
> +  int offset = subreg_lowpart_offset (outer_mode, inner_mode);
> +  rtx ret = simplify_gen_subreg (outer_mode, expr, inner_mode, offset);
> +  if (ret)
> +    return ret;
> +  return gen_rtx_raw_SUBREG (outer_mode, expr, offset);
> +}
> +
>  /* If UREGNO is referenced by any entry in DEBUG, emit a debug insn
>     before or after INSN (depending on WHERE), that binds a (possibly
>     global) debug temp to the widest-mode use of UREGNO, if WHERE is
> @@ -662,9 +678,9 @@ dead_debug_insert_temp (struct dead_debu
>  	  /* Ok, it's the same (hardware) REG, but with a different
>  	     mode, so SUBREG it.  */
>  	  else
> -	    breg = lowpart_subreg (GET_MODE (reg),
> -				   cleanup_auto_inc_dec (src, VOIDmode),
> -				   GET_MODE (dest));
> +	    breg = debug_lowpart_subreg (GET_MODE (reg),
> +					 cleanup_auto_inc_dec (src, VOIDmode),
> +					 GET_MODE (dest));
>  	}
>        else if (GET_CODE (dest) == SUBREG)
>  	{
> @@ -684,9 +700,9 @@ dead_debug_insert_temp (struct dead_debu
>  	    breg = NULL;
>  	  /* Yay, we can use SRC, just adjust its mode.  */
>  	  else
> -	    breg = lowpart_subreg (GET_MODE (reg),
> -				   cleanup_auto_inc_dec (src, VOIDmode),
> -				   GET_MODE (dest));
> +	    breg = debug_lowpart_subreg (GET_MODE (reg),
> +					 cleanup_auto_inc_dec (src, VOIDmode),
> +					 GET_MODE (dest));
>  	}
>        /* Oh well, we're out of luck.  */
>        else
> @@ -740,7 +756,8 @@ dead_debug_insert_temp (struct dead_debu
>  	*DF_REF_REAL_LOC (cur->use) = dval;
>        else
>  	*DF_REF_REAL_LOC (cur->use)
> -	  = gen_lowpart_SUBREG (GET_MODE (*DF_REF_REAL_LOC (cur->use)), dval);
> +	  = debug_lowpart_subreg (GET_MODE (*DF_REF_REAL_LOC (cur->use)), dval,
> +				  GET_MODE (dval));
>        /* ??? Should we simplify subreg of subreg?  */
>        bitmap_set_bit (debug->to_rescan, INSN_UID (DF_REF_INSN (cur->use)));
>        uses = cur->next;
> --- gcc/testsuite/g++.dg/debug/pr65678.C.jj	2015-04-07 11:18:51.701891845 +0200
> +++ gcc/testsuite/g++.dg/debug/pr65678.C	2015-04-07 11:19:50.285943861 +0200
> @@ -0,0 +1,35 @@
> +// PR debug/65678
> +// { dg-do compile }
> +
> +long long v;
> +
> +static int
> +bar (double x)
> +{
> +#if __SIZEOF_DOUBLE__ == __SIZEOF_LONG_LONG__
> +  __builtin_memmove (&v, &x, sizeof v);
> +#else
> +  (void) x;
> +  v = 0;
> +#endif
> +  return v;
> +}
> +
> +struct A
> +{
> +  A (double x) : a (bar (x)) {}
> +  int m1 ();
> +  int m2 () { int b = a; return b; }
> +  int a;
> +};
> +
> +void foo ();
> +
> +void
> +baz (double x)
> +{
> +  int c = A (x).m2 ();
> +  int d = A (x).m1 ();
> +  if (d)
> +    foo ();
> +}
> 
> 	Jakub
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Jennifer Guild,
Dilip Upmanyu, Graham Norton HRB 21284 (AG Nuernberg)

Re: [PATCH] Fix up creation of lowpart subregs in valtrack (PR debug/65678)

[Richard Sandiford]

Jakub Jelinek <jakub@redhat.com> writes:
> Hi!
>
> gen_rtx_SUBREG/gen_lowpart_SUBREG ICE when trying to create a subreg
> the backend doesn't like, lowpart_subreg returns NULL.  But, for debug info
> purposes, all lowpart subregs are valid, a subreg in the debug insns or
> var-tracking notes is simply a mode punning construct, and even when the
> backend doesn't have any instruction to perform such operation, the debugger
> can always do the punning.
>
> var-tracking.c already has code to emit gen_rtx_raw_SUBREG if everything
> else fails, the following patch adjusts valtrack.c to do the same thing.

Looks like this has the potential to introduce nested subregs.

Also, the problem isn't just that the architecture can't do some accesses
easily (such as accessing the high part of a register).  It's also that
GCC's model of subregs doesn't always give the right answer.  E.g.
sometimes the endianess of a particular register is the opposite of what
GCC expects, or the value is modelled as occupying more than one register
but isn't evenly distributed through them.

I take your point about this being done in var-tracking.c, but it always
seemed dangerous there too...

Thanks,
Richard