[PATCH] D46422: [LCSSA] Iteratively remove unused PHI nodes in formLCSSAForInstructions

[Michael Zolotukhin via Phabricator via llvm-commits]

mzolotukhin added a comment.

> Ok, I will make an attempt to reduce it further, but if there was an easy way to trigger the problem, then I guess we would have seen it a long time ago (and bugpoint would have been able to remove some more basic blocks).
>  If it is preferred, then I can remove all the checks and keep it as a "just check that this no longer asserts" kind of test.

Yes, it is much preferred, thank you! If we have more than one problem here, then we need more small tests, but not one huge test.

> Do you think it is possible to remove the PHI nodes directly in the first loop?

I think it's possible.

> Is it guaranteed that a PHI node, without uses, being inserted in PHIsToRemove only can be used by later created PHI nodes

PHI-nodes are only inserted when we call SSAUpdater.RewriteUse. I did not expected that their use lists change afterwards, so frankly, I'm surprised that inserted PHIs get more uses on later iterations - that's why I'm asking for a simpler test that could help to understand how and why this happens.

> Then it might be enough to iterate in reverse insertion order...

Why do we need to reverse the order?

> Another question is if it is important that we remove all unused PHI nodes?
>  If it is OK to do it quick and dirty, and only remove some of the speculatively created and unused PHI nodes, then we can simplify this.

I'm not sure I got this, could you elaborate on this please? What would be a quicker way?

>   I picked a solution that removes all speculatively created PHI nodes that are unused after the first loop, including the PHI nodes that are used by PHI nodes that we remove.

The inline comment I had was about your specific implementation, not about the general idea behind it. How is it possible to iterate that loop more than once?

Thanks,
Michael


Repository:
  rL LLVM

https://reviews.llvm.org/D46422