Re: [PATCH] Fix segfault in FRE during SCC value numbering

[Teresa Johnson <tejohnson@google.com>]

On Wed, Jan 15, 2014 at 12:00 PM, Teresa Johnson <tejohnson@google.com> wrote:
> On Wed, Jan 15, 2014 at 10:46 AM, Jeff Law <law@redhat.com> wrote:
>> On 01/15/14 10:07, Teresa Johnson wrote:
>>>
>>> Handle NULL vdef for call in the case where we have a matching vnresult
>>> that has a vdef (it already handles the NULL vdef case when !vnresult).
>>> This
>>> can happen for promoted indirect calls if the fallback indirect call
>>> (which has a vdef) can be proven equivalent to the promoted direct call
>>> (which might not have a vdef).
>>>
>>> This occurred for a case where we had a promoted indirect call,
>>> where FRE determined that the promoted direct call and the fall-back
>>> indirect
>>> call were equivalent (since earlier it determined that the function
>>> pointer
>>> was always set to that target). The indirect call had been analyzed by
>>> visit_reference_op_call first, and had a VDEF. The direct call did not
>>> have a
>>> VDEF, presumably because it was a leaf function in the same module without
>>> any
>>> stores. But visit_reference_op_call unconditionally calls set_ssa_val_to
>>> when
>>> the previous vnresult had a vdef, leading to a seg fault in this case.
>>> If we had analyzed the direct call first the failure wouldn't have
>>> occurred
>>> since the !vnresult case guards the call to set_ssa_val_to with a check
>>> for a NULL vdef, and the subsequent handling of the indirect call would
>>> also not call set_ssa_val_to since vnresult would have had a null
>>> result_vdef.
>>>
>>> Bootstrapped and tested on x86_64-unknown-linux-gnu. Ok for trunk?
>>>
>>> 2014-01-15  Teresa Johnson  <tejohnson@google.com>
>>>
>>>          * tree-ssa-sccvn.c (visit_reference_op_call): Handle NULL vdef.
>>
>> The patch is OK.  Given this was an ICE, do you have a reduced test we can
>> add to the regression suite?  I realize that order of visiting in the SCC is
>> important to trigger, but a regression test would still be useful.
>
> Unfortunately it was hit using LIPO on the google/4_8 branch, and only
> occurred with a specific profile. That's why I don't have a trunk test
> case. I suppose I could create a test case that has a similar
> opportunity. It does look like there are some indirect call promotion
> with FDO tests already (e.g. gcc.dg/tree-prof/indir-call-prof.c), but
> I'm not sure whether they even trigger the same type of FRE
> opportunity. I will take a look.

I'm having a hard time getting the right combination of early/late
inlining and indirect call promotion on trunk to occur to even allow
this optimization to kick in. It's possible I could do so with a
sufficiently complicated test, but I'm not sure it is worth it. I'll
commit the fix right now though.

Thanks,
Teresa

>
> Teresa
>
>>
>> Thanks,
>> Jeff
>>
>>
>
>
>
> --
> Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413



-- 
Teresa Johnson | Software Engineer | tejohnson@google.com | 408-460-2413