-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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: convert some const shifts into load offsets #19286
Comments
Can all the chips we care about do store-to-load forwarding with mismatched sizes? Might be better to keep this out of the generic rules unless we are certain that they all can. |
This come up often? |
Good point. I'm not even sure that amd64 handles this well. Moving to unplanned for now.
Haven't investigated. |
Also, each additional rule incurs a cost - and it's not clear the cost here (extra check for this case at compile-time) will outweigh the benefit. |
I've prototyped this as a target-specific rule, it triggers once during compilation, and provides no performance benefit for simple benchmark (calling top in a loop). I think this is not worth implementing. |
Thanks for running it down, @TocarIP. Closing. |
Not proposing to reopen this, but FWIW, this does occur in the hashmap implementation. Pulling the repeated tophash calculation into its own function for demo purposes: func tophash(hash uintptr) uint8 {
top := uint8(hash >> (sys.PtrSize*8 - 8))
if top < minTopHash {
top += minTopHash
}
return top
} compiles to:
The MOVQ/SHRQ at the beginning could just be a MOVB. |
Currently compiles to:
But instead of shifting, the load offset could be adjusted:
Probably could even be done in the generic rules.
cc @randall77 @mundaym @martisch @TocarIP
The text was updated successfully, but these errors were encountered: