[X86] Combine uitofp <v x i32> to <v x half>

[abhishek-kaushik22]

Closes #121793

[llvmbot]

@llvm/pr-subscribers-llvm-selectiondag

@llvm/pr-subscribers-backend-x86

Author: None (abhishek-kaushik22)

Changes

Closes #121793

---

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

1 Files Affected:

• (modified) llvm/lib/Target/X86/X86ISelLowering.cpp (+13-2)

diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp
index 68bdeb1cebeb9c..156ad47efcbf4f 100644
--- a/llvm/lib/Target/X86/X86ISelLowering.cpp
+++ b/llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -56098,8 +56098,19 @@ static SDValue combineUIntToFP(SDNode *N, SelectionDAG &DAG,
   if (InVT.isVector() && VT.getVectorElementType() == MVT::f16) {
     unsigned ScalarSize = InVT.getScalarSizeInBits();
     if ((ScalarSize == 16 && Subtarget.hasFP16()) || ScalarSize == 32 ||
-        ScalarSize >= 64)
-      return SDValue();
+        ScalarSize >= 64) {
+      if (ScalarSize != 32 || VT.getScalarSizeInBits() != 16 ||
+          Subtarget.hasFP16())
+        return SDValue();
+      // UINT_TO_FP(vXi32 to vXf16) -> FP_ROUND(UINT_TO_FP(vXi32 to vXf32), 0)
+      return DAG.getNode(
+          ISD::FP_ROUND, SDLoc(N), VT,
+          DAG.getNode(ISD::UINT_TO_FP, SDLoc(N),
+                      InVT.changeVectorElementType(MVT::f32), Op0),
+          DAG.getTargetConstant(
+              0, SDLoc(N),
+              DAG.getTargetLoweringInfo().getPointerTy(DAG.getDataLayout())));
+    }
     SDLoc dl(N);
     EVT DstVT =
         EVT::getVectorVT(*DAG.getContext(),

[abhishek-kaushik22]

@RKSimon @phoebewang @e-kud can you please review?

[e-kud]

@abhishek-kaushik22 any tests?

[RKSimon]

I'd prefer this was handled in VectorLegalizer::ExpandUINT_TO_FLOAT since the issue isn't x86-specific

[abhishek-kaushik22]

I'd prefer this was handled in VectorLegalizer::ExpandUINT_TO_FLOAT since the issue isn't x86-specific

Can you please provide an example for other targets that this is failing on?

[RKSimon]

test cases?

[arsenm]

Needs tests

[abhishek-kaushik22]

Needs tests

I am not sure what to add in the tests. Should the tests include check-not for the inf constant or are auto-generated assertions enough? @RKSimon @arsenm

Edit: Added tests with auto assertions to get a review

[abhishek-kaushik22]

With +f16c I still see vmulps .LCPI0_0(%rip), %ymm1, %ymm1 where

.LCPI0_0:
	.long	0x7f800000                      # float +Inf
	.long	0x7f800000                      # float +Inf
	.long	0x7f800000                      # float +Inf
	.long	0x7f800000                      # float +Inf
	.long	0x7f800000                      # float +Inf
	.long	0x7f800000                      # float +Inf
	.long	0x7f800000                      # float +Inf
	.long	0x7f800000                      # float +Inf

in the assembly. Thanks @phoebewang for the comments.

[phoebewang]

LGTM with one nit.

[RKSimon]

Thanks @abhishek-kaushik22 !