Skip to content

Fix CanonNaN template to use correct type for f64 NaN canonicalization#2701

Merged
sbc100 merged 1 commit into
WebAssembly:mainfrom
sumleo:fix/canon-nan-template-type
Feb 25, 2026
Merged

Fix CanonNaN template to use correct type for f64 NaN canonicalization#2701
sbc100 merged 1 commit into
WebAssembly:mainfrom
sumleo:fix/canon-nan-template-type

Conversation

@sumleo

@sumleo sumleo commented Feb 25, 2026

Copy link
Copy Markdown
Contributor

Summary

  • The CanonNaN<T> template function in interp-math.h hardcodes std::numeric_limits<f32>::quiet_NaN() instead of using the template parameter T.
  • When T is f64, this returns an f32 quiet NaN implicitly promoted to f64, which has a different bit pattern than the correct f64 canonical NaN.
  • This patch replaces f32 with T so that std::numeric_limits<T>::quiet_NaN() produces the correct canonical NaN for both f32 and f64.

Test plan

  • cmake --build build --target wabt-unittests builds successfully
  • Verified the fix matches the pattern already used elsewhere in the same file (e.g., FloatDiv, FloatMin, FloatMax all correctly use std::numeric_limits<T>::quiet_NaN())

The CanonNaN<T> template function was hardcoding
std::numeric_limits<f32>::quiet_NaN() instead of using the template
parameter T. When T is f64, this returns the wrong canonical NaN bit
pattern (an f32 quiet NaN implicitly promoted to f64), instead of the
correct f64 quiet NaN.

Replace f32 with T so that std::numeric_limits<T>::quiet_NaN() is used,
returning the correct canonical NaN for both f32 and f64.

@sbc100 sbc100 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder why we don't see any test failures because of this?

@sbc100 sbc100 merged commit 02ece84 into WebAssembly:main Feb 25, 2026
17 checks passed
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.

2 participants