[PATCH] D12334: [ARM] Do not use vtrn for vectorshuffle if the order is reversed

Jeroen Ketema via llvm-commits llvm-commits at lists.llvm.org
Wed Sep 2 03:30:05 PDT 2015


jketema added inline comments.

================
Comment at: lib/Target/ARM/ARMISelLowering.cpp:5059
@@ -5050,3 +5058,3 @@
   for (unsigned i = 0; i < M.size(); i += NumElts) {
     WhichResult = M[i] == 0 ? 0 : 1;
     for (unsigned j = 0; j < NumElts; j += 2) {
----------------
ab wrote:
> Looking at this again, I realize: isn't this a tad too conservative? What happens when you have:
> 
> 
> ```
> <-1, 4, 2, 6, 1, 5, 3, 7>
> ```
> 
> More importantly: could this break, say with:
> 
> ```
> <-1, 5, 3, 7, 1, 5, 3, 7>
> ```
> 
> which isn't a vtrn, but looks like it will match?  After this patch the '>=' should catch this, I think.  If so, could you add a testcase? 
The first one still generates a vtrn as expected, using both the upper and lower part. The second one also generates a vtrn but uses the upper part twice and not the lower part. Curiously, there don't seem to be any test cases for this behavior.


http://reviews.llvm.org/D12334





More information about the llvm-commits mailing list