This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix PR83418
On 12/15/2017 09:30 AM, Richard Biener wrote:
> On December 15, 2017 5:27:14 PM GMT+01:00, Jeff Law <law@redhat.com> wrote:
>> On 12/15/2017 01:10 AM, Richard Biener wrote:
>>> On Thu, 14 Dec 2017, Richard Biener wrote:
>>>
>>>> On December 14, 2017 4:43:42 PM GMT+01:00, Jeff Law <law@redhat.com>
>> wrote:
>>>>> On 12/14/2017 01:54 AM, Richard Biener wrote:
>>>>>>
>>>>>> IVOPTs (at least) leaves unfolded stmts in the IL and VRP
>>>>> overzealously
>>>>>> asserts they cannot happen.
>>>>>>
>>>>>> Bootstrap and regtest running on x86_64-unknown-linux-gnu.
>>>>>>
>>>>>> Richard.
>>>>>>
>>>>>> 2017-12-14 Richard Biener <rguenther@suse.de>
>>>>>>
>>>>>> PR tree-optimization/83418
>>>>>> * vr-values.c
>>>>> (vr_values::extract_range_for_var_from_comparison_expr):
>>>>>> Instead of asserting we don't get unfolded comparisons deal with
>>>>>> them.
>>>>>>
>>>>>> * gcc.dg/torture/pr83418.c: New testcase.
>>>>> I think this also potentially affects dumping. I've seen the
>> dumper
>>>>> crash trying to access a INTEGER_CST where we expected to find an
>>>>> SSA_NAME while iterating over a statement's operands.
>>>>>
>>>>> I haven't submitted the workaround because I hadn't tracked down
>> the
>>>>> root cause to verify something deeper isn't wrong.
>>>>
>>>> Yes, I've seen this as well, see my comment in the PR. The issue is
>> that DOM calls VRP analyze (and dump) routines with not up to date
>> operands during optimize_stmt.
>>>
>>> I had the following in my tree to allow dumping.
>>>
>>> Richard.
>>>
>>> Index: gcc/tree-ssa-dom.c
>>> ===================================================================
>>> --- gcc/tree-ssa-dom.c (revision 255640)
>>> +++ gcc/tree-ssa-dom.c (working copy)
>>> @@ -2017,6 +2017,7 @@ dom_opt_dom_walker::optimize_stmt (basic
>>> undefined behavior that get diagnosed if they're
>> left in
>>> the
>>> IL because we've attached range information to new
>>> SSA_NAMES. */
>>> + update_stmt_if_modified (stmt);
>>> edge taken_edge = NULL;
>>> evrp_range_analyzer.vrp_visit_cond_stmt (as_a <gcond *>
>>
>>> (stmt),
>>> &taken_edge);
>>>
>> I think this implies something earlier changed a statement without
>> updating it.
>
> Dom itself does this and delays updating on purpose as an optimization. That doesn't work quite well when dispatching into different code.
I honestly can't remember the history around not updating -- and I've
always found the explicit need to mark and update clunky.
I'd rather have the IL be consistent -- it's hard to believe there's a
major benefit to deferring the operand update. I'll add your patch to
one of my build/test cycles.
Jeff
>
> Richard.
>
>> jeff
>