1
0
mirror of https://github.com/golang/go synced 2024-11-14 06:50:21 -07:00

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 <austin@google.com>
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Paul Murphy <murp@ibm.com>
TryBot-Result: Go Bot <gobot@golang.org>
This commit is contained in:
Austin Clements 2020-10-26 14:32:13 -04:00
parent 22d2b984a6
commit c305e49e96
3 changed files with 21 additions and 13 deletions

View File

@ -3569,13 +3569,17 @@ func init() {
alias("runtime/internal/atomic", "LoadAcq", "runtime/internal/atomic", "Load", lwatomics...) alias("runtime/internal/atomic", "LoadAcq", "runtime/internal/atomic", "Load", lwatomics...)
alias("runtime/internal/atomic", "LoadAcq64", "runtime/internal/atomic", "Load64", lwatomics...) alias("runtime/internal/atomic", "LoadAcq64", "runtime/internal/atomic", "Load64", lwatomics...)
alias("runtime/internal/atomic", "LoadAcquintptr", "runtime/internal/atomic", "LoadAcq", p4...) alias("runtime/internal/atomic", "LoadAcquintptr", "runtime/internal/atomic", "LoadAcq", p4...)
alias("sync", "runtime_LoadAcquintptr", "runtime/internal/atomic", "LoadAcq", p4...) // linknamed
alias("runtime/internal/atomic", "LoadAcquintptr", "runtime/internal/atomic", "LoadAcq64", p8...) alias("runtime/internal/atomic", "LoadAcquintptr", "runtime/internal/atomic", "LoadAcq64", p8...)
alias("sync", "runtime_LoadAcquintptr", "runtime/internal/atomic", "LoadAcq64", p8...) // linknamed
alias("runtime/internal/atomic", "Storeuintptr", "runtime/internal/atomic", "Store", p4...) alias("runtime/internal/atomic", "Storeuintptr", "runtime/internal/atomic", "Store", p4...)
alias("runtime/internal/atomic", "Storeuintptr", "runtime/internal/atomic", "Store64", p8...) alias("runtime/internal/atomic", "Storeuintptr", "runtime/internal/atomic", "Store64", p8...)
alias("runtime/internal/atomic", "StoreRel", "runtime/internal/atomic", "Store", lwatomics...) alias("runtime/internal/atomic", "StoreRel", "runtime/internal/atomic", "Store", lwatomics...)
alias("runtime/internal/atomic", "StoreRel64", "runtime/internal/atomic", "Store64", lwatomics...) alias("runtime/internal/atomic", "StoreRel64", "runtime/internal/atomic", "Store64", lwatomics...)
alias("runtime/internal/atomic", "StoreReluintptr", "runtime/internal/atomic", "StoreRel", p4...) alias("runtime/internal/atomic", "StoreReluintptr", "runtime/internal/atomic", "StoreRel", p4...)
alias("sync", "runtime_StoreReluintptr", "runtime/internal/atomic", "StoreRel", p4...) // linknamed
alias("runtime/internal/atomic", "StoreReluintptr", "runtime/internal/atomic", "StoreRel64", p8...) alias("runtime/internal/atomic", "StoreReluintptr", "runtime/internal/atomic", "StoreRel64", p8...)
alias("sync", "runtime_StoreReluintptr", "runtime/internal/atomic", "StoreRel64", p8...) // linknamed
alias("runtime/internal/atomic", "Xchguintptr", "runtime/internal/atomic", "Xchg", p4...) alias("runtime/internal/atomic", "Xchguintptr", "runtime/internal/atomic", "Xchg", p4...)
alias("runtime/internal/atomic", "Xchguintptr", "runtime/internal/atomic", "Xchg64", p8...) alias("runtime/internal/atomic", "Xchguintptr", "runtime/internal/atomic", "Xchg64", p8...)
alias("runtime/internal/atomic", "Xadduintptr", "runtime/internal/atomic", "Xadd", p4...) alias("runtime/internal/atomic", "Xadduintptr", "runtime/internal/atomic", "Xadd", p4...)

View File

@ -1338,11 +1338,6 @@ func disallowInternal(srcDir string, importer *Package, importerPath string, p *
return p return p
} }
// Allow sync package to access lightweight atomic functions limited to the runtime.
if p.Standard && strings.HasPrefix(importerPath, "sync") && p.ImportPath == "runtime/internal/atomic" {
return p
}
// Internal is present. // Internal is present.
// Map import path back to directory corresponding to parent of internal. // Map import path back to directory corresponding to parent of internal.
if i > 0 { if i > 0 {

View File

@ -7,7 +7,6 @@ package sync
import ( import (
"internal/race" "internal/race"
"runtime" "runtime"
runtimeatomic "runtime/internal/atomic"
"sync/atomic" "sync/atomic"
"unsafe" "unsafe"
) )
@ -153,7 +152,7 @@ func (p *Pool) Get() interface{} {
func (p *Pool) getSlow(pid int) interface{} { func (p *Pool) getSlow(pid int) interface{} {
// See the comment in pin regarding ordering of the loads. // See the comment in pin regarding ordering of the loads.
size := runtimeatomic.LoadAcquintptr(&p.localSize) // load-acquire size := runtime_LoadAcquintptr(&p.localSize) // load-acquire
locals := p.local // load-consume locals := p.local // load-consume
// Try to steal one element from other procs. // Try to steal one element from other procs.
for i := 0; i < int(size); i++ { for i := 0; i < int(size); i++ {
@ -166,7 +165,7 @@ func (p *Pool) getSlow(pid int) interface{} {
// Try the victim cache. We do this after attempting to steal // Try the victim cache. We do this after attempting to steal
// from all primary caches because we want objects in the // from all primary caches because we want objects in the
// victim cache to age out if at all possible. // victim cache to age out if at all possible.
size = runtimeatomic.Loaduintptr(&p.victimSize) size = atomic.LoadUintptr(&p.victimSize)
if uintptr(pid) >= size { if uintptr(pid) >= size {
return nil return nil
} }
@ -199,7 +198,7 @@ func (p *Pool) pin() (*poolLocal, int) {
// Since we've disabled preemption, GC cannot happen in between. // Since we've disabled preemption, GC cannot happen in between.
// Thus here we must observe local at least as large localSize. // Thus here we must observe local at least as large localSize.
// We can observe a newer/larger local, it is fine (we must observe its zero-initialized-ness). // We can observe a newer/larger local, it is fine (we must observe its zero-initialized-ness).
s := runtimeatomic.LoadAcquintptr(&p.localSize) // load-acquire s := runtime_LoadAcquintptr(&p.localSize) // load-acquire
l := p.local // load-consume l := p.local // load-consume
if uintptr(pid) < s { if uintptr(pid) < s {
return indexLocal(l, pid), pid return indexLocal(l, pid), pid
@ -227,7 +226,7 @@ func (p *Pool) pinSlow() (*poolLocal, int) {
size := runtime.GOMAXPROCS(0) size := runtime.GOMAXPROCS(0)
local := make([]poolLocal, size) local := make([]poolLocal, size)
atomic.StorePointer(&p.local, unsafe.Pointer(&local[0])) // store-release atomic.StorePointer(&p.local, unsafe.Pointer(&local[0])) // store-release
runtimeatomic.StoreReluintptr(&p.localSize, uintptr(size)) // store-release runtime_StoreReluintptr(&p.localSize, uintptr(size)) // store-release
return &local[pid], pid return &local[pid], pid
} }
@ -283,3 +282,13 @@ func indexLocal(l unsafe.Pointer, i int) *poolLocal {
func runtime_registerPoolCleanup(cleanup func()) func runtime_registerPoolCleanup(cleanup func())
func runtime_procPin() int func runtime_procPin() int
func runtime_procUnpin() func runtime_procUnpin()
// The below are implemented in runtime/internal/atomic and the
// compiler also knows to intrinsify the symbol we linkname into this
// package.
//go:linkname runtime_LoadAcquintptr runtime/internal/atomic.LoadAcquintptr
func runtime_LoadAcquintptr(ptr *uintptr) uintptr
//go:linkname runtime_StoreReluintptr runtime/internal/atomic.StoreReluintptr
func runtime_StoreReluintptr(ptr *uintptr, val uintptr) uintptr