This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Fix PR83418


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. 

Richard. 

>jeff


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]