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.

Sign up for GitHub

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

Jump to bottom

cmd/compile: failed to make Go on riscv64 CPU with numa [1.20 backport] #61471

Closed
gopherbot opened this issue Jul 20, 2023 · 3 comments
Closed

cmd/compile: failed to make Go on riscv64 CPU with numa [1.20 backport] #61471

gopherbot opened this issue Jul 20, 2023 · 3 comments
Labels
arch-riscv Issues solely affecting the riscv64 architecture. CherryPickApproved Used during the release process for point releases compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge
Milestone
Go1.20.7

Comments

@gopherbot
Copy link
Contributor

@mengzhuo requested issue #61295 to be considered for backport to the next 1.20 minor release.

@gopherbot Please open backport issues.
@gopherbot gopherbot added the CherryPickCandidate Used during the release process for point releases label Jul 20, 2023
@gopherbot gopherbot mentioned this issue Jul 20, 2023
Closed
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jul 20, 2023
@gopherbot gopherbot added this to the Go1.20.7 milestone Jul 20, 2023
@gopherbot
Copy link
Contributor Author

Change https://go.dev/cl/511515 mentions this issue: [release-branch.go1.20] cmd/asm, cmd/internal/obj: generate proper atomic ops for riscv64

@mdempsky
Copy link
Contributor

Approved, since the CL only affects riscv64 codegen.

@mdempsky mdempsky added CherryPickApproved Used during the release process for point releases and removed CherryPickCandidate Used during the release process for point releases labels Jul 26, 2023
@gopherbot
Copy link
Contributor Author

Closed by merging 10d85fa to release-branch.go1.20.

gopherbot pushed a commit that referenced this issue Jul 26, 2023
@mengzhuo @gopherbot
[release-branch.go1.20] cmd/asm, cmd/internal/obj: generate proper at…
10d85fa 
…omic ops for riscv64

Go's memory model closely follows the approach C++ concurrency memory
model (https://go.dev/ref/mem) and Go atomic "has the same semantics as C++'s
sequentially consistent atomics".

Meanwhile according to RISCV manual A.6 "Mappings from C/C++ primitives to RISC-V primitives".
C/C++ atomic operations (memory_order_acq_rel) should be map to "amo<op>.{w|d}.aqrl"
LR/SC (memory_order_acq_rel) should map to "lr.{w|d}.aq; <op>; sc.{w|d}.rl"

goos: linux
goarch: riscv64
pkg: runtime/internal/atomic
                │ atomic.old.bench │          atomic.new.bench           │
                │      sec/op      │   sec/op     vs base                │
AtomicLoad64-4         4.216n ± 1%   4.202n ± 0%        ~ (p=0.127 n=10)
AtomicStore64-4        5.040n ± 0%   6.718n ± 0%  +33.30% (p=0.000 n=10)
AtomicLoad-4           4.217n ± 0%   4.213n ± 0%        ~ (p=0.145 n=10)
AtomicStore-4          5.040n ± 0%   6.718n ± 0%  +33.30% (p=0.000 n=10)
And8-4                 9.237n ± 0%   9.240n ± 0%        ~ (p=0.582 n=10)
And-4                  5.878n ± 0%   6.719n ± 0%  +14.31% (p=0.000 n=10)
And8Parallel-4         28.44n ± 0%   28.46n ± 0%   +0.07% (p=0.000 n=10)
AndParallel-4          28.40n ± 0%   28.43n ± 0%   +0.11% (p=0.000 n=10)
Or8-4                  8.399n ± 0%   8.398n ± 0%        ~ (p=0.357 n=10)
Or-4                   5.879n ± 0%   6.718n ± 0%  +14.27% (p=0.000 n=10)
Or8Parallel-4          28.43n ± 0%   28.45n ± 0%   +0.09% (p=0.000 n=10)
OrParallel-4           28.40n ± 0%   28.43n ± 0%   +0.11% (p=0.000 n=10)
Xadd-4                 30.05n ± 0%   30.10n ± 0%   +0.18% (p=0.000 n=10)
Xadd64-4               30.05n ± 0%   30.09n ± 0%   +0.12% (p=0.000 n=10)
Cas-4                  60.48n ± 0%   61.13n ± 0%   +1.08% (p=0.000 n=10)
Cas64-4                62.28n ± 0%   62.34n ± 0%        ~ (p=0.810 n=10)
Xchg-4                 30.05n ± 0%   30.09n ± 0%   +0.15% (p=0.000 n=10)
Xchg64-4               30.05n ± 0%   30.09n ± 0%   +0.13% (p=0.000 n=10)
geomean                15.42n        16.17n        +4.89%

Fixes #61471

Change-Id: I97b5325db50467eeec36fb079bded7b09a32330f
Reviewed-on: https://go-review.googlesource.com/c/go/+/508715
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: Joel Sing <joel@sing.id.au>
Run-TryBot: M Zhuo <mzh@golangcn.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
(cherry picked from commit 890b96f)
Reviewed-on: https://go-review.googlesource.com/c/go/+/511515
Auto-Submit: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: M Zhuo <mzh@golangcn.org>
@dmitshur dmitshur added the arch-riscv Issues solely affecting the riscv64 architecture. label Jul 27, 2023
@golang golang locked and limited conversation to collaborators Jul 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
arch-riscv Issues solely affecting the riscv64 architecture. CherryPickApproved Used during the release process for point releases compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge
Projects
None yet
Milestone
Go1.20.7
Development

No branches or pull requests
3 participants
@mdempsky @dmitshur @gopherbot