| Message ID | 20130305163023.GK12913@tucnak.redhat.com |
|---|---|
| State | New |
| Headers | show |
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
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
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 > >
--- 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])) (); +}