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

Issue compiling for ESP32-C3 (AtomicUsize used in std_detect) #3

Closed
igrr opened this issue Jun 11, 2021 · 14 comments
Closed

Issue compiling for ESP32-C3 (AtomicUsize used in std_detect) #3

igrr opened this issue Jun 11, 2021 · 14 comments

Comments

@igrr
Copy link
Member

igrr commented Jun 11, 2021

With stable or beta Rust toolchains, I get the following error when running cargo build --release:

error[E0463]: can't find crate for `std`
  |
  = note: the `riscv32i-unknown-none-elf` target may not be installed

error: aborting due to previous error

For more information about this error, try `rustc --explain E0463`.
error: could not compile `rustlib`
`rustup show` output:
Default host: x86_64-apple-darwin
rustup home:  /Users/ivan/.rustup

installed toolchains
--------------------

stable-x86_64-apple-darwin (default)
beta-x86_64-apple-darwin
nightly-x86_64-apple-darwin

installed targets for active toolchain
--------------------------------------

riscv32i-unknown-none-elf
x86_64-apple-darwin

active toolchain
----------------

stable-x86_64-apple-darwin (default)
rustc 1.48.0 (7eac88abb 2020-11-16)

Switching to nightly channel, as indicated in quick start for Xtensa, and installing riscv32i-unknown-none-elf target, i get a different error:

   Compiling rustlib v0.1.0 (/Users/ivan/e/rust-esp32-example/rustlib)
error[E0432]: unresolved import `core::sync::atomic::AtomicUsize`
 --> /Users/ivan/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/stdarch/crates/std_detect/src/detect/cache.rs:8:5
  |
