[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#1088261, @mzolotukhin wrote:

> > 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?


As seen in my earlier post we remove these PHI nodes when using my patch:

  %value.lcssa2 = phi i16 [ %value3.lcssa, %exit1.pred ], [ %value, %gazonk ]
  %value3.lcssa = phi i16 [ %value3, %foo ]
  %value3.lcssa4 = phi i16 [ undef, %unreachable1 ], [ %value3, %bar ]
  %value.lcssa.lcssa = phi i16 [ %value3.lcssa4, %exit2.pred ], [ %value.lcssa, %qqq ]

My idea was that if we iterated in reverse order we would remove the use of `%value3.lcssa4` before trying to remove the PHI node defining that value.
But as seen above we wouldn't be able to remove the PHI node defining `%value3.lcssa` instead (as it is used by the first PHI node in the list).
So it will not help by changing the iteration order in case we desire to remove all those four PHI nodes.

> 
> 
>> 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?

Well, it would be simpler/quicker to just do:

  for (PHINode *PN : PHIsToRemoveIfNotUsed)
     if (PN->use_empty())
       PN->eraseFromParent();

if we do not care about leaving some unused PHI nodes around (for later passes to clean up).
But then I guess we could skip the cleanup completely.


Repository:
  rL LLVM

https://reviews.llvm.org/D46422