This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [RFC][PATCH] Extend store ccp
- From: Revital1 Eres <ERES at il dot ibm dot com>
- To: Diego Novillo <dnovillo at google dot com>
- Cc: Daniel Berlin <dberlin at dberlin dot org>, gcc-patches at gcc dot gnu dot org, rguenther at suse dot de
- Date: Tue, 19 Jun 2007 22:14:29 +0300
- Subject: Re: [RFC][PATCH] Extend store ccp
Hello,
Thanks for the review!
> > -/* We have just defined a new value for VAR. If IS_VARYING is true,
> > - add all immediate uses of VAR to VARYING_SSA_EDGES, otherwise add
> > +static bool
> > +do_store_ccp (void)
> > +{
> > + return (flag_tree_store_ccp != 0);
> > +}
>
> No. This will return 'true' every time we run CCP. We only want this
> to return 'true' when flag_tree_store_ccp is set *and* we are running
> pass_store_ccp.
This relates to the comment I posted in the original message regarding
the extra parameter; the problem is I do not know how to check what pass
currently running. I did not find a similar example.
> > +
> > +/* Given STMT add it to the edge worklist. If IS_VARYING is true,
> > + add all the uses of stmt's lhs to VARYING_SSA_EDGES, otherwise add
>
> s/lhs/LHS/
I took this comment from add_ssa_edge () function so for consistency
reasons it might stay lhs.
> > + if (offset_overlaps_with_access (offset, size, offset2,
maxsize2))
> > + {
> > + /* If the DEF is not exact and matches, bail out. */
> > + if (maxsize2 != size2 || size2 != size || offset2 != offset)
> > + return NULL_TREE;
> > +
> > + /* For now just handle a distance of one. If the RHS of the
DEF
> > + is another ref, we could recurse here.
> > + ??? Recurse here? */
>
> A distance of one? What do you mean in this comment?
Actually I took this comment from Richard's code... I guess it refers
to a recursive propgation of constants, for example -
1) *p = 5;
...
2) a[i] = *p;
...
3) x = a[i];
Visiting stmt 3 we walk to it's definition in stmt 2 and because it's
rhs is also a ref we recursively find it's def stmt to propagate 5.
It seems this comment is redundant as it's the propagator job to take
care of such propagation...
Thanks again,
Revital