-
Notifications
You must be signed in to change notification settings - Fork 18k
internal: add package internal/float #42848
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
Comments
How would the If using:
This might cause additional inlining budget to be used affecting performance. IsNaN some others are also so simple that having it defined multiple times to e.g. avoid adding multiple new dependency seems fine. |
This was my concern as well. Unless there's a way to apply a peephole or a rewrite for this that bypasses the inlining budget, this would affect it.
I agree that |
If so, we should fix the inlining heuristics. Such a trivial forwarding function should be a zero-cost abstraction. |
If this doesn't affect public API, I don't think this needs to go through the proposal process. |
Change https://golang.org/cl/293794 mentions this issue: |
I sent CL 293794 for this. I am not convinced the deduplication is worth the effort unless we know if future that #33440 will be resolved. I was slightly conservative -- there are packages that import math for these functions, where we could strip that import by having them import internal/float. |
Currently, functions that rely on properties of a floating-point number like its sign-bit, nan-checking, and integer representation get duplicated across packages. For instance,
For instance,
isFinite
,isInf
,abs
,copysign
,float64bits
, andfloat64frombits
are defined in bothmath
andruntime
.isNaN
itself is defined inmath
,math/bits
,runtime
,sort
.I propose that we move the declarations in
runtime/float.go
tointernal/float
and reuse them in bothmath
,runtime
, and anyplace else.I assume this hasn't been done already because internal packages didn't exist at the time most of this code was written. In the case of
sort
, we wanted to remove its dependency onmath
, but if we end up adopting #33440,sort
will need more access to a float64's underlying representation.The text was updated successfully, but these errors were encountered: