[llvm] [X86] Fix miscompile of fptosi.sat.iN.f16 for NaN under avx512fp16 (PR #210556)
via llvm-commits
llvm-commits at lists.llvm.org
Sat Jul 18 15:35:17 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-backend-x86
Author: Nathaniel McCallum (npmccallum)
<details>
<summary>Changes</summary>
## 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
---
Full diff: https://github.com/llvm/llvm-project/pull/210556.diff
2 Files Affected:
- (modified) llvm/lib/Target/X86/X86ISelLowering.cpp (+16-3)
- (added) llvm/test/CodeGen/X86/avx512fp16-fptosi-sat-nan.ll (+244)
``````````diff
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
new file mode 100644
index 0000000000000..75d709364234b
--- /dev/null
+++ b/llvm/test/CodeGen/X86/avx512fp16-fptosi-sat-nan.ll
@@ -0,0 +1,244 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mattr=+avx512fp16 | FileCheck %s
+
+; 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.
+;
+; 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: 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: 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
+; 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: 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: 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
+; 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: 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: 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
+; 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: 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: 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
+; 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: 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: 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
+; 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: 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: 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
+; 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: 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: 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
+; 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)
``````````
</details>
https://github.com/llvm/llvm-project/pull/210556
More information about the llvm-commits
mailing list