Skip to content

Commit c305e49

Browse files
committed
cmd/go,cmd/compile,sync: remove special import case in cmd/go
CL 253748 introduced a special case in cmd/go to allow sync to import runtime/internal/atomic. Besides introducing unnecessary complexity into cmd/go, this breaks other packages (like gopls) that understand how imports work, but don't understand this special case. Fix this by using the more standard linkname-based approach to pull the necessary functions from runtime/internal/atomic into sync. Since these are compiler intrinsics, we also have to tell the compiler that the linknamed symbols are intrinsics to get this optimization in sync. Fixes #42196. Change-Id: I1f91498c255c91583950886a89c3c9adc39a32f0 Reviewed-on: https://go-review.googlesource.com/c/go/+/265124 Trust: Austin Clements <[email protected]> Run-TryBot: Austin Clements <[email protected]> Reviewed-by: Bryan C. Mills <[email protected]> Reviewed-by: Keith Randall <[email protected]> Reviewed-by: Paul Murphy <[email protected]> TryBot-Result: Go Bot <[email protected]>
1 parent 22d2b98 commit c305e49

File tree

3 files changed

+21
-13
lines changed

3 files changed

+21
-13
lines changed

src/cmd/compile/internal/gc/ssa.go

+4
Original file line numberDiff line numberDiff line change
@@ -3569,13 +3569,17 @@ func init() {
35693569
alias("runtime/internal/atomic", "LoadAcq", "runtime/internal/atomic", "Load", lwatomics...)
35703570
alias("runtime/internal/atomic", "LoadAcq64", "runtime/internal/atomic", "Load64", lwatomics...)
35713571
alias("runtime/internal/atomic", "LoadAcquintptr", "runtime/internal/atomic", "LoadAcq", p4...)
3572+
alias("sync", "runtime_LoadAcquintptr", "runtime/internal/atomic", "LoadAcq", p4...) // linknamed
35723573
alias("runtime/internal/atomic", "LoadAcquintptr", "runtime/internal/atomic", "LoadAcq64", p8...)
3574+
alias("sync", "runtime_LoadAcquintptr", "runtime/internal/atomic", "LoadAcq64", p8...) // linknamed
35733575
alias("runtime/internal/atomic", "Storeuintptr", "runtime/internal/atomic", "Store", p4...)
35743576
alias("runtime/internal/atomic", "Storeuintptr", "runtime/internal/atomic", "Store64", p8...)
35753577
alias("runtime/internal/atomic", "StoreRel", "runtime/internal/atomic", "Store", lwatomics...)
35763578
alias("runtime/internal/atomic", "StoreRel64", "runtime/internal/atomic", "Store64", lwatomics...)
35773579
alias("runtime/internal/atomic", "StoreReluintptr", "runtime/internal/atomic", "StoreRel", p4...)
3580+
alias("sync", "runtime_StoreReluintptr", "runtime/internal/atomic", "StoreRel", p4...) // linknamed
35783581
alias("runtime/internal/atomic", "StoreReluintptr", "runtime/internal/atomic", "StoreRel64", p8...)
3582+
alias("sync", "runtime_StoreReluintptr", "runtime/internal/atomic", "StoreRel64", p8...) // linknamed
35793583
alias("runtime/internal/atomic", "Xchguintptr", "runtime/internal/atomic", "Xchg", p4...)
35803584
alias("runtime/internal/atomic", "Xchguintptr", "runtime/internal/atomic", "Xchg64", p8...)
35813585
alias("runtime/internal/atomic", "Xadduintptr", "runtime/internal/atomic", "Xadd", p4...)

src/cmd/go/internal/load/pkg.go

-5
Original file line numberDiff line numberDiff line change
@@ -1338,11 +1338,6 @@ func disallowInternal(srcDir string, importer *Package, importerPath string, p *
13381338
return p
13391339
}
13401340

1341-
// Allow sync package to access lightweight atomic functions limited to the runtime.
1342-
if p.Standard && strings.HasPrefix(importerPath, "sync") && p.ImportPath == "runtime/internal/atomic" {
1343-
return p
1344-
}
1345-
13461341
// Internal is present.
13471342
// Map import path back to directory corresponding to parent of internal.
13481343
if i > 0 {

src/sync/pool.go

+17-8
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ package sync
77
import (
88
"internal/race"
99
"runtime"
10-
runtimeatomic "runtime/internal/atomic"
1110
"sync/atomic"
1211
"unsafe"
1312
)
@@ -153,8 +152,8 @@ func (p *Pool) Get() interface{} {
153152

154153
func (p *Pool) getSlow(pid int) interface{} {
155154
// See the comment in pin regarding ordering of the loads.
156-
size := runtimeatomic.LoadAcquintptr(&p.localSize) // load-acquire
157-
locals := p.local // load-consume
155+
size := runtime_LoadAcquintptr(&p.localSize) // load-acquire
156+
locals := p.local // load-consume
158157
// Try to steal one element from other procs.
159158
for i := 0; i < int(size); i++ {
160159
l := indexLocal(locals, (pid+i+1)%int(size))
@@ -166,7 +165,7 @@ func (p *Pool) getSlow(pid int) interface{} {
166165
// Try the victim cache. We do this after attempting to steal
167166
// from all primary caches because we want objects in the
168167
// victim cache to age out if at all possible.
169-
size = runtimeatomic.Loaduintptr(&p.victimSize)
168+
size = atomic.LoadUintptr(&p.victimSize)
170169
if uintptr(pid) >= size {
171170
return nil
172171
}
@@ -199,8 +198,8 @@ func (p *Pool) pin() (*poolLocal, int) {
199198
// Since we've disabled preemption, GC cannot happen in between.
200199
// Thus here we must observe local at least as large localSize.
201200
// We can observe a newer/larger local, it is fine (we must observe its zero-initialized-ness).
202-
s := runtimeatomic.LoadAcquintptr(&p.localSize) // load-acquire
203-
l := p.local // load-consume
201+
s := runtime_LoadAcquintptr(&p.localSize) // load-acquire
202+
l := p.local // load-consume
204203
if uintptr(pid) < s {
205204
return indexLocal(l, pid), pid
206205
}
@@ -226,8 +225,8 @@ func (p *Pool) pinSlow() (*poolLocal, int) {
226225
// If GOMAXPROCS changes between GCs, we re-allocate the array and lose the old one.
227226
size := runtime.GOMAXPROCS(0)
228227
local := make([]poolLocal, size)
229-
atomic.StorePointer(&p.local, unsafe.Pointer(&local[0])) // store-release
230-
runtimeatomic.StoreReluintptr(&p.localSize, uintptr(size)) // store-release
228+
atomic.StorePointer(&p.local, unsafe.Pointer(&local[0])) // store-release
229+
runtime_StoreReluintptr(&p.localSize, uintptr(size)) // store-release
231230
return &local[pid], pid
232231
}
233232

@@ -283,3 +282,13 @@ func indexLocal(l unsafe.Pointer, i int) *poolLocal {
283282
func runtime_registerPoolCleanup(cleanup func())
284283
func runtime_procPin() int
285284
func runtime_procUnpin()
285+
286+
// The below are implemented in runtime/internal/atomic and the
287+
// compiler also knows to intrinsify the symbol we linkname into this
288+
// package.
289+
290+
//go:linkname runtime_LoadAcquintptr runtime/internal/atomic.LoadAcquintptr
291+
func runtime_LoadAcquintptr(ptr *uintptr) uintptr
292+
293+
//go:linkname runtime_StoreReluintptr runtime/internal/atomic.StoreReluintptr
294+
func runtime_StoreReluintptr(ptr *uintptr, val uintptr) uintptr

0 commit comments

Comments
 (0)