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>

diff --git a/src/cmd/compile/internal/gc/ssa.go b/src/cmd/compile/internal/gc/ssa.go
index f840ef40663858e0aac0469bd03732ee82c26886..a1b5a03687eff3beeebac988e643f2c3d22467c9 100644 (file)
--- a/src/cmd/compile/internal/gc/ssa.go
+++ b/src/cmd/compile/internal/gc/ssa.go
@@ -3569,13 +3569,17 @@ func init() {
        alias("runtime/internal/atomic", "LoadAcq", "runtime/internal/atomic", "Load", lwatomics...)
        alias("runtime/internal/atomic", "LoadAcq64", "runtime/internal/atomic", "Load64", lwatomics...)
        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("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", "Store64", p8...)
        alias("runtime/internal/atomic", "StoreRel", "runtime/internal/atomic", "Store", lwatomics...)
        alias("runtime/internal/atomic", "StoreRel64", "runtime/internal/atomic", "Store64", lwatomics...)
        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("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", "Xchg64", p8...)
        alias("runtime/internal/atomic", "Xadduintptr", "runtime/internal/atomic", "Xadd", p4...)

diff --git a/src/cmd/go/internal/load/pkg.go b/src/cmd/go/internal/load/pkg.go
index fcd7728c7b4694384a001f243bb548b8445fe456..615b5ef769bb60f3447cb7b681763842ff0bb865 100644 (file)
--- a/src/cmd/go/internal/load/pkg.go
+++ b/src/cmd/go/internal/load/pkg.go
@@ -1338,11 +1338,6 @@ func disallowInternal(srcDir string, importer *Package, importerPath string, 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.
        // Map import path back to directory corresponding to parent of internal.
        if i > 0 {

diff --git a/src/sync/pool.go b/src/sync/pool.go
index 137413fdc4f92607750c502c0af9ec4751d23252..1ae70127ac24ec137906c7c87dfc28e6ab02d413 100644 (file)
--- a/src/sync/pool.go
+++ b/src/sync/pool.go
@@ -7,7 +7,6 @@ package sync
 import (
        "internal/race"
        "runtime"
-       runtimeatomic "runtime/internal/atomic"
        "sync/atomic"
        "unsafe"
 )
@@ -153,8 +152,8 @@ func (p *Pool) Get() interface{} {
 
 func (p *Pool) getSlow(pid int) interface{} {
        // See the comment in pin regarding ordering of the loads.
-       size := runtimeatomic.LoadAcquintptr(&p.localSize) // load-acquire
-       locals := p.local                                  // load-consume
+       size := runtime_LoadAcquintptr(&p.localSize) // load-acquire
+       locals := p.local                            // load-consume
        // Try to steal one element from other procs.
        for i := 0; i < int(size); i++ {
                l := indexLocal(locals, (pid+i+1)%int(size))
@@ -166,7 +165,7 @@ func (p *Pool) getSlow(pid int) interface{} {
        // Try the victim cache. We do this after attempting to steal
        // from all primary caches because we want objects in the
        // victim cache to age out if at all possible.
-       size = runtimeatomic.Loaduintptr(&p.victimSize)
+       size = atomic.LoadUintptr(&p.victimSize)
        if uintptr(pid) >= size {
                return nil
        }
@@ -199,8 +198,8 @@ func (p *Pool) pin() (*poolLocal, int) {
        // Since we've disabled preemption, GC cannot happen in between.
        // 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).
-       s := runtimeatomic.LoadAcquintptr(&p.localSize) // load-acquire
-       l := p.local                                    // load-consume
+       s := runtime_LoadAcquintptr(&p.localSize) // load-acquire
+       l := p.local                              // load-consume
        if uintptr(pid) < s {
                return indexLocal(l, pid), pid
        }
@@ -226,8 +225,8 @@ func (p *Pool) pinSlow() (*poolLocal, int) {
        // If GOMAXPROCS changes between GCs, we re-allocate the array and lose the old one.
        size := runtime.GOMAXPROCS(0)
        local := make([]poolLocal, size)
-       atomic.StorePointer(&p.local, unsafe.Pointer(&local[0]))   // store-release
-       runtimeatomic.StoreReluintptr(&p.localSize, uintptr(size)) // store-release
+       atomic.StorePointer(&p.local, unsafe.Pointer(&local[0])) // store-release
+       runtime_StoreReluintptr(&p.localSize, uintptr(size))     // store-release
        return &local[pid], pid
 }
 
@@ -283,3 +282,13 @@ func indexLocal(l unsafe.Pointer, i int) *poolLocal {
 func runtime_registerPoolCleanup(cleanup func())
 func runtime_procPin() int
 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