-
Notifications
You must be signed in to change notification settings - Fork 17.7k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
math: Add Round function (ties away from zero)
This function avoids subtle faults found in many ad-hoc implementations, and is simple enough to be inlined by the compiler. Fixes #20100 Change-Id: Ib320254e9b1f1f798c6ef906b116f63bc29e8d08 Reviewed-on: https://go-review.googlesource.com/43652 Reviewed-by: Robert Griesemer <[email protected]>
- Loading branch information
Showing
2 changed files
with
93 additions
and
1 deletion.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
03c3bb5
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can someone ELI5 why can't we just use native processor instructions for some of these?
03c3bb5
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dansouza We already use a native instruction for the S390X backend. Other backends are welcome to do so also.
We can't naively use amd64's ROUND instructions because we assume only SSE2. ROUND[SP][SD] are SSE4.1. So to use them we'd need to condition its use on a CPUID check and have a fallback path.
03c3bb5
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @randall77, thank you for taking the time to explain it. Are there already mechanisms in the compiler to, based on CPUID, change the pointers to certain functions to be used just once at bootstrap, instead of calling and branching on CPUID everytime Math.Round is called?
For example, when you start a Go program, a bootstrap routine that runs before
main()
rewrites some function table based on CPUID parameters - if the current CPU has SSE4.1, we change the address of the 'Math.Round' function in the ELF symbol table to an optimized version that uses ROUND(SP|SD), otherwise keep the fallback. Is that a thing already?03c3bb5
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we do that. See alginit in src/runtime/alg.go.
You can also just generate the conditional code and fallback. We do it for POPCOUNT, which is also not SSE2. See src/cmd/compile/internal/gc/ssa.go, search for popcnt.
03c3bb5
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@randall77 thank you for the pointers, I'll check it out and add this optimization.