[PATCH] D83880: AMDGPU/GlobalISel: Legalize s64->s16 G_SITOFP/G_UITOFP

Wed Jul 15 08:45:26 PDT 2020

arsenm added a comment.

In D83880#2153414 <https://reviews.llvm.org/D83880#2153414>, @foad wrote:

> > Legailize as s64->s32 G_SITOFP/G_UITOFP followed by s32->s16 G_FPTRUNC.
>
> It seems like this would give the wrong result in some cases because of the double rounding. What does SelectionDAG do?

It looks like it does the same thing. AMDGPU and AArch64 both seem to do the same thing in custom lowering. I think since the source is an integer, there really isn't a whole lot of opportunity for rounding to do much.

================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp:647
     .lowerIf(typeIs(1, S1))
-    .customFor({{S64, S64}});
+    .customFor({{S64, S64}, {S16, S64}});
   if (ST.has16BitInsts())
----------------
Could you move this to widenScalar for TypeIdx == 0 in LegalizerHelper instead of making this custom?

================
Comment at: llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp:1821
+  // i64 -> f16
+  if (MRI.getType(Src) == S64 && MRI.getType(Dst)==S16) {
+    B.buildFPTrunc(Dst, B.buildInstr(MI.getOpcode(), {S32}, {Src}));
----------------
Missing spaces around ==

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83880/new/

https://reviews.llvm.org/D83880