cmd/compile: rewrite f(g()) to use OAS2FUNC earlier

[mdempsky]

Currently, there's a bunch of complexity from handling f(g()) where g is multi-valued.

For example, there's #15992, where it doesn't work when f is copy or delete. Until 1602e49 it didn't work with complex either.

It makes variadic functions and implicit conversions to interface type more complicated too. For example, given

//go:noescape
func f(a, b interface{})
func g() (a, b [4]int)

it should be possible to stack allocate the [4]int boxes for f(g()), but the OCONVIFACE nodes aren't even introduced until order.go, so esc.go has no way to mark them as non-escaping and currently they're heap allocated.

order.go (callArgs/copyRet) already rewrites f(g()) into t1, t2, .., tn := g(); f(t1, t2, ..., tn). I think we should do that during type checking instead.

A side benefit will be that we can also rewrite calls to variadic functions to always use a slice parameter (i.e., the ... call form), which should simplify various call-site logic too.

I feel like there have been other subtleties around multi-value and variadic functions, so I expect this should help improve compiler robustness.

[gopherbot]

Change https://golang.org/cl/153841 mentions this issue: cmd/compile: rewrite f(g()) for multi-value g() during typecheck

[mdempsky]

I uploaded CL 153841 as a proof-of-concept of the idea. It mostly works with two notable caveats:

1. Handling global variable initializers like var v = f(g()) is tricky, because we can't generate autotmps outside of function context. The CL currently uses statictmps instead as a hacky workaround, but something better needs to be done before this can be committed.

2. Error messages need a way to print f(g()) instead of f(.autotmp_1, .autotmp_2).

[mdempsky]

CL 153841 addresses these concerns now:

1. Temporary variables for globals are allocated to the init function. (Actually, since this function isn't created until later, they're allocated to a dummy ODCLFUNC node; later they're moved and reassociated with the actual init function.)

2. The version from before rewriting is saved in n.Orig, like how constant expressions work.

Inlining sees the post-rewrite form, which means .autotmp_XXX variables can now end up in export bodies, and it increases the naive count-the-nodes cost heuristic used for determining inlinability. Neither of these seem like major issues to me though.

I'll send the CL out for review once the next dev cycle opens.

[mdempsky]

And for the record, here's a memory corruption bug due to escape analysis mishandling implicit conversions in f(g()) calls:

package main

import "runtime"

type T [4]int

//go:noinline
func f(x, y interface{}) interface{} {
	return x
}

//go:noinline
func g(x []*T) ([]*T, []*T) {
	return x, x
}

func main() {
	c := make(chan int)
	p := new(T)
	runtime.SetFinalizer(p, func(q *T) { println("finalized", q); c <- 0 })
	var s [10]*T
	s[0] = p
	h := f(g(s[:]))
	runtime.GC()
	<-c
	println("accessing", h.([]*T)[0])
}

(Note that this is distinct from the test case in #29353.)

[gopherbot]

Change https://golang.org/cl/166983 mentions this issue: cmd/compile: rewrite f(g()) for multi-value g() during typecheck