introduce --param max-vartrack-expr-depth

introduce --param max-vartrack-expr-depth

[Alexandre Oliva]

[-- Attachment #1: Type: text/plain, Size: 532 bytes --]

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?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: vta-expr-depth-param.patch --]
[-- Type: text/x-diff, Size: 3752 bytes --]

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;

[-- Attachment #3: Type: text/plain, Size: 258 bytes --]



-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist      Red Hat Brazil Compiler Engineer

Re: introduce --param max-vartrack-expr-depth

[Jakub Jelinek]

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

Re: introduce --param max-vartrack-expr-depth

[Bernd Schmidt]

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

Re: introduce --param max-vartrack-expr-depth

[Alexandre Oliva]

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.

-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist      Red Hat Brazil Compiler Engineer

Re: introduce --param max-vartrack-expr-depth

[Jakub Jelinek]

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

Re: introduce --param max-vartrack-expr-depth

[Alexandre Oliva]

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.

-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist      Red Hat Brazil Compiler Engineer

Re: introduce --param max-vartrack-expr-depth

[Alexandre Oliva]

[-- Attachment #1: Type: text/plain, Size: 1361 bytes --]

On Jun  1, 2011, Alexandre Oliva <aoliva@redhat.com> wrote:

> 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.

Such as this one...


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: vta-param-vartrack-expr-depth-int-max.patch --]
[-- Type: text/x-diff, Size: 1086 bytes --]

Index: gcc/params.def
===================================================================
--- gcc/params.def.orig	2011-05-31 18:28:05.348070586 -0300
+++ gcc/params.def	2011-06-01 17:09:41.117140944 -0300
@@ -845,7 +845,7 @@ DEFPARAM (PARAM_MAX_VARTRACK_SIZE,
 DEFPARAM (PARAM_MAX_VARTRACK_EXPR_DEPTH,
 	  "max-vartrack-expr-depth",
 	  "Max. recursion depth for expanding var tracking expressions",
-	  10, 0, 0)
+	  20, 0, 0)
 
 /* Set minimum insn uid for non-debug insns.  */
 
Index: gcc/var-tracking.c
===================================================================
--- gcc/var-tracking.c.orig	2011-05-31 20:06:25.604477956 -0300
+++ gcc/var-tracking.c	2011-05-31 23:56:06.578450957 -0300
@@ -5288,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, EXPR_DEPTH);
+	  arg = cselib_expand_value_rtx (arg, scratch_regs, 5);
 	  if (arg == NULL_RTX)
 	    return NULL_RTX;
 	  if (!CONST_INT_P (arg) && GET_CODE (arg) != SYMBOL_REF)

[-- Attachment #3: Type: text/plain, Size: 258 bytes --]



-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist      Red Hat Brazil Compiler Engineer

Re: introduce --param max-vartrack-expr-depth

[Jakub Jelinek]

On Wed, Jun 01, 2011 at 07:25:39PM -0300, Alexandre Oliva wrote:
> Such as this one...

I'd appreciate if this could go in...

> Index: gcc/params.def
> ===================================================================
> --- gcc/params.def.orig	2011-05-31 18:28:05.348070586 -0300
> +++ gcc/params.def	2011-06-01 17:09:41.117140944 -0300
> @@ -845,7 +845,7 @@ DEFPARAM (PARAM_MAX_VARTRACK_SIZE,
>  DEFPARAM (PARAM_MAX_VARTRACK_EXPR_DEPTH,
>  	  "max-vartrack-expr-depth",
>  	  "Max. recursion depth for expanding var tracking expressions",
> -	  10, 0, 0)
> +	  20, 0, 0)
>  
>  /* Set minimum insn uid for non-debug insns.  */
>  
> Index: gcc/var-tracking.c
> ===================================================================
> --- gcc/var-tracking.c.orig	2011-05-31 20:06:25.604477956 -0300
> +++ gcc/var-tracking.c	2011-05-31 23:56:06.578450957 -0300
> @@ -5288,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, EXPR_DEPTH);
> +	  arg = cselib_expand_value_rtx (arg, scratch_regs, 5);
>  	  if (arg == NULL_RTX)
>  	    return NULL_RTX;
>  	  if (!CONST_INT_P (arg) && GET_CODE (arg) != SYMBOL_REF)

	Jakub

Re: introduce --param max-vartrack-expr-depth

[Bernd Schmidt]

