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: Duplicated sequence of instructions in runtime.adjustframe on AMD64 #49167

Closed
jake-ciolek opened this issue Oct 26, 2021 · 3 comments
Closed

cmd/compile: Duplicated sequence of instructions in runtime.adjustframe on AMD64 #49167

jake-ciolek opened this issue Oct 26, 2021 · 3 comments
Labels
FrozenDueToAge

Comments

@jake-ciolek
Copy link
Contributor

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

go tip

Does this issue reproduce with the latest release?

Yes

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

go env Output
Darwin AMD64

What did you do?

Looked at the assembly code of runtime.adjustframe.

What did you expect to see?

movl	$1, %eax
movq	128(%rsp), %rbp
addq	$136, %rsp
retq

I expected to see this once since there's a ret.

What did you see instead?

movl	$1, %eax
movq	128(%rsp), %rbp
addq	$136, %rsp
retq
movl	$1, %eax
movq	128(%rsp), %rbp
addq	$136, %rsp
retq
movl	$1, %eax
movq	128(%rsp), %rbp
addq	$136, %rsp
retq

Perhaps I am missing something and there's a valid reason for this, but it seems to me that we could use this sequence once and save 40+ bytes of instructions. Not sure if it's occuring anywhere else.
@ianlancetaylor
Copy link
Contributor

If you look at the line table information, you'll see that there are different line numbers associated with each of those instruction blocks. There are three of them because there are three different return true statements in the function. It's not possible for a Go compiler to merge instruction sequences in the general case, because that will cause a panic to show the wrong line number. This particular instruction sequence probably can't panic, but a signal is still a possibility. It's not clear to me whether these instructions can be merged or not.

@randall77
Copy link
Contributor

See #24936 (#24936 (comment))
I had a CL to do something like this, never really got around to completing it https://go-review.googlesource.com/c/go/+/265100

@ianlancetaylor
Copy link
Contributor

Thanks, closing as dup of #24936.

@golang golang locked and limited conversation to collaborators Oct 26, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@ianlancetaylor @randall77 @gopherbot @jake-ciolek