cmd/compile: soften type matching when allocating stack slots

Currently we use pointer equality on types when deciding whether we can
reuse a stack slot. That's too strict, as we don't guarantee pointer
equality for the same type. In particular, it can vary based on whether
PtrTo has been called in the frontend or not.

Instead, use the type's LinkString, which is guaranteed to both be
unique for a type, and to not vary given two different type structures
describing the same type.

Update #65783

Change-Id: I64f55138475f04bfa30cfb819b786b7cc06aebe4
Reviewed-on: https://go-review.googlesource.com/c/go/+/565436
Reviewed-by: Keith Randall <khr@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Keith Randall <khr@golang.org>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>

diff --git a/src/cmd/compile/internal/ssa/stackalloc.go b/src/cmd/compile/internal/ssa/stackalloc.go
index c9ca778b3aa47785a6445f1dfdba4140ef13651c..8290e1730efbe9aae80c360ef77f41861087b210 100644 (file)
--- a/src/cmd/compile/internal/ssa/stackalloc.go
+++ b/src/cmd/compile/internal/ssa/stackalloc.go
@@ -202,11 +202,11 @@ func (s *stackAllocState) stackalloc() {
        }
 
        // For each type, we keep track of all the stack slots we
-       // have allocated for that type.
-       // TODO: share slots among equivalent types. We would need to
-       // only share among types with the same GC signature. See the
-       // type.Equal calls below for where this matters.
-       locations := map[*types.Type][]LocalSlot{}
+       // have allocated for that type. This map is keyed by
+       // strings returned by types.LinkString. This guarantees
+       // type equality, but also lets us match the same type represented
+       // by two different types.Type structures. See issue 65783.
+       locations := map[string][]LocalSlot{}
 
        // Each time we assign a stack slot to a value v, we remember
        // the slot we used via an index into locations[v.Type].
@@ -258,7 +258,8 @@ func (s *stackAllocState) stackalloc() {
 
                noname:
                        // Set of stack slots we could reuse.
-                       locs := locations[v.Type]
+                       typeKey := v.Type.LinkString()
+                       locs := locations[typeKey]
                        // Mark all positions in locs used by interfering values.
                        for i := 0; i < len(locs); i++ {
                                used[i] = false
@@ -281,7 +282,7 @@ func (s *stackAllocState) stackalloc() {
                        if i == len(locs) {
                                s.nAuto++
                                locs = append(locs, LocalSlot{N: f.NewLocal(v.Pos, v.Type), Type: v.Type, Off: 0})
-                               locations[v.Type] = locs
+                               locations[typeKey] = locs
                        }
                        // Use the stack variable at that index for v.
                        loc := locs[i]

diff --git a/test/codegen/stack.go b/test/codegen/stack.go
index eebbbf1677f5e35df4528d4e67c6ac463cbcb462..65c9868d67086d950efc7a97175ddcd684525f2c 100644 (file)
--- a/test/codegen/stack.go
+++ b/test/codegen/stack.go
@@ -113,3 +113,32 @@ func Defer() {
        // amd64:`CALL\truntime\.deferprocStack`
        defer func() {}()
 }
+
+// Check that stack slots are shared among values of the same
+// type, but not pointer-identical types. See issue 65783.
+
+func spillSlotReuse() {
+       // The return values of getp1 and getp2 need to be
+       // spilled around the calls to nopInt. Make sure that
+       // spill slot gets reused.
+
+       //arm64:`.*autotmp_2-8\(SP\)`
+       getp1()[nopInt()] = 0
+       //arm64:`.*autotmp_2-8\(SP\)`
+       getp2()[nopInt()] = 0
+}
+
+//go:noinline
+func nopInt() int {
+       return 0
+}
+
+//go:noinline
+func getp1() *[4]int {
+       return nil
+}
+
+//go:noinline
+func getp2() *[4]int {
+       return nil
+}