RFC: Merging the avr-rust fork upstream

[dylanmckay]

Hello all,

I would like to know the general opinions on merging the avr-rust fork into Rust proper.

The fork itself has became a lot more stable with less bugs in the recent months. It has also started attracting a number of people interested in using it.

You can find one of the more interesting projects using avr-rust on GitHub.

Blockers

LLVM 5.0

Rust is currently on LLVM 4.0, which contains a working AVR backend, but there have been many bugfixes since then. We would need to wait for LLVM 5.0 to be supported (nearly finished: #43370) before we get a version of the AVR backend that has a few important bugs fixed.

This is no longer a blocker. Upstream Rust is at LLVM 6.0 as of 2018-02-20.

Questions

Cherry-picking fixes

If AVR support was built into mainline, we'd need to be able to cherry-pick patches into Rust's LLVM fork. I don't imagine this would be much of a problem, as we already cherry-pick a number of important fixes into there.

All of the bugfixes cherry-picked into the avr-rust repository have already been upstreamed into LLVM trunk, which would continue to be the case if we merged the fork, as I am not a fan of the LLVM fork diverging too far from trunk.

Cherry-picking is necessary because of LLVM's 6-month release cycle.

Current issues in the AVR backend

There aren't any known bugs in the avr-rust/rust repository - all of the known bugs are issues in the AVR LLVM backend; here are some of the more interesting/impactful ones.

libcore cannot be compiled without modification

There is a milestone set up to track what bugs need to be fixed in order to get libcore compiling successfully without modification.

This hasn't been much of a problem for users so far, as xargo will transparently compile libcore on the fly when needed, and we can override libcore in Xargo.toml.

I am unsure what the Rust team thinks of merging a target which can't use the stock libcore.

Any operations on function pointers other than 'call' access RAM, not program memory (avr-rust#68)

This is a symptom of AVR being the very first in-tree LLVM backend for a Harvard architecture. LLVM currently assumes that all functions reside in "the generic address space", which corresponds to RAM. Because of this, if you attempt to load/store through a function pointer, it will access RAM instead of the program memory.

Good news is that I have pending upstream LLVM patches to fix it (D37052, D37053, D37054, D37057).

32-bit shifts generate calls to a compiler-rt routine that doesn't exist (avr-llvm/llvm#163)

Because there aren't many (if any) targets that don't support 32-bit shifts natively, libgcc and compiler-rt do not have 32-bit versions of shift routine, even though LLVM still happily generates a call to it.

This causes an undefined symbol error whilst linking. This will only happen if you actually write code or use code that performs 32-bit shifts, as the linker is quite good at removing all dead code.

Note that I've had one user hit the missing shift bug due to compiling in release mode, which promoted a multiplication to a shift as an "optimisation".

Actual changes to merge

You can find all AVR-specific differences by looking at this diff.

Note that over half of that diff is just the README and Travis CI configuration - the actual code being upstreamed is very small; just some glue code to enable the AVR backend and a target specification.

This diff also conditionally disables parts of libcore for AVR - these fixes would not upstreamed, and are not strictly required as downstream users can use Xargo to compile a minified libcore for AVR).

Links

AVR-Rust on Gitter
AVR-Rust on GitHub

[Restioson]

+1! Avr rust built into the compiler proper would be super useful. It's almost free of bugs now.

[dylanmckay]

Not completely free of bugs :)

I will update the description to include some information about the state of the backend w.r.t bugs

[Restioson]

Almost though 😄. Just a couple to go

[alexcrichton]

In general we're quite welcoming of platforms upstream in rust-lang/rust so long as they don't place a maintenance burden on us! Some specific thoughts from what you're thinking:

• Cherry-picking LLVM commits every so often is totally OK, I think y'all've gone through the process a few times already as well :)
• Missing intrinsics in compiler-rt is fine by us, you have the option of implementing them in Rust as well through the compiler-builtins project.
• The patch you have looks pretty good, although I think we'd want to work more through the various #[cfg] in libcore. Are these items left out due to "bugs in LLVM" or because they're fundamentally not supported at all on AVR? The former would be best done by "fixing LLVM" somehow whereas the latter makes things more tricky.

In general we currently have all platforms with the same uniform interface of libcore/libstd, but it's not clear that'll remain true into as we keep picking up more platforms.

[dylanmckay]

The patch you have looks pretty good, although I think we'd want to work more through the various #[cfg] in libcore. Are these items left out due to "bugs in LLVM" or because they're fundamentally not supported at all on AVR?

It is because of bugs in LLVM.

The only question in my mind is on support of i128/u128 types - I think that for AVR, these aren't really useful. The i128 support in libcore is currently commented out because of a bug, but there may be other undiscovered bugs as it is not a well tested codepath, and really exercises the register allocator as the AVR only has 32 bytes worth of general purpose registers.

It is still quite likely that we could get i128 working on AVR though, I don't know too much of the specific problems it triggers in the backend.

I think that the best way forward would be to propose the patch for real once libcore does compile without modification, or at least without many.

[alexcrichton]

I think that the best way forward would be to propose the patch for real once libcore does compile without modification, or at least without many.

Sounds reasonable to me!

[est31]

The only question in my mind is on support of i128/u128 types - I think that for AVR, these aren't really useful.

My main fear of not supporting i128 on AVR is that it will have a chilling effect on adoption of 128 bit integers for the sake of compatibility with AVR or other embedded platforms. E.g. if there is a library that uses 128 bit integers, AVR users that will want to use it will file issues to drop the usage. This might make 128 bit integers not "safe" to use in Rust code that strives to be portable, which I wouldn't like to happen.

[shepmaster]

a chilling effect on adoption of 128 bit integers for the sake of compatibility with AVR or other embedded platforms

I don't think that it's a terribly huge deal here. Small embedded devices already have giant limitations (no stdin / stdout, generally no allocator, etc.) that make it real hard to just drop in an arbitrary library. I recently learned that AVR GCC's double is actually an alias for float! I don't know if we are going to keep that strangeness or not, but it would affect crates way more than i128.

I think we are always going to have special features that are used to make a crate suitable for embedded, just like we do for no-std.

[est31]

no stdin / stdout, generally no allocator, etc.

You are describing the #![no_std] ecosystem. There are libraries that target this ecosystem. And the general rule for that ecosystem is to take libcore as a given, which also includes i128. Each target that doesn't support i128 has a larger chilling effect inside this ecosystem, because an embedded target's "market share" is bigger inside the subset of the entire Rust ecosystem where the x86 family is not a very relevant player.

I don't know if we are going to keep that strangeness or not, but it would affect crates way more than i128.

Interesting! I do agree that if we'd alias f64 to f32 (or not provide it), it would affect the ecosystem more. However, if we can strive for consistency, why shouldn't we? It is definitely possible for us to implement i128.

[shepmaster]

It is definitely possible for us to implement i128.

Absolutely, and I realize I didn't clearly state that I think we should implement i128 for AVR. However, any code that actually uses an i128 on an AVR is going to be in for a world of pain.

However, if we can strive for consistency, why shouldn't we?

Consistency with what, is the question. Consistency with GCC (f64 == f32) or with every other Rust target (f64 != f32)?

You are describing the #![no_std] ecosystem.

Yep, which is why I said "special features that are used to make a crate suitable for embedded, just like we do for no-std." 😇

[shepmaster]

A larger problem that's been in the back of my mind since we originally landed the 16-bit usize patch is that, fundamentally, Rust and Rust programmers tend to assume that usize is the "native" size of a register. AFAIK, this is true for all the other platforms Rust targets, but not for AVR.