[PATCH] D18546: Prevent X86IselLowering from merging volatile loads

[James Y Knight via llvm-commits]

jyknight added a subscriber: jyknight.
jyknight added a comment.

In http://reviews.llvm.org/D18546#386204, @RKSimon wrote:

> I'm not sure if this is necessary - we already check that we're not merging consecutive loads where any/all the loads are volatiles. DAG.isConsecutiveLoad handles these cases for us (and its why we already have tests for them) - we shouldn't need to check again in EltsFromConsecutiveLoads.


Except, it doesn't actually check that.

The only reason why "load volatile; load; load; load" is not being merged by that code already is the incoming Chain must match on the nodes for isConsecutiveLoad to return true. With the current code, the chains do not get rearranged to the proper shape.

With the new code, they do: all 4 loads can be determined to not alias, and thus they are all given the same incoming Chain (EntryToken). This is correct. Then, isConsecutiveLoad says yes.


http://reviews.llvm.org/D18546