Closed igrr closed 2 years ago
The other thing is, I couldn't find where the std_detect
dependency comes from. cargo tree
doesn't show it in the list.
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.
Actually it turns out the whole std relies on atomics: https://github.com/rust-lang/stdarch/issues/1181#issuecomment-859542062. 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.
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).
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. https://github.com/rust-lang/rust/issues/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 https://github.com/rust-lang/rust/pull/66548.
There was a PR to allow atomics, but it didn't get merged: https://github.com/rust-lang/rust/pull/81752.
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.
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"
]
}
// 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
.
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.
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.
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:
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.
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.
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.
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...).
I think we can close this now?
-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).-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).
With
stable
orbeta
Rust toolchains, I get the following error when runningcargo build --release
:`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 installingriscv32i-unknown-none-elf
target, i get a different error:`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 https://github.com/rust-lang/rust/issues/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?