introduce --param max-vartrack-expr-depth

One of my patches for PR 48866 regressed guality/asm-1.c on
x86_64-linux-gnu because what used to be a single complex debug value
expression became a chain of debug temps holding simpler expressions,
and this chain exceeded the default recursion depth in resolving
location expressions.

This patch introduces a param to control this depth, so that one can
trade compile time for better debug info, and bumps up the default depth
a bit, avoiding the regression.

Regstrapped on x86_64-linux-gnu and i686-linux-gnu.  Ok to install?

On Mon, May 30, 2011 at 07:35:58AM -0300, Alexandre Oliva wrote:
> One of my patches for PR 48866 regressed guality/asm-1.c on
> x86_64-linux-gnu because what used to be a single complex debug value
> expression became a chain of debug temps holding simpler expressions,
> and this chain exceeded the default recursion depth in resolving
> location expressions.
> 
> This patch introduces a param to control this depth, so that one can
> trade compile time for better debug info, and bumps up the default depth
> a bit, avoiding the regression.
> 
> Regstrapped on x86_64-linux-gnu and i686-linux-gnu.  Ok to install?

I support this, after all I had to bump it from 8 to 10 already in
my http://gcc.gnu.org/ml/gcc-patches/2011-05/msg01182.html pending review.

> for  gcc/ChangeLog
> from  Alexandre Oliva  <aoliva@redhat.com>
> 
> 	* params.def (PARAM_MAX_VARTRACK_EXPR_DEPTH): New.
> 	* doc/invoke.texi: Document max-vartrack-expr-depth.
> 	* var-tracking.c (EXPR_DEPTH): New.
> 	(reverse_op, vt_expand_loc, vt_expand_loc_dummy): Use it.
> 
> Index: gcc/params.def
> ===================================================================
> --- gcc/params.def.orig	2011-05-30 03:16:26.496342009 -0300
> +++ gcc/params.def	2011-05-30 03:35:17.852414343 -0300
> @@ -839,6 +839,14 @@ DEFPARAM (PARAM_MAX_VARTRACK_SIZE,
>  	  "Max. size of var tracking hash tables",
>  	  50000000, 0, 0)
>  
> +/* Set maximum recursion depth for var tracking expression expansion
> +   and resolution.  */
> +
> +DEFPARAM (PARAM_MAX_VARTRACK_EXPR_DEPTH,
> +	  "max-vartrack-expr-depth",
> +	  "Max. recursion depth for expanding var tracking expressions",
> +	  10, 0, 0)
> +
>  /* Set minimum insn uid for non-debug insns.  */
>  
>  DEFPARAM (PARAM_MIN_NONDEBUG_INSN_UID,
> Index: gcc/doc/invoke.texi
> ===================================================================
> --- gcc/doc/invoke.texi.orig	2011-05-30 03:40:14.716854423 -0300
> +++ gcc/doc/invoke.texi	2011-05-30 03:46:29.349288790 -0300
> @@ -8866,6 +8866,16 @@ the function.  If the limit is exceeded 
>  tracking analysis is completely disabled for the function.  Setting
>  the parameter to zero makes it unlimited.
>  
> +@item max-vartrack-expr-depth
> +Sets a maximum number of recursion levels when attempting to map
> +variable names or debug temporaries to value expressions.  This trades
> +compile time for more complete debug information.  If this is set too
> +low, value expressions that are available and could be represented in
> +debug information may end up not being used; setting this higher may
> +enable the compiler to find more complex debug expressions, but compile
> +time may grow exponentially, and even then, it may fail to find more
> +usable expressions.  The default is 10.
> +
>  @item min-nondebug-insn-uid
>  Use uids starting at this parameter for nondebug insns.  The range below
>  the parameter is reserved exclusively for debug insns created by
> Index: gcc/var-tracking.c
> ===================================================================
> --- gcc/var-tracking.c.orig	2011-05-30 03:40:46.135898766 -0300
> +++ gcc/var-tracking.c	2011-05-30 03:39:35.745798000 -0300
> @@ -5215,6 +5215,8 @@ add_uses_1 (rtx *x, void *cui)
>    for_each_rtx (x, add_uses, cui);
>  }
>  
> +#define EXPR_DEPTH (PARAM_VALUE (PARAM_MAX_VARTRACK_EXPR_DEPTH))
> +
>  /* Attempt to reverse the EXPR operation in the debug info.  Say for
>     reg1 = reg2 + 6 even when reg2 is no longer live we
>     can express its value as VAL - 6.  */
> @@ -5286,7 +5288,7 @@ reverse_op (rtx val, const_rtx expr)
>        arg = XEXP (src, 1);
>        if (!CONST_INT_P (arg) && GET_CODE (arg) != SYMBOL_REF)
>  	{
> -	  arg = cselib_expand_value_rtx (arg, scratch_regs, 5);
> +	  arg = cselib_expand_value_rtx (arg, scratch_regs, EXPR_DEPTH);
>  	  if (arg == NULL_RTX)
>  	    return NULL_RTX;
>  	  if (!CONST_INT_P (arg) && GET_CODE (arg) != SYMBOL_REF)
> @@ -7416,7 +7418,7 @@ vt_expand_loc (rtx loc, htab_t vars, boo
>    data.dummy = false;
>    data.cur_loc_changed = false;
>    data.ignore_cur_loc = ignore_cur_loc;
> -  loc = cselib_expand_value_rtx_cb (loc, scratch_regs, 8,
> +  loc = cselib_expand_value_rtx_cb (loc, scratch_regs, EXPR_DEPTH,
>  				    vt_expand_loc_callback, &data);
>  
>    if (loc && MEM_P (loc))
> @@ -7438,7 +7440,7 @@ vt_expand_loc_dummy (rtx loc, htab_t var
>    data.dummy = true;
>    data.cur_loc_changed = false;
>    data.ignore_cur_loc = false;
> -  ret = cselib_dummy_expand_value_rtx_cb (loc, scratch_regs, 8,
> +  ret = cselib_dummy_expand_value_rtx_cb (loc, scratch_regs, EXPR_DEPTH,
>  					  vt_expand_loc_callback, &data);
>    *pcur_loc_changed = data.cur_loc_changed;
>    return ret;

	Jakub

On 05/30/2011 12:35 PM, Alexandre Oliva wrote:
> One of my patches for PR 48866 regressed guality/asm-1.c on
> x86_64-linux-gnu because what used to be a single complex debug value
> expression became a chain of debug temps holding simpler expressions,
> and this chain exceeded the default recursion depth in resolving
> location expressions.

What's the worst that can happen if you remove the limit altogether?
Assuming that would make the compiler blow up, the patch is ok.


Bernd

On May 30, 2011, Bernd Schmidt <bernds@codesourcery.com> wrote:

> On 05/30/2011 12:35 PM, Alexandre Oliva wrote:
>> One of my patches for PR 48866 regressed guality/asm-1.c on
>> x86_64-linux-gnu because what used to be a single complex debug value
>> expression became a chain of debug temps holding simpler expressions,
>> and this chain exceeded the default recursion depth in resolving
>> location expressions.

> What's the worst that can happen if you remove the limit altogether?

Exponential behavior comes to mind.  I will try a bootstrap with a very
high value, but for pathological cases we'd probably still need the
param anyway, so I'll check it in.  Thanks for the reviews.

On Tue, May 31, 2011 at 01:13:31PM -0300, Alexandre Oliva wrote:
> On May 30, 2011, Bernd Schmidt <bernds@codesourcery.com> wrote:
> 
> > On 05/30/2011 12:35 PM, Alexandre Oliva wrote:
> >> One of my patches for PR 48866 regressed guality/asm-1.c on
> >> x86_64-linux-gnu because what used to be a single complex debug value
> >> expression became a chain of debug temps holding simpler expressions,
> >> and this chain exceeded the default recursion depth in resolving
> >> location expressions.
> 
> > What's the worst that can happen if you remove the limit altogether?
> 
> Exponential behavior comes to mind.  I will try a bootstrap with a very
> high value, but for pathological cases we'd probably still need the
> param anyway, so I'll check it in.  Thanks for the reviews.

Yeah, before introduction of cselib_dummy_expand_value_rtx_cb
it used to be far more urgent, because with high depth values we'd create
tons of garbage even when it wouldn't lead to anything reasonable, but still
with very high depth values we could end up eating too much memory
and compiler time.  That said, perhaps default of say 20 instead of 10 wouldn't
be bad...
I'm not sure you want to bump the depth in reversible_op, because there it
blindly creates new rtx and folds them (on the other side, it doesn't have
a callback there, so it really doesn't matter how long chain of
DEBUG_EXPR_DECLs is).  I think 5 should be enough to get through LO_SUM
etc., perhaps it could be bumped a tiny bit but not too much.

	Jakub

On May 31, 2011, Alexandre Oliva <aoliva@redhat.com> wrote:

> On May 30, 2011, Bernd Schmidt <bernds@codesourcery.com> wrote:
>> On 05/30/2011 12:35 PM, Alexandre Oliva wrote:
>>> One of my patches for PR 48866 regressed guality/asm-1.c on
>>> x86_64-linux-gnu because what used to be a single complex debug value
>>> expression became a chain of debug temps holding simpler expressions,
>>> and this chain exceeded the default recursion depth in resolving
>>> location expressions.

>> What's the worst that can happen if you remove the limit altogether?

> Exponential behavior comes to mind.

It's unusual, but debug/pr41264-1.c exhibits it, given INT_MAX for the
param, even though under such a (lack of) limit bootstrap doesn't go
slower or faster, after restoring depth 5 for the reverse_op() use.  As
Jakub pointed out, that one probably shouldn't be affected by the
parameter, as depth 5 is exactly what we want for the kind of expression
we're looking for.  With unlimited depth for that one, not even
libiberty/md5.c compiles successfully, exhausting memory on a box with
some 40GB of total VM (8+32).

So I guess I'll stick with what I checked in, but keep a patch handy to
bump the limit a little bit up and revert to 5 in reverse_op.