Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cmd/compile: mips/mips64 floating point negation does not handle NaN correctly #58466

Open
ianlancetaylor opened this issue Feb 10, 2023 · 5 comments
Assignees
Labels
arch-mips compiler/runtime Issues related to the Go compiler and/or runtime. help wanted NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone

Comments

@ianlancetaylor
Copy link
Contributor

As discussed on #56491 (comment), on MIPS the gc compiler uses neg.d (which it calls NEGD) to negate a floating-point value. However, that instruction does not actually negate a NaN. That suggests that we should not use neg.d for floating point negation, but should instead xor with the high bit.

CC @randall77 @golang/mips @golang/runtime

@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Feb 10, 2023
@randall77
Copy link
Contributor

It's not entirely clear to me that negating a NaN should result in a negative NaN.

NaN means something went wrong when computing this number. Its sign is kind of meaningless.

Operations with a NaN argument tend to return that NaN argument unchanged (or one of the NaN arguments, if there are several, not sure how the choice is made). For instance, explain this program:

-x for floats is clearly special on most archs, as it actually computes on the NaN instead of returning it unchanged. Except on mips, which to my mind is more consistent because it returns its NaN arg, even when the operation is negation.

Does ieee754 say anything about this?

@smasher164
Copy link
Member

smasher164 commented Feb 11, 2023

Does ieee754 say anything about this?

6.3 The sign bit
When either an input or result is a NaN, this standard does not interpret the sign of a NaN. However,
operations on bit strings—copy, negate, abs, copySign—specify the sign bit of a NaN result, sometimes
based upon the sign bit of a NaN operand. The logical predicates totalOrder and isSignMinus are also
affected by the sign bit of a NaN operand.
For all other operations, this standard does not specify the sign
bit of a NaN result, even when there is only one input NaN, or when the NaN is produced from an invalid
operation.


It's unclear to me what "sometimes based upon the sign bit of a NaN operand" means here. Does that mean negate takes into account the sign bit or not?

@smasher164
Copy link
Member

Okay from section 5.5.1:

For x a NaN, the results of the operations have the same representation as x (qNaN or sNaN).

The operations it's referring to are copy, negate, abs, and copySign. So I guess the sign for NaN is undefined.

@randall77
Copy link
Contributor

Does that mean (-1) * x and -x have different semantics? Ugh. We do that optimization in the generic rewrite rules.

Indeed: https://go.dev/play/p/W2R_eSU1X9G

@ianlancetaylor
Copy link
Contributor Author

I don't think that the multiplication and the negation have to be implemented differently. IEEE doesn't specify the sign bit of a NaN result of multiplication, so it's OK to treat it as negation.

@prattmic prattmic added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Feb 14, 2023
@prattmic prattmic added this to the Backlog milestone Feb 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arch-mips compiler/runtime Issues related to the Go compiler and/or runtime. help wanted NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
Development

No branches or pull requests

8 participants