This is the mail archive of the
mailing list for the GCC project.
Re: [PATCH] Simplify some PRE stuff
On Sun, 9 Mar 2008, Daniel Berlin wrote:
> On Sun, Mar 9, 2008 at 7:13 AM, Richard Guenther <email@example.com> wrote:
> > While munging with PRE I noticed the following. get_sccvn_value seems
> > overly complicated (and has this strange defstmt = NULL and later assert
> > for it code), so we just simplify it do do what the comment says
> > (I notice the commend in the deleted block does no longer apply as we
> > now lookup the vn without vuses).
> Yes, since you changed it to use vn_lookup_or_add, the rest no longer applies.
> > (for unrecognized stmts) adding uses to the sets. This doesn't make
> > sense to me as they should already be in as we iterate in dominator
> > order and the avail sets get initialized with the dominating definition
> > blocks of these uses. If valid this iteration needs a comment at least.
> If you want to remove the adding to AVAIL, that is fine.
> You cannot remove the adding to EXP_GEN.
> EXP_GEN represents those expressions that appear on the RHS of
> statements in a given block.
> Even "unrecognized" uses should appear in EXP_GEN.
Ok. It isn't intutive to me that using something is "generating"
an expression. I would connect this more to the definition point
of the used SSA_NAME. I'll add a comment ...
> > Bootstrapped and tested on x86_64-unknown-linux-gnu, ok for mainline?
> You should be aware that I made a lot of the changes you are undoing
> after analyzing PRE's behavior on a large number of testcases I can't
> submit (or easily minimize). I also used to ensure the number of
> eliminations made during a gcc bootstrap didn't change. You should be
> very careful about changing it and expecting okay results just because
> it bootstraps and passes the small number of optimization regtests we
> have for it.
> It is generallly a good idea to add asserts to make sure you are not
> changing behavior and bootstrap with those, and for overall behavior,
> ensuring the number of eliminations made during a bootstrap doesn't
... and look at this with retaining EXP_GEN.
> In any case, this patch is fine except for the adding to EXP_GEN part,
> which you shouldn't remove.
> (The reason it does not necessarily change anything in most cases is
> because I corrected the dataflow equations for ANTIC, so the default
> ANTIC set usually includes most of EXP_GEN)