Re: [RFC][PATCH] Extend store ccp

[Revital1 Eres <ERES at il dot ibm dot com>]

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