introduce --param max-vartrack-expr-depth

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.

1960 Park Blvd., Palo Alto, CA 94306  650-325-8077

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

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.

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

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.