[Bug tree-optimization/68234] tree-vrp pass need to be improved when handling ASSERT_EXPR
jiwang at gcc dot gnu.org
gcc-bugzilla@gcc.gnu.org
Mon Nov 9 13:12:00 GMT 2015
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68234
--- Comment #4 from Jiong Wang <jiwang at gcc dot gnu.org> ---
(In reply to rguenther@suse.de from comment #3)
> 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?
OK.
now I understand this patch actually done the range correction not in vrp1 but
in vrp2, as after vrp1 those trouble maker ASSERT_EXPR are removed, then in
vrp2, the tree is quite simplified that we invoke the scalar evolution to do a
final calculation to make sure we find as accurate information as possible. I
feel this is what we should have done originally, it is a general improvement
to range info calcuation.
Will combine this block of code with above similar code then send out the patch
with testcase.
More information about the Gcc-bugs
mailing list