On 06/02/2011 10:46 AM, Jakub Jelinek wrote:
> On Wed, Jun 01, 2011 at 07:25:39PM -0300, Alexandre Oliva wrote:
>> Such as this one...
> 
> I'd appreciate if this could go in...

Go on then.

Bernd

Re: introduce --param max-vartrack-expr-depth

[Alexandre Oliva]

[-- Attachment #1: Type: text/plain, Size: 303 bytes --]

On Jun  2, 2011, Bernd Schmidt <bernds@codesourcery.com> wrote:

> On 06/02/2011 10:46 AM, Jakub Jelinek wrote:
>> On Wed, Jun 01, 2011 at 07:25:39PM -0300, Alexandre Oliva wrote:
>>> Such as this one...
>> 
>> I'd appreciate if this could go in...

> Go on then.

Ok, here's what I've just installed.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: vta-param-vartrack-expr-depth-int-max.patch --]
[-- Type: text/x-diff, Size: 1276 bytes --]

for  gcc/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	* params.def (PARAM_MAX_VARTRACK_EXPR_DEPTH): Bump default to 10.
	* var-tracking.c (reverse_op): Limite recurse depth to 5.

Index: gcc/params.def
===================================================================
--- gcc/params.def.orig	2011-05-31 18:28:05.348070586 -0300
+++ gcc/params.def	2011-06-01 17:09:41.117140944 -0300
@@ -845,7 +845,7 @@ DEFPARAM (PARAM_MAX_VARTRACK_SIZE,
 DEFPARAM (PARAM_MAX_VARTRACK_EXPR_DEPTH,
 	  "max-vartrack-expr-depth",
 	  "Max. recursion depth for expanding var tracking expressions",
-	  10, 0, 0)
+	  20, 0, 0)
 
 /* Set minimum insn uid for non-debug insns.  */
 
Index: gcc/var-tracking.c
===================================================================
--- gcc/var-tracking.c.orig	2011-05-31 20:06:25.604477956 -0300
+++ gcc/var-tracking.c	2011-05-31 23:56:06.578450957 -0300
@@ -5288,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, EXPR_DEPTH);
+	  arg = cselib_expand_value_rtx (arg, scratch_regs, 5);
 	  if (arg == NULL_RTX)
 	    return NULL_RTX;
 	  if (!CONST_INT_P (arg) && GET_CODE (arg) != SYMBOL_REF)

[-- Attachment #3: Type: text/plain, Size: 258 bytes --]



-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist      Red Hat Brazil Compiler Engineer

Re: introduce --param max-vartrack-expr-depth

[Michael Eager]

On 05/31/2011 09:13 AM, 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.

I've run into a problem with this change when building microblaze-xilinx-elf.

When compiling _divdi3.o, cselib_expand_value_rtx_1 returns a huge rtx
tree for variable _r1 when max_depth is greater than 17.  If -g is
specified, this later results in attempting to generate a DWARF location
list much larger than the 0xffff size limit, resulting in an assert failure.


Changelog:

2011-07-20  Michael Eager  <eager@eagercon.com>

         * params.def (PARAM_MAX_VARTRACK_EXPR_DEPTH): Default to 10.

Index: gcc/params.def
===================================================================
--- gcc/params.def      (revision 176533)
+++ gcc/params.def      (working copy)
@@ -845,7 +845,7 @@
  DEFPARAM (PARAM_MAX_VARTRACK_EXPR_DEPTH,
           "max-vartrack-expr-depth",
           "Max. recursion depth for expanding var tracking expressions",
-         20, 0, 0)
+         10, 0, 0)


-- 
Michael Eager	 eager@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306  650-325-8077

Re: introduce --param max-vartrack-expr-depth

[Jakub Jelinek]

On Wed, Jul 20, 2011 at 01:07:40PM -0700, Michael Eager wrote:
> I've run into a problem with this change when building microblaze-xilinx-elf.
> 
> When compiling _divdi3.o, cselib_expand_value_rtx_1 returns a huge rtx
> tree for variable _r1 when max_depth is greater than 17.  If -g is
> specified, this later results in attempting to generate a DWARF location
> list much larger than the 0xffff size limit, resulting in an assert failure.

I think Alex is working on a patch which will hopefully improve it.
In 4.6-RH in the mean time we are defaulting max-vartrack-expr-depth to 12
instead, perhaps that would be better than 10.

As for the 0xffff size limit, I'd say we should handle it instead of
asserting, e.g. if the size is 64K or more, we could emit there a single
DW_OP_call4 + DW_TAG_dwarf_procedure that would contain it.
Or just drop the range on the floor, still better than ICE.

> 2011-07-20  Michael Eager  <eager@eagercon.com>
> 
>         * params.def (PARAM_MAX_VARTRACK_EXPR_DEPTH): Default to 10.
> 
> Index: gcc/params.def
> ===================================================================
> --- gcc/params.def      (revision 176533)
> +++ gcc/params.def      (working copy)
> @@ -845,7 +845,7 @@
>  DEFPARAM (PARAM_MAX_VARTRACK_EXPR_DEPTH,
>           "max-vartrack-expr-depth",
>           "Max. recursion depth for expanding var tracking expressions",
> -         20, 0, 0)
> +         10, 0, 0)

	Jakub

Re: introduce --param max-vartrack-expr-depth

[Michael Eager]

On 07/20/2011 01:23 PM, Jakub Jelinek wrote:
> On Wed, Jul 20, 2011 at 01:07:40PM -0700, Michael Eager wrote:
>> I've run into a problem with this change when building microblaze-xilinx-elf.
>>
>> When compiling _divdi3.o, cselib_expand_value_rtx_1 returns a huge rtx
>> tree for variable _r1 when max_depth is greater than 17.  If -g is
>> specified, this later results in attempting to generate a DWARF location
>> list much larger than the 0xffff size limit, resulting in an assert failure.
>
> I think Alex is working on a patch which will hopefully improve it.
> In 4.6-RH in the mean time we are defaulting max-vartrack-expr-depth to 12
> instead, perhaps that would be better than 10.

I'm OK with a value of 12.

> As for the 0xffff size limit, I'd say we should handle it instead of
> asserting, e.g. if the size is 64K or more, we could emit there a single
> DW_OP_call4 + DW_TAG_dwarf_procedure that would contain it.
> Or just drop the range on the floor, still better than ICE.

I don't think that a huge location list is a good result, even if you
package it into a DWARF procedure.


-- 
Michael Eager	 eager@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306  650-325-8077

Re: introduce --param max-vartrack-expr-depth

[Jakub Jelinek]

On Wed, Jul 20, 2011 at 01:28:46PM -0700, Michael Eager wrote:
> On 07/20/2011 01:23 PM, Jakub Jelinek wrote:
> >On Wed, Jul 20, 2011 at 01:07:40PM -0700, Michael Eager wrote:
> >>I've run into a problem with this change when building microblaze-xilinx-elf.
> >>
> >>When compiling _divdi3.o, cselib_expand_value_rtx_1 returns a huge rtx
> >>tree for variable _r1 when max_depth is greater than 17.  If -g is
> >>specified, this later results in attempting to generate a DWARF location
> >>list much larger than the 0xffff size limit, resulting in an assert failure.
> >
> >I think Alex is working on a patch which will hopefully improve it.
> >In 4.6-RH in the mean time we are defaulting max-vartrack-expr-depth to 12
> >instead, perhaps that would be better than 10.
> 
> I'm OK with a value of 12.

The patch with s/10/12/g is preapproved.

	Jakub

Re: introduce --param max-vartrack-expr-depth

[Michael Eager]

On 07/20/2011 01:48 PM, Jakub Jelinek wrote:
> On Wed, Jul 20, 2011 at 01:28:46PM -0700, Michael Eager wrote:
>> On 07/20/2011 01:23 PM, Jakub Jelinek wrote:
>>> On Wed, Jul 20, 2011 at 01:07:40PM -0700, Michael Eager wrote:
>>>> I've run into a problem with this change when building microblaze-xilinx-elf.
>>>>
>>>> When compiling _divdi3.o, cselib_expand_value_rtx_1 returns a huge rtx
>>>> tree for variable _r1 when max_depth is greater than 17.  If -g is
>>>> specified, this later results in attempting to generate a DWARF location
>>>> list much larger than the 0xffff size limit, resulting in an assert failure.
>>>
>>> I think Alex is working on a patch which will hopefully improve it.
>>> In 4.6-RH in the mean time we are defaulting max-vartrack-expr-depth to 12
>>> instead, perhaps that would be better than 10.
>>
>> I'm OK with a value of 12.
>
> The patch with s/10/12/g is preapproved.
>
> 	Jakub

Checked in revision 176538.

-- 
Michael Eager	 eager@eagercon.com
1960 Park Blvd., Palo Alto, CA 94306  650-325-8077