-
Notifications
You must be signed in to change notification settings - Fork 18k
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: missed opportunity to inline runtime.memmove #41662
Comments
Is the issue still actual? Let me take it. |
Here's a sample of a similar optimization where small memequals calls are optimized. |
@Kels9009 great! Ask here if you have questions. Please be sure to add a codegen test (see test/codegen). |
@dr2chase @josharian OK, thanks for useful information. |
@kels-ng A note for the future: when pasting text, please just paste plain text, not an image. At least for me your images are nearly illegible, whereas everyone using this issue tracker can read plain text. Thanks. |
Thanks, @kels-ng. We should do option (1). New rules are pretty cheap. There are a fair number of rules duplicated in generic and lowered opt, for similar reasons. We don't want to introduce new opt passes without a very compelling need, and moving around existing passes is risky. |
Change https://golang.org/cl/289151 mentions this issue: |
Change https://golang.org/cl/352054 mentions this issue: |
Add rule to PPC64.rules to inline runtime.memmove in more cases, as is done for other target architectures Updated tests in codegen/copy.go to verify changes are done on ppc64/ppc64le Updates #41662 Change-Id: Id937ce21f9b4f4047b3e66dfa3c960128ee16a2a Reviewed-on: https://go-review.googlesource.com/c/go/+/352054 Run-TryBot: Lynn Boger <laboger@linux.vnet.ibm.com> TryBot-Result: Go Bot <gobot@golang.org> Reviewed-by: Cherry Mui <cherryyz@google.com> Trust: Lynn Boger <laboger@linux.vnet.ibm.com>
This should compile down to a pair of MOVQs, one to load from x and one to write to b. It doesn't; it still contains a call to runtime.memmove. From a quick glance at ssa.html, the problem is that the lowering of runtime.memmove to Move happens during generic.rules, but we haven't detected that the size of the memmove is a constant until after lowering.
To fix this, we could either improve the analysis in the generic stages or add arch-specific runtime.memmove-to-Move lowering.
cc @randall77 @dr2chase @martisch @mundaym
The text was updated successfully, but these errors were encountered: