Skip to content

Commit

Permalink
[release-branch.go1.19] cmd/compile: sign-extend the 2nd argument of …
Browse files Browse the repository at this point in the history
…the LoweredAtomicCas32 on loong64,mips64x,riscv64

The function LoweredAtomicCas32 is implemented using the LL-SC instruction pair
on loong64, mips64x, riscv64. However,the LL instruction on loong64, mips64x,
riscv64 is sign-extended, so it is necessary to sign-extend the 2nd parameter
"old" of the LoweredAtomicCas32, so that the instruction BNE after LL can get
the desired result.

The function prototype of LoweredAtomicCas32 in golang:
    func Cas32(ptr *uint32, old, new uint32) bool

When using an intrinsify implementation:
    case 1: (*ptr) <= 0x80000000 && old < 0x80000000
        E.g: (*ptr) = 0x7FFFFFFF, old = Rarg1= 0x7FFFFFFF

        After run the instruction "LL (Rarg0), Rtmp": Rtmp = 0x7FFFFFFF
        Rtmp ! = Rarg1(old) is false, the result we expect

    case 2: (*ptr) >= 0x80000000 && old >= 0x80000000
        E.g: (*ptr) = 0x80000000, old = Rarg1= 0x80000000

        After run the instruction "LL (Rarg0), Rtmp": Rtmp = 0xFFFFFFFF_80000000
        Rtmp ! = Rarg1(old) is true, which we do not expect

When using an non-intrinsify implementation:
    Because Rarg1 is loaded from the stack using sign-extended instructions
    ld.w, the situation described in Case 2 above does not occur

Benchmarks on linux/loong64:
name     old time/op  new time/op  delta
Cas      50.0ns ± 0%  50.1ns ± 0%   ~     (p=1.000 n=1+1)
Cas64    50.0ns ± 0%  50.1ns ± 0%   ~     (p=1.000 n=1+1)
Cas-4    56.0ns ± 0%  56.0ns ± 0%   ~     (p=1.000 n=1+1)
Cas64-4  56.0ns ± 0%  56.0ns ± 0%   ~     (p=1.000 n=1+1)

Benchmarks on Loongson 3A4000 (GOARCH=mips64le, 1.8GHz)
name     old time/op  new time/op  delta
Cas      70.4ns ± 0%  70.3ns ± 0%   ~     (p=1.000 n=1+1)
Cas64    70.7ns ± 0%  70.6ns ± 0%   ~     (p=1.000 n=1+1)
Cas-4    81.1ns ± 0%  80.8ns ± 0%   ~     (p=1.000 n=1+1)
Cas64-4  80.9ns ± 0%  80.9ns ± 0%   ~     (p=1.000 n=1+1)

Fixes #57345

Change-Id: I190a7fc648023b15fa392f7fdda5ac18c1561bac
Reviewed-on: https://go-review.googlesource.com/c/go/+/457135
Run-TryBot: Than McIntosh <thanm@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Wayne Zuo <wdvxdr@golangcn.org>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: David Chase <drchase@google.com>
Reviewed-on: https://go-review.googlesource.com/c/go/+/458355
Run-TryBot: David Chase <drchase@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
  • Loading branch information
dr2chase authored and gopherbot committed Dec 28, 2022
1 parent 24963e5 commit c176029
Show file tree
Hide file tree
Showing 7 changed files with 101 additions and 9 deletions.
3 changes: 2 additions & 1 deletion src/cmd/compile/internal/ssa/gen/LOONG64.rules
Expand Up @@ -392,7 +392,8 @@

(AtomicAdd(32|64) ...) => (LoweredAtomicAdd(32|64) ...)

(AtomicCompareAndSwap(32|64) ...) => (LoweredAtomicCas(32|64) ...)
(AtomicCompareAndSwap32 ptr old new mem) => (LoweredAtomicCas32 ptr (SignExt32to64 old) new mem)
(AtomicCompareAndSwap64 ...) => (LoweredAtomicCas64 ...)

// checks
(NilCheck ...) => (LoweredNilCheck ...)
Expand Down
3 changes: 2 additions & 1 deletion src/cmd/compile/internal/ssa/gen/MIPS64.rules
Expand Up @@ -392,7 +392,8 @@

(AtomicAdd(32|64) ...) => (LoweredAtomicAdd(32|64) ...)

(AtomicCompareAndSwap(32|64) ...) => (LoweredAtomicCas(32|64) ...)
(AtomicCompareAndSwap32 ptr old new mem) => (LoweredAtomicCas32 ptr (SignExt32to64 old) new mem)
(AtomicCompareAndSwap64 ...) => (LoweredAtomicCas64 ...)

// checks
(NilCheck ...) => (LoweredNilCheck ...)
Expand Down
2 changes: 1 addition & 1 deletion src/cmd/compile/internal/ssa/gen/RISCV64.rules
Expand Up @@ -568,7 +568,7 @@

(AtomicAnd32 ...) => (LoweredAtomicAnd32 ...)

(AtomicCompareAndSwap32 ...) => (LoweredAtomicCas32 ...)
(AtomicCompareAndSwap32 ptr old new mem) => (LoweredAtomicCas32 ptr (SignExt32to64 old) new mem)
(AtomicCompareAndSwap64 ...) => (LoweredAtomicCas64 ...)

(AtomicExchange32 ...) => (LoweredAtomicExchange32 ...)
Expand Down
24 changes: 22 additions & 2 deletions src/cmd/compile/internal/ssa/rewriteLOONG64.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

24 changes: 22 additions & 2 deletions src/cmd/compile/internal/ssa/rewriteMIPS64.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

24 changes: 22 additions & 2 deletions src/cmd/compile/internal/ssa/rewriteRISCV64.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

30 changes: 30 additions & 0 deletions src/runtime/internal/atomic/atomic_test.go
Expand Up @@ -345,6 +345,36 @@ func TestBitwiseContended(t *testing.T) {
}
}

func TestCasRel(t *testing.T) {
const _magic = 0x5a5aa5a5
var x struct {
before uint32
i uint32
after uint32
o uint32
n uint32
}

x.before = _magic
x.after = _magic
for j := 0; j < 32; j += 1 {
x.i = (1 << j) + 0
x.o = (1 << j) + 0
x.n = (1 << j) + 1
if !atomic.CasRel(&x.i, x.o, x.n) {
t.Fatalf("should have swapped %#x %#x", x.o, x.n)
}

if x.i != x.n {
t.Fatalf("wrong x.i after swap: x.i=%#x x.n=%#x", x.i, x.n)
}

if x.before != _magic || x.after != _magic {
t.Fatalf("wrong magic: %#x _ %#x != %#x _ %#x", x.before, x.after, _magic, _magic)
}
}
}

func TestStorepNoWB(t *testing.T) {
var p [2]*int
for i := range p {
Expand Down

0 comments on commit c176029

Please sign in to comment.