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

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope added a comment.

In https://reviews.llvm.org/D46422#1088801, @mzolotukhin wrote:

> I looked deeper into this, and I think you exposed several issues here:
>
> - The entire thing is triggered by the fact that the value we're rewriting is used in an unreachable block. We might just ignore such cases (see the loop on lines 102-110).
> - The loops in this test are not in simplified form. While they are not guaranteed to be in it, it might indicate that we have a bug in some other pass that fails to preserve the loop in a simplified form. Whatever fix for the current issue we choose, it would be interesting to understand what led to this situation.


I think the program originates from some fuzzer (csmith?) and using a slightly modified pass-pipeline such as `opt -sroa -O3`.

> Returning back to the issue in question, here is my understanding of what happens in this case. LCSSA adds a phi-node for the `%value`. The phi-nodes it creates in the process happen to land into another loop, and thus we need to build LCSSA for those loop too (that's why we iterate several times in the worklist loop). When we add LCSSA phi-nodes for the second loop, we again insert phi-nodes into yet another loop. This continues until we convert all touched loops to LCSSA form. What happens in this process is that a PHI-node might have 0 uses at the time it was inserted first time, but as we keep inserting new phis, it might happen to be used in some of the latter phis - which is why the assertion fails. I think the fix should be to just replace the assert with the check (+add a comment about it). After this some redundant phi-nodes might remain in the program, but that could be fixed by adding operands of the phi to `PHIsToRemove` if they are not used anywhere except the phi, which we're removing.

I checked what would happen if I erase unused PHI nodes directly at the end of loop that is inserting the PHI nodes.
Then test/Transforms/LCSSA/pr28608.ll started to fail again, unless I also started to ignore unreachable-from-entry users in the loop on lines 102-110.
With both of those fixes we get an unused `%value.lcssa5 = phi i16 [ undef, %unreachable1 ], [ %value3, %bar ]` in `exit2.pred`. That one was added by SSAUpdate.RewriteUse. So that PHI node is not even becoming a candidate for removal.

I'll update the patch with the other suggestion (simply replacing the assert with a check).


Repository:
  rL LLVM

https://reviews.llvm.org/D46422