[PATCH] D56875: [DAGCombiner] narrow vector binop with 2 insert subvector operands

Mon Jan 21 14:19:45 PST 2019

spatel added a comment.

In D56875#1365621 <https://reviews.llvm.org/D56875#1365621>, @efriedma wrote:

> > 'xor undef, undef --> 0' (not undef).
>
> Strictly speaking, `xor undef, undef ---> undef` is correct.  It's just that as a practical matter, we try to fold obvious cases to zero so we don't have to argue with people who write silly constructs like `__m128i a = _mm_xor_si128(a, a);`.  The fold here seems unlikely to cause problems in practice.

Ah, interesting. Either way, I think we want a test to document the behavior, so I added 1 here:
rL351763 <https://reviews.llvm.org/rL351763>

Now, the funny thing about this particular case is that we'll generate the optimal code with x86 AVX either way because 'vxorps' with 128-bit operands zeros the upper half...oops! So I added another test here:
rL351764 <https://reviews.llvm.org/rL351764>

I think I should check in the more conservative version of the patch first since I already wrote it. Then, if we decide it's worth loosening to the form shown here currently, I can make that a follow-up.

After looking a bit more closely at other patterns that I was hoping to fix, I'm actually not sure if we will end up keeping this code. I think we might be better off trying more general vector-demanded-elements enhancements for binops which would make this explicit pattern-matching unnecessary.

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

https://reviews.llvm.org/D56875