[X86][MMX] Add support for MMX zero vector creation

[RKSimon]

Summary

As mentioned on PR35869, (and came up recently on D41517) we don't create a MMX zero register via the PXOR but instead perform a spill to stack from a xmm zero register.

This patch adds support for direct MMX zero vector creation and should make it easier to add better constant vector creation in the future as well.

[craig.topper]

lib/Target/X86/X86InstrInfo.cpp

8964

Do the test cases cover this?

[RKSimon]

lib/Target/X86/X86InstrInfo.cpp

8964

No I don't think they do. Any suggestions on suitable tests? I'm a little unclear as to when this kicks in given that the zero registers are all flagged as isReMaterializable.

[reames]

Reading along for my own education, feel free to ignore if I'm way off base.

I think I'm missing something here. If we have a bitcast of a zero element, why don't we have a generic combine to convert that to a zeroinitializer of the target type? Wouldn't we already need/have handling for a zeroinitializer constant of x86_mmx type?

More generally, why doesn't that generic combine exist for *any* constant?

[efriedma]

Wouldn't we already need/have handling for a zeroinitializer constant of x86_mmx type?

From LangRef: "There are no arrays, vectors or constants of this type."

[RKSimon]

In D41908#972788, @efriedma wrote:

Wouldn't we already need/have handling for a zeroinitializer constant of x86_mmx type?

From LangRef: "There are no arrays, vectors or constants of this type."

Yes, we can't attach constants to the x86_mmx type, hence having to alias it via MMX_MOVW2D

lib/Target/X86/X86InstrInfo.cpp

8964

I still haven't been able to cause this to fire, copying the approach from fold-pcmpeqd-2.ll doesn't seem to work - maybe just drop this part or add an assertion/unreachable?

[reames]

In D41908#972788, @efriedma wrote:

Wouldn't we already need/have handling for a zeroinitializer constant of x86_mmx type?

From LangRef: "There are no arrays, vectors or constants of this type."

Should we consider changing that? We can always revise the LangRef to allow constants if that simplifies the implementation.

[RKSimon]

In D41908#973621, @reames wrote:

In D41908#972788, @efriedma wrote:

Wouldn't we already need/have handling for a zeroinitializer constant of x86_mmx type?

From LangRef: "There are no arrays, vectors or constants of this type."

Should we consider changing that? We can always revise the LangRef to allow constants if that simplifies the implementation.

It's worth looking into, but I'd prefer to get this in first.

[craig.topper]

This seems to cause a constant pool to be use for the second _mm_setzero_si64 call.

#include <x86intrin.h>

__m64 bar(__m64 a, __m64 b, __m64 z, __m64 y) {
  __m64 c = _mm_add_pi32(a, b);
  __m64 d = _mm_mul_su32(c, _mm_setzero_si64());
  __m64 e = _mm_add_pi32(d, z);
  __m64 f = _mm_add_pi16(a, e);
  __m64 g = _mm_add_pi16(b, f);
  __m64 h = _mm_mul_su32(g, y);
  __m64 i = _mm_add_pi16(h, z);
  __m64 j = _mm_add_pi16(i, y);
  __m64 k = _mm_add_pi16(j, e);
  __m64 l = _mm_add_pi16(k, f);
  __m64 m = _mm_add_pi16(l, d);
  __m64 n = _mm_mul_su32(m, c);
  __m64 o = _mm_add_pi16(n, _mm_setzero_si64());
  __m64 p = _mm_add_pi16(o, g);
  __m64 q = _mm_mul_su32(p, i);
  __m64 r = _mm_mul_su32(q, b);
  __m64 s = _mm_add_pi16(r, j);
  __m64 t = _mm_add_pi16(s, k);
  return t;
}

[RKSimon]

Updated with zero fold test recommended by @craig.topper

[craig.topper]

LGTM

lib/Target/X86/X86ISelLowering.cpp

