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

cmd/compile: make argument size of -S output more accurate #29334

Open
mmcloughlin opened this issue Dec 19, 2018 · 4 comments
Open

cmd/compile: make argument size of -S output more accurate #29334

mmcloughlin opened this issue Dec 19, 2018 · 4 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@mmcloughlin
Copy link
Contributor

mmcloughlin commented Dec 19, 2018

I would like to seek clarification about the correct argument size for assembly functions. Specifically, asmdecl requires different sizes from those reported by go tool compile -S. See the following gist for an example:

https://gist.github.com/mmcloughlin/598774c255f40605e613ffb3361e240c

I'll copy the main points here. The function implemented is:

package argsize

func Index3(a [7]byte) byte

func Expect(a [7]byte) byte { return a[3] }

with assembly

TEXT ·Index3(SB),0,$0-16
	MOVB	a_3+3(FP), AL
	MOVB	AL, ret+8(FP)
	RET

go tool compile -S reports argument size 16

"".Expect STEXT nosplit size=10 args=0x10 locals=0x0
	0x0000 00000 (argsize.go:6)	TEXT	"".Expect(SB), NOSPLIT|ABIInternal, $0-16

while asmdecl requires size 9

./argsize.s:1:1: [amd64] Index3: wrong argument size 16; expected $...-9

Is this behaving as intended?

cc @alandonovan @josharian

@gopherbot gopherbot added this to the Unreleased milestone Dec 19, 2018
@mmcloughlin
Copy link
Contributor Author

I guess the question is whether this code block requires an additional alignment (to arch.maxAlign) before setting fn.size.

https://github.com/golang/tools/blob/88e3b261f28032aa04435fc7aeae5d27d26bf777/go/analysis/passes/asmdecl/asmdecl.go#L578-L583

@cherrymui
Copy link
Member

I think 9 is technically the more correct answer. The compiler's -S output rounds it up, which probably doesn't really matter in practice. I guess we could change the compiler's -S output, if you think it would be good.

@agnivade agnivade changed the title x/tools/go/analysis/passes/asmdecl: is required argument size correct? cmd/compile: make argument size of -S output more accurate Jan 7, 2019
@agnivade agnivade added the NeedsFix The path to resolution is known, but the work has not been done. label Jan 7, 2019
@agnivade agnivade modified the milestones: Unreleased, Unplanned Jan 7, 2019
@agnivade
Copy link
Contributor

agnivade commented Jan 7, 2019

Re-titled appropriately.

@mmcloughlin
Copy link
Contributor Author

This issue seems like a special case of #29538

@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jul 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

4 participants