[PATCH] D10683: AVX-512 vector shuffle lowering

[Chandler Carruth]

chandlerc added a comment.



In http://reviews.llvm.org/D10683#207927, @delena wrote:

> AVX-512 shuffles are more symmetric for FP and Int types, than divergent. 
>  So if I split them, I'll write the same code twice. I know this because I have the full shuffle implementation on my table.
>  You can look at it here, but on the left side, in red.
>  http://reviews.llvm.org/D10502
>
> The only "if" you'll see at line 10274, left side.
>
> All vperm/vpermil/valign/unpack are symmetric.


But there are other forms that aren't symmetric (and Simon mentioned some in his email).

You can still have both fp and int sides delegate to common routines for perm(il), align, and unpack (much as the AVX2 code does).

I assume the duplicated code you're frustrated by is the UNPCK lowering code? The interesting this is how much duplication would actually be removed by factoring that out. It might be worth doing, especially as the masks become huge for AVX-512. Still, that should be factored into an unpack lowering helper.

> 

> 

> > Also, it'd be nice if you could add shuffle decode support for the AVX512 vperm* instructions so there are helpful assembly comments.

> 

> 

> No problem, I will. Do you want to see this in the same patch?


I think this should land first. It makes the tests substantially easier to review. The sequence in which these things should land:

1. basic instruction support in the .td files
2. shuffle mask encode/decode which is tested using target intrinsics or tested purely in MC
3. shuffle lowering as in this patch

That way, the support each one needs to be tested lands with it or in a prior patch, and each patch is as reasonably small of a chunk for the reviewer.


Repository:
  rL LLVM

http://reviews.llvm.org/D10683