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

cmd/compile: eliminate runtime.conv* calls with unused results #22971

Open
pavel opened this Issue Dec 2, 2017 · 13 comments

Comments

Projects
None yet
10 participants
@pavel

pavel commented Dec 2, 2017

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

go version go1.9.2 linux/amd64

Does this issue reproduce with the latest release?

Yes

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

linux/amd64

What did you do?

I'm using the following file (noop.go):

package main

import "fmt"

func debug(data ...interface{}) {}

func op() {
  fmt.Println("Op")
}

func main() {
  debug("123")
  op()
}

I compile the file using: go tool compile noop.go.
Then I inspect the object file using: go tool objdump noop.o.

What did you expect to see?

Ideally I expect to see no traces of the debug function.
In a non-ideal, but a good outcome I expect to see no calls to the debug function.

What did you see instead?

A call to the debug function.

noop.go:12 0x569 e800000000 CALL 0x56e [1:5]R_CALL:%22%22.debug
noop.go:13 0x56e e800000000 CALL 0x573 [1:5]R_CALL:%22%22.op

Additional information

If I replace signature of the debug function to:

func debug(data string) {}

I get the "good" scenarion:

  • the debug function is present
  • no calls to the debug function exist
@ALTree

This comment has been minimized.

Member

ALTree commented Dec 2, 2017

I suspect "certain cases" == "when the function is not inlined". Variadic functions (... arguments) are currently not inlined in the default configuration. This looks like a bad interaction between the inliner and the dead-code-elimination pass. IIRC we have a few issues about this.

@cherrymui cherrymui added this to the Go1.11 milestone Dec 2, 2017

@pavel

This comment has been minimized.

pavel commented Dec 2, 2017

@ALTree I'm not familiar with inline expansion rules of the Go compiler.

Here's what I did, to see if inline expansion mechanism worked on the code above.

I've changed the debug function to be:

func debug(data string) {
  fmt.Println(string)
}

I expected to see the call to the debug function to be inlined after compilation, but looking at objdump output it does not seem to be the case on my environment.

noop.go:14 0x640 e800000000 CALL 0x645 [1:5]R_CALL:%22%22.debug
noop.go:15 0x645 e800000000 CALL 0x64a [1:5]R_CALL:%22%22.op

I guess inline expansion is not happening here due to variadic nature of fmt.Println.

In case when the debug function looks like this:

func debug(data string) {
  data = data + "123"
}

I see inline expansion happening, though the debug function is still present.

noop.go:6 0x6cd e800000000 CALL 0x6d2 [1:5]R_CALL:runtime.concatstring2
noop.go:15 0x6d2 e800000000 CALL 0x6d7 [1:5]R_CALL:%22%22.op
@davecheney

This comment has been minimized.

Contributor

davecheney commented Dec 2, 2017

@ALTree

This comment has been minimized.

Member

ALTree commented Dec 3, 2017

@pavel as Dave pointed out, it's a different issue.

func debug(data ...interface{}) {}

vs

func debug(data string) {
  fmt.Println(string)
}

The former function is variadic, the latter isn't. The former has an empty body, the latter doesn't.

It's still not clear to me what issue you are reporting here. Is that we don't inline variadic functions? That we don't inline nonleaf functions? We already have issues for those things, and work has been done to fix them (for example inlining nonleafs). Or is this just about you wanting to understand inlining rules?

@pavel

This comment has been minimized.

pavel commented Dec 3, 2017

@ALTree Sorry, I've deviated with this inline expansion comments of mine.

Discard all my comments regarding inline expansion. My issue is described in the title and my first comment (i.e. compiler does not remove functions with empty body and calls to them).

@cznic

This comment has been minimized.

Contributor

cznic commented Dec 3, 2017

(i.e. compiler does not remove functions with empty body and calls to them).

FTR: Empty function body is not a sufficient condition for eliminating the call.

@dr2chase dr2chase modified the milestones: Go1.11, Go1.12 Jun 26, 2018

@cherrymui

This comment has been minimized.

Contributor

cherrymui commented Jul 25, 2018

At tip, the no-op function now is indeed elided:

