cmd/compile: more precise analysis of method values

Previously for a method value "x.M", we always flowed x directly to
the heap, which led to the receiver argument generally needing to be
heap allocated.

This CL changes it to flow x to the closure and M's receiver
parameter. This allows receiver arguments to be stack allocated as
long as (1) the closure never escapes, *and* (2) method doesn't leak
its receiver parameter.

Within the standard library, this allows a handful of objects to be
stack allocated instead. Listed here are diagnostics that were
previously emitted by "go build -gcflags=-m std cmd" that are no
longer emitted:

archive/tar/writer.go:118:6: moved to heap: f
archive/tar/writer.go:208:6: moved to heap: f
archive/tar/writer.go:248:6: moved to heap: f
cmd/compile/internal/gc/initorder.go:252:2: moved to heap: d
cmd/compile/internal/gc/initorder.go:75:2: moved to heap: s
cmd/go/internal/generate/generate.go:206:7: &Generator literal escapes to heap
cmd/internal/obj/arm64/asm7.go:910:2: moved to heap: c
cmd/internal/obj/mips/asm0.go:415:2: moved to heap: c
cmd/internal/obj/pcln.go:294:22: new(pcinlineState) escapes to heap
cmd/internal/obj/s390x/asmz.go:459:2: moved to heap: c
crypto/tls/handshake_server.go:56:2: moved to heap: hs

Thanks to Cuong Manh Le for help coming up with this solution.

Fixes #27557.

Change-Id: I8c85d671d07fb9b53e11d2dd05949a34dbbd7e17
Reviewed-on: https://go-review.googlesource.com/c/go/+/228263
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>

diff --git a/src/cmd/compile/internal/gc/closure.go b/src/cmd/compile/internal/gc/closure.go
index f00fd59f86d0c2bb51934e63f955716b5399f2dc..9d71c1e2ef2dddb093ceb806071b9793c491d7cb 100644 (file)
--- a/src/cmd/compile/internal/gc/closure.go
+++ b/src/cmd/compile/internal/gc/closure.go
@@ -566,3 +566,20 @@ func walkpartialcall(n *Node, init *Nodes) *Node {
 
        return walkexpr(clos, init)
 }
+
+// callpartMethod returns the *types.Field representing the method
+// referenced by method value n.
+func callpartMethod(n *Node) *types.Field {
+       if n.Op != OCALLPART {
+               Fatalf("expected OCALLPART, got %v", n)
+       }
+
+       // TODO(mdempsky): Optimize this. If necessary,
+       // makepartialcall could save m for us somewhere.
+       var m *types.Field
+       if lookdot0(n.Right.Sym, n.Left.Type, &m, false) != 1 {
+               Fatalf("failed to find field for OCALLPART")
+       }
+
+       return m
+}

diff --git a/src/cmd/compile/internal/gc/escape.go b/src/cmd/compile/internal/gc/escape.go
index 598750fd8da3195001a54fe7bcb18e5a329deb80..ac6fe67e4bfbaaa931f66a331261554f8bfab4c4 100644 (file)
--- a/src/cmd/compile/internal/gc/escape.go
+++ b/src/cmd/compile/internal/gc/escape.go
@@ -522,10 +522,26 @@ func (e *Escape) exprSkipInit(k EscHole, n *Node) {
                // nop
 
        case OCALLPART:
-               e.spill(k, n)
+               // Flow the receiver argument to both the closure and
+               // to the receiver parameter.
+
+               closureK := e.spill(k, n)
+
+               m := callpartMethod(n)
+
+               // We don't know how the method value will be called
+               // later, so conservatively assume the result
+               // parameters all flow to the heap.
+               //
+               // TODO(mdempsky): Change ks into a callback, so that
+               // we don't have to create this dummy slice?
+               var ks []EscHole
+               for i := m.Type.NumResults(); i > 0; i-- {
+                       ks = append(ks, e.heapHole())
+               }
+               paramK := e.tagHole(ks, asNode(m.Type.Nname()), m.Type.Recv())
 
-               // TODO(mdempsky): We can do better here. See #27557.
-               e.assignHeap(n.Left, "call part", n)
+               e.expr(e.teeHole(paramK, closureK), n.Left)
 
        case OPTRLIT:
                e.expr(e.spill(k, n), n.Left)

diff --git a/src/cmd/compile/internal/gc/scc.go b/src/cmd/compile/internal/gc/scc.go
index 0428a6af8d66e45fe0808b5fc1ef4bcc5b7b4fdc..60e0a9b8b5106b3067bd77047122e897b1e5350f 100644 (file)
--- a/src/cmd/compile/internal/gc/scc.go
+++ b/src/cmd/compile/internal/gc/scc.go
@@ -82,6 +82,13 @@ func (v *bottomUpVisitor) visit(n *Node) uint32 {
                                        min = m
                                }
                        }
+               case OCALLPART:
+                       fn := asNode(callpartMethod(n).Type.Nname())
+                       if fn != nil && fn.Op == ONAME && fn.Class() == PFUNC && fn.Name.Defn != nil {
+                               if m := v.visit(fn.Name.Defn); m < min {
+                                       min = m
+                               }
+                       }
                case OCLOSURE:
                        if m := v.visit(n.Func.Closure); m < min {
                                min = m

diff --git a/test/escape2.go b/test/escape2.go
index 4e30331380c7e553475072ef179bd03081bacde5..cf24f4bebc8efc52f4761eaf6547c9ded1aeb797 100644 (file)
--- a/test/escape2.go
+++ b/test/escape2.go
@@ -1386,8 +1386,7 @@ func (t *Tm) M() { // ERROR "t does not escape$"
 func foo141() {
        var f func()
 
-       // BAD: new(Tm) should not escape
-       t := new(Tm) // ERROR "new\(Tm\) escapes to heap$"
+       t := new(Tm) // ERROR "new\(Tm\) does not escape$"
        f = t.M      // ERROR "t.M does not escape$"
        _ = f
 }

diff --git a/test/escape2n.go b/test/escape2n.go
index 26b0a1d8c53faad65add7a1d17e8747ab4b96cee..f771e0aef26faae228f276c5951ed5a2792bbcaa 100644 (file)
--- a/test/escape2n.go
+++ b/test/escape2n.go
@@ -1386,8 +1386,7 @@ func (t *Tm) M() { // ERROR "t does not escape$"
 func foo141() {
        var f func()
 
-       // BAD: new(Tm) should not escape
-       t := new(Tm) // ERROR "new\(Tm\) escapes to heap$"
+       t := new(Tm) // ERROR "new\(Tm\) does not escape$"
        f = t.M      // ERROR "t.M does not escape$"
        _ = f
 }

diff --git a/test/fixedbugs/issue21709.go b/test/fixedbugs/issue21709.go
index abc9e767e5c31026ab0315191924d62d793067eb..cc5896ab53e317bfa67be7d8a13892e39c6082a4 100644 (file)
--- a/test/fixedbugs/issue21709.go
+++ b/test/fixedbugs/issue21709.go
@@ -14,8 +14,7 @@ func (s *S) Inc() {} // ERROR "s does not escape"
 var N int
 
 func F1() {
-       // BAD: s should not escape
-       var s S // ERROR "moved to heap: s"
+       var s S
        for i := 0; i < N; i++ {
                fs := []func(){ // ERROR "\[\]func\(\) literal does not escape"
                        s.Inc, // ERROR "s.Inc does not escape"
@@ -27,8 +26,7 @@ func F1() {
 }
 
 func F2() {
-       // BAD: s should not escape
-       var s S // ERROR "moved to heap: s"
+       var s S
        for i := 0; i < N; i++ {
                for _, f := range []func(){ // ERROR "\[\]func\(\) literal does not escape"
                        s.Inc, // ERROR "s.Inc does not escape"

diff --git a/test/fixedbugs/issue27557.go b/test/fixedbugs/issue27557.go
index 11a23f6932625b40679f9f6d449283fed9376f5f..e35ab5a1690fb03dc01faff01b56a1302ad1f5e6 100644 (file)
--- a/test/fixedbugs/issue27557.go
+++ b/test/fixedbugs/issue27557.go
@@ -9,8 +9,7 @@ package p
 var sink interface{}
 
 func _() {
-       // BAD: t should not escape
-       var t T         // ERROR "moved to heap"
+       var t T
        f := t.noescape // ERROR "t.noescape does not escape"
        f()
 }