https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68234
--- Comment #4 from Jiong Wang <jiwang at gcc dot gnu.org> --- (In reply to rguent...@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.
