[PATCH] D30558: Fix value numbers in successor blocks if liveout number has changed

[Matthias Braun via Phabricator via llvm-commits]

MatzeB added a comment.

In https://reviews.llvm.org/D30558#698061, @rampitec wrote:

> In https://reviews.llvm.org/D30558#697994, @MatzeB wrote:
>
> > Thanks for putting up the testcase!
> >
> > I am still not convinced this is the best fix. Before invoking handleMove there must have been 1 value number living out of the block, we may create new value number in the process, however we should always be able to implement handleMove in a way that we re-use the value number that was live-out previous for whatever new value is live-out now. This means we have to fix `handleMoveUp()` and not add more core to work around a bug that really is in `handleMoveUp()`.
>
>
> It is not only move up, move down can do the same I guess. It worth nothing I see no actual case when it happens. So I think it needs to be fixed in handleMove itself.
>  Do you think we can just swap value numbers of old and new liveouts instead of propagating a new value number to successors?


I have fixed similar bugs to this in the past, there is an expectations for handleMoveUp/handleMoveDown to maintain the same value number for the live-out value. The code there already juggles value number to achieve this probably gets mixed up in of the many cases.


Repository:
  rL LLVM

https://reviews.llvm.org/D30558