[PATCH] D23808: [X86][SSE] Add lowering to cvttpd2dq/cvttps2dq for sitofp v2f64/2f32 to 2i32

[Simon Pilgrim via llvm-commits]

RKSimon added inline comments.

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:22229
@@ -22227,1 +22228,3 @@
 
+    if (IsSigned && N->getValueType(0) == MVT::v2i32) {
+      assert(Subtarget.hasSSE2() && "Requires at least SSE2!");
----------------
delena wrote:
> RKSimon wrote:
> > As the result type is the illegal v2i32 type we need to use ReplaceNodeResults - we can't just go direct to Lower_FP_TO_SINT as type legalization will get in the way.
> If you specify 
> setOperationAction(ISD::FP_TO_SINT,         MVT::v2f32, Custom) - specify source type, because type legalizer looks at it first.
> it takes you directly from type legalizer to LowerFP_TO_SINT().
> 
> Please look at LowerSINT_TO_FP(), we do the same there.
> 
> Additionally, you do
> setOperationAction(ISD::FP_TO_SINT,         MVT::v2i32, Custom) - it will call LowerFP_TO_SINT() for v2f64 -> v2i32
> 
I think the difference is that we are returning a illegal type (v2i32) while your SINT_TO_FP examples are taking it as an input.

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:22234
@@ +22233,3 @@
+        SDValue Idx = DAG.getIntPtrConstant(0, dl);
+        SDValue Res = DAG.getNode(X86ISD::CVTTPD2DQ, dl, MVT::v4i32, Src);
+        Res = DAG.getNode(ISD::EXTRACT_SUBVECTOR, dl, MVT::v2i32, Res, Idx);
----------------
delena wrote:
> RKSimon wrote:
> > See above.
> I meant using ISD::FP_TO_SINT instead of X86ISD::CVTTPD2DQ
This is similar to the need for X86ISD::CVTDQ2PD - can't use (v2f32 SINT_TO_FP (v2i32)) would return an illegal type.


Repository:
  rL LLVM

https://reviews.llvm.org/D23808