Re: [PATCH] Fix PR83418

2018-01-02 Thread Jeff Law
On 01/02/2018 02:26 AM, Richard Biener wrote:
> On Tue, 19 Dec 2017, Jeff Law wrote:
> 
>> On 12/15/2017 09:30 AM, Richard Biener wrote:
>>> On December 15, 2017 5:27:14 PM GMT+01:00, Jeff Law  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 
 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  

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 

> (stmt),
>_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.
> 
> SSA operand updating is/was one of the major slownesses at some point
> so we started micro-optimizing it.
> 
> There's the odd modified bit in GIMPLE stmts which we neither consistently
> set when modifying operands in a way that require update_stmt nor
> do we avoid re-scanning the stmt when doing update_stmt (which
> just sets the modified bit and then calls update_stmt_if_modified).
> 
> IMHO we should either get rid of the modified bit or use / update it
> consistently.  But there's the general data structure issue of
> the immediate use list of SSA names and the SSA operand list of
> stmts where the former keys back to the latter but not the other
> way around - this means we can't update the stmt operand list without
> re-scanning the whole stmt when substituting SSA names for example
> (but we could set the modified bit there).
> 
> Note that update_stmt fixes both the stmt operand list and also
> immediate use data in case somebody changed a stmt via the
> set_rhs/lhs interfaces (which _also_ don't set the modified bit).
> 
> So it's somewhat of a mess but rather than trying to "fix" the
> modified bit thing I'd see to fix the data structures to allow
> O(1) updating of the stmt SSA operand list from places that use
> SET_USE (use_p, X) (propagate_value and friends).  Maybe we can
> get rid of the stmt SSA operand list and instead walk the
> operand slots (for single-rhs we'd need to linearly walk
> the GENERIC tree in a quite constrained way).  We've got rid of
> the DEFs list already after all...  (I know this was on Michas
> TODO list)
The core of the operand scanning pre-dates tuples -- so finding operands
was fairly painful as we had to walk the entire expression.  Thus the
operand cache was born.

As you mention, we ought to be able to look at the operand slots these
days which may change things enough that we fundamentally don't need the
cache anymore.

We'd probably need to special case expressions/references in the operand
slots (ie, MEM_REF, perhaps others).  But if 

Re: [PATCH] Fix PR83418

2018-01-02 Thread Richard Biener
On Tue, 19 Dec 2017, Jeff Law wrote:

> On 12/15/2017 09:30 AM, Richard Biener wrote:
> > On December 15, 2017 5:27:14 PM GMT+01:00, Jeff Law  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 
> >> 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  
> >>
> >>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 
> >>
> >>> (stmt),
> >>>_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.

SSA operand updating is/was one of the major slownesses at some point
so we started micro-optimizing it.

There's the odd modified bit in GIMPLE stmts which we neither consistently
set when modifying operands in a way that require update_stmt nor
do we avoid re-scanning the stmt when doing update_stmt (which
just sets the modified bit and then calls update_stmt_if_modified).

IMHO we should either get rid of the modified bit or use / update it
consistently.  But there's the general data structure issue of
the immediate use list of SSA names and the SSA operand list of
stmts where the former keys back to the latter but not the other
way around - this means we can't update the stmt operand list without
re-scanning the whole stmt when substituting SSA names for example
(but we could set the modified bit there).

Note that update_stmt fixes both the stmt operand list and also
immediate use data in case somebody changed a stmt via the
set_rhs/lhs interfaces (which _also_ don't set the modified bit).

So it's somewhat of a mess but rather than trying to "fix" the
modified bit thing I'd see to fix the data structures to allow
O(1) updating of the stmt SSA operand list from places that use
SET_USE (use_p, X) (propagate_value and friends).  Maybe we can
get rid of the stmt SSA operand list and instead walk the
operand slots (for single-rhs we'd need to linearly walk
the GENERIC tree in a quite constrained way).  We've got rid of
the DEFs list already after all...  (I know this was on Michas
TODO list)

Richard.



Re: [PATCH] Fix PR83418

2017-12-21 Thread Richard Biener
On December 21, 2017 5:40:48 AM GMT+01:00, Jeff Law  wrote:
>On 12/15/2017 09:30 AM, Richard Biener wrote:
>> On December 15, 2017 5:27:14 PM GMT+01:00, Jeff Law 
>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
>
>>> 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  
>>>
>>> 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 *>
>>>
 (stmt),
   
>_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. 
>So I went ahead with a bootstrap and regression test with this patch.
>If (of course) worked fine.  I'm installing it on the trunk.

Thanks. 

Richard. 

>jeff



Re: [PATCH] Fix PR83418

2017-12-20 Thread Jeff Law
On 12/15/2017 09:30 AM, Richard Biener wrote:
> On December 15, 2017 5:27:14 PM GMT+01:00, Jeff Law  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 
>> 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  
>>
>>  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 
>>
>>> (stmt),
>>>_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. 
So I went ahead with a bootstrap and regression test with this patch.
If (of course) worked fine.  I'm installing it on the trunk.

jeff



Re: [PATCH] Fix PR83418

2017-12-19 Thread Jeff Law
On 12/15/2017 09:30 AM, Richard Biener wrote:
> On December 15, 2017 5:27:14 PM GMT+01:00, Jeff Law  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 
>> 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  
>>
>>  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 
>>
>>> (stmt),
>>>_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
> 



Re: [PATCH] Fix PR83418

2017-12-15 Thread Richard Biener
On December 15, 2017 5:27:14 PM GMT+01:00, Jeff Law  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 
>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  
>
>   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 
>
>> (stmt),
>>_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



Re: [PATCH] Fix PR83418

2017-12-15 Thread Jeff Law
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  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  

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  
> (stmt),
>_edge);
> 
I think this implies something earlier changed a statement without
updating it.

jeff


Re: [PATCH] Fix PR83418

2017-12-15 Thread Richard Biener
On Thu, 14 Dec 2017, Richard Biener wrote:

> On December 14, 2017 4:43:42 PM GMT+01:00, Jeff Law  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  
> >> 
> >>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  
(stmt),
   _edge);



