[PATCH] D33994: [DAGCombiner] Add another combine from build vector to shuffle

[Guy Blank via Phabricator via llvm-commits]

guyblank added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:14010
       }
+    } else if (InVT2.getSizeInBits() * 2 == VT.getSizeInBits() &&
+               InVT1.getSizeInBits() == VT.getSizeInBits()) {
----------------
delena wrote:
> In the case above you'll find (VT.getSizeInBits() % InVT1.getSizeInBits() == 0). Is this your case
>  (VT.getSizeInBits() % InVT2.getSizeInBits() == 0) ?
The case in line 13962 also checks that InVT1 == InVT2.

```

 if ((VT.getSizeInBits() % InVT1.getSizeInBits() == 0) && InVT1 == InVT2)

```
It then concats InVT1,InVT2 and a bunch of undef vectors to reach the size of VT.
The shuffle will be on the result of the concats and an undef vector of the same size.



In the case i've added, InVT2 is smaller than InVT1. So it is concated with undef to reach the size of InVT1 and VT.
The shuffle will be on InVT1 and the result of the concat.




================
Comment at: test/CodeGen/X86/vector-shuffle-v48.ll:39
   %5 = lshr <16 x i32> %4, %x2
   %6 = trunc <16 x i32> %5 to <16 x i8>
   ret <16 x i8> %6
----------------
RKSimon wrote:
> Why do you have all this extra code (zext/mul/lshr/trunc)? Isn't the problem just in the shufflevector?
specifically in this case it causes the build vector result to be v32i8, and be caught by the if i've added.
but i should probably change the shufflevector to just return <32 x i8>, since it would demonstrate the issue as well, without the extra code.

Thanks! 


https://reviews.llvm.org/D33994