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

[Stanislav Mekhanoshin via Phabricator via llvm-commits]

rampitec added a comment.

In https://reviews.llvm.org/D30558#698090, @MatzeB wrote:

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


In fact if I change my fix to swap liveout VN if changed instead of iterating successors that should be fast and safe if I understand it right. Also less cryptic source.


Repository:
  rL LLVM

https://reviews.llvm.org/D30558