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
Labels
Milestone

Comments

@mmcloughlin
Copy link
Contributor

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

This comment has been minimized.

Copy link
Contributor Author

@mmcloughlin mmcloughlin commented Dec 19, 2018

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

This comment has been minimized.

Copy link
Contributor

@cherrymui cherrymui commented Dec 21, 2018

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 label Jan 7, 2019
@agnivade agnivade modified the milestones: Unreleased, Unplanned Jan 7, 2019
@agnivade

This comment has been minimized.

Copy link
Contributor

@agnivade agnivade commented Jan 7, 2019

Re-titled appropriately.

@mmcloughlin

This comment has been minimized.

Copy link
Contributor Author

@mmcloughlin mmcloughlin commented Jan 7, 2019

This issue seems like a special case of #29538

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.