[X86] lowerFPToIntToFP - handle signedness for fp->int and int->fp independently

[RKSimon]

We were assuming that both conversions were for the same integer signedness

Fixes #217355

[llvmorg-github-actions]

@llvm/pr-subscribers-backend-x86

Author: Simon Pilgrim (RKSimon)

Changes

We were assuming that both conversions were for the same integer signedness

Fixes #217355

---

Full diff: https://github.com/llvm/llvm-project/pull/217404.diff

2 Files Affected:

• (modified) llvm/lib/Target/X86/X86ISelLowering.cpp (+10-5)
• (modified) llvm/test/CodeGen/X86/fp-int-fp-cvt.ll (+30-8)

diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp
index 922657ca9e17c..6144e1f0bf99d 100644
--- a/llvm/lib/Target/X86/X86ISelLowering.cpp
+++ b/llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -20344,7 +20344,9 @@ static SDValue lowerFPToIntToFP(SDValue CastToFP, const SDLoc &DL,
                                 const X86Subtarget &Subtarget) {
   SDValue CastToInt = CastToFP.getOperand(0);
   MVT VT = CastToFP.getSimpleValueType();
-  if ((CastToInt.getOpcode() != ISD::FP_TO_SINT &&
+  if ((CastToFP.getOpcode() != ISD::SINT_TO_FP &&
+       CastToFP.getOpcode() != ISD::UINT_TO_FP) ||
+      (CastToInt.getOpcode() != ISD::FP_TO_SINT &&
        CastToInt.getOpcode() != ISD::FP_TO_UINT) ||
       VT.isVector())
     return SDValue();
@@ -20364,7 +20366,8 @@ static SDValue lowerFPToIntToFP(SDValue CastToFP, const SDLoc &DL,
   unsigned SrcSize = SrcVT.getSizeInBits();
   unsigned IntSize = IntVT.getSizeInBits();
   unsigned VTSize = VT.getSizeInBits();
-  bool IsUnsigned = CastToInt.getOpcode() == ISD::FP_TO_UINT;
+  bool FromUnsigned = CastToFP.getOpcode() == ISD::UINT_TO_FP;
+  bool ToUnsigned = CastToInt.getOpcode() == ISD::FP_TO_UINT;
   unsigned ToIntOpcode =
       SrcSize != IntSize ? X86ISD::CVTTP2SI : (unsigned)ISD::FP_TO_SINT;
   unsigned ToFPOpcode =
@@ -20373,14 +20376,16 @@ static SDValue lowerFPToIntToFP(SDValue CastToFP, const SDLoc &DL,
 
   if (Subtarget.hasVLX() && Subtarget.hasDQI()) {
     // AVX512DQ+VLX
-    if (IsUnsigned) {
+    if (ToUnsigned) {
       ToIntOpcode =
           SrcSize != IntSize ? X86ISD::CVTTP2UI : (unsigned)ISD::FP_TO_UINT;
+    }
+    if (FromUnsigned) {
       ToFPOpcode =
           IntSize != VTSize ? X86ISD::CVTUI2P : (unsigned)ISD::UINT_TO_FP;
     }
   } else {
-    if (IsUnsigned || IntVT == MVT::i64) {
+    if (FromUnsigned || ToUnsigned || IntVT == MVT::i64) {
       // SSE2 can only perform f64/f32 <-> i32 signed.
       if (!Subtarget.useAVX512Regs() || !Subtarget.hasDQI())
         return SDValue();
@@ -20388,7 +20393,7 @@ static SDValue lowerFPToIntToFP(SDValue CastToFP, const SDLoc &DL,
       // Need to extend width for AVX512DQ without AVX512VL.
       Width = 512;
       ToIntOpcode = CastToInt.getOpcode();
-      ToFPOpcode = IsUnsigned ? ISD::UINT_TO_FP : ISD::SINT_TO_FP;
+      ToFPOpcode = FromUnsigned ? ISD::UINT_TO_FP : ISD::SINT_TO_FP;
     }
   }
 
diff --git a/llvm/test/CodeGen/X86/fp-int-fp-cvt.ll b/llvm/test/CodeGen/X86/fp-int-fp-cvt.ll
index 631e2562b0e80..9d9ed66c79d46 100644
--- a/llvm/test/CodeGen/X86/fp-int-fp-cvt.ll
+++ b/llvm/test/CodeGen/X86/fp-int-fp-cvt.ll
@@ -339,19 +339,41 @@ define float @ucvtf32_i64(float %a0) {
   ret float %ff
 }
 
-; FIXME: Negative test - signed/unsigned mismatch
+; Signed/unsigned mismatch
 define float @PR217355(float %x) {
 ; SSE-LABEL: PR217355:
 ; SSE:       # %bb.0:
-; SSE-NEXT:    cvttps2dq %xmm0, %xmm0
-; SSE-NEXT:    cvtdq2ps %xmm0, %xmm0
+; SSE-NEXT:    cvttss2si %xmm0, %eax
+; SSE-NEXT:    xorps %xmm0, %xmm0
+; SSE-NEXT:    cvtsi2ss %rax, %xmm0
 ; SSE-NEXT:    retq
 ;
-; AVX-LABEL: PR217355:
-; AVX:       # %bb.0:
-; AVX-NEXT:    vcvttps2dq %xmm0, %xmm0
-; AVX-NEXT:    vcvtdq2ps %xmm0, %xmm0
-; AVX-NEXT:    retq
+; AVX2-LABEL: PR217355:
+; AVX2:       # %bb.0:
+; AVX2-NEXT:    vcvttss2si %xmm0, %eax
+; AVX2-NEXT:    vcvtsi2ss %rax, %xmm15, %xmm0
+; AVX2-NEXT:    retq
+;
+; AVX512-VL-LABEL: PR217355:
+; AVX512-VL:       # %bb.0:
+; AVX512-VL-NEXT:    vcvttps2dq %xmm0, %xmm0
+; AVX512-VL-NEXT:    vcvtudq2ps %xmm0, %xmm0
+; AVX512-VL-NEXT:    retq
+;
+; AVX512-NOVL-LABEL: PR217355:
+; AVX512-NOVL:       # %bb.0:
+; AVX512-NOVL-NEXT:    # kill: def $xmm0 killed $xmm0 def $zmm0
+; AVX512-NOVL-NEXT:    vcvttps2dq %zmm0, %zmm0
+; AVX512-NOVL-NEXT:    vcvtudq2ps %zmm0, %zmm0
+; AVX512-NOVL-NEXT:    # kill: def $xmm0 killed $xmm0 killed $zmm0
+; AVX512-NOVL-NEXT:    vzeroupper
+; AVX512-NOVL-NEXT:    retq
+;
+; AVX512F-LABEL: PR217355:
+; AVX512F:       # %bb.0:
+; AVX512F-NEXT:    vcvttss2si %xmm0, %eax
+; AVX512F-NEXT:    vcvtusi2ss %eax, %xmm15, %xmm0
+; AVX512F-NEXT:    retq
   %integer = fptosi float %x to i32
   %result = uitofp i32 %integer to float
   ret float %result

[marxin]

Thanks for the quick fix!

[KavinTheG]

LGTM!