Rahix / avr-device

Register access crate for AVR microcontrollers
Apache License 2.0
176 stars 67 forks source link

llvm_asm! removed in latest nightly rust #91

Closed Outurnate closed 2 years ago

Outurnate commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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