cmd/compile: non-symmetric inline cost when using named vs non-named returns

[qmuntal]

What version of Go are you using (go version)?

$ go version
go version go1.14.2 windows/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output

$ go env
set GO111MODULE=
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\Dante\AppData\Local\go-build
set GOENV=C:\Users\Dante\AppData\Roaming\go\env
set GOEXE=.exe
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOINSECURE=
set GONOPROXY=
set GONOSUMDB=
set GOOS=windows
set GOPATH=C:\Users\Dante\go
set GOPRIVATE=
set GOPROXY=https://proxy.golang.org,direct
set GOROOT=c:\go
set GOSUMDB=sum.golang.org
set GOTMPDIR=
set GOTOOLDIR=c:\go\pkg\tool\windows_amd64
set GCCGO=gccgo
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=C:\Users\Dante\Documents\Code\go3mf\go.mod
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -fmessage-length=0 -fdebug-prefix-map=C:\Users\Dante\AppData\Local\Temp\go-build018008438=/tmp/go-build -gno-record-gcc-switches

What did you do?

Code: https://play.golang.org/p/bM-akhk0QXG
go build -gcflags "-m=2"

What did you expect to see?

NewTriangleInline is equivalent to NewTriangleNoInline, the only difference is that the first uses a named return and the second one doesn´t, therefore I would expect that both have the same inline cost.

func NewTriangleInline(v1, v2, v3 uint32) (t Triangle) {
	t.SetIndex(0, v1)
	t.SetIndex(1, v2)
	t.SetIndex(2, v3)
	return
}

func NewTriangleNoInline(v1, v2, v3 uint32) Triangle {
	var t Triangle
	t.SetIndex(0, v1)
	t.SetIndex(1, v2)
	t.SetIndex(2, v3)
	return t
}

What did you see instead?

.\foo.go:11:6: can inline NewTriangleInline as: func(uint32, uint32, uint32) Triangle { t.SetIndex(0, v1); t.SetIndex(1, v2); t.SetIndex(2, v3); return  }
.\foo.go:18:6: cannot inline NewTriangleNoInline: function too complex: cost 84 exceeds budget 80

[andybons]

@randall77 @josharian

[randall77]

Yes, the inlining cost estimate is not very sophisticated. Explicit declarations count, implicit ones (args+returns) don't.
This particular problem can be fixed.

I worry about the more general issue though. The inlining cost model is just an estimate. It's always going to be wrong in some situations, even in otherwise silly ways (e.g. x := 5; y += x vs. y += 5).
To really understand the cost of the inlined function we'd need to compile for real, and that can be a lot more expensive (but we compile it anyway at some point, so maybe?). Above that, we really want to know the cost at the call site, which would depend on things like arguments which are constant.

So I guess the question is - where do we draw the line? What constitutes an inlining cost "bug" and what doesn't? Do we really want to fix this one bug, only to have lots of similar bugs dribble in over time?

I'm kind of inclined to say "not a bug". Even if we fix this bug, there's no guarantee we won't reweight the inliner next release and break your inlinings anyway. But I'm open to other ideas.

[qmuntal]

@randall77 I agree with you, if the inline model is designed to give rough estimates, then this is not a bug but an improvement request.

I would also agree on closing this issue if a more generic one is created (if not there yet) for tracking a complete inliner. Else I´m inclined to leave it opened until this specific case is correctly managed.

Even if we fix this bug, there's no guarantee we won't reweight the inliner next release and break your inlinings anyway

Thanks for pointing me out this other issue. I will iterate some more time on NewTriangle to try to reduce its cost even more even if this request is implemented, as it is on my critical path. Anyway, this issue is not about fixing my specific code but to track the named returns issue.