Skip to content

Commit 5480eb8

Browse files
committed
[Legalizer] Fix fp-to-uint to fp-tosint promotion assertion.
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 cause miscompile. Note that the floating point value must be in [0, 65535), otherwise the behavior is undefined. 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. Reviewers: jmolloy, nadav, echristo, kbarton Subscribers: mehdi_amini, nemanjai, llvm-commits Differential Revision: https://reviews.llvm.org/D28284 llvm-svn: 291015
1 parent 363ae81 commit 5480eb8

File tree

3 files changed

+29
-2
lines changed

3 files changed

+29
-2
lines changed

llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -428,7 +428,11 @@ SDValue DAGTypeLegalizer::PromoteIntRes_FP_TO_XINT(SDNode *N) {
428428
// Assert that the converted value fits in the original type. If it doesn't
429429
// (eg: because the value being converted is too big), then the result of the
430430
// original operation was undefined anyway, so the assert is still correct.
431-
return DAG.getNode(NewOpc == ISD::FP_TO_UINT ?
431+
//
432+
// NOTE: fp-to-uint to fp-to-sint promotion guarantees zero extend. For example:
433+
// before legalization: fp-to-uint16, 65534. -> 0xfffe
434+
// after legalization: fp-to-sint32, 65534. -> 0x0000fffe
435+
return DAG.getNode(N->getOpcode() == ISD::FP_TO_UINT ?
432436
ISD::AssertZext : ISD::AssertSext, dl, NVT, Res,
433437
DAG.getValueType(N->getValueType(0).getScalarType()));
434438
}

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,11 @@
33
target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
44
target triple = "aarch64"
55

6+
; If the float value is negative or too large, the result is undefined anyway;
7+
; otherwise, fcvtzs must returns a value in [0, 256), which guarantees zext.
8+
69
; CHECK-LABEL: float_char_int_func:
710
; CHECK: fcvtzs [[A:w[0-9]+]], s0
8-
; CHECK-NEXT: and w0, [[A]], #0xff
911
; CHECK-NEXT: ret
1012
define i32 @float_char_int_func(float %infloatVal) {
1113
entry:
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
2+
; RUN: llc -O0 < %s | FileCheck %s
3+
target triple = "powerpc64le--linux-gnu"
4+
5+
define i1 @Test(double %a) {
6+
; CHECK-LABEL: Test:
7+
; CHECK: # BB#0: # %entry
8+
; CHECK-NEXT: xscvdpsxws 1, 1
9+
; CHECK-NEXT: mfvsrwz 3, 1
10+
; CHECK-NEXT: xori 3, 3, 65534
11+
; CHECK-NEXT: cntlzw 3, 3
12+
; CHECK-NEXT: srwi 3, 3, 5
13+
; CHECK-NEXT: # implicit-def: %X4
14+
; CHECK-NEXT: mr 4, 3
15+
; CHECK-NEXT: mr 3, 4
16+
; CHECK-NEXT: blr
17+
entry:
18+
%conv = fptoui double %a to i16
19+
%cmp = icmp eq i16 %conv, -2
20+
ret i1 %cmp
21+
}

0 commit comments

Comments
 (0)