Skip to content

Use LLVM intrinsics for NaN-propagating float min and max#5114

Merged
SeanTAllen merged 1 commit intomainfrom
fix-float-min-max-nan
Apr 4, 2026
Merged

Use LLVM intrinsics for NaN-propagating float min and max#5114
SeanTAllen merged 1 commit intomainfrom
fix-float-min-max-nan

Conversation

@SeanTAllen
Copy link
Copy Markdown
Member

F32/F64 min/max used if this < y then this else y end, which is asymmetric with NaN. IEEE 754 comparisons involving NaN return false, so the else branch always fires when this is NaN but not when y is NaN — NaN.min(5.0) returned 5.0 but F32(5.0).min(NaN) returned NaN.

Replaced with LLVM llvm.minimum/llvm.maximum intrinsics which implement IEEE 754-2019 NaN-propagating semantics: if either operand is NaN, the result is NaN. Integer min/max are unchanged.

Closes #5089

@ponylang-main ponylang-main added the discuss during sync Should be discussed during an upcoming sync label Apr 4, 2026
SeanTAllen added a commit that referenced this pull request Apr 4, 2026
@SeanTAllen SeanTAllen force-pushed the fix-float-min-max-nan branch from 812db8d to f1c9cdd Compare April 4, 2026 14:55
The conditional implementation (`if this < y then this else y end`) is
asymmetric when NaN is involved: IEEE 754 comparisons with NaN return
false, so the else branch always fires when `this` is NaN but not when
`y` is NaN. The result depends on argument order, which is wrong.

LLVM's `llvm.minimum`/`llvm.maximum` intrinsics implement IEEE 754-2019
NaN-propagating semantics — if either operand is NaN, the result is NaN.

Closes #5089
@SeanTAllen SeanTAllen force-pushed the fix-float-min-max-nan branch from f1c9cdd to 702ea79 Compare April 4, 2026 17:33
@SeanTAllen SeanTAllen merged commit fd470be into main Apr 4, 2026
16 checks passed
@SeanTAllen SeanTAllen deleted the fix-float-min-max-nan branch April 4, 2026 17:34
@ponylang-main ponylang-main removed the discuss during sync Should be discussed during an upcoming sync label Apr 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

min and max for floating points don't respect IEEE 754 treatment of NaN

2 participants