[PATCH] D55974: [GVN] Force a sequence point when comparing two DenseMap values.

[Matt Davis via Phabricator via llvm-commits]

mattd added a comment.

Thanks for the feedback @efriedma.

In D55974#1338652 <https://reviews.llvm.org/D55974#1338652>, @efriedma wrote:

> If we're calling grow() on BlockRPONumber in this code, something has gone wrong. Both "P" and "CurrentBlock" should have an RPO number at this point in the code, so it shouldn't actually grow.


In the body of `GVN::performPRE()` there is case where a block will split via a call to `GVN::splitCriticalEdges`.  If the crit edge(s) are split, new basic blocks are introduced to the function but are not added to the BlockRPONumber map.  In fact the test `test/Transform/GVN/PRE/pre-basic-add.ll` creates such a situation.  This means that BlockRPONumber might be accessed for a basic block that has not been added to it.

> It looks like r225536 added a new caller to performScalarPRE(), and BlockRPONumber is not correctly initialized at that point.  I'm surprised this hasn't led to any miscompiles.

The message in r225536 does seem to suggest a periodic LTO related failure, and that is similar to what we were seeing.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55974/new/

https://reviews.llvm.org/D55974