This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: PATCH RFC: Don't move to infinity until you've seen all edges
- From: Ian Lance Taylor <iant at google dot com>
- To: Diego Novillo <dnovillo at redhat dot com>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: 12 Apr 2007 12:11:45 -0700
- Subject: Re: PATCH RFC: Don't move to infinity until you've seen all edges
- References: <m37ish5yxe.fsf@localhost.localdomain> <461E7EAF.7070207@redhat.com>
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