internal/maps,cmd/compile/internal/walk: replace calls to mapaccess1* with mapaccess2*

mapaccess1* and mapaccess2* functions share the same implementation and differ only in whether the boolean "found" is returned.

This change replaces mapaccess1* calls with mapaccess2*.
We can do this transparently, since the call site can safely discard the second (boolean) result.

Ideally, mapacces1* functions could be removed entirely, but this change keeps them as thin wrappers for compatibility.

Fixes #73196
Change-Id: I07c3423d22ed1095ac3666d00e134c2747b2f9c1
Reviewed-on: https://go-review.googlesource.com/c/go/+/736020
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Auto-Submit: Keith Randall <khr@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Keith Randall <khr@golang.org>

diff --git a/src/cmd/compile/internal/walk/assign.go b/src/cmd/compile/internal/walk/assign.go
index 63b6a1d2c14163c216974daf9704e4f1c9dda8bb..f640cd83d142701dd4279e40e6a8dacc23ebb20f 100644 (file)
--- a/src/cmd/compile/internal/walk/assign.go
+++ b/src/cmd/compile/internal/walk/assign.go
@@ -154,12 +154,14 @@ func walkAssignMapRead(init *ir.Nodes, n *ir.AssignListStmt) ir.Node {
 
        r := n.Rhs[0].(*ir.IndexExpr)
        walkExprListSafe(n.Lhs, init)
+
        r.X = walkExpr(r.X, init)
        r.Index = walkExpr(r.Index, init)
+       map_ := r.X
        t := r.X.Type()
-
        fast := mapfast(t)
        key := mapKeyArg(fast, r, r.Index, false)
+       args := []ir.Node{reflectdata.IndexMapRType(base.Pos, r), map_, key}
 
        // from:
        //   a,b = m[i]
@@ -168,15 +170,14 @@ func walkAssignMapRead(init *ir.Nodes, n *ir.AssignListStmt) ir.Node {
        //   a = *var
        a := n.Lhs[0]
 
-       var call *ir.CallExpr
-       if w := t.Elem().Size(); w <= abi.ZeroValSize {
-               fn := mapfn(mapaccess2[fast], t, false)
-               call = mkcall1(fn, fn.Type().ResultsTuple(), init, reflectdata.IndexMapRType(base.Pos, r), r.X, key)
+       var mapFn ir.Node
+       if t.Elem().Size() > abi.ZeroValSize {
+               args = append(args, reflectdata.ZeroAddr(t.Elem().Size()))
+               mapFn = mapfn("mapaccess2_fat", t, true)
        } else {
-               fn := mapfn("mapaccess2_fat", t, true)
-               z := reflectdata.ZeroAddr(w)
-               call = mkcall1(fn, fn.Type().ResultsTuple(), init, reflectdata.IndexMapRType(base.Pos, r), r.X, key, z)
+               mapFn = mapfn(mapaccess[fast], t, false)
        }
+       call := mkcall1(mapFn, mapFn.Type().ResultsTuple(), init, args...)
 
        // mapaccess2* returns a typed bool, but due to spec changes,
        // the boolean result of i.(T) is now untyped so we make it the

diff --git a/src/cmd/compile/internal/walk/expr.go b/src/cmd/compile/internal/walk/expr.go
index 2794671c73bc7bf59ea8f713f1c4aa3b988b0cfd..125ffd53b1eba1ca31767f181fd90c95b2c77ed7 100644 (file)
--- a/src/cmd/compile/internal/walk/expr.go
+++ b/src/cmd/compile/internal/walk/expr.go
@@ -868,20 +868,43 @@ func walkIndexMap(n *ir.IndexExpr, init *ir.Nodes) ir.Node {
        key := mapKeyArg(fast, n, n.Index, n.Assigned)
        args := []ir.Node{reflectdata.IndexMapRType(base.Pos, n), map_, key}
 
+       if n.Assigned {
+               mapFn := mapfn(mapassign[fast], t, false)
+               call := mkcall1(mapFn, nil, init, args...)
+               call.SetType(types.NewPtr(t.Elem()))
+               call.MarkNonNil() // mapassign always return non-nil pointers.
+               star := ir.NewStarExpr(base.Pos, call)
+               star.SetType(t.Elem())
+               star.SetTypecheck(1)
+               return star
+       }
+
+       // from:
+       //   m[i]
+       // to:
+       //   var, _ = mapaccess2*(t, m, i)
+       //   *var
        var mapFn ir.Node
-       switch {
-       case n.Assigned:
-               mapFn = mapfn(mapassign[fast], t, false)
-       case t.Elem().Size() > abi.ZeroValSize:
+       if t.Elem().Size() > abi.ZeroValSize {
                args = append(args, reflectdata.ZeroAddr(t.Elem().Size()))
-               mapFn = mapfn("mapaccess1_fat", t, true)
-       default:
-               mapFn = mapfn(mapaccess1[fast], t, false)
+               mapFn = mapfn("mapaccess2_fat", t, true)
+       } else {
+               mapFn = mapfn(mapaccess[fast], t, false)
        }
-       call := mkcall1(mapFn, nil, init, args...)
-       call.SetType(types.NewPtr(t.Elem()))
-       call.MarkNonNil() // mapaccess1* and mapassign always return non-nil pointers.
-       star := ir.NewStarExpr(base.Pos, call)
+       call := mkcall1(mapFn, mapFn.Type().ResultsTuple(), init, args...)
+
+       var_ := typecheck.TempAt(base.Pos, ir.CurFunc, types.NewPtr(t.Elem()))
+       var_.SetTypecheck(1)
+       var_.MarkNonNil() // mapaccess always returns a non-nill pointer
+
+       bool_ := typecheck.TempAt(base.Pos, ir.CurFunc, types.Types[types.TBOOL])
+       bool_.SetTypecheck(1)
+
+       r := ir.NewAssignListStmt(base.Pos, ir.OAS2FUNC, []ir.Node{var_, bool_}, []ir.Node{call})
+       r.SetTypecheck(1)
+       init.Append(walkExpr(r, init))
+
+       star := ir.NewStarExpr(base.Pos, var_)
        star.SetType(t.Elem())
        star.SetTypecheck(1)
        return star

diff --git a/src/cmd/compile/internal/walk/walk.go b/src/cmd/compile/internal/walk/walk.go
index 08f36095a55f2fa6d3dddb1b28ac0759c1bd2ea5..a97263d2bfc2d949689c0eb25060939853ad851c 100644 (file)
--- a/src/cmd/compile/internal/walk/walk.go
+++ b/src/cmd/compile/internal/walk/walk.go
@@ -184,8 +184,7 @@ func mkmapnames(base string, ptr string) mapnames {
        return mapnames{base, base + "_fast32", base + "_fast32" + ptr, base + "_fast64", base + "_fast64" + ptr, base + "_faststr"}
 }
 
-var mapaccess1 = mkmapnames("mapaccess1", "")
-var mapaccess2 = mkmapnames("mapaccess2", "")
+var mapaccess = mkmapnames("mapaccess2", "")
 var mapassign = mkmapnames("mapassign", "ptr")
 var mapdelete = mkmapnames("mapdelete", "")
 

diff --git a/src/internal/runtime/maps/runtime.go b/src/internal/runtime/maps/runtime.go
index 8bba23f07003bd036b75a2ac473775240998d0ab..039f4f051023b023119dd9845336c8a3f19c2ccd 100644 (file)
--- a/src/internal/runtime/maps/runtime.go
+++ b/src/internal/runtime/maps/runtime.go
@@ -38,9 +38,6 @@ func newobject(typ *abi.Type) unsafe.Pointer
 //go:linkname errNilAssign
 var errNilAssign error
 
-// Pull from runtime. It is important that is this the exact same copy as the
-// runtime because runtime.mapaccess1_fat compares the returned pointer with
-// &runtime.zeroVal[0].
 // TODO: move zeroVal to internal/abi?
 //
 //go:linkname zeroVal runtime.zeroVal
@@ -54,84 +51,15 @@ var zeroVal [abi.ZeroValSize]byte
 //
 //go:linkname runtime_mapaccess1 runtime.mapaccess1
 func runtime_mapaccess1(typ *abi.MapType, m *Map, key unsafe.Pointer) unsafe.Pointer {
-       if race.Enabled && m != nil {
-               callerpc := sys.GetCallerPC()
-               pc := abi.FuncPCABIInternal(runtime_mapaccess1)
-               race.ReadPC(unsafe.Pointer(m), callerpc, pc)
-               race.ReadObjectPC(typ.Key, key, callerpc, pc)
-       }
-       if msan.Enabled && m != nil {
-               msan.Read(key, typ.Key.Size_)
-       }
-       if asan.Enabled && m != nil {
-               asan.Read(key, typ.Key.Size_)
-       }
-
-       if m == nil || m.Used() == 0 {
-               if err := mapKeyError(typ, key); err != nil {
-                       panic(err) // see issue 23734
-               }
-               return unsafe.Pointer(&zeroVal[0])
-       }
-
-       if m.writing != 0 {
-               fatal("concurrent map read and map write")
-       }
-
-       hash := typ.Hasher(key, m.seed)
-
-       if m.dirLen <= 0 {
-               _, elem, ok := m.getWithKeySmall(typ, hash, key)
-               if !ok {
-                       return unsafe.Pointer(&zeroVal[0])
-               }
-               return elem
-       }
-
-       // Select table.
-       idx := m.directoryIndex(hash)
-       t := m.directoryAt(idx)
-
-       // Probe table.
-       seq := makeProbeSeq(h1(hash), t.groups.lengthMask)
-       h2Hash := h2(hash)
-       for ; ; seq = seq.next() {
-               g := t.groups.group(typ, seq.offset)
-
-               match := g.ctrls().matchH2(h2Hash)
-
-               for match != 0 {
-                       i := match.first()
-
-                       slotKey := g.key(typ, i)
-                       slotKeyOrig := slotKey
-                       if typ.IndirectKey() {
-                               slotKey = *((*unsafe.Pointer)(slotKey))
-                       }
-                       if typ.Key.Equal(key, slotKey) {
-                               slotElem := unsafe.Pointer(uintptr(slotKeyOrig) + typ.ElemOff)
-                               if typ.IndirectElem() {
-                                       slotElem = *((*unsafe.Pointer)(slotElem))
-                               }
-                               return slotElem
-                       }
-                       match = match.removeFirst()
-               }
-
-               match = g.ctrls().matchEmpty()
-               if match != 0 {
-                       // Finding an empty slot means we've reached the end of
-                       // the probe sequence.
-                       return unsafe.Pointer(&zeroVal[0])
-               }
-       }
+       p, _ := runtime_mapaccess2(typ, m, key)
+       return p
 }
 
 //go:linkname runtime_mapaccess2 runtime.mapaccess2
 func runtime_mapaccess2(typ *abi.MapType, m *Map, key unsafe.Pointer) (unsafe.Pointer, bool) {
        if race.Enabled && m != nil {
                callerpc := sys.GetCallerPC()
-               pc := abi.FuncPCABIInternal(runtime_mapaccess1)
+               pc := abi.FuncPCABIInternal(runtime_mapaccess2)
                race.ReadPC(unsafe.Pointer(m), callerpc, pc)
                race.ReadObjectPC(typ.Key, key, callerpc, pc)
        }

diff --git a/src/internal/runtime/maps/runtime_fast32.go b/src/internal/runtime/maps/runtime_fast32.go
index d5be04afd450c0e1a2b65f881705b4849b36febe..a185dca5104a39394dee3555587b046e71fa0232 100644 (file)
--- a/src/internal/runtime/maps/runtime_fast32.go
+++ b/src/internal/runtime/maps/runtime_fast32.go
@@ -13,72 +13,8 @@ import (
 
 //go:linkname runtime_mapaccess1_fast32 runtime.mapaccess1_fast32
 func runtime_mapaccess1_fast32(typ *abi.MapType, m *Map, key uint32) unsafe.Pointer {
-       if race.Enabled && m != nil {
-               callerpc := sys.GetCallerPC()
-               pc := abi.FuncPCABIInternal(runtime_mapaccess1_fast32)
-               race.ReadPC(unsafe.Pointer(m), callerpc, pc)
-       }
-
-       if m == nil || m.Used() == 0 {
-               return unsafe.Pointer(&zeroVal[0])
-       }
-
-       if m.writing != 0 {
-               fatal("concurrent map read and map write")
-               return nil
-       }
-
-       if m.dirLen == 0 {
-               g := groupReference{
-                       data: m.dirPtr,
-               }
-               full := g.ctrls().matchFull()
-               slotKey := g.key(typ, 0)
-               slotSize := typ.SlotSize
-               for full != 0 {
-                       if key == *(*uint32)(slotKey) && full.lowestSet() {
-                               slotElem := unsafe.Pointer(uintptr(slotKey) + typ.ElemOff)
-                               return slotElem
-                       }
-                       slotKey = unsafe.Pointer(uintptr(slotKey) + slotSize)
-                       full = full.shiftOutLowest()
-               }
-               return unsafe.Pointer(&zeroVal[0])
-       }
-
-       k := key
-       hash := typ.Hasher(abi.NoEscape(unsafe.Pointer(&k)), m.seed)
-
-       // Select table.
-       idx := m.directoryIndex(hash)
-       t := m.directoryAt(idx)
-
-       // Probe table.
-       seq := makeProbeSeq(h1(hash), t.groups.lengthMask)
-       h2Hash := h2(hash)
-       for ; ; seq = seq.next() {
-               g := t.groups.group(typ, seq.offset)
-
-               match := g.ctrls().matchH2(h2Hash)
-
-               for match != 0 {
-                       i := match.first()
-
-                       slotKey := g.key(typ, i)
-                       if key == *(*uint32)(slotKey) {
-                               slotElem := unsafe.Pointer(uintptr(slotKey) + typ.ElemOff)
-                               return slotElem
-                       }
-                       match = match.removeFirst()
-               }
-
-               match = g.ctrls().matchEmpty()
-               if match != 0 {
-                       // Finding an empty slot means we've reached the end of
-                       // the probe sequence.
-                       return unsafe.Pointer(&zeroVal[0])
-               }
-       }
+       p, _ := runtime_mapaccess2_fast32(typ, m, key)
+       return p
 }
 
 //go:linkname runtime_mapaccess2_fast32 runtime.mapaccess2_fast32

diff --git a/src/internal/runtime/maps/runtime_fast64.go b/src/internal/runtime/maps/runtime_fast64.go
index 2bee2d4be019b3febfa84abb8bf708784048729a..6e1e0ea25d2b5b7ff5e172a8386128bcbcee3472 100644 (file)
--- a/src/internal/runtime/maps/runtime_fast64.go
+++ b/src/internal/runtime/maps/runtime_fast64.go
@@ -13,72 +13,8 @@ import (
 
 //go:linkname runtime_mapaccess1_fast64 runtime.mapaccess1_fast64
 func runtime_mapaccess1_fast64(typ *abi.MapType, m *Map, key uint64) unsafe.Pointer {
-       if race.Enabled && m != nil {
-               callerpc := sys.GetCallerPC()
-               pc := abi.FuncPCABIInternal(runtime_mapaccess1_fast64)
-               race.ReadPC(unsafe.Pointer(m), callerpc, pc)
-       }
-
-       if m == nil || m.Used() == 0 {
-               return unsafe.Pointer(&zeroVal[0])
-       }
-
-       if m.writing != 0 {
-               fatal("concurrent map read and map write")
-               return nil
-       }
-
-       if m.dirLen == 0 {
-               g := groupReference{
-                       data: m.dirPtr,
-               }
-               full := g.ctrls().matchFull()
-               slotKey := g.key(typ, 0)
-               slotSize := typ.SlotSize
-               for full != 0 {
-                       if key == *(*uint64)(slotKey) && full.lowestSet() {
-                               slotElem := unsafe.Pointer(uintptr(slotKey) + 8)
-                               return slotElem
-                       }
-                       slotKey = unsafe.Pointer(uintptr(slotKey) + slotSize)
-                       full = full.shiftOutLowest()
-               }
-               return unsafe.Pointer(&zeroVal[0])
-       }
-
-       k := key
-       hash := typ.Hasher(abi.NoEscape(unsafe.Pointer(&k)), m.seed)
-
-       // Select table.
-       idx := m.directoryIndex(hash)
-       t := m.directoryAt(idx)
-
-       // Probe table.
-       seq := makeProbeSeq(h1(hash), t.groups.lengthMask)
-       h2Hash := h2(hash)
-       for ; ; seq = seq.next() {
-               g := t.groups.group(typ, seq.offset)
-
-               match := g.ctrls().matchH2(h2Hash)
-
-               for match != 0 {
-                       i := match.first()
-
-                       slotKey := g.key(typ, i)
-                       if key == *(*uint64)(slotKey) {
-                               slotElem := unsafe.Pointer(uintptr(slotKey) + 8)
-                               return slotElem
-                       }
-                       match = match.removeFirst()
-               }
-
-               match = g.ctrls().matchEmpty()
-               if match != 0 {
-                       // Finding an empty slot means we've reached the end of
-                       // the probe sequence.
-                       return unsafe.Pointer(&zeroVal[0])
-               }
-       }
+       p, _ := runtime_mapaccess2_fast64(typ, m, key)
+       return p
 }
 
 //go:linkname runtime_mapaccess2_fast64 runtime.mapaccess2_fast64

diff --git a/src/internal/runtime/maps/runtime_faststr.go b/src/internal/runtime/maps/runtime_faststr.go
index 374468b66438a6c02691e89998f8c0d300a71bf1..4a50c46106e2656024b2f349f7b8d9084ad9240d 100644 (file)
--- a/src/internal/runtime/maps/runtime_faststr.go
+++ b/src/internal/runtime/maps/runtime_faststr.go
@@ -99,62 +99,8 @@ func stringPtr(s string) unsafe.Pointer {
 
 //go:linkname runtime_mapaccess1_faststr runtime.mapaccess1_faststr
 func runtime_mapaccess1_faststr(typ *abi.MapType, m *Map, key string) unsafe.Pointer {
-       if race.Enabled && m != nil {
-               callerpc := sys.GetCallerPC()
-               pc := abi.FuncPCABIInternal(runtime_mapaccess1_faststr)
-               race.ReadPC(unsafe.Pointer(m), callerpc, pc)
-       }
-
-       if m == nil || m.Used() == 0 {
-               return unsafe.Pointer(&zeroVal[0])
-       }
-
-       if m.writing != 0 {
-               fatal("concurrent map read and map write")
-               return nil
-       }
-
-       if m.dirLen <= 0 {
-               elem := m.getWithoutKeySmallFastStr(typ, key)
-               if elem == nil {
-                       return unsafe.Pointer(&zeroVal[0])
-               }
-               return elem
-       }
-
-       k := key
-       hash := typ.Hasher(abi.NoEscape(unsafe.Pointer(&k)), m.seed)
-
-       // Select table.
-       idx := m.directoryIndex(hash)
-       t := m.directoryAt(idx)
-
-       // Probe table.
-       seq := makeProbeSeq(h1(hash), t.groups.lengthMask)
-       h2Hash := h2(hash)
-       for ; ; seq = seq.next() {
-               g := t.groups.group(typ, seq.offset)
-
-               match := g.ctrls().matchH2(h2Hash)
-
-               for match != 0 {
-                       i := match.first()
-
-                       slotKey := g.key(typ, i)
-                       if key == *(*string)(slotKey) {
-                               slotElem := unsafe.Pointer(uintptr(slotKey) + 2*goarch.PtrSize)
-                               return slotElem
-                       }
-                       match = match.removeFirst()
-               }
-
-               match = g.ctrls().matchEmpty()
-               if match != 0 {
-                       // Finding an empty slot means we've reached the end of
-                       // the probe sequence.
-                       return unsafe.Pointer(&zeroVal[0])
-               }
-       }
+       p, _ := runtime_mapaccess2_faststr(typ, m, key)
+       return p
 }
 
 //go:linkname runtime_mapaccess2_faststr runtime.mapaccess2_faststr

diff --git a/src/runtime/map.go b/src/runtime/map.go
index 4a0713cfc4c05ea96d7dfc008e2a5ed774826157..0b61430441eccc5851bbb7cf53cd96c7a903e1ad 100644 (file)
--- a/src/runtime/map.go
+++ b/src/runtime/map.go
@@ -91,16 +91,16 @@ func mapaccess1(t *abi.MapType, m *maps.Map, key unsafe.Pointer) unsafe.Pointer
 func mapaccess2(t *abi.MapType, m *maps.Map, key unsafe.Pointer) (unsafe.Pointer, bool)
 
 func mapaccess1_fat(t *abi.MapType, m *maps.Map, key, zero unsafe.Pointer) unsafe.Pointer {
-       e := mapaccess1(t, m, key)
-       if e == unsafe.Pointer(&zeroVal[0]) {
+       e, ok := mapaccess2(t, m, key)
+       if !ok {
                return zero
        }
        return e
 }
 
 func mapaccess2_fat(t *abi.MapType, m *maps.Map, key, zero unsafe.Pointer) (unsafe.Pointer, bool) {
-       e := mapaccess1(t, m, key)
-       if e == unsafe.Pointer(&zeroVal[0]) {
+       e, ok := mapaccess2(t, m, key)
+       if !ok {
                return zero, false
        }
        return e, true