if ((SrcVT == MVT::v4i1 || SrcVT == MVT::v2i1) && VT.isScalarInteger() &&
       return DAG.getNode(ISD::TRUNCATE, dl, VT, N0);
     }
   }
 
   // Since MMX types are special and don't usually play with other vector types,
   // it's better to handle them early to be sure we emit efficient code by
   // avoiding store-load conversions.
   if (VT == MVT::x86mmx) {
+    // Detect zero MMX vectors.
+    if (X86::isZeroNode(N0) || ISD::isBuildVectorAllZeros(N0.getNode())) {
+      SDLoc DL(N0);
+      return DAG.getNode(X86ISD::MMX_MOVW2D, DL, VT,
+                         DAG.getConstant(0, DL, MVT::i32));
+    }
+
     // Detect bitcasts between i32 to x86mmx low word.
     if (N0.getOpcode() == ISD::BUILD_VECTOR && SrcVT == MVT::v2i32 &&
         isNullConstant(N0.getOperand(1))) {
       SDValue N00 = N0.getOperand(0);
       if (N00.getValueType() == MVT::i32)
         return DAG.getNode(X86ISD::MMX_MOVW2D, SDLoc(N00), VT, N00);
     }
 

lib/Target/X86/X86InstrInfo.cpp

bool X86InstrInfo::expandPostRAPseudo(MachineInstr &MI) const {
   case X86::SETB_C8r:
     return Expand2AddrUndef(MIB, get(X86::SBB8rr));
   case X86::SETB_C16r:
     return Expand2AddrUndef(MIB, get(X86::SBB16rr));
   case X86::SETB_C32r:
     return Expand2AddrUndef(MIB, get(X86::SBB32rr));
   case X86::SETB_C64r:
     return Expand2AddrUndef(MIB, get(X86::SBB64rr));
+  case X86::MMX_SET0:
+    return Expand2AddrUndef(MIB, get(X86::MMX_PXORirr));
   case X86::V_SET0:
   case X86::FsFLD0SS:
   case X86::FsFLD0SD:
     return Expand2AddrUndef(MIB, get(HasAVX ? X86::VXORPSrr : X86::XORPSrr));
   case X86::AVX_SET0: {
     assert(HasAVX && "AVX not supported");
     const TargetRegisterInfo *TRI = &getRegisterInfo();
     unsigned SrcReg = MIB->getOperand(0).getReg();
else
     case X86::AVX512_256_SET0:
       Alignment = 32;
       break;
     case X86::V_SET0:
     case X86::V_SETALLONES:
     case X86::AVX512_128_SET0:
       Alignment = 16;
       break;
+    case X86::MMX_SET0:
     case X86::FsFLD0SD:
     case X86::AVX512_FsFLD0SD:
       Alignment = 8;
       break;
     case X86::FsFLD0SS:
     case X86::AVX512_FsFLD0SS:
       Alignment = 4;
       break;
MachineInstr *X86InstrInfo::foldMemoryOperandImpl(
 
   // Make sure the subregisters match.
   // Otherwise we risk changing the size of the load.
   if (LoadMI.getOperand(0).getSubReg() != MI.getOperand(Ops[0]).getSubReg())
     return nullptr;
 
   SmallVector<MachineOperand,X86::AddrNumOperands> MOs;
   switch (LoadMI.getOpcode()) {
+  case X86::MMX_SET0:
   case X86::V_SET0:
   case X86::V_SETALLONES:
   case X86::AVX2_SETALLONES:
   case X86::AVX1_SETALLONES:
   case X86::AVX_SET0:
   case X86::AVX512_128_SET0:
   case X86::AVX512_256_SET0:
   case X86::AVX512_512_SET0:
if (Opc == X86::FsFLD0SS || Opc == X86::AVX512_FsFLD0SS)
       Ty = Type::getFloatTy(MF.getFunction().getContext());
     else if (Opc == X86::FsFLD0SD || Opc == X86::AVX512_FsFLD0SD)
       Ty = Type::getDoubleTy(MF.getFunction().getContext());
     else if (Opc == X86::AVX512_512_SET0 || Opc == X86::AVX512_512_SETALLONES)
       Ty = VectorType::get(Type::getInt32Ty(MF.getFunction().getContext()),16);
     else if (Opc == X86::AVX2_SETALLONES || Opc == X86::AVX_SET0 ||
              Opc == X86::AVX512_256_SET0 || Opc == X86::AVX1_SETALLONES)
       Ty = VectorType::get(Type::getInt32Ty(MF.getFunction().getContext()), 8);
+    else if (Opc == X86::MMX_SET0)
+      Ty = VectorType::get(Type::getInt32Ty(MF.getFunction().getContext()), 2);
     else
       Ty = VectorType::get(Type::getInt32Ty(MF.getFunction().getContext()), 4);
 
     bool IsAllOnes = (Opc == X86::V_SETALLONES || Opc == X86::AVX2_SETALLONES ||
                       Opc == X86::AVX512_512_SETALLONES ||
                       Opc == X86::AVX1_SETALLONES);
     const Constant *C = IsAllOnes ? Constant::getAllOnesValue(Ty) :
                                     Constant::getNullValue(Ty);

lib/Target/X86/X86InstrMMX.td

def MMX_CVT_PD_ITINS : OpndItins<
   IIC_MMX_CVT_PD_RR, IIC_MMX_CVT_PD_RM
 >;
 
 def MMX_CVT_PS_ITINS : OpndItins<
   IIC_MMX_CVT_PS_RR, IIC_MMX_CVT_PS_RM
 >;
 }
 
+// Alias instruction that maps zero vector to pxor mmx.
+// This is expanded by ExpandPostRAPseudos to an pxor.
+// We set canFoldAsLoad because this can be converted to a constant-pool
+// load of an all-zeros value if folding it would be beneficial.
+let isReMaterializable = 1, isAsCheapAsAMove = 1, canFoldAsLoad = 1,
+    isPseudo = 1, SchedRW = [WriteZero] in {
+def MMX_SET0 : I<0, Pseudo, (outs VR64:$dst), (ins), "", []>;
+}
+
 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,
                                X86MemOperand OType = i64mem> {
     def irr : MMXI<opc, MRMSrcReg, (outs VR64:$dst),
                  (ins VR64:$src1, VR64:$src2),
def MMX_MOVD64rm : MMXI<0x6E, MRMSrcMem, (outs VR64:$dst), (ins i32mem:$src),
                         [(set VR64:$dst,
                         (x86mmx (scalar_to_vector (loadi32 addr:$src))))],
                         IIC_MMX_MOV_MM_RM>, Sched<[WriteLoad]>;
 
 let Predicates = [HasMMX] in {
   let AddedComplexity = 15 in
     def : Pat<(x86mmx (MMX_X86movw2d GR32:$src)),
               (MMX_MOVD64rr GR32:$src)>;
-  let AddedComplexity = 20 in
+  let AddedComplexity = 20 in {
+    def : Pat<(x86mmx (MMX_X86movw2d (i32 0))),
+              (MMX_SET0)>;
     def : Pat<(x86mmx (MMX_X86movw2d (loadi32 addr:$src))),
               (MMX_MOVD64rm addr:$src)>;
   }
+}
 
 let mayStore = 1 in
 def MMX_MOVD64mr : MMXI<0x7E, MRMDestMem, (outs), (ins i32mem:$dst, VR64:$src),
                         "movd\t{$src, $dst|$dst, $src}", [], IIC_MMX_MOV_MM_RM>,
                    Sched<[WriteStore]>;
 
 def MMX_MOVD64grr : MMXI<0x7E, MRMDestReg, (outs GR32:$dst), (ins VR64:$src),
                          "movd\t{$src, $dst|$dst, $src}",

test/CodeGen/X86/mmx-fold-zero.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc < %s -mtriple=i686-unknown-unknown -mattr=+mmx,+sse2 | FileCheck %s --check-prefixes=CHECK,X86
 ; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mattr=+mmx,+sse2 | FileCheck %s --check-prefixes=CHECK,X64
 
 define double @mmx_zero(double, double, double, double) nounwind {
 ; X86-LABEL: mmx_zero:
 ; X86:       # %bb.0:
 ; X86-NEXT:    pushl %ebp
 ; X86-NEXT:    movl %esp, %ebp
 ; X86-NEXT:    andl $-8, %esp
-; X86-NEXT:    subl $24, %esp
+; X86-NEXT:    subl $16, %esp
 ; X86-NEXT:    movq 8(%ebp), %mm0
 ; X86-NEXT:    movq 16(%ebp), %mm5
-; X86-NEXT:    movq %mm5, {{[0-9]+}}(%esp) # 8-byte Spill
+; X86-NEXT:    movq %mm5, (%esp) # 8-byte Spill
 ; X86-NEXT:    movq %mm0, %mm3
 ; X86-NEXT:    paddd %mm5, %mm3
-; X86-NEXT:    xorps %xmm0, %xmm0
-; X86-NEXT:    movdq2q %xmm0, %mm1
-; X86-NEXT:    movq %mm1, (%esp) # 8-byte Spill
+; X86-NEXT:    pxor %mm1, %mm1
 ; X86-NEXT:    movq %mm3, %mm6
 ; X86-NEXT:    pmuludq %mm1, %mm6
 ; X86-NEXT:    movq 24(%ebp), %mm4
 ; X86-NEXT:    movq %mm6, %mm2
 ; X86-NEXT:    paddd %mm4, %mm2
 ; X86-NEXT:    paddw %mm2, %mm0
 ; X86-NEXT:    movq %mm5, %mm1
 ; X86-NEXT:    paddw %mm0, %mm1
 ; X86-NEXT:    movq 32(%ebp), %mm5
 ; X86-NEXT:    movq %mm1, %mm7
 ; X86-NEXT:    pmuludq %mm5, %mm7
 ; X86-NEXT:    paddw %mm4, %mm7
 ; X86-NEXT:    paddw %mm7, %mm5
 ; X86-NEXT:    paddw %mm5, %mm2
 ; X86-NEXT:    paddw %mm2, %mm0
 ; X86-NEXT:    paddw %mm6, %mm0
 ; X86-NEXT:    pmuludq %mm3, %mm0
-; X86-NEXT:    paddw (%esp), %mm0 # 8-byte Folded Reload
+; X86-NEXT:    paddw {{\.LCPI.*}}, %mm0
 ; X86-NEXT:    paddw %mm1, %mm0
 ; X86-NEXT:    pmuludq %mm7, %mm0
-; X86-NEXT:    pmuludq {{[0-9]+}}(%esp), %mm0 # 8-byte Folded Reload
+; X86-NEXT:    pmuludq (%esp), %mm0 # 8-byte Folded Reload
 ; X86-NEXT:    paddw %mm5, %mm0
 ; X86-NEXT:    paddw %mm2, %mm0
 ; X86-NEXT:    movq2dq %mm0, %xmm0
 ; X86-NEXT:    movsd %xmm0, {{[0-9]+}}(%esp)
 ; X86-NEXT:    fldl {{[0-9]+}}(%esp)
 ; X86-NEXT:    movl %ebp, %esp
 ; X86-NEXT:    popl %ebp
 ; X86-NEXT:    retl
 ;
 ; X64-LABEL: mmx_zero:
 ; X64:       # %bb.0:
 ; X64-NEXT:    movdq2q %xmm0, %mm0
 ; X64-NEXT:    movdq2q %xmm1, %mm5
 ; X64-NEXT:    movq %mm5, -{{[0-9]+}}(%rsp) # 8-byte Spill
 ; X64-NEXT:    movq %mm0, %mm3
 ; X64-NEXT:    paddd %mm5, %mm3
-; X64-NEXT:    xorps %xmm0, %xmm0
-; X64-NEXT:    movdq2q %xmm0, %mm1
-; X64-NEXT:    movq %mm1, -{{[0-9]+}}(%rsp) # 8-byte Spill
+; X64-NEXT:    pxor %mm1, %mm1
 ; X64-NEXT:    movq %mm3, %mm6
 ; X64-NEXT:    pmuludq %mm1, %mm6
 ; X64-NEXT:    movdq2q %xmm2, %mm4
 ; X64-NEXT:    movq %mm6, %mm2
 ; X64-NEXT:    paddd %mm4, %mm2
 ; X64-NEXT:    paddw %mm2, %mm0
 ; X64-NEXT:    movq %mm5, %mm1
 ; X64-NEXT:    paddw %mm0, %mm1
 ; X64-NEXT:    movdq2q %xmm3, %mm5
 ; X64-NEXT:    movq %mm1, %mm7
 ; X64-NEXT:    pmuludq %mm5, %mm7
 ; X64-NEXT:    paddw %mm4, %mm7
 ; X64-NEXT:    paddw %mm7, %mm5
 ; X64-NEXT:    paddw %mm5, %mm2
 ; X64-NEXT:    paddw %mm2, %mm0
 ; X64-NEXT:    paddw %mm6, %mm0
 ; X64-NEXT:    pmuludq %mm3, %mm0
-; X64-NEXT:    paddw -{{[0-9]+}}(%rsp), %mm0 # 8-byte Folded Reload
+; X64-NEXT:    paddw {{\.LCPI.*}}, %mm0
 ; X64-NEXT:    paddw %mm1, %mm0
 ; X64-NEXT:    pmuludq %mm7, %mm0
 ; X64-NEXT:    pmuludq -{{[0-9]+}}(%rsp), %mm0 # 8-byte Folded Reload
 ; X64-NEXT:    paddw %mm5, %mm0
 ; X64-NEXT:    paddw %mm2, %mm0
 ; X64-NEXT:    movq2dq %mm0, %xmm0
 ; X64-NEXT:    retq
   %5 = bitcast double %0 to x86_mmx

test/CodeGen/X86/vector-shuffle-mmx.ll

entry:
   ret void
 }
 
 define void @test1() {
 ; X32-LABEL: test1:
 ; X32:       ## %bb.0: ## %entry
 ; X32-NEXT:    pushl %edi
 ; X32-NEXT:    .cfi_def_cfa_offset 8
-; X32-NEXT:    subl $16, %esp
-; X32-NEXT:    .cfi_def_cfa_offset 24
+; X32-NEXT:    subl $8, %esp
+; X32-NEXT:    .cfi_def_cfa_offset 16
 ; X32-NEXT:    .cfi_offset %edi, -8
-; X32-NEXT:    xorps %xmm0, %xmm0
-; X32-NEXT:    movlps %xmm0, (%esp)
-; X32-NEXT:    movq (%esp), %mm0
+; X32-NEXT:    pxor %mm0, %mm0
 ; X32-NEXT:    movsd {{.*#+}} xmm0 = mem[0],zero
-; X32-NEXT:    movsd %xmm0, {{[0-9]+}}(%esp)
-; X32-NEXT:    movq {{[0-9]+}}(%esp), %mm1
+; X32-NEXT:    movsd %xmm0, (%esp)
+; X32-NEXT:    movq (%esp), %mm1
 ; X32-NEXT:    xorl %edi, %edi
 ; X32-NEXT:    maskmovq %mm1, %mm0
-; X32-NEXT:    addl $16, %esp
+; X32-NEXT:    addl $8, %esp
 ; X32-NEXT:    popl %edi
 ; X32-NEXT:    retl
 ;
 ; X64-LABEL: test1:
 ; X64:       ## %bb.0: ## %entry
-; X64-NEXT:    xorps %xmm0, %xmm0
-; X64-NEXT:    movlps %xmm0, -{{[0-9]+}}(%rsp)
-; X64-NEXT:    movq -{{[0-9]+}}(%rsp), %mm0
+; X64-NEXT:    pxor %mm0, %mm0
 ; X64-NEXT:    movq {{.*}}(%rip), %rax
 ; X64-NEXT:    movq %rax, -{{[0-9]+}}(%rsp)
 ; X64-NEXT:    movq -{{[0-9]+}}(%rsp), %mm1
 ; X64-NEXT:    xorl %edi, %edi
 ; X64-NEXT:    maskmovq %mm1, %mm0
 ; X64-NEXT:    retq
 entry:
   %tmp528 = bitcast <8 x i8> zeroinitializer to <2 x i32>
entry:
   %1 = shufflevector <2 x i32> %0, <2 x i32> undef, <2 x i32> zeroinitializer
   store <2 x i32> %1, <2 x i32>* @tmp_V2i, align 8
   ret void
 }
 
 define <4 x float> @pr35869() nounwind {
 ; X32-LABEL: pr35869:
 ; X32:       ## %bb.0:
-; X32-NEXT:    subl $28, %esp
+; X32-NEXT:    subl $12, %esp
 ; X32-NEXT:    movl $64, %eax
 ; X32-NEXT:    movd %eax, %xmm0
 ; X32-NEXT:    movq %xmm0, (%esp)
-; X32-NEXT:    pxor %xmm0, %xmm0
-; X32-NEXT:    movq %xmm0, {{[0-9]+}}(%esp)
 ; X32-NEXT:    movq (%esp), %mm0
-; X32-NEXT:    punpcklbw {{[0-9]+}}(%esp), %mm0 ## mm0 = mm0[0],mem[0],mm0[1],mem[1],mm0[2],mem[2],mm0[3],mem[3]
-; X32-NEXT:    movq %xmm0, {{[0-9]+}}(%esp)
-; X32-NEXT:    movq {{[0-9]+}}(%esp), %mm1
+; X32-NEXT:    pxor %mm1, %mm1
+; X32-NEXT:    punpcklbw %mm1, %mm0 ## mm0 = mm0[0],mm1[0],mm0[1],mm1[1],mm0[2],mm1[2],mm0[3],mm1[3]
+; X32-NEXT:    pxor %xmm0, %xmm0
 ; X32-NEXT:    pcmpgtw %mm0, %mm1
 ; X32-NEXT:    movq %mm0, %mm2
 ; X32-NEXT:    punpckhwd %mm1, %mm2 ## mm2 = mm2[2],mm1[2],mm2[3],mm1[3]
 ; X32-NEXT:    cvtpi2ps %mm2, %xmm0
 ; X32-NEXT:    movlhps {{.*#+}} xmm0 = xmm0[0,0]
 ; X32-NEXT:    punpcklwd %mm1, %mm0 ## mm0 = mm0[0],mm1[0],mm0[1],mm1[1]
 ; X32-NEXT:    cvtpi2ps %mm0, %xmm0
-; X32-NEXT:    addl $28, %esp
+; X32-NEXT:    addl $12, %esp
 ; X32-NEXT:    retl
 ;
 ; X64-LABEL: pr35869:
 ; X64:       ## %bb.0:
 ; X64-NEXT:    movl $64, %eax
 ; X64-NEXT:    movd %eax, %xmm0
 ; X64-NEXT:    movq %xmm0, -{{[0-9]+}}(%rsp)
-; X64-NEXT:    pxor %xmm0, %xmm0
-; X64-NEXT:    movq %xmm0, -{{[0-9]+}}(%rsp)
 ; X64-NEXT:    movq -{{[0-9]+}}(%rsp), %mm0
-; X64-NEXT:    punpcklbw -{{[0-9]+}}(%rsp), %mm0 ## mm0 = mm0[0],mem[0],mm0[1],mem[1],mm0[2],mem[2],mm0[3],mem[3]
-; X64-NEXT:    movq %xmm0, -{{[0-9]+}}(%rsp)
-; X64-NEXT:    movq -{{[0-9]+}}(%rsp), %mm1
+; X64-NEXT:    pxor %mm1, %mm1
+; X64-NEXT:    punpcklbw %mm1, %mm0 ## mm0 = mm0[0],mm1[0],mm0[1],mm1[1],mm0[2],mm1[2],mm0[3],mm1[3]
+; X64-NEXT:    pxor %xmm0, %xmm0
 ; X64-NEXT:    pcmpgtw %mm0, %mm1
 ; X64-NEXT:    movq %mm0, %mm2
 ; X64-NEXT:    punpckhwd %mm1, %mm2 ## mm2 = mm2[2],mm1[2],mm2[3],mm1[3]
 ; X64-NEXT:    cvtpi2ps %mm2, %xmm0
 ; X64-NEXT:    movlhps {{.*#+}} xmm0 = xmm0[0,0]
 ; X64-NEXT:    punpcklwd %mm1, %mm0 ## mm0 = mm0[0],mm1[0],mm0[1],mm1[1]
 ; X64-NEXT:    cvtpi2ps %mm0, %xmm0
 ; X64-NEXT:    retq