Re: [PATCH] Fix PR83418

2017-12-14 Thread Richard Biener
On December 14, 2017 4:43:42 PM GMT+01:00, Jeff Law  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  
>> 
>>  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. 

Richard. 

>Jeff



Re: [PATCH] Fix PR83418

2017-12-14 Thread Jeff Law
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  
> 
>   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.

Jeff


[PATCH] Fix PR83418

2017-12-14 Thread Richard Biener

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  

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.

Index: gcc/vr-values.c
===
--- gcc/vr-values.c (revision 255622)
+++ gcc/vr-values.c (working copy)
@@ -445,11 +445,12 @@ vr_values::extract_range_for_var_from_co
   tree  min, max, type;
   value_range *limit_vr;
   type = TREE_TYPE (var);
-  gcc_assert (limit != var);
 
   /* For pointer arithmetic, we only keep track of pointer equality
- and inequality.  */
-  if (POINTER_TYPE_P (type) && cond_code != NE_EXPR && cond_code != EQ_EXPR)
+ and inequality.  If we arrive here with unfolded conditions like
+ _1 > _1 do not derive anything.  */
+  if ((POINTER_TYPE_P (type) && cond_code != NE_EXPR && cond_code != EQ_EXPR)
+  || limit == var)
 {
   set_value_range_to_varying (vr_p);
   return;
Index: gcc/testsuite/gcc.dg/torture/pr83418.c
===
--- gcc/testsuite/gcc.dg/torture/pr83418.c  (nonexistent)
+++ gcc/testsuite/gcc.dg/torture/pr83418.c  (working copy)
@@ -0,0 +1,17 @@
+/* { dg-do compile } */
+
+void
+yj (int j4)
+{
+  int t3;
+
+  for (t3 = 0; t3 < 6; ++t3)
+{
+  short int v4 = t3;
+
+  if (v4 == j4 || v4 > t3)
+   for (;;)
+ {
+ }
+}
+}