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: rotate instructions are not always emitted when they could be #18254

Closed
dgryski opened this issue Dec 8, 2016 · 7 comments
Closed

Comments

@dgryski
Copy link
Contributor

dgryski commented Dec 8, 2016

go version devel +41908a5453 Thu Dec 1 02:54:21 2016 +0000 linux/amd64
(1.8beta1)

What did you do?

https://play.golang.org/p/eyIlsgnmc4

What did you expect to see?

Both of the rotate functions can be inlined. For rot7, a ROLL instruction is generated:

ROLL    $7, AX

However, the constant propagation is incomplete for rot:

MOVQ    AX, CX
SHLL    $7, AX
SHRL    $25, CX
ORL     CX, AX

Rotate instructions are common in crypto code and hashing algorithms. Here's an example of a significant speedup by forcing the compiler to emit the ROLQ:
cespare/xxhash@4a66a12

@bradfitz bradfitz added this to the Unplanned milestone Dec 8, 2016
@bradfitz
Copy link
Contributor

bradfitz commented Dec 8, 2016

/cc @randall77

@randall77
Copy link
Contributor

This issue is a side effect of doing rotate detection in walk instead of the SSA rewriter.
We just need to put rotate-generation rules in the SSA rewriter and remove them from walk.
Then we can retire the OLROT opcode. And for that matter, all of walkrotate.

@randall77 randall77 self-assigned this Dec 8, 2016
@randall77 randall77 modified the milestones: Go1.9Early, Unplanned Dec 8, 2016
@dgryski
Copy link
Contributor Author

dgryski commented Dec 9, 2016

I might try to tackle this.

@randall77
Copy link
Contributor

See https://go-review.googlesource.com/#/c/34232/

@gopherbot
Copy link
Contributor

CL https://golang.org/cl/34232 mentions this issue.

@rasky
Copy link
Member

rasky commented Jan 8, 2017

This function does not generate a rotation, even after CL 34232 is applied:

func testRotate(op uint32) uint32 {
	rot := uint((op >> 7) & 0x1E)
	return ((op & 0xFF) >> rot) | ((op & 0xFF) << (32 - rot))
}

Should I open a different issue?

@randall77
Copy link
Contributor

@rasky Yes, if you could please open a different issue.
This issue is about rotates by a constant amount.
Your issue is rotates by a non-constant amount.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants