[PATCH] D17041: [X86] Don't assume that a shuffle operand is #0: it isn't for VPERMV.

Ahmed Bougacha via llvm-commits llvm-commits at lists.llvm.org
Wed Feb 10 10:38:35 PST 2016


ab added subscribers: delena, igorb, AsafBadouh.
ab added a comment.

In http://reviews.llvm.org/D17041#348673, @spatel wrote:

> Hi Ahmed -
>
> I'm still blissfully ignorant of AVX-512, so my opinion shouldn't have as much weight as people who are working on that (cc some of the others that were on http://reviews.llvm.org/D10683?).
>
> But I would lean towards the first solution: swap the operands for the X86ISD::VPERMV node. If I'm understanding the problem, this would (mostly?) limit the changes to the td defs.


Yep

> We barely document the DAG node operands or their orders anyway, so adding that kind of info seems fair to me. I think it's better to preserve the software uniformity as long as possible, even if the hardware instructions are a mess. Ie, the C instrinsics keep the expected order:

>  https://software.intel.com/en-us/node/524011

>  ...so let's preserve that illusion as long as we can.


I agree; I'm mostly worried about us developers expecting the SD op to match the instruction.
I didn't realize the C intrinsics were swapped. IMO that's enough to justify #1.

..but now that I look it up, the AVX-512 intrinsics use the instruction order *sigh*
Back to square one.

-Ahmed


http://reviews.llvm.org/D17041





More information about the llvm-commits mailing list