Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

cmd/compile: empty variadic function causes argument to escape #30898

Closed
tsavola opened this issue Mar 18, 2019 · 7 comments
Closed

cmd/compile: empty variadic function causes argument to escape #30898

tsavola opened this issue Mar 18, 2019 · 7 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Performance
Milestone
Unplanned

Comments

@tsavola
Copy link
Contributor

tsavola commented Mar 18, 2019

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

Tested with latest release and latest master:

go version go1.12.1 linux/amd64
go version devel +bedb6a18d8 Sun Mar 17 19:13:01 2019 +0000 linux/amd64

Does this issue reproduce with the latest release?

Yes.

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

go env Output
GOARCH="amd64"
GOBIN=""
GOCACHE="/home/user/.cache/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/user"
GOPROXY=""
GORACE=""
GOROOT="/usr/local/go"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build390631282=/tmp/go-build -gno-record-gcc-switches"

What did you do?

$ cat example.go
package main

func debugf(format string, args ...interface{}) {
	// Dummy implementation for non-debug build.
	// A non-empty implementation would be enabled with a build tag.
}

func main() {
	value := 10
	debugf("value is %d", value)
}
$ go build -gcflags="-m -S" example.go
# command-line-arguments
./example.go:3:6: can inline debugf
./example.go:7:6: can inline main
./example.go:9:8: inlining call to debugf
/tmp/go-build383765417/b001/_gomod_.go:6:6: can inline init.0
./example.go:3:13: debugf format does not escape
./example.go:3:28: debugf args does not escape
./example.go:9:9: value escapes to heap
./example.go:9:8: main []interface {} literal does not escape
"".debugf STEXT nosplit size=1 args=0x28 locals=0x0
	0x0000 00000 (/home/user/example.go:3)	TEXT	"".debugf(SB), NOSPLIT|ABIInternal, $0-40
	0x0000 00000 (/home/user/example.go:3)	FUNCDATA	$0, gclocals·54241e171da8af6ae173d69da0236748(SB)
	0x0000 00000 (/home/user/example.go:3)	FUNCDATA	$1, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
	0x0000 00000 (/home/user/example.go:3)	FUNCDATA	$3, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
	0x0000 00000 (/home/user/example.go:5)	RET
	0x0000 c3                                               .
"".main STEXT size=60 args=0x0 locals=0x18
	0x0000 00000 (/home/user/example.go:7)	TEXT	"".main(SB), ABIInternal, $24-0
	0x0000 00000 (/home/user/example.go:7)	MOVQ	(TLS), CX
	0x0009 00009 (/home/user/example.go:7)	CMPQ	SP, 16(CX)
	0x000d 00013 (/home/user/example.go:7)	JLS	53
	0x000f 00015 (/home/user/example.go:7)	SUBQ	$24, SP
	0x0013 00019 (/home/user/example.go:7)	MOVQ	BP, 16(SP)
	0x0018 00024 (/home/user/example.go:7)	LEAQ	16(SP), BP
	0x001d 00029 (/home/user/example.go:7)	FUNCDATA	$0, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
	0x001d 00029 (/home/user/example.go:7)	FUNCDATA	$1, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
	0x001d 00029 (/home/user/example.go:7)	FUNCDATA	$3, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
	0x001d 00029 (/home/user/example.go:9)	PCDATA	$2, $0
	0x001d 00029 (/home/user/example.go:9)	PCDATA	$0, $0
	0x001d 00029 (/home/user/example.go:9)	MOVQ	$10, (SP)
	0x0025 00037 (/home/user/example.go:9)	CALL	runtime.convT64(SB)
	0x002a 00042 (/home/user/example.go:9)	XCHGL	AX, AX
	0x002b 00043 (<unknown line number>)	MOVQ	16(SP), BP
	0x0030 00048 (<unknown line number>)	ADDQ	$24, SP
	0x0034 00052 (<unknown line number>)	RET
	0x0035 00053 (<unknown line number>)	NOP
	0x0035 00053 (/home/user/example.go:7)	PCDATA	$0, $-1
	0x0035 00053 (/home/user/example.go:7)	PCDATA	$2, $-1
	0x0035 00053 (/home/user/example.go:7)	CALL	runtime.morestack_noctxt(SB)
	0x003a 00058 (/home/user/example.go:7)	JMP	0
	0x0000 64 48 8b 0c 25 00 00 00 00 48 3b 61 10 76 26 48  dH..%....H;a.v&H
	0x0010 83 ec 18 48 89 6c 24 10 48 8d 6c 24 10 48 c7 04  ...H.l$.H.l$.H..
	0x0020 24 0a 00 00 00 e8 00 00 00 00 90 48 8b 6c 24 10  $..........H.l$.
	0x0030 48 83 c4 18 c3 e8 00 00 00 00 eb c4              H...........
	rel 5+4 t=16 TLS+0
	rel 38+4 t=8 runtime.convT64+0
	rel 54+4 t=8 runtime.morestack_noctxt+0
...

What did you expect to see?

No memory allocations.

What did you see instead?

  • ./example.go:9:9: value escapes to heap
  • CALL runtime.convT64(SB)

The issue goes away if debugf signature is changed to

  • debugf(format string, args ...int),
  • debugf(format string, arg interface{}), or
  • debugf(format string, args []interface{}) and the slice is created explicitly by the caller.
@randall77
Copy link
Contributor

The allocation comes from converting an int to an interface{}. The result of that conversion is never used.

For your three no-bug cases there is either no conversion (the ...int case), or the compiler was able to remove the conversion as dead code. For some reason it is not dead-code eliminated in the original case.

I'm surprised the original case and no-bug case 3 are different. That leads me to believe this issue should be fixable.

@mdempsky @dr2chase

@mdempsky
Copy link
Member

Very weird that the explicit slice case is different from the implicit slice case. Makes me think we should be normalizing that earlier on in the compiler.

@randall77
Copy link
Contributor

@mdempsky Great minds think alike.

@bradfitz bradfitz changed the title Empty variadic function causes argument to escape cmd/compile: empty variadic function causes argument to escape Mar 18, 2019
@bradfitz bradfitz added this to the Unplanned milestone Mar 18, 2019
@mdempsky
Copy link
Member

mdempsky commented Mar 18, 2019 •
edited

CL 168238 is an experimental take at normalizing variadic calls during typechecking.

It still fails a handful of regress tests. Most of them look harmless, though the uintptrescapes2.go changes are worth a double look.

Notably, it eliminates the convT64 call as expected.

@cherrymui cherrymui mentioned this issue Mar 20, 2019
Closed
@bcmills bcmills added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 12, 2019
@mariecurried
Copy link

I think this issue has been fixed by @mdempsky's escape analysis rewrite.

@mdempsky
Copy link
Member

@marigonzes Indeed it does. If someone wants to add a regress test that would be great.

@gopherbot
Copy link

Change https://golang.org/cl/172422 mentions this issue: test: add escape regress for empty variadic function

@golang golang locked and limited conversation to collaborators Apr 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Performance
Projects
None yet
Milestone
Unplanned
Development

No branches or pull requests
7 participants
@bradfitz @mdempsky @tsavola @bcmills @randall77 @gopherbot @mariecurried