[Legalizer] Fix fp-to-uint to fp-tosint promotion assertion.

[timshen]

Summary

When promoting fp-to-uint16 to fp-to-sint32, the result is actually zero
extended. For example, given double 65534.0, without legalization:

fp-to-uint16: 65534.0 -> 0xfffe

With the legalization:

fp-to-sint32: 65534.0 -> 0x0000fffe

Without this patch, legalization wrongly emits a signed extend assertion,
which is consumed by later icmp instruction, and causes miscompile.

Note that the user needs to ensure that floating point value must be in [0, 65535),
otherwise we are allowed to have undefined behavior.

This patch reverts r279223 behavior and adds more tests and
documentations.

In PR29041's context, James Molloy mentioned that:

We don't need to mask because conversion from float->uint8_t is
undefined if the integer part of the float value is not representable in
uint8_t. Therefore we can assume this doesn't happen!

which is totally true and good, because fptoui is documented clearly to
have undefined behavior when overflow/underflow happens. We should take
the advantage of this behavior so that we can save unnecessary mask
instructions.

[jmolloy]

Hi,

I agree with this. Thanks for fixing this!

James

[efriedma]

llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp

436 ↗

(On Diff #83026)

Not a comment about your patch, exactly, but I don't think the assertion here is safe; AssertZext(UNDEF) can lead to undefined behavior. For example, LOAD(ADD(global_array, AND(SomeUndefValue, 255))) produces undef, assuming global_array has at least 256 elements. However, LOAD(ADD(global_array, AND(AssertZext(SomeUndefValue, i8), 255))) simplifies to LOAD(ADD(global_array, AssertZext(SomeUndefValue, i8))), which has undefined behavior.

[timshen]

llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp

436 ↗

(On Diff #83026)

I agree, and I think being undefined in such a scenario matches part of the fptoui/fptosi contract:

If the value cannot fit in ty2, the results are undefined.

Since the value undef cannot fit in "ty2", the integer type, we allow undefined behavior here.

llvm/trunk/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp

if (N->getOpcode() == ISD::FP_TO_UINT &&
       TLI.isOperationLegalOrCustom(ISD::FP_TO_SINT, NVT))
     NewOpc = ISD::FP_TO_SINT;
 
   SDValue Res = DAG.getNode(NewOpc, dl, NVT, N->getOperand(0));
 
   // Assert that the converted value fits in the original type.  If it doesn't
   // (eg: because the value being converted is too big), then the result of the
   // original operation was undefined anyway, so the assert is still correct.
-  return DAG.getNode(NewOpc == ISD::FP_TO_UINT ?
+  //
+  // NOTE: fp-to-uint to fp-to-sint promotion guarantees zero extend. For example:
+  //   before legalization: fp-to-uint16, 65534. -> 0xfffe
+  //   after legalization: fp-to-sint32, 65534. -> 0x0000fffe
+  return DAG.getNode(N->getOpcode() == ISD::FP_TO_UINT ?
                      ISD::AssertZext : ISD::AssertSext, dl, NVT, Res,
                      DAG.getValueType(N->getValueType(0).getScalarType()));
 }
 
 SDValue DAGTypeLegalizer::PromoteIntRes_FP_TO_FP16(SDNode *N) {
   EVT NVT = TLI.getTypeToTransformTo(*DAG.getContext(), N->getValueType(0));
   SDLoc dl(N);
 

llvm/trunk/test/CodeGen/AArch64/fptouint-i8-zext.ll

 ; RUN: llc < %s | FileCheck %s
 
 target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
 target triple = "aarch64"
 
+; If the float value is negative or too large, the result is undefined anyway;
+; otherwise, fcvtzs must returns a value in [0, 256), which guarantees zext.
+
 ; CHECK-LABEL: float_char_int_func:
 ; CHECK: fcvtzs [[A:w[0-9]+]], s0
-; CHECK-NEXT: and w0, [[A]], #0xff
 ; CHECK-NEXT: ret
 define i32 @float_char_int_func(float %infloatVal) {
 entry:
   %conv = fptoui float %infloatVal to i8
   %conv1 = zext i8 %conv to i32
   ret i32 %conv1
 }

llvm/trunk/test/CodeGen/PowerPC/fp64-to-int16.ll

+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -O0 < %s | FileCheck %s
+target triple = "powerpc64le--linux-gnu"
+
+define i1 @Test(double %a) {
+; CHECK-LABEL: Test:
+; CHECK:       # BB#0: # %entry
+; CHECK-NEXT:    xscvdpsxws 1, 1
+; CHECK-NEXT:    mfvsrwz 3, 1
+; CHECK-NEXT:    xori 3, 3, 65534
+; CHECK-NEXT:    cntlzw 3, 3
+; CHECK-NEXT:    srwi 3, 3, 5
+; CHECK-NEXT:    # implicit-def: %X4
+; CHECK-NEXT:    mr 4, 3
+; CHECK-NEXT:    mr 3, 4
+; CHECK-NEXT:    blr
+entry:
+  %conv = fptoui double %a to i16
+  %cmp = icmp eq i16 %conv, -2
+  ret i1 %cmp
+}