[PATCH] D111901: [VectorCombine] fold shuffle-of-binops with common operand

[Sanjay Patel via Phabricator via llvm-commits]

spatel marked an inline comment as done.
spatel added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/VectorCombine.cpp:1084
+      TargetTransformInfo::SK_PermuteSingleSrc, VecTy, UnaryMask);
+  if (ShufCost > BinopCost)
+    return false;
----------------
RKSimon wrote:
> Are we OK with accepting the fold if ShufCost == BinopCost ?
This follows the lead of other vector combines and instcombine in general: if we can rearrange code without incurring cost, then it might unlock further transforms, so we try it.

Direct motivation is seen in the example from D111800 - in that case, we get shuffle-of-shuffle as the 1st instructions in the function and that can be reduced by the backend (x86 at least).

In a minimal case where there's no further optimization, we end up with something that is probably neutral. For example, here's the 'and' v2i64 test on x86 and aarch64:

```
define <2 x i64> @and_and_shuf_v2i64_yy_swap(<2 x i64> %x, <2 x i64> %y, <2 x i64> %z) {
  %b0 = and <2 x i64> %x, %y
  %b1 = and <2 x i64> %y, %z
  %r = shufflevector <2 x i64> %b0, <2 x i64> %b1, <2 x i32> <i32 3, i32 0>
  ret <2 x i64> %r
}

define <2 x i64> @shuf_shuf_and_v2i64_yy_swap(<2 x i64> %x, <2 x i64> %y, <2 x i64> %z) {
  %a0 = shufflevector <2 x i64> %y, <2 x i64> poison, <2 x i32> <i32 1, i32 0>
  %a1 = shufflevector <2 x i64> %x, <2 x i64> %z, <2 x i32> <i32 3, i32 0>
  %r = and <2 x i64> %a0, %a1
  ret <2 x i64> %r
}
```
```
before:
	andps	%xmm1, %xmm0
	andps	%xmm1, %xmm2
	shufps	$78, %xmm0, %xmm2               ## xmm2 = xmm2[2,3],xmm0[0,1]
after:	
	pshufd	$78, %xmm1, %xmm1               ## xmm1 = xmm1[2,3,0,1]
	shufps	$78, %xmm0, %xmm2               ## xmm2 = xmm2[2,3],xmm0[0,1]
	pand	%xmm2, %xmm1
```

```
before:
	and	v2.16b, v1.16b, v2.16b
	and	v0.16b, v0.16b, v1.16b
	ext	v0.16b, v2.16b, v0.16b, #8
after:
	ext	v1.16b, v1.16b, v1.16b, #8
	ext	v0.16b, v2.16b, v0.16b, #8
	and	v0.16b, v1.16b, v0.16b
```



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

https://reviews.llvm.org/D111901