8 | use core::sync::atomic::AtomicUsize;
  |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no `AtomicUsize` in `sync::atomic`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0432`.
error: could not compile `std_detect`
`rustup show` output:
Default host: x86_64-apple-darwin
rustup home:  /Users/ivan/.rustup

installed toolchains
--------------------

stable-x86_64-apple-darwin
beta-x86_64-apple-darwin
nightly-x86_64-apple-darwin (default)

installed targets for active toolchain
--------------------------------------

riscv32i-unknown-none-elf
riscv32imc-unknown-none-elf
x86_64-apple-darwin

active toolchain
----------------

nightly-x86_64-apple-darwin (default)
rustc 1.54.0-nightly (16e18395c 2021-06-10)

The failure makes sense, since RV32I or RV32IMC don't support atomic instructions.

This looks like rust-lang/rust#62269, except this time in std_detect crate. I guess this is something that needs to be reported in https://github.com/rust-lang/stdarch/blob/82106e84345ee25366cd8a49a228d69fde795047/crates/std_detect/src/detect/cache.rs#L8?

@igrr igrr changed the title Issue compiling for ESP32-C3 Issue compiling for ESP32-C3 (AtomicUsize used in std_detect) Jun 11, 2021
@igrr
Copy link
Member Author

igrr commented Jun 11, 2021

The other thing is, I couldn't find where the std_detect dependency comes from. cargo tree doesn't show it in the list.

@MabezDev
Copy link
Collaborator

MabezDev commented Jun 11, 2021

I think this is an upstream bug, as it assumes all targets building the standard library have atomic support. I've filed an issue upstream.

std_detect is a dep of the standard library. As for why this doesn't show up with cargo tree is because of a hack in the way most rust code is built; most of the core crates, alloc, core std etc and passed to the compiler via --extern instead of the usual cargo method.

The next release of the rust-xtensa fork will fully support using the cargo build-std feature which should make cargo tree show std deps.

@MabezDev
Copy link
Collaborator

Actually it turns out the whole std relies on atomics: rust-lang/stdarch#1181 (comment). Correct me if I'm wrong but the esp32c3 only have the imc extensions so might be a bit stuck.

It might be possible to work around it with crates such as https://github.com/embassy-rs/atomic-polyfill, though I am unsure if that sort of solution would be accepted upstream.

@igrr
Copy link
Member Author

igrr commented Jun 11, 2021

Thanks for explaining this @MabezDev! Yes, ESP32-C3 doesn't support the A extension.

We've discussed in the past about adding an Illegal Instruction exception handler, and emulating "A" extension instructions in software. Then the Rust part can be built with atomics support (rv32imac).

@igrr
Copy link
Member Author

igrr commented Jun 13, 2021

I started looking if it is possible to configure the Rust compiler to emit atomics to LLVM and let LLVM convert them to builtins, which could later be provided by the runtime. It seems this is possible.
rust-lang/rust#58500
https://docs.rust-embedded.org/embedonomicon/custom-target.html#fill-the-target-file (starting from "configure atomics").

The rv32imc target is specified to have no atomics: https://github.com/rust-lang/rust/blob/fb3ea63d9b4c3e9bb90d4250b870faaffb9c8fd2/compiler/rustc_target/src/spec/riscv32imc_unknown_none_elf.rs#L15-L16. If max_atomic_width was changed e.g. to Some(64), then we could provide the implementations of __atomic_* and __sync_* builtins to emulate the atomics. IDF already does this: https://github.com/espressif/esp-idf/blob/master/components/newlib/stdatomic.c.

max_atomic_width was set to 0 in rust-lang/rust#66548.
There was a PR to allow atomics, but it didn't get merged: rust-lang/rust#81752.

@MabezDev
Copy link
Collaborator

Interesting find, I wasn't aware of that PR, it seems to have been closed prematurely imo; it might be possible to get a similar PR merged. Regardless we can always create our own esp32c3 target definition and set max atomic width.

FYI if you would like to try this without having to rebuild the compiler you can pass the target definition as json file.

  1. Dump the current config to start from rustc -Z unstable-options --print target-spec-json --target=riscv32imc-unknown-none-elf, which should get you something like so:
{
  "arch": "riscv32",
  "atomic-cas": false,
  "cpu": "generic-rv32",
  "data-layout": "e-m:e-p:32:32-i64:64-n32-S128",
  "eh-frame-header": false,
  "emit-debug-gdb-scripts": false,
  "executables": true,
  "features": "+m,+c",
  "is-builtin": true,
  "linker": "rust-lld",
  "linker-flavor": "ld.lld",
  "llvm-target": "riscv32",
  "max-atomic-width": 0,
  "panic-strategy": "abort",
  "relocation-model": "static",
  "target-pointer-width": "32",
  "unsupported-abis": [
    "cdecl",
    "stdcall",
    "stdcall-unwind",
    "fastcall",
    "vectorcall",
    "thiscall",
    "thiscall-unwind",
    "aapcs",
    "win64",
    "sysv64",
    "ptx-kernel",
    "msp430-interrupt",
    "x86-interrupt",
    "amdgpu-kernel"
  ]
}
  1. Modify and save to a file
// esp32c3.json
{
  "arch": "riscv32",
  "atomic-cas": false,
  "cpu": "generic-rv32",
  "data-layout": "e-m:e-p:32:32-i64:64-n32-S128",
  "eh-frame-header": false,
  "emit-debug-gdb-scripts": false,
  "executables": true,
  "features": "+m,+c",
  "is-builtin": true,
  "linker": "rust-lld",
  "linker-flavor": "ld.lld",
  "llvm-target": "riscv32",
  "max-atomic-width": 64,
  "panic-strategy": "abort",
  "relocation-model": "static",
  "target-pointer-width": "32",
  "unsupported-abis": [
    "cdecl",
    "stdcall",
    "stdcall-unwind",
    "fastcall",
    "vectorcall",
    "thiscall",
    "thiscall-unwind",
    "aapcs",
    "win64",
    "sysv64",
    "ptx-kernel",
    "msp430-interrupt",
    "x86-interrupt",
    "amdgpu-kernel"
  ]
}

Then to use this target def, specify the filename as the target. E.g cargo build --target esp32c3.json --release.

@MabezDev
Copy link
Collaborator

MabezDev commented Jun 14, 2021

Interestingly, using this target file does not produce any linker errors, which I did not expect.

fn main() -> ! {
    let a = AtomicUsize::new(0);
    let v = a.load(core::sync::atomic::Ordering::SeqCst);

    unsafe {
        // don't optimize our atomics out
        let _ = core::ptr::read_volatile(&v);
    }

    loop {}
}

Emits the follwing asm:

40380082 <main>:
40380082:	1141                	addi	sp,sp,-16
40380084:	c602                	sw	zero,12(sp)
40380086:	4532                	lw	a0,12(sp)
40380088:	a001                	j	40380088 <main+0x6>

It seems llvm is smart enough to emit store and loads, as they are guaranteed to be atomic provided they are aligned, which is this case they are (I wonder if llvm forces the alignment). Whoops, that was the volatile read assembly. It turns out nothing is emitted.

@MabezDev
Copy link
Collaborator

MabezDev commented Jun 14, 2021

Going a step further, and enabling CAS in the config, I finally get the linker errors I was expecting:

error: linking with `rust-lld` failed: exit status: 1
  |
  = note: "rust-lld" "-flavor" "gnu" "-L" "/home/mabez/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/esp32c3/lib" "/home/mabez/development/rust/embedded/experiments/esp32c3-atomics/target/esp32c3/release/deps/esp32c3_atomics-d92b03ebe1d3913a.esp32c3_atomics.a7oowben-cgu.0.rcgu.o" "-o" "/home/mabez/development/rust/embedded/experiments/esp32c3-atomics/target/esp32c3/release/deps/esp32c3_atomics-d92b03ebe1d3913a" "--gc-sections" "-L" "/home/mabez/development/rust/embedded/experiments/esp32c3-atomics/target/esp32c3/release/deps" "-L" "/home/mabez/development/rust/embedded/experiments/esp32c3-atomics/target/release/deps" "-L" "/home/mabez/development/rust/embedded/experiments/esp32c3-atomics/target/esp32c3/release/build/esp32c3-atomics-9b2108d32392835a/out" "-L" "/home/mabez/development/rust/embedded/experiments/esp32c3-atomics/target/esp32c3/release/build/riscv-rt-74a9ba62e1e76916/out" "-L" "/home/mabez/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib/rustlib/esp32c3/lib" "-Bstatic" "/home/mabez/development/rust/embedded/experiments/esp32c3-atomics/target/esp32c3/release/deps/libriscv_rt-beab8a2adb2c5edd.rlib" "/home/mabez/development/rust/embedded/experiments/esp32c3-atomics/target/esp32c3/release/deps/libriscv-1426107be137617d.rlib" "/home/mabez/development/rust/embedded/experiments/esp32c3-atomics/target/esp32c3/release/deps/libbit_field-5f74332d802b4356.rlib" "/home/mabez/development/rust/embedded/experiments/esp32c3-atomics/target/esp32c3/release/deps/libbare_metal-f9134e2b930ea2a4.rlib" "/home/mabez/development/rust/embedded/experiments/esp32c3-atomics/target/esp32c3/release/deps/libr0-d777e05f6903c3e8.rlib" "--start-group" "/home/mabez/development/rust/embedded/experiments/esp32c3-atomics/target/esp32c3/release/deps/libpanic_halt-cec86e4695542601.rlib" "/home/mabez/development/rust/embedded/experiments/esp32c3-atomics/target/esp32c3/release/deps/librustc_std_workspace_core-e3595355621bb146.rlib" "/home/mabez/development/rust/embedded/experiments/esp32c3-atomics/target/esp32c3/release/deps/libcore-7ab8af7a7e03fa9c.rlib" "--end-group" "/home/mabez/development/rust/embedded/experiments/esp32c3-atomics/target/esp32c3/release/deps/libcompiler_builtins-94ed1e7bbe5329ba.rlib" "-Tesp32c3-link.x" "-Bdynamic"
  = note: rust-lld: error: undefined symbol: __atomic_load_4
          >>> referenced by esp32c3_atomics.a7oowben-cgu.0
          >>>               /home/mabez/development/rust/embedded/experiments/esp32c3-atomics/target/esp32c3/release/deps/esp32c3_atomics-d92b03ebe1d3913a.esp32c3_atomics.a7oowben-cgu.0.rcgu.o:(main)
          
          rust-lld: error: undefined symbol: __atomic_compare_exchange_4
          >>> referenced by esp32c3_atomics.a7oowben-cgu.0
          >>>               /home/mabez/development/rust/embedded/experiments/esp32c3-atomics/target/esp32c3/release/deps/esp32c3_atomics-d92b03ebe1d3913a.esp32c3_atomics.a7oowben-cgu.0.rcgu.o:(main)
          

error: aborting due to previous error

error: could not compile `esp32c3-atomics`

Hope this is somewhat helpful.

@rgcottrell
Copy link
Collaborator

Interesting. I thought the error was because the std library was not being built for the esp32c3, but it sounds like it is available but that the atomics are not present.

My workaround to get the example building on both Xtensa and RISCV platforms was to conditionally require no_std on RISCV projects. Ideally we will be able to remove this restriction in the future.

Depending on where we want to take this example in the future, I can think of a few options:

  • Limit the example to Xtensa platforms only
  • Limit the example to no_std functionality
  • Conditionally enable std on Xtensa platforms

@igrr
Copy link
Member Author

igrr commented Jun 14, 2021

Thanks for the tip about customizing the target json file @MabezDev! Do I understand it right that this can be considered a workaround but not a good long-term solution, since eventually we would like users to run cargo without the extra --target argument?

Interesting. I thought the error was because the std library was not being built for the esp32c3, but it sounds like it is available but that the atomics are not present.

If I understand that correctly, it's both: we do need atomics to compile std, and we do need to use a fork (https://github.com/ivmarkov/rust/commits/stable) to get std to work on top of IDF.

By the way, in the meantime I also got the atomics emulation working: catching Illegal Instruction exception, decoding the instruction, emulating it in C, returning to the next instruction. It is a standalone test, next I'll look into integrating it into IDF.

On another note, we will meet the same issue with missing atomics on ESP32-S2 — it is single core and doesn't have the hardware S32C1I (atomic compare and set) instruction.

@MabezDev
Copy link
Collaborator

Thanks for the tip about customizing the target json file @MabezDev! Do I understand it right that this can be considered a workaround but not a good long-term solution, since eventually we would like users to run cargo without the extra --target argument?

Indeed, it's mainly for testing really. Once you have a suitable spec it should go into the compiler itself along with all the other built in targets, I just saves time on compiler rebuilds.

By the way, in the meantime I also got the atomics emulation working: catching Illegal Instruction exception, decoding the instruction, emulating it in C, returning to the next instruction. It is a standalone test, next I'll look into integrating it into IDF.

Awesome, I think that's probably the cleanest solution.

@igrr
Copy link
Member Author

igrr commented Jul 13, 2021

Attaching a couple of ESP-IDF patches for RISC-V atomic instruction emulation support. Currently these can be applied on top of ESP-IDF master branch. Still need to do some work to clean up the patches for review, add test cases, and write some docs to get them merged.

riscv-atomic-emulation-patches.zip

@ivmarkov
Copy link

ivmarkov commented Jul 18, 2021

Seems I'm stuck with release/v4.3 because PlatformIO cannot compile ESP-IDF master (some *.lf scripts are now gone, and they try to use them from their SCons-derived-from-CMake build scripts; perhaps there are other issues too).

But I decided to bite the bullet and apply the patch to V4.3.
Surprisingly it applied (almost) cleanly! I had to fix one line in a CMakeLists.txt file.

Does it work correctly? No idea, but evidence shows it does not crash so perhaps it even works!

Some details:
I've implemented this new target in my stable_V1.53 branch of the Rust STD compiler fork which has the Some(32) and atomics_cas = false properties and does seem to generate riscv32 atomics instructions:

#[no_mangle]
extern "C" fn my_main() -> ! {
    let a = AtomicUsize::new(0);
    let v = a.compare_and_swap(0, 1, Ordering::SeqCst);
    let v2 = a.swap(2, Ordering::SeqCst);

    unsafe {
        // don't optimize our atomics out
        let _ = core::ptr::read_volatile(&v);
        let _ = core::ptr::read_volatile(&v2);
    }

    loop {}
}

... generates this assembly:

00000000 <my_main>:
   0:	1141                	addi	sp,sp,-16
   2:	c002                	sw	zero,0(sp)
   4:	4505                	li	a0,1
   6:	858a                	mv	a1,sp
   8:	1605a62f          	lr.w.aqrl	a2,(a1)
   c:	e601                	bnez	a2,14 <my_main+0x14>
   e:	1ea5a6af          	sc.w.aqrl	a3,a0,(a1)
  12:	fafd                	bnez	a3,8 <my_main+0x8>
  14:	c432                	sw	a2,8(sp)
  16:	4509                	li	a0,2
  18:	0ea5a52f          	amoswap.w.aqrl	a0,a0,(a1)
  1c:	c62a                	sw	a0,12(sp)
  1e:	4522                	lw	a0,8(sp)
  20:	4532                	lw	a0,12(sp)
  22:	a001                	j	22 <my_main+0x22>

Judging from the presence of amoswap & .aqrl I think the atomic instructions are generated correctly.

Running this code in the cargo-first STD demo does not crash the program.

(By the way, I have two additional Rust targets, one also for ESP32C3 and another for ESP32S2 that implement the "other" approach with Some(32) and atomics_cas = true and rely on atomics implemented as libcalls...).

@ivmarkov
Copy link

ivmarkov commented Oct 12, 2021

I think we can close this now?

  • In no_std mode, -Z build-std=std should not be passed to cargo, as it does not make sense to build the Rust standard library when it is not planned to be used (-Z build-std=core should be used instead; core is free of atomic requirements).
  • In std mode, -Z build-std=std should be passed to cargo, but that's now OK for all *-espidf targets, as in the meantime, these do support atomics either natively (esp32/esp32s3) or by lowering these to libcalls which are implemented in the ESP-IDF itself (esp32s2/esp32c3).

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