internal/runtime/atomic: add Xchg8 to all GOOS=linux platforms #69735
Comments
rhysh
added
NeedsFix
|
Change https://go.dev/cl/617496 mentions this issue: |
|
Thanks for the ping, Rhys! I have some code ready for ARM64 and will open a CL soon. I'll try to look into the remaining architectures later this week. |
|
Change https://go.dev/cl/617595 mentions this issue: |
This is minor extension of the existing support for 32 and 64 bit types. For #69735 Change-Id: I6828ec223951d2b692e077dc507b000ac23c32a1 Reviewed-on: https://go-review.googlesource.com/c/go/+/617496 Reviewed-by: Rhys Hiltner <[email protected]> Reviewed-by: Cherry Mui <[email protected]> Reviewed-by: Keith Randall <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Mauri de Souza Meneguzzo <[email protected]> Reviewed-by: Keith Randall <[email protected]>
For #69735 Change-Id: I61a2e561684c538eea705e60c8ebda6be3ef31a7 GitHub-Last-Rev: 3c7f4ec GitHub-Pull-Request: #69751 Reviewed-on: https://go-review.googlesource.com/c/go/+/617595 LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Keith Randall <[email protected]> Reviewed-by: Keith Randall <[email protected]> Reviewed-by: Michael Knyszek <[email protected]>
|
Change https://go.dev/cl/620756 mentions this issue: |
|
Change https://go.dev/cl/620855 mentions this issue: |
For #69735 Change-Id: I5b9f57315d693d613dc88dc02c10bee39aeeef76 GitHub-Last-Rev: 690337e GitHub-Pull-Request: #69923 Reviewed-on: https://go-review.googlesource.com/c/go/+/620756 TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Keith Randall <[email protected]> Auto-Submit: Keith Randall <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Dmitri Shuralyov <[email protected]> Reviewed-by: Keith Randall <[email protected]>
|
Change https://go.dev/cl/621395 mentions this issue: |
These are 8-bit ARM Load/Store atomics and are available starting from armv6k. See https://developer.arm.com/documentation/dui0379/e/arm-and-thumb-instructions/strex For #69735 Change-Id: I12623433c89070495c178208ee4758b3cdefd368 GitHub-Last-Rev: d6a7978 GitHub-Pull-Request: #69959 Cq-Include-Trybots: luci.golang.try:gotip-linux-arm Reviewed-on: https://go-review.googlesource.com/c/go/+/621395 Reviewed-by: Keith Randall <[email protected]> Reviewed-by: Keith Randall <[email protected]> Reviewed-by: Cherry Mui <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]>
For #69735 Change-Id: I18c0ca15d94a9b1751c1e55459283e01dc114150 GitHub-Last-Rev: dd9a39a GitHub-Pull-Request: #69924 Cq-Include-Trybots: luci.golang.try:gotip-linux-arm Reviewed-on: https://go-review.googlesource.com/c/go/+/620855 Reviewed-by: Keith Randall <[email protected]> Reviewed-by: Keith Randall <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Michael Pratt <[email protected]>
|
@rhysh We still have a few GOARCHs without implementations for Xchg8. Specifically, these are s390x, loong64, riscv64, and all MIPS architectures. Since these architectures lack 8-bit atomics, it complicates things slightly, requiring us to use fences and 32-bit load/store operations with proper alignment in assembly. See https://godbolt.org/z/Kqh4oWbWM. I don't have plans to work on these at the moment, but I wonder if we could use a Go implementation based on atomic.Cas for the remaining GOARCHs until a native implementation is available. |
|
Thanks for your work on these, @mauri870 . I'm looking into having the caller prepare its own CAS-based alternative, with an explicit (const) branch to choose the That would mean enough of an Xchg8 implementation to allow compilation, even if the body is a panic. So far, all of the calls are in one file, so the complexity of that is well under control. Let's revisit this after landing a fix for #68578 ; I've proposed a few alternatives there with different tradeoffs and different levels of reliance on Xchg8 (including one that, for temporary simplicity at the cost of zero-contention throughput, only does full-uintptr updates with CAS). |
|
Change https://go.dev/cl/623058 mentions this issue: |
In Loongson's new microstructure LA664 (Loongson-3A6000) and later, the atomic
instruction AMSWAP[DB]{B,H} [1] is supported. Therefore, the implementation of
the atomic operation exchange can be selected according to the CPUCFG flag LAM_BH:
AMSWAPDBB(full barrier) instruction is used on new microstructures, and traditional
LL-SC is used on LA464 (Loongson-3A5000) and older microstructures. This can
significantly improve the performance of Go programs on new microstructures.
Because Xchg8 implemented using traditional LL-SC uses too many temporary
registers, it is not suitable for intrinsics.
goos: linux
goarch: loong64
pkg: internal/runtime/atomic
cpu: Loongson-3A6000 @ 2500.00MHz
BenchmarkXchg8 100000000 10.41 ns/op
BenchmarkXchg8-2 100000000 10.41 ns/op
BenchmarkXchg8-4 100000000 10.41 ns/op
BenchmarkXchg8Parallel 96647592 12.41 ns/op
BenchmarkXchg8Parallel-2 58376136 20.60 ns/op
BenchmarkXchg8Parallel-4 78458899 17.97 ns/op
goos: linux
goarch: loong64
pkg: internal/runtime/atomic
cpu: Loongson-3A5000-HV @ 2500.00MHz
BenchmarkXchg8 38323825 31.23 ns/op
BenchmarkXchg8-2 38368219 31.23 ns/op
BenchmarkXchg8-4 37154156 31.26 ns/op
BenchmarkXchg8Parallel 37908301 31.63 ns/op
BenchmarkXchg8Parallel-2 30413440 39.42 ns/op
BenchmarkXchg8Parallel-4 30737626 39.03 ns/op
For #69735
[1]: https://loongson.github.io/LoongArch-Documentation/LoongArch-ELF-ABI-EN.html
Change-Id: I02ba68f66a2210b6902344fdc9975eb62de728ab
Reviewed-on: https://go-review.googlesource.com/c/go/+/623058
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: sophie zhao <[email protected]>
Reviewed-by: Meidan Li <[email protected]>
Reviewed-by: Cherry Mui <[email protected]>
Reviewed-by: Mauri de Souza Meneguzzo <[email protected]>
Reviewed-by: Dmitri Shuralyov <[email protected]>
|
Change https://go.dev/cl/631615 mentions this issue: |
|
Change https://go.dev/cl/631756 mentions this issue: |
For #69735 Change-Id: I34ca2b027494525ab64f94beee89ca373a5031ae Reviewed-on: https://go-review.googlesource.com/c/go/+/631615 Reviewed-by: Michael Knyszek <[email protected]> Reviewed-by: Keith Randall <[email protected]> Reviewed-by: Mark Ryan <[email protected]> Reviewed-by: Mauri de Souza Meneguzzo <[email protected]> Reviewed-by: Keith Randall <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]>
For #69735 Change-Id: Ide6b3077768a96b76078e5d4f6460596b8ff1560 Reviewed-on: https://go-review.googlesource.com/c/go/+/631756 Reviewed-by: Keith Randall <[email protected]> Reviewed-by: Michael Knyszek <[email protected]> Reviewed-by: Keith Randall <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]> Auto-Submit: Keith Randall <[email protected]>
|
Change https://go.dev/cl/651315 mentions this issue: |
For #69735 Change-Id: I2a0336214786e14b9a37834d81a0a0d14231451c Reviewed-on: https://go-review.googlesource.com/c/go/+/651315 Reviewed-by: Michael Knyszek <[email protected]> Reviewed-by: Keith Randall <[email protected]> Auto-Submit: Keith Randall <[email protected]> Reviewed-by: Keith Randall <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]>
|
Change https://go.dev/cl/657936 mentions this issue: |
|
Thank you all for the help! |
Uh oh!
There was an error while loading. Please reload this page.
Pending acceptance of CL 601597 (for #68578), we'll want an atomic.Xchg8 implementation for all GOARCH values that support it — at least those that overlap with GOOS=linux. This issue tracks that work, including expanding the test from CL 606900 to cover those platforms and eventually to move into atomic_test.go.
CC @golang/runtime
CC @mauri870 (who showed interest in doing some of the work)
The text was updated successfully, but these errors were encountered: