Patchwork Avoid too complex debug insns during expansion (PR debug/56510)

login
register
mail settings
Submitter Jakub Jelinek
Date March 5, 2013, 4:30 p.m.
Message ID <20130305163023.GK12913@tucnak.redhat.com>
Download mbox | patch
Permalink /patch/225084/
State New
Headers show

Comments

Jakub Jelinek - March 5, 2013, 4:30 p.m.
Hi!

cselib (probably among others) isn't prepared to handle arbitrarily
complex debug insns.  The debug insns are usually created from debug stmts
which shouldn't have unbound complexity, but with TER we can actually end up
with arbitrarily large debug insns.

This patch fixes that up during expansion, by splitting subexpressions of
too large debug insn expressions into their own debug temporaries.

So far bootstrapped/regtested on x86_64-linux and i686-linux without the
first two hunks (it caused one failure on the latter because of invalid RTL
sharing), I'm going to bootstrap/regtest it again, ok for trunk if it
passes?

2013-03-05  Jakub Jelinek  <jakub@redhat.com>

	PR debug/56510
	* cfgexpand.c (expand_debug_parm_decl): Call copy_rtx on incoming.
	(avoid_complex_debug_insns): New function.
	(expand_debug_locations): Call it.

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


	Jakub
Jeff Law - March 5, 2013, 9:40 p.m.
On 03/05/2013 09:30 AM, Jakub Jelinek wrote:
> Hi!
>
> cselib (probably among others) isn't prepared to handle arbitrarily
> complex debug insns.  The debug insns are usually created from debug stmts
> which shouldn't have unbound complexity, but with TER we can actually end up
> with arbitrarily large debug insns.
>
> This patch fixes that up during expansion, by splitting subexpressions of
> too large debug insn expressions into their own debug temporaries.
>
> So far bootstrapped/regtested on x86_64-linux and i686-linux without the
> first two hunks (it caused one failure on the latter because of invalid RTL
> sharing), I'm going to bootstrap/regtest it again, ok for trunk if it
> passes?
>
> 2013-03-05  Jakub Jelinek  <jakub@redhat.com>
>
> 	PR debug/56510
> 	* cfgexpand.c (expand_debug_parm_decl): Call copy_rtx on incoming.
> 	(avoid_complex_debug_insns): New function.
> 	(expand_debug_locations): Call it.
>
> 	* gcc.dg/pr56510.c: New test.
So it's not that cselib (and possibly others) can't handle these complex 
RTL expressions, it's just unbearably slow.  Right?


>
>   }
>
> +/* Ensure INSN_VAR_LOCATION_LOC (insn) doesn't have unbound complexity.
> +   Allow 4 levels of rtl nesting for most rtl codes, and if we see anything
> +   deeper than that, create DEBUG_EXPRs and emit DEBUG_INSNs before INSN.  */
:-)  Similar to a comment I made in someone else's patch, I don't like 
the magic number "4", but I don't think this is worth creating a PARAM 
for controlling its behaviour.


> +
> +static void
> +avoid_complex_debug_insns (rtx insn, rtx *exp_p, int depth)
> +{
> +  rtx exp = *exp_p;
> +  if (exp == NULL_RTX)
> +    return;
> +  if ((OBJECT_P (exp) && !MEM_P (exp)) || GET_CODE (exp) == CLOBBER)
> +    return;
A blank line or two seems to be missing above.


Fine with the trivial formatting fix assuming your bootstrap/regtest is OK.


Jeff
Jakub Jelinek - March 5, 2013, 10:37 p.m.
On Tue, Mar 05, 2013 at 02:40:34PM -0700, Jeff Law wrote:
> So it's not that cselib (and possibly others) can't handle these
> complex RTL expressions, it's just unbearably slow.  Right?

They handle it, but with bad compile time complexity, so on some testcases
it might take years or centuries etc.

> Fine with the trivial formatting fix assuming your bootstrap/regtest is OK.

Thanks, bootstraps/regtests finished fine, I've added the two blank lines
and committed.

	Jakub
Richard Guenther - March 6, 2013, 8:30 a.m.
On Tue, 5 Mar 2013, Jakub Jelinek wrote:

> Hi!
> 
> cselib (probably among others) isn't prepared to handle arbitrarily
> complex debug insns.  The debug insns are usually created from debug stmts
> which shouldn't have unbound complexity, but with TER we can actually end up
> with arbitrarily large debug insns.
> 
> This patch fixes that up during expansion, by splitting subexpressions of
> too large debug insn expressions into their own debug temporaries.
> 
> So far bootstrapped/regtested on x86_64-linux and i686-linux without the
> first two hunks (it caused one failure on the latter because of invalid RTL
> sharing), I'm going to bootstrap/regtest it again, ok for trunk if it
> passes?

Works for me.

Richard.

> 2013-03-05  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR debug/56510
> 	* cfgexpand.c (expand_debug_parm_decl): Call copy_rtx on incoming.
> 	(avoid_complex_debug_insns): New function.
> 	(expand_debug_locations): Call it.
> 
> 	* gcc.dg/pr56510.c: New test.
> 
> --- gcc/cfgexpand.c.jj	2013-03-05 15:12:15.071565689 +0100
> +++ gcc/cfgexpand.c	2013-03-05 17:21:55.683602432 +0100
> @@ -2622,6 +2622,8 @@ expand_debug_parm_decl (tree decl)
>  	      reg = gen_raw_REG (GET_MODE (reg), OUTGOING_REGNO (REGNO (reg)));
>  	      incoming = replace_equiv_address_nv (incoming, reg);
>  	    }
> +	  else
> +	    incoming = copy_rtx (incoming);
>  	}
>  #endif
>  
> @@ -2637,7 +2639,7 @@ expand_debug_parm_decl (tree decl)
>  	  || (GET_CODE (XEXP (incoming, 0)) == PLUS
>  	      && XEXP (XEXP (incoming, 0), 0) == virtual_incoming_args_rtx
>  	      && CONST_INT_P (XEXP (XEXP (incoming, 0), 1)))))
> -    return incoming;
> +    return copy_rtx (incoming);
>  
>    return NULL_RTX;
>  }
> @@ -3704,6 +3706,54 @@ expand_debug_source_expr (tree exp)
>    return op0;
>  }
>  
> +/* Ensure INSN_VAR_LOCATION_LOC (insn) doesn't have unbound complexity.
> +   Allow 4 levels of rtl nesting for most rtl codes, and if we see anything
> +   deeper than that, create DEBUG_EXPRs and emit DEBUG_INSNs before INSN.  */
> +
> +static void
> +avoid_complex_debug_insns (rtx insn, rtx *exp_p, int depth)
> +{
> +  rtx exp = *exp_p;
> +  if (exp == NULL_RTX)
> +    return;
> +  if ((OBJECT_P (exp) && !MEM_P (exp)) || GET_CODE (exp) == CLOBBER)
> +    return;
> +
> +  if (depth == 4)
> +    {
> +      /* Create DEBUG_EXPR (and DEBUG_EXPR_DECL).  */
> +      rtx dval = make_debug_expr_from_rtl (exp);
> +
> +      /* Emit a debug bind insn before INSN.  */
> +      rtx bind = gen_rtx_VAR_LOCATION (GET_MODE (exp),
> +				       DEBUG_EXPR_TREE_DECL (dval), exp,
> +				       VAR_INIT_STATUS_INITIALIZED);
> +
> +      emit_debug_insn_before (bind, insn);
> +      *exp_p = dval;
> +      return;
> +    }
> +
> +  const char *format_ptr = GET_RTX_FORMAT (GET_CODE (exp));
> +  int i, j;
> +  for (i = 0; i < GET_RTX_LENGTH (GET_CODE (exp)); i++)
> +    switch (*format_ptr++)
> +      {
> +      case 'e':
> +	avoid_complex_debug_insns (insn, &XEXP (exp, i), depth + 1);
> +	break;
> +
> +      case 'E':
> +      case 'V':
> +	for (j = 0; j < XVECLEN (exp, i); j++)
> +	  avoid_complex_debug_insns (insn, &XVECEXP (exp, i, j), depth + 1);
> +	break;
> +
> +      default:
> +	break;
> +      }
> +}
> +
>  /* Expand the _LOCs in debug insns.  We run this after expanding all
>     regular insns, so that any variables referenced in the function
>     will have their DECL_RTLs set.  */
> @@ -3724,7 +3774,7 @@ expand_debug_locations (void)
>      if (DEBUG_INSN_P (insn))
>        {
>  	tree value = (tree)INSN_VAR_LOCATION_LOC (insn);
> -	rtx val;
> +	rtx val, prev_insn, insn2;
>  	enum machine_mode mode;
>  
>  	if (value == NULL_TREE)
> @@ -3753,6 +3803,9 @@ expand_debug_locations (void)
>  	  }
>  
>  	INSN_VAR_LOCATION_LOC (insn) = val;
> +	prev_insn = PREV_INSN (insn);
> +	for (insn2 = insn; insn2 != prev_insn; insn2 = PREV_INSN (insn2))
> +	  avoid_complex_debug_insns (insn2, &INSN_VAR_LOCATION_LOC (insn2), 0);
>        }
>  
>    flag_strict_aliasing = save_strict_alias;
> --- gcc/testsuite/gcc.dg/pr56510.c.jj	2013-03-05 16:57:54.498939220 +0100
> +++ gcc/testsuite/gcc.dg/pr56510.c	2013-03-05 16:57:54.499939214 +0100
> @@ -0,0 +1,37 @@
> +/* PR debug/56510 */
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -g" } */
> +
> +struct S { unsigned long s1; void **s2[0]; };
> +void **a, **b, **c, **d, **e, **f;
> +
> +static void **
> +baz (long x, long y)
> +{
> +  void **s = f;
> +  *f = (void **) (y << 8 | (x & 0xff));
> +  f += y + 1;
> +  return s;
> +}
> +
> +void bar (void);
> +void
> +foo (void)
> +{
> +  void **g = b[4];
> +  a = b[2];
> +  b = b[1];
> +  g[2] = e;
> +  void **h
> +    = ((void **************************)
> +       a)[1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][66];
> +  void **i = ((struct S *) h)->s2[4];
> +  d = baz (4, 3);
> +  d[1] = b;
> +  d[2] = a;
> +  d[3] = bar;
> +  b = d;
> +  g[1] = i[2];
> +  a = g;
> +  ((void (*) (void)) (i[1])) ();
> +}
> 
> 	Jakub
> 
>

Patch

--- gcc/cfgexpand.c.jj	2013-03-05 15:12:15.071565689 +0100
+++ gcc/cfgexpand.c	2013-03-05 17:21:55.683602432 +0100
@@ -2622,6 +2622,8 @@  expand_debug_parm_decl (tree decl)
 	      reg = gen_raw_REG (GET_MODE (reg), OUTGOING_REGNO (REGNO (reg)));
 	      incoming = replace_equiv_address_nv (incoming, reg);
 	    }
+	  else
+	    incoming = copy_rtx (incoming);
 	}
 #endif
 
@@ -2637,7 +2639,7 @@  expand_debug_parm_decl (tree decl)
 	  || (GET_CODE (XEXP (incoming, 0)) == PLUS
 	      && XEXP (XEXP (incoming, 0), 0) == virtual_incoming_args_rtx
 	      && CONST_INT_P (XEXP (XEXP (incoming, 0), 1)))))
-    return incoming;
+    return copy_rtx (incoming);
 
   return NULL_RTX;
 }
@@ -3704,6 +3706,54 @@  expand_debug_source_expr (tree exp)
   return op0;
 }
 
+/* Ensure INSN_VAR_LOCATION_LOC (insn) doesn't have unbound complexity.
+   Allow 4 levels of rtl nesting for most rtl codes, and if we see anything
+   deeper than that, create DEBUG_EXPRs and emit DEBUG_INSNs before INSN.  */
+
+static void
+avoid_complex_debug_insns (rtx insn, rtx *exp_p, int depth)
+{
+  rtx exp = *exp_p;
+  if (exp == NULL_RTX)
+    return;
+  if ((OBJECT_P (exp) && !MEM_P (exp)) || GET_CODE (exp) == CLOBBER)
+    return;
+
+  if (depth == 4)
+    {
+      /* Create DEBUG_EXPR (and DEBUG_EXPR_DECL).  */
+      rtx dval = make_debug_expr_from_rtl (exp);
+
+      /* Emit a debug bind insn before INSN.  */
+      rtx bind = gen_rtx_VAR_LOCATION (GET_MODE (exp),
+				       DEBUG_EXPR_TREE_DECL (dval), exp,
+				       VAR_INIT_STATUS_INITIALIZED);
+
+      emit_debug_insn_before (bind, insn);
+      *exp_p = dval;
+      return;
+    }
+
+  const char *format_ptr = GET_RTX_FORMAT (GET_CODE (exp));
+  int i, j;
+  for (i = 0; i < GET_RTX_LENGTH (GET_CODE (exp)); i++)
+    switch (*format_ptr++)
+      {
+      case 'e':
+	avoid_complex_debug_insns (insn, &XEXP (exp, i), depth + 1);
+	break;
+
+      case 'E':
+      case 'V':
+	for (j = 0; j < XVECLEN (exp, i); j++)
+	  avoid_complex_debug_insns (insn, &XVECEXP (exp, i, j), depth + 1);
+	break;
+
+      default:
+	break;
+      }
+}
+
 /* Expand the _LOCs in debug insns.  We run this after expanding all
    regular insns, so that any variables referenced in the function
    will have their DECL_RTLs set.  */
@@ -3724,7 +3774,7 @@  expand_debug_locations (void)
     if (DEBUG_INSN_P (insn))
       {
 	tree value = (tree)INSN_VAR_LOCATION_LOC (insn);
-	rtx val;
+	rtx val, prev_insn, insn2;
 	enum machine_mode mode;
 
 	if (value == NULL_TREE)
@@ -3753,6 +3803,9 @@  expand_debug_locations (void)
 	  }
 
 	INSN_VAR_LOCATION_LOC (insn) = val;
+	prev_insn = PREV_INSN (insn);
+	for (insn2 = insn; insn2 != prev_insn; insn2 = PREV_INSN (insn2))
+	  avoid_complex_debug_insns (insn2, &INSN_VAR_LOCATION_LOC (insn2), 0);
       }
 
   flag_strict_aliasing = save_strict_alias;
--- gcc/testsuite/gcc.dg/pr56510.c.jj	2013-03-05 16:57:54.498939220 +0100
+++ gcc/testsuite/gcc.dg/pr56510.c	2013-03-05 16:57:54.499939214 +0100
@@ -0,0 +1,37 @@ 
+/* PR debug/56510 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -g" } */
+
+struct S { unsigned long s1; void **s2[0]; };
+void **a, **b, **c, **d, **e, **f;
+
+static void **
+baz (long x, long y)
+{
+  void **s = f;
+  *f = (void **) (y << 8 | (x & 0xff));
+  f += y + 1;
+  return s;
+}
+
+void bar (void);
+void
+foo (void)
+{
+  void **g = b[4];
+  a = b[2];
+  b = b[1];
+  g[2] = e;
+  void **h
+    = ((void **************************)
+       a)[1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][1][66];
+  void **i = ((struct S *) h)->s2[4];
+  d = baz (4, 3);
+  d[1] = b;
+  d[2] = a;
+  d[3] = bar;
+  b = d;
+  g[1] = i[2];
+  a = g;
+  ((void (*) (void)) (i[1])) ();
+}