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.

Sign up for GitHub

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

Open
tzneal opened this issue May 10, 2017 · 6 comments
Open

cmd/compile: inlining isn't performed on generated init functions #20321

tzneal opened this issue May 10, 2017 · 6 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. early-in-cycle A change that should be done early in the 3 month dev cycle.
Milestone
Backlog

Comments

@tzneal
Copy link
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
Copy link
Contributor

mdempsky commented Nov 29, 2017 •
edited
Loading

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
Copy link
Contributor

@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
Copy link
Contributor

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
@dr2chase dr2chase added the early-in-cycle A change that should be done early in the 3 month dev cycle. label Jun 26, 2018
@griesemer
Copy link
Contributor

Too late for 1.12 cycle. Moving to 1.13.

@griesemer griesemer modified the milestones: Go1.12, Go1.13 Dec 5, 2018
@mdempsky
Copy link
Contributor

@agnivade

Do you think this is as trivial as just adding that one line ? (I can send the CL in that case)

Yes, I think it's that simple.

Or would you want tests too ? (I would need some guidance in that case).

Tests would be good. I'd expect you could write an errorcheck -m test and make sure we emit inlining call to foo at the variable declaration statement.

@josharian
Copy link
Contributor

Too late for 1.13. Let's try making the trivial fix right when the 1.14 tree opens and see how it fares. (And check on binary size and compilation time impact.)

@josharian josharian modified the milestones: Go1.13, Go1.14 Jun 3, 2019
@rsc rsc modified the milestones: Go1.14, Backlog Oct 9, 2019
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jul 13, 2022
@mknyszek mknyszek moved this to Triage Backlog in Go Compiler / Runtime Jul 15, 2022
@gabyhelp gabyhelp mentioned this issue Nov 6, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. early-in-cycle A change that should be done early in the 3 month dev cycle.
Projects
Go Compiler / Runtime
Status: Triage Backlog
Milestone
Backlog
Development

No branches or pull requests
8 participants
@mdempsky @tzneal @josharian @rsc @agnivade @dr2chase @griesemer @gopherbot