Incorrect operand sizes for some MMX instructions: punpcklwd, punpcklbw and punpckldq

[avt77]

Summary

It's a fix for Bug 33838 - Incorrect operand sizes for MMX instructions: I changed the description of the instrs in td file. When I was working with this issue I seriously changed the debug logging for asm-matcher but I decided to introduce those changes in a separate patch which will be published soon.

[craig.topper]

lib/Target/X86/X86InstrMMX.td

112 ↗

(On Diff #128227)

Can we add a X86MemOperand parameter to MMXI_binop_rm_int that defaults to i64mem. Then just make MMXI_binop_rm32_int inherit from MMXI_binop_rm_int and pass the i32mem to that parameter.

[craig.topper]

test/MC/X86/punpack-intel.s

1 ↗

(On Diff #128227)

Just add these to intel-syntax.s and intel-syntax-error.s I don't think its worth a new test file.

[avt77]

I changed the implementation accodingly Craig's requiremetns: now it's shorter and more effective.

[RKSimon]

Please remember to prefix phab subjects, in this case "[X86][MMX]" - and tag it with PR33838 as well:

"[X86][MMX] Incorrect operand sizes for some MMX instructions: punpcklwd, punpcklbw and punpckldq (PR33838)"

Repeat on the commit message as well - it makes email filtering a lot easier - cheers!

[craig.topper]

LGTM

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

def MMX_CVT_PS_ITINS : OpndItins<
   IIC_MMX_CVT_PS_RR, IIC_MMX_CVT_PS_RM
 >;
 }
 
 let Constraints = "$src1 = $dst" in {
   // MMXI_binop_rm_int - Simple MMX binary operator based on intrinsic.
   // When this is cleaned up, remove the FIXME from X86RecognizableInstr.cpp.
   multiclass MMXI_binop_rm_int<bits<8> opc, string OpcodeStr, Intrinsic IntId,
-                               OpndItins itins, bit Commutable = 0> {
+                               OpndItins itins, bit Commutable = 0,
+                               X86MemOperand OType = i64mem> {
     def irr : MMXI<opc, MRMSrcReg, (outs VR64:$dst),
                  (ins VR64:$src1, VR64:$src2),
                  !strconcat(OpcodeStr, "\t{$src2, $dst|$dst, $src2}"),
                  [(set VR64:$dst, (IntId VR64:$src1, VR64:$src2))], itins.rr>,
               Sched<[itins.Sched]> {
       let isCommutable = Commutable;
     }
     def irm : MMXI<opc, MRMSrcMem, (outs VR64:$dst),
-                 (ins VR64:$src1, i64mem:$src2),
+                 (ins VR64:$src1, OType:$src2),
                  !strconcat(OpcodeStr, "\t{$src2, $dst|$dst, $src2}"),
                  [(set VR64:$dst, (IntId VR64:$src1,
                                    (bitconvert (load_mmx addr:$src2))))],
                  itins.rm>, Sched<[itins.Sched.Folded, ReadAfterLd]>;
   }
 
   multiclass MMXI_binop_rmi_int<bits<8> opc, bits<8> opc2, Format ImmForm,
                                 string OpcodeStr, Intrinsic IntId,

 defm MMX_PUNPCKHWD : MMXI_binop_rm_int<0x69, "punpckhwd",
                                        int_x86_mmx_punpckhwd,
                                        MMX_UNPCK_H_ITINS>;
 defm MMX_PUNPCKHDQ : MMXI_binop_rm_int<0x6A, "punpckhdq",
                                        int_x86_mmx_punpckhdq,
                                        MMX_UNPCK_H_ITINS>;
 defm MMX_PUNPCKLBW : MMXI_binop_rm_int<0x60, "punpcklbw",
                                        int_x86_mmx_punpcklbw,
-                                       MMX_UNPCK_L_ITINS>;
+                                       MMX_UNPCK_L_ITINS,
+                                       0, i32mem>;
 defm MMX_PUNPCKLWD : MMXI_binop_rm_int<0x61, "punpcklwd",
                                        int_x86_mmx_punpcklwd,
-                                       MMX_UNPCK_L_ITINS>;
+                                       MMX_UNPCK_L_ITINS,
+                                       0, i32mem>;
 defm MMX_PUNPCKLDQ : MMXI_binop_rm_int<0x62, "punpckldq",
                                        int_x86_mmx_punpckldq,
-                                       MMX_UNPCK_L_ITINS>;
+                                       MMX_UNPCK_L_ITINS,
+                                       0, i32mem>;
 
 // -- Pack Instructions
 defm MMX_PACKSSWB : MMXI_binop_rm_int<0x63, "packsswb", int_x86_mmx_packsswb,
                                       MMX_PCK_ITINS>;
 defm MMX_PACKSSDW : MMXI_binop_rm_int<0x6B, "packssdw", int_x86_mmx_packssdw,
                                       MMX_PCK_ITINS>;
 defm MMX_PACKUSWB : MMXI_binop_rm_int<0x67, "packuswb", int_x86_mmx_packuswb,
                                       MMX_PCK_ITINS>;

llvm/trunk/test/MC/X86/intel-syntax-error.s

 //CHECK: error: scale factor in address must be 1, 2, 4 or 8
 lea RDX, [unknown_number * RAX + RBX + _foo]
 //CHECK: error: BaseReg/IndexReg already set!
 lea RDX, [4 * RAX + 27 * RBX + _pat]
 //CHECK: error: unexpected bracket encountered
 lea RDX, [[arr]
 //CHECK: error: unexpected bracket encountered
 lea RDX, [arr[]
+
+.intel_syntax
+
+// CHECK: error: invalid operand for instruction
+punpcklbw mm0, qword ptr [rsp]
+// CHECK: error: invalid operand for instruction
+punpcklwd mm0, word ptr [rsp]
+// CHECK: error: invalid operand for instruction
+punpckldq mm0, qword ptr [rsp]
+

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

 
 movsd  qword ptr [rax], xmm0
 // CHECK: movsd %xmm0, (%rax)
 // CHECK-STDERR-NOT: movsd qword ptr [rax], xmm0
 
 xlat byte ptr [eax]
 // CHECK: xlatb
 // CHECK-STDERR: memory operand is only for determining the size, (R|E)BX will be used for the location
+
+// CHECK:   punpcklbw
+punpcklbw mm0, dword ptr [rsp]
+// CHECK:   punpcklwd
+punpcklwd mm0, dword ptr [rsp]
+// CHECK:   punpckldq
+punpckldq mm0, dword ptr [rsp]
+

llvm/trunk/test/MC/X86/x86_64-asm-match.s

 // CHECK:  Matching formal operand class MCK_GR32 against actual operand at index 2 (): match success using generic matcher
 // CHECK:  Matching formal operand class InvalidMatchClass against actual operand at index 3: actual operand index out of range Opcode result: complete match, selecting this opcode
 // CHECK:AsmMatcher: found 4 encodings with mnemonic 'punpcklbw'
 // CHECK:Trying to match opcode MMX_PUNPCKLBWirr
 // CHECK:  Matching formal operand class MCK_VR64 against actual operand at index 1 (): match success using generic matcher
 // CHECK:  Matching formal operand class MCK_VR64 against actual operand at index 2 (): Opcode result: multiple operand mismatches, ignoring this opcode
 // CHECK:Trying to match opcode MMX_PUNPCKLBWirm
 // CHECK:  Matching formal operand class MCK_VR64 against actual operand at index 1 (): match success using generic matcher
-// CHECK:  Matching formal operand class MCK_Mem64 against actual operand at index 2 (): match success using generic matcher
+// CHECK:  Matching formal operand class MCK_Mem32 against actual operand at index 2 (): match success using generic matcher
 // CHECK:  Matching formal operand class InvalidMatchClass against actual operand at index 3: actual operand index out of range Opcode result: complete match, selecting this opcode
 
 
 pshufb    CPI1_0(%rip), %xmm1
 sha1rnds4 $1, %xmm1, %xmm2
 pinsrw    $3, %ecx, %xmm5
 crc32l    %gs:0xdeadbeef(%rbx,%rcx,8),%ecx
 
 .intel_syntax
-punpcklbw mm0, qword ptr [rsp]
+punpcklbw mm0, dword ptr [rsp]