"".main STEXT size=48 args=0x0 locals=0x8
	0x0000 00000 (x.go:11)	TEXT	"".main(SB), $8-0
	0x0000 00000 (x.go:11)	MOVQ	(TLS), CX
	0x0009 00009 (x.go:11)	CMPQ	SP, 16(CX)
	0x000d 00013 (x.go:11)	JLS	41
	0x000f 00015 (x.go:11)	SUBQ	$8, SP
	0x0013 00019 (x.go:11)	MOVQ	BP, (SP)
	0x0017 00023 (x.go:11)	LEAQ	(SP), BP
	0x001b 00027 (x.go:11)	FUNCDATA	$0, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
	0x001b 00027 (x.go:11)	FUNCDATA	$1, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
	0x001b 00027 (x.go:11)	FUNCDATA	$3, gclocals·33cdeccccebe80329f1fdbee7f5874cb(SB)
	0x001b 00027 (x.go:13)	PCDATA	$2, $0
	0x001b 00027 (x.go:13)	PCDATA	$0, $0
	0x001b 00027 (x.go:13)	CALL	"".op(SB)
	0x0020 00032 (x.go:14)	MOVQ	(SP), BP
	0x0024 00036 (x.go:14)	ADDQ	$8, SP
	0x0028 00040 (x.go:14)	RET
	0x0029 00041 (x.go:14)	NOP
	0x0029 00041 (x.go:11)	PCDATA	$0, $-1
	0x0029 00041 (x.go:11)	PCDATA	$2, $-1
	0x0029 00041 (x.go:11)	CALL	runtime.morestack_noctxt(SB)
	0x002e 00046 (x.go:11)	JMP	0

There is no call to debug.

So this is fixed?

@cherrymui

This comment has been minimized.

Contributor

cherrymui commented Jul 26, 2018

@cherrymui cherrymui closed this Jul 26, 2018

@mirtchovski

This comment has been minimized.

Contributor

mirtchovski commented Jul 26, 2018

I wonder if this is working as expected. When I add a couple of more arguments to the call to debug I see calls to convT2E. i would have expected those to be gone if the function call is elided:

