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

Weak linkage for aeabi memory functions #378

Closed
pca006132 opened this issue Sep 29, 2020 · 5 comments
Closed

Weak linkage for aeabi memory functions #378

pca006132 opened this issue Sep 29, 2020 · 5 comments

Comments

@pca006132
Copy link
Contributor

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.

@josephlr
Copy link
Contributor

josephlr commented Oct 6, 2020

@japaric has a comment that explains why things were done this way: #164 (comment)

@pca006132
Copy link
Contributor Author

@japaric has a comment that explains why things were done this way: #164 (comment)

Yes I've read that, what he means is weak linkage is needed for thumb targets to compile, and normal linkage is enough for other targets to compile. What I don't understand is why he would change from using weak linkage for all arm targets, into restricting weak linkage to thumb targets... IDK if using weak linkage everywhere would cause problems or something, I am not experienced in this kind of things.

Not sure if I should just open a PR to change the functions to weak linkage, and let others review/run on CI...

@josephlr
Copy link
Contributor

josephlr commented Oct 6, 2020

Not sure if I should just open a PR to change the functions to weak linkage, and let others review/run on CI...

That seems reasonable to me

@lifning
Copy link

lifning commented Dec 29, 2020

When can this be expected to hit nightly? I see there was a version bump last month, but I'm still evidently getting a version without weak linkage from rustup.

@bjorn3
Copy link
Member

bjorn3 commented Dec 29, 2020

rust-lang/rust#79863 will update compiler-builtins, but it is currently blocked on a bug.

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

No branches or pull requests

4 participants