Skip to content

Conversation

saghul
Copy link
Contributor

@saghul saghul commented Feb 3, 2025

No description provided.

@saghul saghul requested a review from bnoordhuis February 3, 2025 09:37
@bnoordhuis
Copy link
Contributor

Oh, by the way: *unneeded

@saghul saghul merged commit 7e5d085 into master Feb 3, 2025
61 checks passed
@saghul saghul deleted the no-nan branch February 3, 2025 09:51
@chqrlie
Copy link
Collaborator

chqrlie commented Feb 3, 2025

We originally used JS_FLOAT64_NAN instead of NAN to ensure the NaN values were normaiized even on architectures were NAN does not always expand to a normalized value. This may be necessary for 32-bit NAN boxing on some exotic targets. Defining JS_FLOAT64_NAN appropriately for these targets would solve the problem.

Removing such wrappers increases the adherence on target specific behavior. Trying to keep these to a minimum helps with portability.

@bnoordhuis
Copy link
Contributor

I wondered about that. I suspected it was to signal that there's a "canonical" NaN but then we don't normalize NaNs (or not consistently anyway), rendering it moot. It mostly seems like baggage at this point.

@saghul
Copy link
Contributor Author

saghul commented Feb 3, 2025

@chqrlie Do you know what those excotic targets are?

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.

3 participants