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
math: use Abs rather than if x < 0 { x = -x } #21812
Comments
I'll work on this one. Currently looking through the math library for other instances of this. |
I have a change ready to be submitted to optimize the Abs and Copysign for ppc64x, because as you mention above, we have a single instruction for those. Not your if statement however. |
@laboger I made some changes though using Abs following the suggestions of @mundaym above. You could find the PR here: https://go-review.googlesource.com/c/go/+/63050. I think we could synchronize our efforts to fix this. |
Change https://golang.org/cl/63050 mentions this issue: |
Change https://golang.org/cl/63190 mentions this issue: |
Here is my CL to make Abs an intrinsic on ppc64x. https://go-review.googlesource.com/c/go/+/67130 |
Change https://golang.org/cl/84437 mentions this issue: |
Now that #13095 is closed we should consider using the
Abs
function where possible in the math library (and possibly elsewhere in the std lib).For example, this statement in j0.go could presumably be replaced:
There are subtle differences between the
if
statement andAbs
(NaN handling for example) but most of the math functions handle those cases explicitly anyway.We should also be aware that don't fully optimize
Abs
on all architectures yet, but I think this will come with time. s390x and ppc64x, for example, can doAbs
in a single instruction whereas the if statement is more difficult to optimize in the general case (because of NaN/-0.0 inputs mainly).This could be a good issue for a first time contributor, though be aware that you should include before/after benchmarks and for some functions you may need to manually disable assembly implementations to exercise the Go code.
The text was updated successfully, but these errors were encountered: