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: inlining isn't performed on generated init functions #20321

Open
tzneal opened this Issue May 10, 2017 · 4 comments

Comments

Projects
None yet
5 participants
@tzneal
Member

tzneal commented May 10, 2017

Please answer these questions before submitting your issue. Thanks!

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

go version devel +c061f51 Wed May 10 20:19:50 2017 +0000 linux/amd64

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

amd64/linux

What did you do?

package main

import "fmt"

func double(x int) int {
	return x * 2
}

var Y = []int{double(1), double(2), double(3)}

func main() {
	fmt.Println(Y[0])
}
$ go build -gcflags="-m" -o test test.go && go tool objdump -s main.init test
# command-line-arguments
./test.go:5:6: can inline double
./test.go:12:15: Y[0] escapes to heap
./test.go:12:13: main ... argument does not escape
TEXT main.init(SB) <autogenerated>
  <autogenerated>:1	0x481700		64488b0c25f8ffffff	MOVQ FS:0xfffffff8, CX			
  <autogenerated>:1	0x481709		483b6110		CMPQ 0x10(CX), SP			
  <autogenerated>:1	0x48170d		0f8694000000		JBE 0x4817a7				
  <autogenerated>:1	0x481713		4883ec18		SUBQ $0x18, SP				
  <autogenerated>:1	0x481717		48896c2410		MOVQ BP, 0x10(SP)			
  <autogenerated>:1	0x48171c		488d6c2410		LEAQ 0x10(SP), BP			
  <autogenerated>:1	0x481721		0fb605bcd80a00		MOVZX 0xad8bc(IP), AX			
  <autogenerated>:1	0x481728		3c01			CMPL $0x1, AL				
  <autogenerated>:1	0x48172a		760a			JBE 0x481736				
  <autogenerated>:1	0x48172c		488b6c2410		MOVQ 0x10(SP), BP			
  <autogenerated>:1	0x481731		4883c418		ADDQ $0x18, SP				
  <autogenerated>:1	0x481735		c3			RET					
  <autogenerated>:1	0x481736		7507			JNE 0x48173f				
  <autogenerated>:1	0x481738		e80344faff		CALL runtime.throwinit(SB)		
  <autogenerated>:1	0x48173d		0f0b			UD2					
  <autogenerated>:1	0x48173f		c6059ed80a0001		MOVB $0x1, 0xad89e(IP)			
  <autogenerated>:1	0x481746		e895fcffff		CALL fmt.init(SB)			
  test.go:9		0x48174b		48c7042401000000	MOVQ $0x1, 0(SP)			
  test.go:9		0x481753		e8e8feffff		CALL main.double(SB)			
  test.go:9		0x481758		488b442408		MOVQ 0x8(SP), AX			
  test.go:9		0x48175d		488905acda0a00		MOVQ AX, 0xadaac(IP)			
  test.go:9		0x481764		48c7042402000000	MOVQ $0x2, 0(SP)			
  test.go:9		0x48176c		e8cffeffff		CALL main.double(SB)			
  test.go:9		0x481771		488b442408		MOVQ 0x8(SP), AX			
  test.go:9		0x481776		4889059bda0a00		MOVQ AX, 0xada9b(IP)			
  test.go:9		0x48177d		48c7042403000000	MOVQ $0x3, 0(SP)			
  test.go:9		0x481785		e8b6feffff		CALL main.double(SB)			
  test.go:9		0x48178a		488b442408		MOVQ 0x8(SP), AX			
  test.go:9		0x48178f		4889058ada0a00		MOVQ AX, 0xada8a(IP)			
  <autogenerated>:1	0x481796		c60547d80a0002		MOVB $0x2, 0xad847(IP)			
  <autogenerated>:1	0x48179d		488b6c2410		MOVQ 0x10(SP), BP			
  <autogenerated>:1	0x4817a2		4883c418		ADDQ $0x18, SP				
  <autogenerated>:1	0x4817a6		c3			RET					
  <autogenerated>:1	0x4817a7		e8e4d1fcff		CALL runtime.morestack_noctxt(SB)	
  <autogenerated>:1	0x4817ac		e94fffffff		JMP main.init(SB)			

What did you expect to see?

No calls to main.double

@tzneal tzneal added this to the Go1.10 milestone May 10, 2017

@mdempsky mdempsky modified the milestones: Go1.10, Go1.11 Nov 29, 2017

@mdempsky

This comment has been minimized.

Member

mdempsky commented Nov 29, 2017

It looks like this is as trivial as adding inlcalls(fn) right before funccompile(fn) in fninit.

However, not release critical, and I'm concerned this might subtly interact with other passes. Let's do early in 1.11 instead.

@agnivade

This comment has been minimized.

Member

agnivade commented Mar 27, 2018

@mdempsky - I have tested that your fix works.

I have not touched the compiler before. Do you think this is as trivial as just adding that one line ? (I can send the CL in that case) Or would you want tests too ? (I would need some guidance in that case).

Or if you think it's better you do it yourself, that is fine too. Let me know.

@dr2chase

This comment has been minimized.

Contributor

dr2chase commented Jun 26, 2018

Early in 1.12 now, we need to be sure this is not too terrible for binary size.

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

@griesemer

This comment has been minimized.

Contributor

griesemer commented Dec 5, 2018

Too late for 1.12 cycle. Moving to 1.13.

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

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