"".main STEXT size=144 args=0x0 locals=0x50
	0x0000 00000 (/tmp/x.go:11)	TEXT	"".main(SB), $80-0
	0x0000 00000 (/tmp/x.go:11)	MOVQ	(TLS), CX
	0x0009 00009 (/tmp/x.go:11)	CMPQ	SP, 16(CX)
	0x000d 00013 (/tmp/x.go:11)	JLS	134
	0x000f 00015 (/tmp/x.go:11)	SUBQ	$80, SP
	0x0013 00019 (/tmp/x.go:11)	MOVQ	BP, 72(SP)
	0x0018 00024 (/tmp/x.go:11)	LEAQ	72(SP), BP
	0x001d 00029 (/tmp/x.go:11)	FUNCDATA	$0, gclocals·69c1753bd5f81501d95132d08af04464(SB)
	0x001d 00029 (/tmp/x.go:11)	FUNCDATA	$1, gclocals·713abd6cdf5e052e4dcd3eb297c82601(SB)
	0x001d 00029 (/tmp/x.go:11)	FUNCDATA	$3, gclocals·1cf923758aae2e428391d1783fe59973(SB)
	0x001d 00029 (/tmp/x.go:12)	PCDATA	$2, $0
	0x001d 00029 (/tmp/x.go:12)	PCDATA	$0, $0
	0x001d 00029 (/tmp/x.go:12)	MOVQ	$0, (SP)
	0x0025 00037 (/tmp/x.go:12)	PCDATA	$2, $1
	0x0025 00037 (/tmp/x.go:12)	LEAQ	go.string."xyzzy"(SB), AX
	0x002c 00044 (/tmp/x.go:12)	PCDATA	$2, $0
	0x002c 00044 (/tmp/x.go:12)	MOVQ	AX, 8(SP)
	0x0031 00049 (/tmp/x.go:12)	MOVQ	$5, 16(SP)
	0x003a 00058 (/tmp/x.go:12)	CALL	runtime.stringtoslicebyte(SB)
	0x003f 00063 (/tmp/x.go:12)	MOVQ	32(SP), AX
	0x0044 00068 (/tmp/x.go:12)	PCDATA	$2, $2
	0x0044 00068 (/tmp/x.go:12)	MOVQ	24(SP), CX
	0x0049 00073 (/tmp/x.go:12)	MOVQ	40(SP), DX
	0x004e 00078 (/tmp/x.go:12)	PCDATA	$2, $0
	0x004e 00078 (/tmp/x.go:12)	PCDATA	$0, $1
	0x004e 00078 (/tmp/x.go:12)	MOVQ	CX, ""..autotmp_4+48(SP)
	0x0053 00083 (/tmp/x.go:12)	MOVQ	AX, ""..autotmp_4+56(SP)
	0x0058 00088 (/tmp/x.go:12)	MOVQ	DX, ""..autotmp_4+64(SP)
	0x005d 00093 (/tmp/x.go:12)	PCDATA	$2, $1
	0x005d 00093 (/tmp/x.go:12)	LEAQ	type.[]uint8(SB), AX
	0x0064 00100 (/tmp/x.go:12)	PCDATA	$2, $0
	0x0064 00100 (/tmp/x.go:12)	MOVQ	AX, (SP)
	0x0068 00104 (/tmp/x.go:12)	PCDATA	$2, $1
	0x0068 00104 (/tmp/x.go:12)	LEAQ	""..autotmp_4+48(SP), AX
	0x006d 00109 (/tmp/x.go:12)	PCDATA	$2, $0
	0x006d 00109 (/tmp/x.go:12)	MOVQ	AX, 8(SP)
	0x0072 00114 (/tmp/x.go:12)	CALL	runtime.convT2Eslice(SB)
	0x0077 00119 (/tmp/x.go:13)	PCDATA	$0, $0
	0x0077 00119 (/tmp/x.go:13)	CALL	"".op(SB)
	0x007c 00124 (/tmp/x.go:14)	MOVQ	72(SP), BP
	0x0081 00129 (/tmp/x.go:14)	ADDQ	$80, SP
	0x0085 00133 (/tmp/x.go:14)	RET
	0x0086 00134 (/tmp/x.go:14)	NOP
	0x0086 00134 (/tmp/x.go:11)	PCDATA	$0, $-1
	0x0086 00134 (/tmp/x.go:11)	PCDATA	$2, $-1
	0x0086 00134 (/tmp/x.go:11)	CALL	runtime.morestack_noctxt(SB)
	0x008b 00139 (/tmp/x.go:11)	JMP	0

here's my modified code:

package main

import "fmt"

func debug(data ...interface{}) {}

func op() {
	fmt.Println("Op")
}

func main() {
	debug("123", 1, []byte("xyzzy"))
	op()
}
@cherrymui

This comment has been minimized.

Contributor

cherrymui commented Jul 26, 2018

Ok, this is not fully resolved. Thanks, @mirtchovski.

@cherrymui cherrymui reopened this Jul 26, 2018

@aclements

This comment has been minimized.

Member

aclements commented Jul 26, 2018

How hard would it be to do purity optimizations for known-pure runtime functions like conv*? (Not necessarily deriving what functions are pure; just using a hand-coded list.) It seems like that should be easy, but maybe we first need a higher-level SSA call representation? /cc @dr2chase since he's been thinking about purity analysis.

@josharian

This comment has been minimized.

Contributor

josharian commented Jul 28, 2018

@aclements I suspect it could be done pretty easily in this special case with a generic rewrite rule that checks for conv functions with unused results and eliminates them. The rewrite rule will be awkward, but that's nothing new.

@josharian josharian changed the title from cmd/compile: noop functions are not compiled away in certain cases to cmd/compile: eliminate runtime.conv* calls with unused results Jul 28, 2018

@randall77

This comment has been minimized.

Contributor

randall77 commented Dec 12, 2018

Punting to 1.13, too late for anything major in 1.12.

@randall77 randall77 modified the milestones: Go1.12, Go1.13 Dec 12, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment