[PATCH] Simplify some PRE stuff

Richard Guenther rguenther@suse.de
Sun Mar 9 15:52:00 GMT 2008


On Sun, 9 Mar 2008, Daniel Berlin wrote:

> On Sun, Mar 9, 2008 at 7:13 AM, Richard Guenther <rguenther@suse.de> 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
> change.

... 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)

Thanks,
Richard.



More information about the Gcc-patches mailing list