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

pavel · 2017-12-02T08:16:41Z

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

The text was updated successfully, but these errors were encountered:

ALTree · 2017-12-02T10:24:12Z

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.

pavel · 2017-12-02T17:30:29Z

@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 · 2017-12-02T19:53:59Z

Then debug won’t be a leaf function as it calls the complex fmt.Println. This is being worked ok, search for mid stack inlining, but afaik is not enabled at the moment.

…

On 3 Dec 2017, at 04:30, pavel ***@***.***> wrote: @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 — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

ALTree · 2017-12-03T10:49:41Z

@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 · 2017-12-03T18:14:45Z

@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 · 2017-12-03T19:54:25Z

(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.

cherrymui · 2018-07-25T23:57:00Z

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 · 2018-07-26T00:02:45Z

https://go-review.googlesource.com/100456 is the fix.

mirtchovski · 2018-07-26T01:12:06Z

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 · 2018-07-26T11:27:03Z

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

aclements · 2018-07-26T14:00:35Z

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 · 2018-07-28T22:17:12Z

@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.

randall77 · 2018-12-12T00:44:44Z

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

danscales · 2019-12-16T23:44:46Z

This bug, as currently specified, was actually fixed when @mdempsky enabled his new escape analysis algorithm. The reason that the runtime.conv* calls were created in the first place was because (I think) the old escape analysis algorithm was not proving that the arguments to debug() were not escaping, probably related to the variadic function call. With the new escape analysis in place, the runtime.conv* routines are not generated. All the args are created on the stack, and therefore all operations related to the debug function call can be and are dead-coded.

I think we probably just want to close this bug. We could create a new bug or proposal for the more general case of understanding when functions (like runtime.conv* or user functions) have no side effects, and optimizing/removing when their results are not used.

Here's an example of a function where the compiler could get rid of the purefunc() call, after inlining inlinedfunc() and doing constant propagation, etc, but currently purefunc() remains in the code (of course, since the compiler doesn't record that purefunc has no side effects):

package main

func main() {
	mytest("abc", 1)
}

//go:noinline
func mytest(s string, n int) int {
	var r int
	a := purefunc(s)
	if inlinedfunc() < 4 {
		r = a
	} else {
		r = 3
	}
	return r
}

//go:noinline
func purefunc(s string) int {
	return 18
}

func inlinedfunc() int {
	return 5
}

ALTree added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Dec 2, 2017

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

ALTree added WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Dec 3, 2017

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

cherrymui closed this as completed Jul 26, 2018

cherrymui reopened this Jul 26, 2018

josharian added Performance NeedsFix The path to resolution is known, but the work has not been done. and removed WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. labels Jul 28, 2018

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

aclements mentioned this issue Nov 25, 2018

cmd/compile: static init maps should never generate code #2559

Open

josharian mentioned this issue Dec 10, 2018

cmd/compile: optimize variables in function calls as equivalent to constant-derived variables #29166

Open

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

andybons removed this from the Go1.13 milestone Jul 8, 2019

andybons added this to the Go1.14 milestone Jul 8, 2019

rsc modified the milestones: Go1.14, Backlog Oct 9, 2019

bradfitz added the binary-size label May 1, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

pavel commented Dec 2, 2017 •

edited

ALTree commented Dec 2, 2017 •

edited

pavel commented Dec 2, 2017 •

edited

davecheney commented Dec 2, 2017 via email

ALTree commented Dec 3, 2017 •

edited

pavel commented Dec 3, 2017 •

edited

cznic commented Dec 3, 2017

cherrymui commented Jul 25, 2018

cherrymui commented Jul 26, 2018

mirtchovski commented Jul 26, 2018

cherrymui commented Jul 26, 2018

aclements commented Jul 26, 2018

josharian commented Jul 28, 2018

randall77 commented Dec 12, 2018

danscales commented Dec 16, 2019

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

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

Comments

pavel commented Dec 2, 2017 • edited

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

Does this issue reproduce with the latest release?

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

What did you do?

What did you expect to see?

What did you see instead?

Additional information

ALTree commented Dec 2, 2017 • edited

pavel commented Dec 2, 2017 • edited

davecheney commented Dec 2, 2017 via email

ALTree commented Dec 3, 2017 • edited

pavel commented Dec 3, 2017 • edited

cznic commented Dec 3, 2017

cherrymui commented Jul 25, 2018

cherrymui commented Jul 26, 2018

mirtchovski commented Jul 26, 2018

cherrymui commented Jul 26, 2018

aclements commented Jul 26, 2018

josharian commented Jul 28, 2018

randall77 commented Dec 12, 2018

danscales commented Dec 16, 2019

pavel commented Dec 2, 2017 •

edited

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

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

ALTree commented Dec 2, 2017 •

edited

pavel commented Dec 2, 2017 •

edited

ALTree commented Dec 3, 2017 •

edited

pavel commented Dec 3, 2017 •

edited