[PATCH] D18546: Prevent X86IselLowering from merging volatile loads

[Nirav Dave via llvm-commits]

niravd added a comment.

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


This is currently not caught by the current tests, but if you apply http://reviews.llvm.org/D18062 and http://reviews.llvm.org/D14834 which clean up Merging of Consecutive Stores (and willl hopefully be commited soon). merge-consecutive-loads-128.ll will produce wrong code. Specifically in "merge_4f32_f32_2345_volatile", there are 4 consecutive loads with the first being volatile preventing merging them but are merged into a single volatile load (this happens in the legalize-types of SelectionDAGISel after dag-combine1).

It doesn't look like isConsecutiveLoad does any checking for volatiles though perhaps it should (though we should rename it to reflect that).

We should also probably dictate that the newLd is always non-volatile.

> If you wish to add additional volatile load tests that'd be great; but please note, we auto-generate the merge-consecutive-loads-*.ll test files (something we're trying to encourage) - manually editting these files with entwined DAG checks is not easy to maintain.


Good point. I'll undo these test changes and rerun the generation for http://reviews.llvm.org/D14834 when behavior changes.


http://reviews.llvm.org/D18546