Patchwork Fix an --enable-checking=rtl issue in var-tracking (PR debug/45849)

login
register
mail settings
Submitter Jakub Jelinek
Date Oct. 4, 2010, 3:42 p.m.
Message ID <20101004154211.GE10466@tyan-ft48-01.lab.bos.redhat.com>
Download mbox | patch
Permalink /patch/66703/
State New
Headers show

Comments

Jakub Jelinek - Oct. 4, 2010, 3:42 p.m.
Hi!

As swap_commutative_operands_p depends on REG_POINTER, and whether some
reg is REG_POINTER might change during var-tracking, simplification of
expressions that are rtx_equal_p might lead into expressions that aren't
rtx_equal_p any longer.  This breaks ENABLE_RTL_CHECKING var-tracking
verification that cur_loc_changed flag works correctly.
As I think that checking is still useful, and I think it is unlikely to
ensure that there won't be REG_POINTER changes through the function on one
REG, either we'd need to modify swap_commutative_operand_p etc.
to disregard REG_POINTER/MEM_POINTER in debug insns/var-tracking (debug info
certainly doesn't care about that stuff), or, as done in this patch, simply
try harder in the var-tracking --enable-checking=rtl check if the simple
check fails.  As it didn't hit until now, I'm not worried too much it will
need to be performed very often.

Bootstrapped/regtested on x86_64-linux and i686-linux.  Ok for trunk?

2010-10-04  Jakub Jelinek  <jakub@redhat.com>

	PR debug/45849
	* var-tracking.c (strip_pointer_flags): New function.
	(emit_note_insn_var_location): If rtx_equal_p check failed,
	retry on locations simplified with simplify_replace_fn_rtx
	and strip_pointer_flags as its callback.

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


	Jakub
Richard Henderson - Oct. 4, 2010, 5:01 p.m.
On 10/04/2010 08:42 AM, Jakub Jelinek wrote:
>  	{
>  	  gcc_assert (pnote);
> -	  gcc_assert (rtx_equal_p (PAT_VAR_LOCATION_LOC (pnote),
> -				   PAT_VAR_LOCATION_LOC (note_vl)));
> +	  if (!rtx_equal_p (PAT_VAR_LOCATION_LOC (pnote),
> +			    PAT_VAR_LOCATION_LOC (note_vl)))
> +	    {
> +	      /* There might be differences caused by REG_POINTER
> +		 differences.  REG_POINTER affects
> +		 swap_commutative_operands_p.  */
> +	      rtx old_vl = PAT_VAR_LOCATION_LOC (pnote);
> +	      rtx new_vl = PAT_VAR_LOCATION_LOC (note_vl);

Move these two variables out above the IF.

Ok with that change.


r~

Patch

--- gcc/var-tracking.c.jj	2010-09-30 15:22:13.000000000 +0200
+++ gcc/var-tracking.c	2010-10-04 12:21:39.549377631 +0200
@@ -7133,6 +7133,19 @@  vt_expand_loc_dummy (rtx loc, htab_t var
 #ifdef ENABLE_RTL_CHECKING
 /* Used to verify that cur_loc_changed updating is safe.  */
 static struct pointer_map_t *emitted_notes;
+
+/* Strip REG_POINTER from REGs and MEM_POINTER from MEMs in order to
+   avoid differences in commutative operand simplification.  */
+static rtx
+strip_pointer_flags (rtx x, const_rtx old_rtx ATTRIBUTE_UNUSED,
+		     void *data ATTRIBUTE_UNUSED)
+{
+  if (REG_P (x) && REG_POINTER (x))
+    return gen_rtx_REG (GET_MODE (x), REGNO (x));
+  if (MEM_P (x) && MEM_POINTER (x))
+    return gen_rtx_MEM (GET_MODE (x), XEXP (x, 0));
+  return NULL_RTX;
+}
 #endif
 
 /* Emit the NOTE_INSN_VAR_LOCATION for variable *VARP.  DATA contains
@@ -7332,8 +7345,21 @@  emit_note_insn_var_location (void **varp
       if (!var->cur_loc_changed && (pnote || PAT_VAR_LOCATION_LOC (note_vl)))
 	{
 	  gcc_assert (pnote);
-	  gcc_assert (rtx_equal_p (PAT_VAR_LOCATION_LOC (pnote),
-				   PAT_VAR_LOCATION_LOC (note_vl)));
+	  if (!rtx_equal_p (PAT_VAR_LOCATION_LOC (pnote),
+			    PAT_VAR_LOCATION_LOC (note_vl)))
+	    {
+	      /* There might be differences caused by REG_POINTER
+		 differences.  REG_POINTER affects
+		 swap_commutative_operands_p.  */
+	      rtx old_vl = PAT_VAR_LOCATION_LOC (pnote);
+	      rtx new_vl = PAT_VAR_LOCATION_LOC (note_vl);
+	      old_vl = simplify_replace_fn_rtx (old_vl, NULL_RTX,
+						strip_pointer_flags, NULL);
+	      new_vl = simplify_replace_fn_rtx (new_vl, NULL_RTX,
+						strip_pointer_flags, NULL);
+	      gcc_assert (rtx_equal_p (old_vl, new_vl));
+	      PAT_VAR_LOCATION_LOC (note_vl) = new_vl;
+	    }
 	}
       *note_slot = (void *) note_vl;
     }
--- gcc/testsuite/gcc.dg/debug/pr45849.c.jj	2010-10-04 12:28:33.255364734 +0200
+++ gcc/testsuite/gcc.dg/debug/pr45849.c	2010-10-04 12:30:27.966658821 +0200
@@ -0,0 +1,31 @@ 
+/* PR debug/45849 */
+/* { dg-do compile } */
+/* { dg-options "-g -Wno-uninitialized" } */
+
+extern void bar (void);
+
+void
+foo (long repllen, char *rp)
+{
+  char *matchend;
+  char *scan;
+  long len;
+  char *matchstart;
+  char *text;
+  char *t;
+
+  repllen--;
+
+  for (;;)
+    {
+      matchstart = t + rp[0];
+      matchend = rp;
+      len = matchstart - text + repllen * (matchend - matchstart);
+      while (len)
+	;
+      for (scan = text; scan != rp; scan++)
+	bar ();
+      if (matchstart)
+	text = matchend;
+    }
+}