go
go copied to clipboard
cmd/compile: Instruction split on amd64
What version of Go are you using (go version
)?
Latest version.
I just want to confirm that this is intended behavior.
Does this issue reproduce with the latest release?
Yes.
What did you do?
I was investigating assembly output from various variations and noticed some peculiar output with go tip amd64 compiler:
Code in question: bits.TrailingZeros64(a^b) >= 51
Godbolt output: https://godbolt.org/z/z1jfE85ax
Playground link of the same code: https://go.dev/play/p/UpuJvUnS1Lm
What did you expect to see?
XORQ CX, AX
BSFQ AX, CX
MOVL $64, DX
CMOVQEQ DX, CX
CMPQ CX, $51
SETGE CL
What did you see instead?
XORQ CX, AX
BSFQ AX, CX
[code not modifying AX or CX, but modifying flags]
BSFQ AX, DX
MOVL $64, DX
CMOVQEQ DX, CX
CMPQ CX, $51
SETGE CL
My main objection is that is re-executes BSF.
It seems to calculate the result on CX, and then later it re-computes to get the flag for the CMOV. If code is simplified (by removing call to A or observing the not-inlined function), this split goes away.
On Intel this is a rather expensive way to check if AX is zero. If this is a more common pattern it seems like it should be avoided. I know I cannot expect perfect assembly, but this seems a bit odd to me.
cc @golang/compiler , though questions might be better suited for golang-dev.
Only see two BSFQ instructions with go version devel go1.20-b726b0cadb Fri Oct 28 23:35:08 2022 +0000 linux/amd64
One is in B() and the other is from B() being in-lined into main().
I find go tip in compiler explorer is not go tip. I add -goversion in compiler options, only to see: compile: version "devel go1.18-088bb4b Tue Nov 2 06:25:39 2021 +0000" does not match go tool version "-nolocalimports"
And I can't reproduce with current go tip(go version devel go1.20-3c17053bba Sat Oct 29 00:40:18 2022 +0000 linux/amd64)
@wdvxdr1123 I can reproduce locally on 1.19.2 - you must check the main() function where it is inlined:
0x00ad 00173 (d:\temp\2\main.go:18) XORQ CX, AX
0x00b0 00176 (d:\temp\2\main.go:22) BSFQ AX, CX
0x00b4 00180 (d:\temp\2\main.go:12) XCHGL AX, AX
0x00b5 00181 (d:\temp\2\main.go:13) XCHGL AX, AX
0x00b6 00182 (d:\temp\2\main.go:14) MOVUPS X15, main..autotmp_35+56(SP)
0x00bc 00188 (d:\temp\2\main.go:14) MOVUPS X15, main..autotmp_35+72(SP)
0x00c2 00194 (d:\temp\2\main.go:14) LEAQ type.bool(SB), DX
0x00c9 00201 (d:\temp\2\main.go:14) MOVQ DX, main..autotmp_35+56(SP)
0x00ce 00206 (d:\temp\2\main.go:18) MOVQ $1125899906842623, BX
0x00d8 00216 (d:\temp\2\main.go:18) TESTQ AX, BX
0x00db 00219 (d:\temp\2\main.go:18) SETEQ BL
0x00de 00222 (d:\temp\2\main.go:14) MOVBLZX BL, BX
0x00e1 00225 (d:\temp\2\main.go:14) LEAQ runtime.staticuint64s(SB), SI
0x00e8 00232 (d:\temp\2\main.go:14) LEAQ (SI)(BX*8), BX
0x00ec 00236 (d:\temp\2\main.go:14) MOVQ BX, main..autotmp_35+64(SP)
0x00f1 00241 (d:\temp\2\main.go:14) MOVQ DX, main..autotmp_35+72(SP)
0x00f6 00246 (d:\temp\2\main.go:22) BSFQ AX, DX
0x00fa 00250 (d:\temp\2\main.go:22) MOVL $64, DX
0x00ff 00255 (d:\temp\2\main.go:22) CMOVQEQ DX, CX
0x0103 00259 (d:\temp\2\main.go:22) CMPQ CX, $51
0x0107 00263 (d:\temp\2\main.go:22) SETGE CL
0x010a 00266 (d:\temp\2\main.go:14) MOVBLZX CL, CX
Output above is produced with:
λ go version
go version go1.19.2 windows/amd64
λ go build -asmflags="-D=GOAMD64_v3 -S" -gcflags=-S . 2>package.asm
@klauspost Yes, I can reproduce with go 1.19.2. I think this issue has been fixed in 669ec549b554ef7fe957bf284271ed3b7db82da1. I revert this commit in my local branch and got the bad assembly output. See comment from @randall77 (https://go-review.googlesource.com/c/go/+/432275/comment/30b66b90_67cd39ef/), I think this commit does improve codegen with flags.
If it is gone in tip we can just close it. Thanks for taking the time to investigate.