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

Use weak linkage for aeabi memory functions #385

Merged
merged 1 commit into from Oct 14, 2020

Conversation

pca006132
Copy link
Contributor

This is the PR for issue #378. Copy pasting from the issue:

I am developing for armv7-none-eabihf targets, and I hope that the aeabi memory functions can be declared as weak symbols so I can replace them with other implementations that are more efficient.

Currently the functions __aeabi_memcpy*, __aeabi_memmove*, __aeabi_memset*, __aeabi_memclr* defined in arm.rs are exported as weak symbols only on thumb mode. There is no way to override the implementation for other arm targets even if we turn off the mem feature, as the implementation would refer to the functions defined in mem.rs. The only workaround I currently know of is to maintain a fork of the compiler_builtins (and patch cargo-xbuild so it would use the forked version).

The implementation here is indeed slower than the optimized version found in other libraries. For example, the newlib implementation of memcpy for armv7a which uses SIMD and other techniques. In our bare-metal project (artiq zynq port) which does quite a lot of copying, we got significant performance improvement for some workload by replacing the memcpy routine with the newlib one (at least doubled the ethernet throughput). I think this proves that there are use cases where we want to replace the implementation.

It seems that in PR #164, the author originally marked all aeabi memory functions as weak symbols, but later changed to thumb targets only (b8a6620), I am not sure about the reason for that.

This PR changes those memory functions to weak symbols, so users can override them if they want for specific purpose or better performance.

For example, in our project https://git.m-labs.hk/pca006132/artiq-zynq/src/branch/test, we overrided the aeabi_memcpy* functions with an implementation from newlib which uses NEON instructions for faster memcpy.

@alexcrichton
Copy link
Member

Thanks for this! Can you add a comment to the top as well?

Also, to be clear, I think it'd be great to land more optimized implementations in this repository. This certainly reduces the likelihood of a clash, but it'd be great if you didn't have to bring along your own memcpy in the first place!

@pca006132
Copy link
Contributor Author

Thanks for this! Can you add a comment to the top as well?

Also, to be clear, I think it'd be great to land more optimized implementations in this repository. This certainly reduces the likelihood of a clash, but it'd be great if you didn't have to bring along your own memcpy in the first place!

Comment added.

It would be great to have more optimized defaults, but in embedded systems, there may be some weird use cases. I think giving some flexibility to the users would still be usefu even if we get more optimized defaults later. (maintaining our own patched compiler builtins is a bit painful, see https://git.m-labs.hk/M-Labs/artiq-zynq/issues/113)

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

Successfully merging this pull request may close these issues.

None yet

2 participants