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>

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
+}

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.
 
-		// TODO(mdempsky): We can do better here. See #27557.
-		e.assignHeap(n.Left, "call part", n)
+		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())
+
+		e.expr(e.teeHole(paramK, closureK), n.Left)
 
 	case OPTRLIT:
 		e.expr(e.spill(k, n), n.Left)

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

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
 }

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
 }

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"

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()
 }