On 01/29/2018 04:37 PM, Jakub Jelinek wrote: > Hi! > > As mentioned in the PR, cunroll sometimes registers some SSA_NAMEs for > renaming and then invokes some SCEV using functions before finally updating > the SSA form. On the testcase we end up with 3 degenerate PHIs pointing at > each other, so follow_copies_to_constant loops forever. I'm at a loss to understand how we could get in that situation. Was it edge removal (if so for which PHI) or the duplicate process that created the loop?
We've seen one case where this happened inside DOM, but it was only because we ignored an unexecutable edge which then caused a PHI to become a degenerate. One or more of the PHI args in the degenerate was marked as EDGE_DFS_BACK on its associated edge. Are any of the PHI args in pr84111 marked in a similar manner? > > The following patch has 2 different fixes for this, one is to avoid looking > at SSA_NAMEs registered for update (in theory all we care are the old ssa > names, but we don't have an API to query just those), the other is to put > some upper bound on how many times we follow SSA_NAMEs (either single defs > or degenerate phis). Either of those changes is sufficient to fix this. Interestingly enough I was looking at a bug several weeks ago where the ability to query one of the sets (I can't remember if it was old or new) of names for rewriting would have been useful. > > During x86_64-linux and i686-linux bootstraps/regtests > follow_copies_to_constant > actually never returned anything useful, so the patch doesn't regress at > least on these targets anything, there are quite a few cases where SSA_NAME > is name_registered_for_update_p but the function would never return anything > but var in those cases. And the highest depth ever seen was 3, except for > the newly added testcase if the && !name_registered_for_update_p (res) part > is commented out. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > > 2018-01-29 Jakub Jelinek <ja...@redhat.com> > > PR tree-optimization/84111 > * tree-scalar-evolution.c: Include tree-into-ssa.h. > (follow_copies_to_constant): Stop on SSA_NAMEs registered for update, > loop at most 128 times. > > * gcc.c-torture/compile/pr84111.c: New test. Probably reasonable. Though I would like to better understand how we got the degenerates forming a loop before final ack. jeff
