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 RFC: Don't move to infinity until you've seen all edges


Diego Novillo <dnovillo@redhat.com> writes:

> Ian Lance Taylor wrote on 04/12/07 12:40:
> 
> > This patch has been bootstrapped and tested on i686-pc-linux-gnu.
> > Does it look OK?
> 
> Hmmm, I think the effects on compile time may be deeper than a single
> additional iteration.  Consider high in-degree PHI nodes, with each
> argument increasing by 1 on each visit.
> 
> But that may not be common enough to worry about.  If there aren't
> noticeable compile time regressions, I'm OK with the change.
> 
> Alternately, why not just use a non-overflow infinity in this case?

We need to use an overflow infinity here, because this is what catches
cases like
    for (i = 1; i > 0; ++i) ++bits;
We need to record that i goes up to +INF(OVF), not just +INF.

> >    /* To prevent infinite iterations in the algorithm, derive ranges
> >       when the new value is slightly bigger or smaller than the
> > -     previous one.  */
> > +     previous one.  We don't do this if we have seen a new edge--we
> > +     only do it the second time we see a new edge.  This helps us
> > +     avoid an overflow infinity for conditionals which are not in a
> > +     loop.  */
> 
> The comment is out of sync.  We don't push to infinity on the second
> edge.  We do it after we've seen *all* edges.

I'm not sure the comment is out of sync, but I will make it clearer.

> >    if (lhs_vr->type == VR_RANGE && vr_result.type == VR_RANGE
> > -      && !all_const)
> > +      && edges <= old_edges)
> 
> Why are you removing the all_const test?  I don't think that's needed.

I don't think the all_const test adds anything above the test for new
executable edges.  If we've already seen [N,N], it's not going to
change the limits the second time we see it.  The test case which
caused Richard Guenther to add all_const--PR 23603--continues to work
as desired after my patch.

Ian


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