[llvm] [X86] Fix miscompile of fptosi.sat.iN.f16 for NaN under avx512fp16 (PR #210556)
Nathaniel McCallum via llvm-commits
llvm-commits at lists.llvm.org
Sat Jul 18 15:34:27 PDT 2026
https://github.com/npmccallum created https://github.com/llvm/llvm-project/pull/210556
## Summary
`X86TargetLowering::LowerFP_TO_INT_SAT` incorrectly lowered
`llvm.fptosi.sat.iN.f16` for N = 13, 14, 15, 16, and the equivalent
fixed-width vector forms, under `avx512fp16` codegen: NaN inputs
produced the destination type's minimum value instead of the 0 that
the intrinsic's documented semantics require.
The two commits are split precommit-test-then-fix: the first commit
adds regression coverage with CHECK lines generated against the
unpatched compiler (pinning the current incorrect output), and the
second applies the fix and regenerates only the CHECK lines that
actually change, so the fix's effect is directly visible in that
commit's diff. See each commit message for the full root-cause
analysis and the reasoning behind the fix, including why exactly
N = 13..16 (and no other width, signedness, or source float type) is
affected.
## Test plan
- [x] New regression test (`llvm/test/CodeGen/X86/avx512fp16-fptosi-sat-nan.ll`)
covering the affected widths, the sibling boundary widths on
both sides, unsigned, and a fixed-width vector form
- [x] Full `test/CodeGen/X86` suite passes, with no change to the
failing-test set before and after this change
- [x] Exhaustively verified against a differential oracle across all
65536 possible f16 bit patterns, for each of i13/i14/i15/i16:
2046/65536 mismatches before the fix (every NaN bit pattern),
0/65536 after
>From 53611a1e4bba8721f533c59557f669af6b935ab4 Mon Sep 17 00:00:00 2001
From: Nathaniel McCallum <nathaniel at mccallum.life>
Date: Sat, 18 Jul 2026 17:39:49 -0400
Subject: [PATCH 1/2] [X86][NFC] Precommit tests for fptosi.sat/fptoui.sat from
f16
Adds coverage for saturating float-to-int conversions from f16 under
avx512fp16, spanning the width boundary relevant to
X86TargetLowering::LowerFP_TO_INT_SAT's promoted-width path: i9 and
i12 (result legalized to an i16 container, exact bounds in f16), i13
through i16 (i16 container, inexact bounds), and i17 (legalized to an
i32 container instead, a structurally different path), plus i8 (exact
bounds via an i8 container), unsigned i16, and a fixed-width vector
form (<8 x half> -> <8 x i16>, scalarized through the same lowering)
as sibling checks.
The generated code for fptosi_sat_i13_f16_qnan, fptosi_sat_i14_f16_qnan,
fptosi_sat_i15_f16_qnan, fptosi_sat_i16_f16_nan/_qnan/_snan, and
fptosi_sat_v8i16_v8f16_qnan is incorrect: NaN inputs produce the
destination type's minimum value instead of the 0 that
@llvm.fptosi.sat's documented semantics require.
---
.../CodeGen/X86/avx512fp16-fptosi-sat-nan.ll | 239 ++++++++++++++++++
1 file changed, 239 insertions(+)
create mode 100644 llvm/test/CodeGen/X86/avx512fp16-fptosi-sat-nan.ll
diff --git a/llvm/test/CodeGen/X86/avx512fp16-fptosi-sat-nan.ll b/llvm/test/CodeGen/X86/avx512fp16-fptosi-sat-nan.ll
new file mode 100644
index 0000000000000..ea24536eab4a5
--- /dev/null
+++ b/llvm/test/CodeGen/X86/avx512fp16-fptosi-sat-nan.ll
@@ -0,0 +1,239 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mattr=+avx512fp16 | FileCheck %s
+
+; Coverage for saturating float-to-int conversions from f16 under
+; avx512fp16, including NaN inputs, spanning the width boundary relevant to
+; X86TargetLowering::LowerFP_TO_INT_SAT's promoted-width path: i9 and i12
+; (legalized to an i16 result container, exact bounds in f16), i13 through
+; i16 (i16 container, inexact bounds), and i17 (legalized to an i32
+; container instead, a structurally different path), plus i8 (exact bounds
+; via an i8 container), unsigned i16, and a fixed-width vector form as
+; sibling checks. Vector saturating conversions are scalarized lane-by-lane
+; through the same scalar lowering, so <8 x half> -> <8 x i16> exercises
+; the identical code path as the scalar i16 cases.
+;
+; fptosi_sat_i13_f16_qnan, fptosi_sat_i14_f16_qnan, fptosi_sat_i15_f16_qnan,
+; fptosi_sat_i16_f16_nan/_qnan/_snan, and fptosi_sat_v8i16_v8f16_qnan
+; produce incorrect code: NaN inputs produce the destination type's minimum
+; value instead of the 0 that @llvm.fptosi.sat's documented semantics
+; require.
+
+define i16 @fptosi_sat_i16_f16_nan(half %x) nounwind {
+; CHECK-LABEL: fptosi_sat_i16_f16_nan:
+; CHECK: # %bb.0:
+; CHECK-NEXT: vcvttsh2si %xmm0, %eax
+; CHECK-NEXT: vucomish {{\.?LCPI[0-9]+_[0-9]+}}(%rip), %xmm0
+; CHECK-NEXT: movl $32768, %ecx # imm = 0x8000
+; CHECK-NEXT: cmovael %eax, %ecx
+; CHECK-NEXT: vucomish {{\.?LCPI[0-9]+_[0-9]+}}(%rip), %xmm0
+; CHECK-NEXT: movl $32767, %eax # imm = 0x7FFF
+; CHECK-NEXT: cmovbel %ecx, %eax
+; CHECK-NEXT: # kill: def $ax killed $ax killed $eax
+; CHECK-NEXT: retq
+ %r = call i16 @llvm.fptosi.sat.i16.f16(half %x)
+ ret i16 %r
+}
+declare i16 @llvm.fptosi.sat.i16.f16(half)
+
+define i16 @fptosi_sat_i16_f16_qnan() nounwind {
+; CHECK-LABEL: fptosi_sat_i16_f16_qnan:
+; CHECK: # %bb.0:
+; CHECK-NEXT: vmovsh {{.*#+}} xmm0 = [NaN,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0]
+; CHECK-NEXT: vcvttsh2si %xmm0, %eax
+; CHECK-NEXT: vucomish {{\.?LCPI[0-9]+_[0-9]+}}(%rip), %xmm0
+; CHECK-NEXT: movl $32768, %ecx # imm = 0x8000
+; CHECK-NEXT: cmovael %eax, %ecx
+; CHECK-NEXT: vucomish {{\.?LCPI[0-9]+_[0-9]+}}(%rip), %xmm0
+; CHECK-NEXT: movl $32767, %eax # imm = 0x7FFF
+; CHECK-NEXT: cmovbel %ecx, %eax
+; CHECK-NEXT: # kill: def $ax killed $ax killed $eax
+; CHECK-NEXT: retq
+ %r = call i16 @llvm.fptosi.sat.i16.f16(half 0xH7E00)
+ ret i16 %r
+}
+
+define i16 @fptosi_sat_i16_f16_snan() nounwind {
+; CHECK-LABEL: fptosi_sat_i16_f16_snan:
+; CHECK: # %bb.0:
+; CHECK-NEXT: vmovsh {{.*#+}} xmm0 = [NaN,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0]
+; CHECK-NEXT: vcvttsh2si %xmm0, %eax
+; CHECK-NEXT: vucomish {{\.?LCPI[0-9]+_[0-9]+}}(%rip), %xmm0
+; CHECK-NEXT: movl $32768, %ecx # imm = 0x8000
+; CHECK-NEXT: cmovael %eax, %ecx
+; CHECK-NEXT: vucomish {{\.?LCPI[0-9]+_[0-9]+}}(%rip), %xmm0
+; CHECK-NEXT: movl $32767, %eax # imm = 0x7FFF
+; CHECK-NEXT: cmovbel %ecx, %eax
+; CHECK-NEXT: # kill: def $ax killed $ax killed $eax
+; CHECK-NEXT: retq
+ %r = call i16 @llvm.fptosi.sat.i16.f16(half 0xH7C01)
+ ret i16 %r
+}
+
+define i13 @fptosi_sat_i13_f16_qnan() nounwind {
+; CHECK-LABEL: fptosi_sat_i13_f16_qnan:
+; CHECK: # %bb.0:
+; CHECK-NEXT: vmovsh {{.*#+}} xmm0 = [NaN,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0]
+; CHECK-NEXT: vcvttsh2si %xmm0, %eax
+; CHECK-NEXT: vucomish {{\.?LCPI[0-9]+_[0-9]+}}(%rip), %xmm0
+; CHECK-NEXT: movl $61440, %ecx # imm = 0xF000
+; CHECK-NEXT: cmovael %eax, %ecx
+; CHECK-NEXT: vucomish {{\.?LCPI[0-9]+_[0-9]+}}(%rip), %xmm0
+; CHECK-NEXT: movl $4095, %eax # imm = 0xFFF
+; CHECK-NEXT: cmovbel %ecx, %eax
+; CHECK-NEXT: # kill: def $ax killed $ax killed $eax
+; CHECK-NEXT: retq
+ %r = call i13 @llvm.fptosi.sat.i13.f16(half 0xH7E00)
+ ret i13 %r
+}
+declare i13 @llvm.fptosi.sat.i13.f16(half)
+
+define i14 @fptosi_sat_i14_f16_qnan() nounwind {
+; CHECK-LABEL: fptosi_sat_i14_f16_qnan:
+; CHECK: # %bb.0:
+; CHECK-NEXT: vmovsh {{.*#+}} xmm0 = [NaN,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0]
+; CHECK-NEXT: vcvttsh2si %xmm0, %eax
+; CHECK-NEXT: vucomish {{\.?LCPI[0-9]+_[0-9]+}}(%rip), %xmm0
+; CHECK-NEXT: movl $57344, %ecx # imm = 0xE000
+; CHECK-NEXT: cmovael %eax, %ecx
+; CHECK-NEXT: vucomish {{\.?LCPI[0-9]+_[0-9]+}}(%rip), %xmm0
+; CHECK-NEXT: movl $8191, %eax # imm = 0x1FFF
+; CHECK-NEXT: cmovbel %ecx, %eax
+; CHECK-NEXT: # kill: def $ax killed $ax killed $eax
+; CHECK-NEXT: retq
+ %r = call i14 @llvm.fptosi.sat.i14.f16(half 0xH7E00)
+ ret i14 %r
+}
+declare i14 @llvm.fptosi.sat.i14.f16(half)
+
+define i15 @fptosi_sat_i15_f16_qnan() nounwind {
+; CHECK-LABEL: fptosi_sat_i15_f16_qnan:
+; CHECK: # %bb.0:
+; CHECK-NEXT: vmovsh {{.*#+}} xmm0 = [NaN,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0]
+; CHECK-NEXT: vcvttsh2si %xmm0, %eax
+; CHECK-NEXT: vucomish {{\.?LCPI[0-9]+_[0-9]+}}(%rip), %xmm0
+; CHECK-NEXT: movl $49152, %ecx # imm = 0xC000
+; CHECK-NEXT: cmovael %eax, %ecx
+; CHECK-NEXT: vucomish {{\.?LCPI[0-9]+_[0-9]+}}(%rip), %xmm0
+; CHECK-NEXT: movl $16383, %eax # imm = 0x3FFF
+; CHECK-NEXT: cmovbel %ecx, %eax
+; CHECK-NEXT: # kill: def $ax killed $ax killed $eax
+; CHECK-NEXT: retq
+ %r = call i15 @llvm.fptosi.sat.i15.f16(half 0xH7E00)
+ ret i15 %r
+}
+declare i15 @llvm.fptosi.sat.i15.f16(half)
+
+; A fixed-width vector form, to cover the scalarized path alongside the
+; scalar cases above -- it hits the identical per-lane lowering.
+define <8 x i16> @fptosi_sat_v8i16_v8f16_qnan() nounwind {
+; CHECK-LABEL: fptosi_sat_v8i16_v8f16_qnan:
+; CHECK: # %bb.0:
+; CHECK-NEXT: vmovsh {{.*#+}} xmm0 = [NaN,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0]
+; CHECK-NEXT: vcvttsh2si %xmm0, %eax
+; CHECK-NEXT: vucomish {{\.?LCPI[0-9]+_[0-9]+}}(%rip), %xmm0
+; CHECK-NEXT: movl $32768, %ecx # imm = 0x8000
+; CHECK-NEXT: cmovael %eax, %ecx
+; CHECK-NEXT: vucomish {{\.?LCPI[0-9]+_[0-9]+}}(%rip), %xmm0
+; CHECK-NEXT: movl $32767, %eax # imm = 0x7FFF
+; CHECK-NEXT: cmovbel %ecx, %eax
+; CHECK-NEXT: vmovd %eax, %xmm0
+; CHECK-NEXT: vpbroadcastw %xmm0, %xmm0
+; CHECK-NEXT: retq
+ %r = call <8 x i16> @llvm.fptosi.sat.v8i16.v8f16(<8 x half> <half 0xH7E00, half 0xH7E00, half 0xH7E00, half 0xH7E00, half 0xH7E00, half 0xH7E00, half 0xH7E00, half 0xH7E00>)
+ ret <8 x i16> %r
+}
+declare <8 x i16> @llvm.fptosi.sat.v8i16.v8f16(<8 x half>)
+
+; i9-i12 share i16's result-legalization container and promoted-width code
+; path but have exact bounds in f16, so they take a different (unaffected)
+; branch entirely; kept here as sibling-width sanity checks.
+define i9 @fptosi_sat_i9_f16_qnan() nounwind {
+; CHECK-LABEL: fptosi_sat_i9_f16_qnan:
+; CHECK: # %bb.0:
+; CHECK-NEXT: vmovsh {{.*#+}} xmm0 = [-2.56E+2,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0]
+; CHECK-NEXT: vmaxsh {{\.?LCPI[0-9]+_[0-9]+}}(%rip), %xmm0, %xmm0
+; CHECK-NEXT: vmovsh {{.*#+}} xmm1 = [2.55E+2,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0]
+; CHECK-NEXT: vminsh %xmm0, %xmm1, %xmm0
+; CHECK-NEXT: vcvttsh2si %xmm0, %eax
+; CHECK-NEXT: # kill: def $ax killed $ax killed $eax
+; CHECK-NEXT: retq
+ %r = call i9 @llvm.fptosi.sat.i9.f16(half 0xH7E00)
+ ret i9 %r
+}
+declare i9 @llvm.fptosi.sat.i9.f16(half)
+
+define i12 @fptosi_sat_i12_f16_qnan() nounwind {
+; CHECK-LABEL: fptosi_sat_i12_f16_qnan:
+; CHECK: # %bb.0:
+; CHECK-NEXT: vmovsh {{.*#+}} xmm0 = [-2.048E+3,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0]
+; CHECK-NEXT: vmaxsh {{\.?LCPI[0-9]+_[0-9]+}}(%rip), %xmm0, %xmm0
+; CHECK-NEXT: vmovsh {{.*#+}} xmm1 = [2.047E+3,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0]
+; CHECK-NEXT: vminsh %xmm0, %xmm1, %xmm0
+; CHECK-NEXT: vcvttsh2si %xmm0, %eax
+; CHECK-NEXT: # kill: def $ax killed $ax killed $eax
+; CHECK-NEXT: retq
+ %r = call i12 @llvm.fptosi.sat.i12.f16(half 0xH7E00)
+ ret i12 %r
+}
+declare i12 @llvm.fptosi.sat.i12.f16(half)
+
+; i17 is legalized to an i32 result container instead of i16, which takes
+; a structurally different (unaffected) path; kept here as a sibling-width
+; sanity check at the other side of the boundary.
+define i17 @fptosi_sat_i17_f16_qnan() nounwind {
+; CHECK-LABEL: fptosi_sat_i17_f16_qnan:
+; CHECK: # %bb.0:
+; CHECK-NEXT: vmovsh {{.*#+}} xmm0 = [NaN,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0]
+; CHECK-NEXT: vcvttsh2si %xmm0, %eax
+; CHECK-NEXT: vucomish {{\.?LCPI[0-9]+_[0-9]+}}(%rip), %xmm0
+; CHECK-NEXT: movl $-65536, %ecx # imm = 0xFFFF0000
+; CHECK-NEXT: cmovael %eax, %ecx
+; CHECK-NEXT: vucomish {{\.?LCPI[0-9]+_[0-9]+}}(%rip), %xmm0
+; CHECK-NEXT: movl $65535, %edx # imm = 0xFFFF
+; CHECK-NEXT: cmovbel %ecx, %edx
+; CHECK-NEXT: xorl %eax, %eax
+; CHECK-NEXT: vucomish %xmm0, %xmm0
+; CHECK-NEXT: cmovnpl %edx, %eax
+; CHECK-NEXT: retq
+ %r = call i17 @llvm.fptosi.sat.i17.f16(half 0xH7E00)
+ ret i17 %r
+}
+declare i17 @llvm.fptosi.sat.i17.f16(half)
+
+; i8 takes a different (exact-bounds) lowering path and is unaffected;
+; kept here as a sibling-width sanity check.
+define i8 @fptosi_sat_i8_f16_qnan() nounwind {
+; CHECK-LABEL: fptosi_sat_i8_f16_qnan:
+; CHECK: # %bb.0:
+; CHECK-NEXT: vmovsh {{.*#+}} xmm0 = [-1.28E+2,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0]
+; CHECK-NEXT: vmaxsh {{\.?LCPI[0-9]+_[0-9]+}}(%rip), %xmm0, %xmm0
+; CHECK-NEXT: vmovsh {{.*#+}} xmm1 = [1.27E+2,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0]
+; CHECK-NEXT: vminsh %xmm0, %xmm1, %xmm0
+; CHECK-NEXT: vcvttsh2si %xmm0, %eax
+; CHECK-NEXT: # kill: def $al killed $al killed $eax
+; CHECK-NEXT: retq
+ %r = call i8 @llvm.fptosi.sat.i8.f16(half 0xH7E00)
+ ret i8 %r
+}
+declare i8 @llvm.fptosi.sat.i8.f16(half)
+
+; Unsigned i16 is already correct (MinInt == 0 coincides with the desired
+; NaN answer); kept here as a sibling sanity check.
+define i16 @fptoui_sat_i16_f16_qnan() nounwind {
+; CHECK-LABEL: fptoui_sat_i16_f16_qnan:
+; CHECK: # %bb.0:
+; CHECK-NEXT: vmovsh {{.*#+}} xmm0 = [NaN,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0]
+; CHECK-NEXT: vcvttsh2si %xmm0, %eax
+; CHECK-NEXT: xorl %ecx, %ecx
+; CHECK-NEXT: vxorps %xmm1, %xmm1, %xmm1
+; CHECK-NEXT: vucomish %xmm1, %xmm0
+; CHECK-NEXT: cmovael %eax, %ecx
+; CHECK-NEXT: vucomish {{\.?LCPI[0-9]+_[0-9]+}}(%rip), %xmm0
+; CHECK-NEXT: movl $65535, %eax # imm = 0xFFFF
+; CHECK-NEXT: cmovbel %ecx, %eax
+; CHECK-NEXT: # kill: def $ax killed $ax killed $eax
+; CHECK-NEXT: retq
+ %r = call i16 @llvm.fptoui.sat.i16.f16(half 0xH7E00)
+ ret i16 %r
+}
+declare i16 @llvm.fptoui.sat.i16.f16(half)
>From 2a4b97943c24e7eb7017df76e597d8704b6c0fee Mon Sep 17 00:00:00 2001
From: Nathaniel McCallum <nathaniel at mccallum.life>
Date: Sat, 18 Jul 2026 17:41:38 -0400
Subject: [PATCH 2/2] [X86] Fix miscompile of fptosi.sat.iN.f16 for NaN under
avx512fp16
X86TargetLowering::LowerFP_TO_INT_SAT incorrectly lowered
llvm.fptosi.sat.iN.f16 so that NaN inputs produced the destination
type's minimum value instead of the 0 that @llvm.fptosi.sat's
documented semantics require. This affects exactly N = 13, 14, 15, 16:
the SelectionDAG type legalizer assigns saturating conversions with a
result width in [9, 16] an i16 result container before this custom
hook runs, which this function then promotes to a 32-bit intermediate
(TmpVT) internally. Within that container, N = 9..12 have saturation
bounds that are exactly representable in f16 (at most 11 significant
bits) and take an unaffected min+max+fptoi path; N = 13..16 need 12-15
bits and take the affected path. Widths 17 through 32 are legalized to
an i32 container instead, which coincides with the 32-bit intermediate
from the start, so the code below takes a different route that happens
to remain correct; wider destinations (e.g. i33, legalized to an i64
container) are unaffected by the same mechanism. Fixed-width vector
forms (e.g. <8 x half> -> <8 x i16>) are scalarized lane-by-lane
through this same lowering and are affected identically.
No other scalar source type is affected. f32 and f64 have enough
mantissa precision (24 and 53 bits) to represent every N in [9, 16]
exactly, so they never reach the affected path regardless of N; f80
and f128 sources are rejected earlier by isScalarFPTypeInSSEReg, and
bf16 (and soft-float f16, when avx512fp16 is unavailable) are rejected
by the isBF16orSoftF16 check just below it, so all three fall back to
the generic expansion instead of reaching this code at all.
The promoted-width fast path first truncates a wider FP_TO_SINT
result, which correctly maps NaN's "integer indefinite" value down to
zero. But the very next MinFloat-clamp select used an unordered
comparison (SETULT), which also matches NaN, silently overwriting that
already-correct zero with MinInt. The explicit final NaN check that
exists later in the function is unreachable for this case, since it
returns early right after the MaxFloat clamp.
Fix by using an ordered comparison (SETOLT) for that MinFloat clamp
specifically when the conversion is signed and was promoted to a wider
TmpVT: in that case NaN is already handled correctly by the earlier
truncation, so the clamp only needs to catch genuine out-of-range
magnitudes, not NaN. Unsigned conversions keep the unordered
comparison (SETULT): there, NaN mapping to MinInt is correct, because
MinInt is always 0 for an unsigned destination. i8 is unaffected for
an unrelated reason regardless of signedness: its bounds are always
exactly representable in f16, so it never reaches this comparison at
all.
The generic, target-independent TargetLowering::expandFP_TO_INT_SAT
that this function is modeled on does not have this bug: its signed
path always applies the equivalent final NaN check unconditionally,
with no early-return shortcut, so no other backend needs a
corresponding fix.
---
llvm/lib/Target/X86/X86ISelLowering.cpp | 19 +++++-
.../CodeGen/X86/avx512fp16-fptosi-sat-nan.ll | 63 ++++++++++---------
2 files changed, 50 insertions(+), 32 deletions(-)
diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp
index 791e04deb9583..767452ff47f12 100644
--- a/llvm/lib/Target/X86/X86ISelLowering.cpp
+++ b/llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -22672,10 +22672,23 @@ X86TargetLowering::LowerFP_TO_INT_SAT(SDValue Op, SelectionDAG &DAG) const {
// result type of the fptoi instructions, INDVAL coincides with integer
// minimum, so we don't need to explicitly check it.
if (!IsSigned || SatWidth != TmpVT.getScalarSizeInBits()) {
- // If Src ULT MinFloat, select MinInt. In particular, this also selects
- // MinInt if Src is NaN.
+ // If Src ULT MinFloat, select MinInt. In particular, for unsigned
+ // conversions this also selects MinInt (== 0) if Src is NaN, which is
+ // the desired final answer.
+ //
+ // For SIGNED conversions promoted to a wider TmpVT above, MinInt != 0,
+ // so this select must NOT also catch NaN here: truncating TmpVT down to
+ // DstVT already mapped NaN's signed INDVAL to zero (see above), and
+ // because this function returns early below without reaching the
+ // explicit NaN check that exists for the non-promoted case, an
+ // unordered comparison here would silently replace that correct zero
+ // with MinInt for NaN inputs. Use an ordered comparison instead so NaN
+ // simply doesn't match, leaving the already-correct truncated zero in
+ // place.
+ ISD::CondCode MinCC = (IsSigned && DstVT != TmpVT) ? ISD::CondCode::SETOLT
+ : ISD::CondCode::SETULT;
Select = DAG.getSelectCC(
- dl, Src, MinFloatNode, MinIntNode, Select, ISD::CondCode::SETULT);
+ dl, Src, MinFloatNode, MinIntNode, Select, MinCC);
}
// If Src OGT MaxFloat, select MaxInt.
diff --git a/llvm/test/CodeGen/X86/avx512fp16-fptosi-sat-nan.ll b/llvm/test/CodeGen/X86/avx512fp16-fptosi-sat-nan.ll
index ea24536eab4a5..75d709364234b 100644
--- a/llvm/test/CodeGen/X86/avx512fp16-fptosi-sat-nan.ll
+++ b/llvm/test/CodeGen/X86/avx512fp16-fptosi-sat-nan.ll
@@ -1,30 +1,29 @@
; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mattr=+avx512fp16 | FileCheck %s
-; Coverage for saturating float-to-int conversions from f16 under
-; avx512fp16, including NaN inputs, spanning the width boundary relevant to
-; X86TargetLowering::LowerFP_TO_INT_SAT's promoted-width path: i9 and i12
-; (legalized to an i16 result container, exact bounds in f16), i13 through
-; i16 (i16 container, inexact bounds), and i17 (legalized to an i32
-; container instead, a structurally different path), plus i8 (exact bounds
-; via an i8 container), unsigned i16, and a fixed-width vector form as
-; sibling checks. Vector saturating conversions are scalarized lane-by-lane
-; through the same scalar lowering, so <8 x half> -> <8 x i16> exercises
-; the identical code path as the scalar i16 cases.
+; Regression test for a miscompile in the promoted-width path of
+; X86TargetLowering::LowerFP_TO_INT_SAT: llvm.fptosi.sat.iN.f16 (for
+; N = 13..16, the widths whose result container is i16 but whose
+; saturation bounds are not exactly representable in f16) and the
+; equivalent fixed-width vector forms, which are scalarized lane-by-lane
+; through the same lowering, produced the destination type's minimum value
+; for every NaN input instead of the 0 that @llvm.fptosi.sat's documented
+; semantics require.
;
-; fptosi_sat_i13_f16_qnan, fptosi_sat_i14_f16_qnan, fptosi_sat_i15_f16_qnan,
-; fptosi_sat_i16_f16_nan/_qnan/_snan, and fptosi_sat_v8i16_v8f16_qnan
-; produce incorrect code: NaN inputs produce the destination type's minimum
-; value instead of the 0 that @llvm.fptosi.sat's documented semantics
-; require.
+; i9 and i12 (same i16 container and promoted-width path, but exact bounds
+; in f16), i17 (legalized to an i32 container instead, a structurally
+; different path), i8 (exact bounds via an i8 container), and unsigned i16
+; are unaffected; kept here as sibling-width and sibling-signedness sanity
+; checks.
define i16 @fptosi_sat_i16_f16_nan(half %x) nounwind {
; CHECK-LABEL: fptosi_sat_i16_f16_nan:
; CHECK: # %bb.0:
; CHECK-NEXT: vcvttsh2si %xmm0, %eax
-; CHECK-NEXT: vucomish {{\.?LCPI[0-9]+_[0-9]+}}(%rip), %xmm0
+; CHECK-NEXT: vmovsh {{.*#+}} xmm1 = [-3.2768E+4,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0]
+; CHECK-NEXT: vucomish %xmm0, %xmm1
; CHECK-NEXT: movl $32768, %ecx # imm = 0x8000
-; CHECK-NEXT: cmovael %eax, %ecx
+; CHECK-NEXT: cmovbel %eax, %ecx
; CHECK-NEXT: vucomish {{\.?LCPI[0-9]+_[0-9]+}}(%rip), %xmm0
; CHECK-NEXT: movl $32767, %eax # imm = 0x7FFF
; CHECK-NEXT: cmovbel %ecx, %eax
@@ -40,9 +39,10 @@ define i16 @fptosi_sat_i16_f16_qnan() nounwind {
; CHECK: # %bb.0:
; CHECK-NEXT: vmovsh {{.*#+}} xmm0 = [NaN,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0]
; CHECK-NEXT: vcvttsh2si %xmm0, %eax
-; CHECK-NEXT: vucomish {{\.?LCPI[0-9]+_[0-9]+}}(%rip), %xmm0
+; CHECK-NEXT: vmovsh {{.*#+}} xmm1 = [-3.2768E+4,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0]
+; CHECK-NEXT: vucomish %xmm0, %xmm1
; CHECK-NEXT: movl $32768, %ecx # imm = 0x8000
-; CHECK-NEXT: cmovael %eax, %ecx
+; CHECK-NEXT: cmovbel %eax, %ecx
; CHECK-NEXT: vucomish {{\.?LCPI[0-9]+_[0-9]+}}(%rip), %xmm0
; CHECK-NEXT: movl $32767, %eax # imm = 0x7FFF
; CHECK-NEXT: cmovbel %ecx, %eax
@@ -57,9 +57,10 @@ define i16 @fptosi_sat_i16_f16_snan() nounwind {
; CHECK: # %bb.0:
; CHECK-NEXT: vmovsh {{.*#+}} xmm0 = [NaN,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0]
; CHECK-NEXT: vcvttsh2si %xmm0, %eax
-; CHECK-NEXT: vucomish {{\.?LCPI[0-9]+_[0-9]+}}(%rip), %xmm0
+; CHECK-NEXT: vmovsh {{.*#+}} xmm1 = [-3.2768E+4,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0]
+; CHECK-NEXT: vucomish %xmm0, %xmm1
; CHECK-NEXT: movl $32768, %ecx # imm = 0x8000
-; CHECK-NEXT: cmovael %eax, %ecx
+; CHECK-NEXT: cmovbel %eax, %ecx
; CHECK-NEXT: vucomish {{\.?LCPI[0-9]+_[0-9]+}}(%rip), %xmm0
; CHECK-NEXT: movl $32767, %eax # imm = 0x7FFF
; CHECK-NEXT: cmovbel %ecx, %eax
@@ -74,9 +75,10 @@ define i13 @fptosi_sat_i13_f16_qnan() nounwind {
; CHECK: # %bb.0:
; CHECK-NEXT: vmovsh {{.*#+}} xmm0 = [NaN,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0]
; CHECK-NEXT: vcvttsh2si %xmm0, %eax
-; CHECK-NEXT: vucomish {{\.?LCPI[0-9]+_[0-9]+}}(%rip), %xmm0
+; CHECK-NEXT: vmovsh {{.*#+}} xmm1 = [-4.096E+3,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0]
+; CHECK-NEXT: vucomish %xmm0, %xmm1
; CHECK-NEXT: movl $61440, %ecx # imm = 0xF000
-; CHECK-NEXT: cmovael %eax, %ecx
+; CHECK-NEXT: cmovbel %eax, %ecx
; CHECK-NEXT: vucomish {{\.?LCPI[0-9]+_[0-9]+}}(%rip), %xmm0
; CHECK-NEXT: movl $4095, %eax # imm = 0xFFF
; CHECK-NEXT: cmovbel %ecx, %eax
@@ -92,9 +94,10 @@ define i14 @fptosi_sat_i14_f16_qnan() nounwind {
; CHECK: # %bb.0:
; CHECK-NEXT: vmovsh {{.*#+}} xmm0 = [NaN,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0]
; CHECK-NEXT: vcvttsh2si %xmm0, %eax
-; CHECK-NEXT: vucomish {{\.?LCPI[0-9]+_[0-9]+}}(%rip), %xmm0
+; CHECK-NEXT: vmovsh {{.*#+}} xmm1 = [-8.192E+3,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0]
+; CHECK-NEXT: vucomish %xmm0, %xmm1
; CHECK-NEXT: movl $57344, %ecx # imm = 0xE000
-; CHECK-NEXT: cmovael %eax, %ecx
+; CHECK-NEXT: cmovbel %eax, %ecx
; CHECK-NEXT: vucomish {{\.?LCPI[0-9]+_[0-9]+}}(%rip), %xmm0
; CHECK-NEXT: movl $8191, %eax # imm = 0x1FFF
; CHECK-NEXT: cmovbel %ecx, %eax
@@ -110,9 +113,10 @@ define i15 @fptosi_sat_i15_f16_qnan() nounwind {
; CHECK: # %bb.0:
; CHECK-NEXT: vmovsh {{.*#+}} xmm0 = [NaN,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0]
; CHECK-NEXT: vcvttsh2si %xmm0, %eax
-; CHECK-NEXT: vucomish {{\.?LCPI[0-9]+_[0-9]+}}(%rip), %xmm0
+; CHECK-NEXT: vmovsh {{.*#+}} xmm1 = [-1.6384E+4,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0]
+; CHECK-NEXT: vucomish %xmm0, %xmm1
; CHECK-NEXT: movl $49152, %ecx # imm = 0xC000
-; CHECK-NEXT: cmovael %eax, %ecx
+; CHECK-NEXT: cmovbel %eax, %ecx
; CHECK-NEXT: vucomish {{\.?LCPI[0-9]+_[0-9]+}}(%rip), %xmm0
; CHECK-NEXT: movl $16383, %eax # imm = 0x3FFF
; CHECK-NEXT: cmovbel %ecx, %eax
@@ -130,9 +134,10 @@ define <8 x i16> @fptosi_sat_v8i16_v8f16_qnan() nounwind {
; CHECK: # %bb.0:
; CHECK-NEXT: vmovsh {{.*#+}} xmm0 = [NaN,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0]
; CHECK-NEXT: vcvttsh2si %xmm0, %eax
-; CHECK-NEXT: vucomish {{\.?LCPI[0-9]+_[0-9]+}}(%rip), %xmm0
+; CHECK-NEXT: vmovsh {{.*#+}} xmm1 = [-3.2768E+4,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0,0.0E+0]
+; CHECK-NEXT: vucomish %xmm0, %xmm1
; CHECK-NEXT: movl $32768, %ecx # imm = 0x8000
-; CHECK-NEXT: cmovael %eax, %ecx
+; CHECK-NEXT: cmovbel %eax, %ecx
; CHECK-NEXT: vucomish {{\.?LCPI[0-9]+_[0-9]+}}(%rip), %xmm0
; CHECK-NEXT: movl $32767, %eax # imm = 0x7FFF
; CHECK-NEXT: cmovbel %ecx, %eax
More information about the llvm-commits
mailing list