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

llvm_asm! removed in latest nightly rust #91

Closed
Outurnate opened this issue Jan 23, 2022 · 8 comments · Fixed by #97
Closed

llvm_asm! removed in latest nightly rust #91

Outurnate opened this issue Jan 23, 2022 · 8 comments · Fixed by #97

Comments

@Outurnate
Copy link

rust-lang/rust#92816 removes the llvm_asm! macro. Since avr_device still uses this macro, the last version that can build this crate is nightly-2022-01-17

asm! is the supported replacement

@Outurnate
Copy link
Author

Of course, rust-lang/compiler-builtins#400 blocks from using anything newer than nightly-2021-01-07, so this is probably not an immediate concern. Not until LLVM closes D114611 (which appears to have recent activity)

@Rahix
Copy link
Owner

Rahix commented Jan 23, 2022

Yeah, for the time being, I would rather keep support for the latest working nightly (=2021-01-07) until the issue you mentioned gets fixed. Otherwise using this project will get even harder because people will need a custom compiler...

That said, if someone wants to build a system which detects the compiler version and switches up the used macros as needed, I'd be okay with merging that. I've seen similar code in anyhow: https://github.com/dtolnay/anyhow/blob/master/build.rs

@Outurnate
Copy link
Author

I may take a stab at that. It looks like anyhow compiles a test program to probe the compiler for support. Given the number of constraints already placed on the compiler version (must be nightly to support "build-std", must =2021-01-07), maybe it's a better developer experience to pull in rustc_version in build.rs and provide some well-written error messages? i.e. if channel != nightly, print a note regarding build-std and kindly ask the user to switch branches. If version > 2021-01-07, link the github issue and kindly ask the user to change. A similar version check (>2022-01-17) could be used to enable a feature on the crate to switch macros?

Let me know your thoughts

@Rahix
Copy link
Owner

Rahix commented Jan 23, 2022

rustc_version in build.rs and provide some well-written error messages?

Sounds good. This here might be even better for our usecase: https://github.com/dtolnay/rustversion

i.e. if channel != nightly, print a note regarding build-std

hm, I wouldn't go that far. Maybe someone in the future wants to use a stable compiler with the necessary support to compile older versions of this crate - we should not put anything into place which would prevent that.

I'd say the error message you'll see from the compiler when build-std is not supported is enough here.

If version > 2021-01-07, link the github issue and kindly ask the user to change.

This will hurt people with patched compilers built from later versions. So I'd rather not error out on it either.

A similar version check (>2022-01-17) could be used to enable a feature on the crate to switch macros?

+1

@Outurnate
Copy link
Author

rustversion seems to be absolutely the closest to what we need. I was looking at autocfg, but it wouldn't support this usecase without cuviper/autocfg#35 merged.

The build-std messaging probably is good enough, you are correct

I would prefer to use probe based detection as well wherever possible. I wonder if it's worth trying to embed a minimum reproducible LLVM source for rust-lang/compiler-builtins#400 ? Check if the compiler can build it rather than relying on dumb version checks

@Outurnate
Copy link
Author

outurnate/avr-device@1cef9a1 is a first pass. Still having some trouble setting up my build environment so I've not tested it yet. According to the rust unstable docs, asm! isn't supported on AVR yet, so that's a separate issue. I think I have the register names/types correct - I'm a bit iffy on this one:

asm!("in {},0x3F", out(reg) sreg, options(nostack))

Thoughts?

@Rahix
Copy link
Owner

Rahix commented Jan 28, 2022

Doesn't look bad, but to be quite honest, I don't really have any experience with the new asm!() macro beyond having skimmed the RFC when it appeared... In the end, the best way forward is to look at the generated assembly here.

@Outurnate
Copy link
Author

Contact with the real world had different opinions. rustversion can't enable crate attributes, so I had to switch to using rustc_version for now. It's already in the dependency tree. It doesn't surface enough information in my opinion. Will probably take another look at autocfg on the next pass. I did build a toolchain with LLVM patched (based off the latest nightly) but I think I made a transcription error when resolving the merge conflicts. rustc SIGSEGVs when building compiler-builtins (so it's definitely hitting the patched path). Once I get a working toolchain, I'll emit LLVM IR for both the old nightly and the patched nightly. Should be a simple comparison

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