[AVX512][MS-compatability][llvm] Amending vpmultishiftqb

[coby]

Summary

The 'vpmultishiftqb' instruction was implemented falsely, this patch amend it.
More specifically - (MS dialect) broadcasting variants were implemented falsely.

Differential Revision: https://reviews.llvm.org/D26257

[craig.topper]

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.

include/llvm/IR/IntrinsicsX86.td

2136 ↗

(On Diff #76768)

I think the result type should still be byte elements to match the number of elements in the write mask.

[coby]

In 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?

include/llvm/IR/IntrinsicsX86.td

2136 ↗

(On Diff #76768)

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.

[delena]

include/llvm/IR/IntrinsicsX86.td

2136 ↗

(On Diff #76768)

Craig is right. The result is a vector of byte elements and the mask type should correspond the result. Please fix.

[craig.topper]

We also need AutoUpgrade support from the old signature with the old argument type to the new signature with the new argument type.

[coby]

1. congregate patch to its agreed-upon set of changes
2. emphasize issue via a dedicated test

llvm/trunk/lib/Target/X86/X86InstrAVX512.td

defm rm : AVX512_maskable<opc, MRMSrcMem, _Dst, (outs _Dst.RC:$dst),
                         (ins _Src.RC:$src1, _Src.MemOp:$src2), OpcodeStr,
                         "$src2, $src1", "$src1, $src2",
                         (_Dst.VT (OpNode (_Src.VT _Src.RC:$src1),
                                       (bitconvert (_Src.LdFrag addr:$src2)))),
                         itins.rm>,
                         AVX512BIBase, EVEX_4V;
 
   defm rmb : AVX512_maskable<opc, MRMSrcMem, _Dst, (outs _Dst.RC:$dst),
-                    (ins _Src.RC:$src1, _Dst.ScalarMemOp:$src2),
+                    (ins _Src.RC:$src1, _Brdct.ScalarMemOp:$src2),
                     OpcodeStr,
                     "${src2}"##_Brdct.BroadcastStr##", $src1",
-                     "$src1, ${src2}"##_Dst.BroadcastStr,
+                     "$src1, ${src2}"##_Brdct.BroadcastStr,
                     (_Dst.VT (OpNode (_Src.VT _Src.RC:$src1), (bitconvert
                                  (_Brdct.VT (X86VBroadcast
                                           (_Brdct.ScalarLdFrag addr:$src2)))))),
                     itins.rm>,
                     AVX512BIBase, EVEX_4V, EVEX_B;
 }
 
 defm VPADD : avx512_binop_rm_vl_all<0xFC, 0xFD, 0xFE, 0xD4, "vpadd", add,

llvm/trunk/test/MC/X86/intel-syntax-x86-avx512vbmi_vl.s

+// RUN: llvm-mc -triple x86_64-unknown-unknown -mcpu=knl -mattr=+avx512vl,+avx512vbmi -x86-asm-syntax=intel -output-asm-variant=1 --show-encoding %s | FileCheck %s
+
+// CHECK:  vpmultishiftqb xmm1, xmm2, qword ptr [rcx]{1to2}
+// CHECK:  encoding: [0x62,0xf2,0xed,0x18,0x83,0x09]
+          vpmultishiftqb xmm1, xmm2, qword ptr [rcx]{1to2}
+
+// CHECK:  vpmultishiftqb xmm1 {k1}, xmm2, qword ptr [rcx]{1to2}
+// CHECK:  encoding: [0x62,0xf2,0xed,0x19,0x83,0x09]
+          vpmultishiftqb xmm1 {k1}, xmm2, qword ptr [rcx]{1to2}
+
+// CHECK:  vpmultishiftqb xmm1 {k1} {z}, xmm2, qword ptr [rcx]{1to2}
+// CHECK:  encoding: [0x62,0xf2,0xed,0x99,0x83,0x09]
+          vpmultishiftqb xmm1 {k1} {z}, xmm2, qword ptr [rcx]{1to2}
+
+// CHECK:  vpmultishiftqb ymm1, ymm2, qword ptr [rcx]{1to4}
+// CHECK:  encoding: [0x62,0xf2,0xed,0x38,0x83,0x09]
+          vpmultishiftqb ymm1, ymm2, qword ptr [rcx]{1to4}
+
+// CHECK:  vpmultishiftqb ymm1 {k1}, ymm2, qword ptr [rcx]{1to4}
+// CHECK:  encoding: [0x62,0xf2,0xed,0x39,0x83,0x09]
+          vpmultishiftqb ymm1 {k1}, ymm2, qword ptr [rcx]{1to4}
+
+// CHECK:  vpmultishiftqb ymm1 {k1} {z}, ymm2, qword ptr [rcx]{1to4}
+// CHECK:  encoding: [0x62,0xf2,0xed,0xb9,0x83,0x09]
+          vpmultishiftqb ymm1 {k1} {z}, ymm2, qword ptr [rcx]{1to4}
+
+// CHECK:  vpmultishiftqb zmm1, zmm2, qword ptr [rcx]{1to8}
+// CHECK:  encoding: [0x62,0xf2,0xed,0x58,0x83,0x09]
+          vpmultishiftqb zmm1, zmm2, qword ptr [rcx]{1to8}
+
+// CHECK:  vpmultishiftqb zmm1 {k1}, zmm2, qword ptr [rcx]{1to8}
+// CHECK:  encoding: [0x62,0xf2,0xed,0x59,0x83,0x09]
+          vpmultishiftqb zmm1 {k1}, zmm2, qword ptr [rcx]{1to8}
+
+// CHECK:  vpmultishiftqb zmm1 {k1} {z}, zmm2, qword ptr [rcx]{1to8}
+// CHECK:  encoding: [0x62,0xf2,0xed,0xd9,0x83,0x09]
+          vpmultishiftqb zmm1 {k1} {z}, zmm2, qword ptr [rcx]{1to8}