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

Support Linker Plugin to inline ASM #139

Closed
jamesmunns opened this issue Apr 11, 2019 · 4 comments · Fixed by #259
Closed

Support Linker Plugin to inline ASM #139

jamesmunns opened this issue Apr 11, 2019 · 4 comments · Fixed by #259

Comments

@jamesmunns
Copy link
Member

See https://github.com/Disasm/inline-test and https://doc.rust-lang.org/nightly/rustc/linker-plugin-lto.html

This should be stable shortly in Rust 1.34.0

CC @Disasm and @pftbest

@jamesmunns
Copy link
Member Author

Also CC @rust-embedded/cortex-m

@jamesmunns
Copy link
Member Author

jamesmunns commented Apr 11, 2019

As a note, I think this would entail the following steps:

  • Move the contents of asm*.s into files called asm*.c, marking each C function as __inline__(always)
  • Wrap the current assembly into explicit C functions, removing the return calls (e.g. bx lr), and respecting the C calling convention for functions that take arguments, such as __psp_w's use of r0
  • Modify the assemble.sh function to match inline-test's behavior using clang to build with -flto=thin. We must also use Clang7 or Clang8 to compile these C files, rather than the arm-none-eabi-* suite.
  • Add documentation to instruct users to add "-C", "linker-plugin-lto" to their .cargo/config's rustflags array.

As far as I know, these changes should be backwards compatible, and should (probably?) not cause regressions by switching from ASM to ASM-in-C. EDIT: See my next comment. I think this could cause breaking changes for some users, though not all. This is probably worth verifying. If it does NOT, I would suggest that we make this a non-breaking change. If it DOES, I would suggest that we add this as a new feature that can be opted-in to, typically only used when the user has added support for linker-plugin-lto

@jamesmunns
Copy link
Member Author

jamesmunns commented Apr 11, 2019

Follow up: I don't think this could be done without a new feature/breaking change. The pre-generated static archives would include LLVM bitcode, which would likely work with any version of rust based on LLVM7 or LLVM8 (I'm not sure if this includes Rust 1.32, which I think is our current MSRV), but this would almost certainly break any projects still using arm-none-eabi-ld.

EDIT: Rust 1.32.0 release is based on LLVM8.

I'd suggest that we make this an opt-in feature, and ship both sets of static archives for now.

bors added a commit to rust-lang/rust that referenced this issue Jul 9, 2019
Update LLVM: apply patch necessary for ThinLTO on RISC-V

This patch allows [using inlined assembly operations](rust-embedded/cortex-m#139) on stable Rust with the help of ThinLTO.
@Disasm
Copy link
Member

Disasm commented Feb 8, 2020

Good news: binary archives compiled with bitcode object files work "as is" in our usual build scenario (without ThinLTO and even LTO).
Bad news: this works only on the specific range of Rust versions compatible with this bitcode.
Another issue is that you never know the last Rust version with the compatible LLVM built-in, so you can't check for the upper supported Rust version in build.rs.

I suggest adding an unstable lto feature that will enable these LTO-ready binary archives instead of the standard ones. This way a user could enable ThinLTO for a specific environment.

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 a pull request may close this issue.

2 participants