[PATCH] D16828: [x86] convert masked store of one element to scalar store

[Sanjay Patel via llvm-commits]

spatel added a comment.

In http://reviews.llvm.org/D16828#343149, @andreadb wrote:

> In http://reviews.llvm.org/D16828#343021, @RKSimon wrote:
>
> > Some minor comments. I wonder if this could be moved to DAGCombiner - possibly with a test for scalar store legality and  TLI.isExtractVectorElementCheap()?
>
>
> I know that on AVX, VMASKMOV does not imply the non-temporal hint. However, what about targets that implement masked stores as non-temporal stores of selected bytes? For those targets, not preserving the non-temporal hint when converting the mask store into a scalar store would affect the hardware write combining logic.


I have preserved the non-temporal hint of the original masked store in the scalar store with this patch, but...

> That said, my knowledge is only limited to x86, so I cannot say that those targets exist.. What I am trying to say is that we probably have to be careful if we decide to move this transformation into the DAGCombiner as different targets may expand those nodes in a slightly different way (not sure if this makes sense..).

>  Basically, I recommend leaving it for now as a target specific combine (just my opinion).


That was my thinking too. Let's make sure this is sane across x86 first. There's a lot of potential variation in how a masked op is implemented. I'll add a 'TODO' comment to remind us that this could be lifted to DAGCombiner once we have more confidence.


http://reviews.llvm.org/D16828