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: suboptimal bits.Rotate* on arm64 #48465

Closed
greatroar opened this issue Sep 19, 2021 · 1 comment
Closed

cmd/compile: suboptimal bits.Rotate* on arm64 #48465

greatroar opened this issue Sep 19, 2021 · 1 comment

Comments

@greatroar
Copy link

@greatroar greatroar commented Sep 19, 2021

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

$ go version
go version go1.17.1 linux/amd64

Does this issue reproduce with the latest release?

Reproduces with

$ gotip version
go version devel go1.18-771b8ea4f4c Sun Sep 19 02:43:09 2021 +0000 linux/amd64

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

GOARCH=arm64

What did you do?

func rotxor1(x, y uint64) uint64 {
	y = (y << 7) | (y >> 57)
	return x ^ y
}

func rotxor2(x, y uint64) uint64 {
	return x ^ bits.RotateLeft64(y, 7)
}

What did you expect to see?

Would have been nice to see a fused rotate-xor (#48002), but at least the generated asm should be the same.

What did you see instead?

For rotxor1:

MOVD	"".y+8(FP), R0
ROR	$57, R0, R0
MOVD	"".x(FP), R1
EOR	R0, R1, R0

For rotxor2:

MOVD	"".y+8(FP), R0
MOVD	$-7, R1
ROR	R1, R0, R0
MOVD	"".x(FP), R1
EOR	R0, R1, R0

The compiler inserts a mov and allocates a register for the shift constant when the intrinsic is used. This is a regression introduced in Go 1.12. It's not limited to these tiny functions: I spotted this when inspecting the generated asm for SipHash.

@greatroar greatroar changed the title Suboptimal rotate+xor on arm64, worse with bits.Rotate* Suboptimal bits.Rotate* on arm64 Sep 19, 2021
@ALTree ALTree changed the title Suboptimal bits.Rotate* on arm64 cmd/compile: suboptimal bits.Rotate* on arm64 Sep 19, 2021
@gopherbot
Copy link

@gopherbot gopherbot commented Sep 19, 2021

Change https://golang.org/cl/350909 mentions this issue: cmd/compile: implement constant rotates on arm64

Loading

@gopherbot gopherbot closed this in 83b36ff Sep 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants