[PATCH] D26257: [AVX512][llvm] Amending vpmultishiftqb

[coby via llvm-commits]

coby added a comment.

In https://reviews.llvm.org/D26257#586935, @craig.topper wrote:

> Can you please clarify what the perceived problem is? The encodings already match your test case before this change and the result type change doesn't make sense given that the instruction really does write bytes.


Simple.

1. While the inst. does accesses/writes bytes, it defined to operate upon qword elements, and to 'emit' qword elements (each 8 selected 'unaligned' bytes is assembled as one 'aligned' qword). So, if nothing else, it should be well represented.
2. consider the following case :

//vpmultishiftqb xmm1, xmm2, [rax]{1to2}//
would it fly?



================
Comment at: include/llvm/IR/IntrinsicsX86.td:2136
         GCCBuiltin<"__builtin_ia32_vpmultishiftqb128_mask">,
-        Intrinsic<[llvm_v16i8_ty], [llvm_v16i8_ty,
-                   llvm_v16i8_ty, llvm_v16i8_ty, llvm_i16_ty], [IntrNoMem]>;
+        Intrinsic<[llvm_v2i64_ty], [llvm_v2i64_ty,
+                   llvm_v2i64_ty, llvm_v2i64_ty, llvm_i16_ty], [IntrNoMem]>;
----------------
craig.topper wrote:
> I think the result type should still be byte elements to match the number of elements in the write mask.
1. As far as practical matters is to be concerned - the 'raw' result will remain the same.
2. One may expect to be supplied with a result which is partitioned as it spec'd to be, i.e. at a qword granularity. hence, I think it should remain as such.


Repository:
  rL LLVM

https://reviews.llvm.org/D26257