https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68234

--- Comment #3 from rguenther at suse dot de <rguenther at suse dot de> ---
On Mon, 9 Nov 2015, jiwang at gcc dot gnu.org wrote:

> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68234
> 
> Jiong Wang <jiwang at gcc dot gnu.org> changed:
> 
>            What    |Removed                     |Added
> ----------------------------------------------------------------------------
>             Summary|tree-vrp pass need to be    |tree-vrp pass need to be
>                    |improved when handling      |improved when handling
>                    |ASSERT/PLUS/MINUS/_EXPR and |ASSERT_EXPR
>                    |Phi node                    |
> 
> --- Comment #2 from Jiong Wang <jiwang at gcc dot gnu.org> ---
> (In reply to Richard Biener from comment #1)
> 
> > I think the issue is that we insert the assert in the first place or
> > that we intersect to a symbolic range this causes us to not use SCEV / loop
> > analysis to get at the range for c_1.  That is, in vrp_visit_phi_node
> > the early outs to varying: shouldn't skip
> > 
> >       /* If we dropped either bound to +-INF then if this is a loop
> >          PHI node SCEV may known more about its value-range.  */
> >       if ((cmp_min > 0 || cmp_min < 0
> >            || cmp_max < 0 || cmp_max > 0)
> >           && (l = loop_containing_stmt (phi))
> >           && l->header == gimple_bb (phi))
> >         adjust_range_with_scev (&vr_result, l, phi, lhs);
> > 
> > sth like
> > 
> > Index: gcc/tree-vrp.c
> > ===================================================================
> > --- gcc/tree-vrp.c      (revision 229804)
> > +++ gcc/tree-vrp.c      (working copy)
> > @@ -8827,6 +8805,24 @@ update_range:
> >  
> >    /* No match found.  Set the LHS to VARYING.  */
> >  varying:
> > +
> > +  /* If we dropped either bound to +-INF then if this is a loop
> > +     PHI node SCEV may known more about its value-range.  */
> > +  if ((l = loop_containing_stmt (phi))
> > +      && l->header == gimple_bb (phi))
> > +    {
> > +      adjust_range_with_scev (&vr_result, l, phi, lhs);
> > +
> > +      /* If we will end up with a (-INF, +INF) range, set it to
> > +        VARYING.  Same if the previous max value was invalid for
> > +        the type and we end up with vr_result.min > vr_result.max.  */
> > +      if (!((vrp_val_is_max (vr_result.max)
> > +            && vrp_val_is_min (vr_result.min))
> > +           || compare_values (vr_result.min,
> > +                              vr_result.max) > 0))
> > +       goto update_range;
> > +    }
> > +
> >    set_value_range_to_varying (lhs_vr);
> >    return SSA_PROP_VARYING;
> >  }
> > 
> > which ends up with
> > 
> > Value ranges after VRP:
> > 
> > c_1: [0, +INF]
> > 
> > as desired.  Maybe you can take the above and put it to testing.
> 
> Thanks for the explanation on those issues.
> 
> The "if" check needs to be guarded by vr_result.type == VR_RANGE, otherwise
> above draft patch passed my testing. bootstrapping on x86-64 and AArch64 OK. 
> no
> regresson on both.

Great.  Can you add a testcase and post the patch?

Reply via email to