Optimized instruction sequence for sitofp operation on X86-32

[delena]

Summary

Optimized
sitofp i64 %x to double
the current sequence

movl %ecx, 8(%esp)
movl %edx, 12(%esp)
fildll 8(%esp)

replaced with

movd %ecx, %xmm0
movd %edx, %xmm1
punpckldq %xmm1, %xmm0
movq %xmm0, 8(%esp)

[mbodart]

Hi Elena,

Just a few minor comments, otherwise LGTM!

• mitch

../lib/Target/X86/X86ISelLowering.cpp

12658 ↗

(On Diff #44200)

It would be useful to have a source comment here to the effect:

Bitcasting to f64 here allows us to do a single 64-bit store from an SSE register,
avoiding the store forwarding penalty that would come with two 32-bit stores.

../test/CodeGen/X86/scalar-int-to-fp.ll

76 ↗

(On Diff #44200)

For both test functions u64_to_f and s64_to_f, we should add the following additional checks
before the fildll:

AVX512_32: punpckldq
SSE_32: punpckldq

120–130 ↗

(On Diff #44200)

Rather than creating a new function, it would seem more simple to just add a check for punpckldq, for both SSE2_32 and AVX512_32, in the existing s64_to_d function.

[delena]

../test/CodeGen/X86/scalar-int-to-fp.ll

120–130 ↗

(On Diff #44200)

This is the code generated for s64_to_d, because the input parameters are already on stack.

pushl   %ebp
movl    %esp, %ebp
andl    $-8, %esp
subl    $8, %esp
fildll  8(%ebp)
fstpl   (%esp)
fldl    (%esp)
movl    %ebp, %esp
popl    %ebp
retl

[delena]

Added the same bitcast for UINT_TO_FP.
Added more tests.

llvm/trunk/lib/Target/X86/X86ISelLowering.cpp

X86TargetLowering::X86TargetLowering(const X86TargetMachine &TM,
   if (!X86ScalarSSEf64) {
     setOperationAction(ISD::BITCAST        , MVT::f32  , Expand);
     setOperationAction(ISD::BITCAST        , MVT::i32  , Expand);
     if (Subtarget->is64Bit()) {
       setOperationAction(ISD::BITCAST      , MVT::f64  , Expand);
       // Without SSE, i64->f64 goes through memory.
       setOperationAction(ISD::BITCAST      , MVT::i64  , Expand);
     }
-  }
+  } else if (!Subtarget->is64Bit())
+    setOperationAction(ISD::BITCAST      , MVT::i64  , Custom);
 
   // Scalar integer divide and remainder are lowered to use operations that
   // produce two results, to match the available instructions. This exposes
   // the two-result form to trivial CSE, which is able to combine x/y and x%y
   // into a single instruction.
   //
   // Scalar integer multiply-high is also lowered to use two-result
   // operations, to match the available instructions. However, plain multiply
SDValue X86TargetLowering::LowerSINT_TO_FP(SDValue Op,
   // Legal.
   if (SrcVT == MVT::i32 && isScalarFPTypeInSSEReg(Op.getValueType()))
     return Op;
   if (SrcVT == MVT::i64 && isScalarFPTypeInSSEReg(Op.getValueType()) &&
       Subtarget->is64Bit()) {
     return Op;
   }
 
+  SDValue ValueToStore = Op.getOperand(0);
+  if (SrcVT == MVT::i64 && isScalarFPTypeInSSEReg(Op.getValueType()) &&
+      !Subtarget->is64Bit())
+    // Bitcasting to f64 here allows us to do a single 64-bit store from
+    // an SSE register, avoiding the store forwarding penalty that would come
+    // with two 32-bit stores.
+    ValueToStore = DAG.getBitcast(MVT::f64, ValueToStore);
+
   unsigned Size = SrcVT.getSizeInBits()/8;
   MachineFunction &MF = DAG.getMachineFunction();
   auto PtrVT = getPointerTy(MF.getDataLayout());
   int SSFI = MF.getFrameInfo()->CreateStackObject(Size, Size, false);
   SDValue StackSlot = DAG.getFrameIndex(SSFI, PtrVT);
   SDValue Chain = DAG.getStore(
-      DAG.getEntryNode(), dl, Op.getOperand(0), StackSlot,
+      DAG.getEntryNode(), dl, ValueToStore, StackSlot,
       MachinePointerInfo::getFixedStack(DAG.getMachineFunction(), SSFI), false,
       false, 0);
   return BuildFILD(Op, SrcVT, Chain, StackSlot, DAG);
 }
 
 SDValue X86TargetLowering::BuildFILD(SDValue Op, EVT SrcVT, SDValue Chain,
                                      SDValue StackSlot,
                                      SelectionDAG &DAG) const {
if (SrcVT == MVT::i32) {
     SDValue Store2 = DAG.getStore(Store1, dl, DAG.getConstant(0, dl, MVT::i32),
                                   OffsetSlot, MachinePointerInfo(),
                                   false, false, 0);
     SDValue Fild = BuildFILD(Op, MVT::i64, Store2, StackSlot, DAG);
     return Fild;
   }
 
   assert(SrcVT == MVT::i64 && "Unexpected type in UINT_TO_FP");
-  SDValue Store = DAG.getStore(DAG.getEntryNode(), dl, Op.getOperand(0),
+  SDValue ValueToStore = Op.getOperand(0);
+  if (isScalarFPTypeInSSEReg(Op.getValueType()) && !Subtarget->is64Bit())
+    // Bitcasting to f64 here allows us to do a single 64-bit store from
+    // an SSE register, avoiding the store forwarding penalty that would come
+    // with two 32-bit stores.
+    ValueToStore = DAG.getBitcast(MVT::f64, ValueToStore);
+  SDValue Store = DAG.getStore(DAG.getEntryNode(), dl, ValueToStore,
                                StackSlot, MachinePointerInfo(),
                                false, false, 0);
   // For i64 source, we need to add the appropriate power of 2 if the input
   // was negative.  This is the same as the optimization in
   // DAGTypeLegalizer::ExpandIntOp_UNIT_TO_FP, and for it to be safe here,
   // we must be careful to do the computation in x87 extended precision, not
   // in SSE. (The generic code can't know it's OK to do this, or how to.)
   int SSFI = cast<FrameIndexSDNode>(StackSlot)->getIndex();
static SDValue LowerCMP_SWAP(SDValue Op, const X86Subtarget *Subtarget,
   return SDValue();
 }
 
 static SDValue LowerBITCAST(SDValue Op, const X86Subtarget *Subtarget,
                             SelectionDAG &DAG) {
   MVT SrcVT = Op.getOperand(0).getSimpleValueType();
   MVT DstVT = Op.getSimpleValueType();
 
-  if (SrcVT == MVT::v2i32 || SrcVT == MVT::v4i16 || SrcVT == MVT::v8i8) {
+  if (SrcVT == MVT::v2i32 || SrcVT == MVT::v4i16 || SrcVT == MVT::v8i8 ||
+      SrcVT == MVT::i64) {
     assert(Subtarget->hasSSE2() && "Requires at least SSE2!");
     if (DstVT != MVT::f64)
       // This conversion needs to be expanded.
       return SDValue();
 
-    SDValue InVec = Op->getOperand(0);
+    SDValue Op0 = Op->getOperand(0);
+    SmallVector<SDValue, 16> Elts;
     SDLoc dl(Op);
-    unsigned NumElts = SrcVT.getVectorNumElements();
-    MVT SVT = SrcVT.getVectorElementType();
+    unsigned NumElts;
+    MVT SVT;
+    if (SrcVT.isVector()) {
+      NumElts = SrcVT.getVectorNumElements();
+      SVT = SrcVT.getVectorElementType();
 
       // Widen the vector in input in the case of MVT::v2i32.
       // Example: from MVT::v2i32 to MVT::v4i32.
-    SmallVector<SDValue, 16> Elts;
       for (unsigned i = 0, e = NumElts; i != e; ++i)
-      Elts.push_back(DAG.getNode(ISD::EXTRACT_VECTOR_ELT, dl, SVT, InVec,
+        Elts.push_back(DAG.getNode(ISD::EXTRACT_VECTOR_ELT, dl, SVT, Op0,
                                    DAG.getIntPtrConstant(i, dl)));
-
+    } else {
+      assert(SrcVT == MVT::i64 && !Subtarget->is64Bit() &&
+             "Unexpected source type in LowerBITCAST");
+      Elts.push_back(DAG.getNode(ISD::EXTRACT_ELEMENT, dl, MVT::i32, Op0,
+                                 DAG.getIntPtrConstant(0, dl)));
+      Elts.push_back(DAG.getNode(ISD::EXTRACT_ELEMENT, dl, MVT::i32, Op0,
+                                 DAG.getIntPtrConstant(1, dl)));
+      NumElts = 2;
+      SVT = MVT::i32;
+    }
     // Explicitly mark the extra elements as Undef.
     Elts.append(NumElts, DAG.getUNDEF(SVT));
 
     EVT NewVT = EVT::getVectorVT(*DAG.getContext(), SVT, NumElts * 2);
     SDValue BV = DAG.getNode(ISD::BUILD_VECTOR, dl, NewVT, Elts);
     SDValue ToV2F64 = DAG.getBitcast(MVT::v2f64, BV);
     return DAG.getNode(ISD::EXTRACT_VECTOR_ELT, dl, MVT::f64, ToV2F64,
                        DAG.getIntPtrConstant(0, dl));

llvm/trunk/test/CodeGen/X86/dagcombine-cse.ll

 ; REQUIRES: asserts
-; RUN: llc < %s -march=x86 -mattr=+sse2 -mtriple=i386-apple-darwin -stats 2>&1 | grep asm-printer | grep 14
+; RUN: llc < %s -march=x86 -mattr=+sse2 -mtriple=i386-apple-darwin -stats 2>&1 | grep asm-printer | grep 13
 
 define i32 @t(i8* %ref_frame_ptr, i32 %ref_frame_stride, i32 %idxX, i32 %idxY) nounwind  {
 entry:
   %tmp7 = mul i32 %idxY, %ref_frame_stride    ; <i32> [#uses=2]
   %tmp9 = add i32 %tmp7, %idxX    ; <i32> [#uses=1]
   %tmp11 = getelementptr i8, i8* %ref_frame_ptr, i32 %tmp9    ; <i8*> [#uses=1]
   %tmp1112 = bitcast i8* %tmp11 to i32*    ; <i32*> [#uses=1]
   %tmp13 = load i32, i32* %tmp1112, align 4    ; <i32> [#uses=1]

llvm/trunk/test/CodeGen/X86/scalar-int-to-fp.ll

 ; CHECK-LABEL: s32_to_x
 ; CHECK: fildl
 define x86_fp80 @s32_to_x(i32 %a) nounwind {
   %r = sitofp i32 %a to x86_fp80
   ret x86_fp80 %r
 }
 
 ; CHECK-LABEL: u64_to_f
+; AVX512_32: vmovq {{.*#+}} xmm0 = mem[0],zero
+; AVX512_32: vmovlpd %xmm0, {{[0-9]+}}(%esp)
 ; AVX512_32: fildll
+
 ; AVX512_64: vcvtusi2ssq
+
+; SSE2_32: movq {{.*#+}} xmm0 = mem[0],zero
+; SSE2_32: movq %xmm0, {{[0-9]+}}(%esp)
 ; SSE2_32: fildll
+
 ; SSE2_64: cvtsi2ssq
 ; X87: fildll
 define float @u64_to_f(i64 %a) nounwind {
   %r = uitofp i64 %a to float
   ret float %r
 }
 
 ; CHECK-LABEL: s64_to_f
 ; AVX512_32: fildll
 ; AVX512_64: vcvtsi2ssq
 ; SSE2_32: fildll
 ; SSE2_64: cvtsi2ssq
 ; X87: fildll
 define float @s64_to_f(i64 %a) nounwind {
   %r = sitofp i64 %a to float
   ret float %r
 }
 
+; CHECK-LABEL: s64_to_f_2
+; SSE2_32:    movd %ecx, %xmm0
+; SSE2_32:    movd %eax, %xmm1
+; SSE2_32:    punpckldq {{.*#+}} xmm1 = xmm1[0],xmm0[0],xmm1[1],xmm0[1]
+; SSE2_32:    movq %xmm1, {{[0-9]+}}(%esp)
+; SSE2_32:    fildll {{[0-9]+}}(%esp)
+
+; AVX512_32:    vmovd %eax, %xmm0
+; AVX512_32:    vpinsrd $1, %ecx, %xmm0, %xmm0
+; AVX512_32:    vmovlpd %xmm0, {{[0-9]+}}(%esp)
+; AVX512_32:    fildll {{[0-9]+}}(%esp)
+
+define float @s64_to_f_2(i64 %a) nounwind {
+  %a1 = add i64 %a, 5
+  %r = sitofp i64 %a1 to float
+  ret float %r
+}
+
 ; CHECK-LABEL: u64_to_d
 ; AVX512_32: vpunpckldq
 ; AVX512_64: vcvtusi2sdq
 ; SSE2_32: punpckldq
 ; SSE2_64: punpckldq
 ; X87: fildll
 define double @u64_to_d(i64 %a) nounwind {
   %r = uitofp i64 %a to double
   ret double %r
 }
 
 ; CHECK-LABEL: s64_to_d
 ; AVX512_32: fildll
 ; AVX512_64: vcvtsi2sdq
 ; SSE2_32: fildll
 ; SSE2_64: cvtsi2sdq
 ; X87: fildll
 define double @s64_to_d(i64 %a) nounwind {
   %r = sitofp i64 %a to double
   ret double %r
 }
 
+; CHECK-LABEL: s64_to_d_2
+; SSE2_32: movd %ecx, %xmm0
+; SSE2_32: movd %eax, %xmm1
+; SSE2_32: punpckldq %xmm0, %xmm1
+; SSE2_32: movq %xmm1, {{[0-9]+}}(%esp)
+; SSE2_32: fildll
+
+; AVX512_32:    vmovd %eax, %xmm0
+; AVX512_32:    vpinsrd $1, %ecx, %xmm0, %xmm0
+; AVX512_32:    vmovlpd %xmm0, {{[0-9]+}}(%esp)
+; AVX512_32: fildll
+
+define double @s64_to_d_2(i64 %a) nounwind {
+  %b = add i64 %a, 5
+  %f = sitofp i64 %b to double
+  ret double %f
+}
+
 ; CHECK-LABEL: u64_to_x
 ; CHECK: fildll
 define x86_fp80 @u64_to_x(i64 %a) nounwind {
   %r = uitofp i64 %a to x86_fp80
   ret x86_fp80 %r
 }
 
 ; CHECK-LABEL: s64_to_x
 ; CHECK: fildll
 define x86_fp80 @s64_to_x(i64 %a) nounwind {
   %r = sitofp i64 %a to x86_fp80
   ret x86_fp80 %r
 }