Re: [PATCH PR41488]Recognize more induction variables by simplifying PEELED chrec in scalar evolution

[Jeff Law <law at redhat dot com>]

On 12/06/13 19:44, Bin.Cheng wrote:
> > Right.  Based on reading the archives, it looks like this stuff is/was
> > generated by PRE.  I also suspect jump threading can create them.  There was
> > talk of throttling PRE to leave things in a form that the IV analysis could
> > more easily digest, but I'm not sure if that was ever completed.
>
> Also could just because it is coded in that way, just as the case I
> added in patch.  I found real examples from ggc-page.c in GCC.
> But it's always good to cleanup input of an optimization, I presume
> that's why Richard tried to move IVOPT later before.
Certainly.  That possibility was mentioned as well in either 41488 or 
elsewhere in my research.

> > I assume tree_to_aff_combination_expand is relatively expensive, thus the
> > two approaches, one which avoids tree_to_aff_combination_expand.
> Considering the dump of case in the patch:
[ snip ]
So it's also a case using the affine stuff gets you get a simpler 
interface to express the value in terms you may be able match.

> Another question, is it acceptable to add an parameter for
> tree_to_aff_combination_expand to limit the number of recursive call
> for it?  Thus we don't need to expand to leaf node every time.
If there's a compelling reason to limit the recursion, then I'd think 
such a parameter would be reasonable.


> > In add_old_iv_candidates, is it the case that the only time
> > SSA_NAME_DEF_STMT (def) is another PHI node is when it's one of these affine
>
> I suppose.  Actually IVOPT make an assert on IP_ORIGINAL candidates in
> function rewrite_use_nonlinear_expr, like:
Well, the reason I ask is after your patch we'd be ignoring anything 
where SSA_NAME_DEF_STMT (def) is a PHI node and not a PEELED_CHREC.  I 
want to make sure there aren't other expected cases where 
SSA_NAME_DEF_STMT (def) is a PHI that we'd want to call add_candidate_1 for.

It sounds like there aren't other cases you'd expect to see here where 
SSA_NAME_DEF_STMT (defFFF) is a PHI.

> IVOPT adds original cand and tries to keep it the original IV is
> because it does have an updating statement.  For IVs picked up by this
> patch, it doesn't have stepping statement, so no need/way to leaving
> it untouched.  It will be represented by candidates for IVs from which
> it is peeled.
Understood.  Thanks for clarifying.  The patch is fine for the trunk.

jeff