WIP: Compile for esp32(s2,c3,..) #20

Remmirad commented 2023年02月21日 13:40:52 +01:00 (Migrated from github.com)

When trying to compile RIOT with Rust for esp32(s2,c3) targets i encountered some problems:

(The first two problems are unrelated to this crate but are mentioned since they should show up when testing this.)

• C2Rust compiled with xtensa compatible llvm needed (esp32,s2)
• C2Rust problematic macro translation https://github.com/immunant/c2rust/issues/803
  causing non compiling code (esp32, s2, c3)
• Some small things here in rust-riot-sys need to be considered:
  • Duplicate definitions are created (activate keep-extern-types to fix)
  • When using e.g. netdev_default C++ files are pulled in and "pollute" compile-commands.json
  • The esps use stdatomic.h when module netdev_default is in

To circumvent the C2Rust macro problem the fork linked in the issue might be used or for simple test-cases the option --translate-const-macros can be removed from build.rs.

For the other 3 things concerning this crate i added ideas how to address them or make them more easily to find, but they definitely need further discussion:

• Duplicate definition of __lock occurs because it is transpiled differently by c2rust and thus doesn't get removed. I only added a warning for this to alert the user that something didn't go as expected
• C++ files: added placeholder just kicking clang++ out not sure how to address this correctly
  • The problem here is that C++ files don't have the std=c11 flag and thus it gets kicked out by the consent strategy
• The atomic case is explained in riot-c2rust.h i thought it might be nice to make this controllable via a feature.

When trying to compile RIOT with Rust for esp32(s2,c3) targets i encountered some problems: (The first two problems are unrelated to this crate but are mentioned since they should show up when testing this.) * C2Rust compiled with xtensa compatible llvm needed (esp32,s2) * C2Rust problematic macro translation https://github.com/immunant/c2rust/issues/803 causing non compiling code (esp32, s2, c3) * Some small things here in `rust-riot-sys` need to be considered: * Duplicate definitions are created (activate `keep-extern-types` to fix) * When using e.g. `netdev_default` C++ files are pulled in and "pollute" `compile-commands.json` * The esps use `stdatomic.h` when module `netdev_default` is in To circumvent the C2Rust macro problem the fork linked in the issue might be used or for simple test-cases the option `--translate-const-macros` can be removed from `build.rs`. For the other 3 things concerning this crate i added ideas how to address them or make them more easily to find, but they definitely need further discussion: * Duplicate definition of `__lock` occurs because it is transpiled differently by c2rust and thus doesn't get removed. I only added a warning for this to alert the user that something didn't go as expected * C++ files: added placeholder just kicking `clang++` out not sure how to address this correctly * The problem here is that C++ files don't have the `std=c11` flag and thus it gets kicked out by the consent strategy * The atomic case is explained in `riot-c2rust.h` i thought it might be nice to make this controllable via a feature.

chrysn commented 2023年02月22日 10:40:07 +01:00 (Migrated from github.com)

Thanks for starting this.

Just for me to get the context right: -S2 and -C3 are the RISC-V variants of the ESP processors? Why do they need an adjusted LLVM toolchain, but more importantly, is there any guide I can follow to get that? Is there a rough schedule when they would be upstreamed? Or can they be installed in parallel? (To have this all testable, we'd eventually need to get that toolchain into riotdocker).

Thanks for starting this. Just for me to get the context right: -S2 and -C3 are the RISC-V variants of the ESP processors? Why do they need an adjusted LLVM toolchain, but more importantly, is there any guide I can follow to get that? Is there a rough schedule when they would be upstreamed? Or can they be installed in parallel? (To have this all testable, we'd eventually need to get that toolchain into riotdocker).

Remmirad commented 2023年02月22日 11:20:33 +01:00 (Migrated from github.com)

The -S2 is still on espressifs lx7 architecture. But yes c3 is risc-v and does not need the special llvm. We somehow got the llvm thing to work, but i did not do that part myself, if you are interested in the details i can ask the person who did that stuff to join us here.

The -S2 is still on espressifs lx7 architecture. But yes c3 is risc-v and does **not** need the special llvm. We somehow got the llvm thing to work, but i did not do that part myself, if you are interested in the details i can ask the person who did that stuff to join us here.

chrysn commented 2023年02月22日 11:25:24 +01:00 (Migrated from github.com)

In this case I'd like to address this in slices, doing -C3 first because it's what should be testable on the images we have now.

Does the C2Rust macro translation issue occur there as well, or is that limited to the -S2 case?

(Depending on how fast you want this to progress, it might be good to have your colleague CC'd as well, or ask them right away if they have any suggestions on what to add to the riotdocker repository -- when opening issues related to Rust there, please tag me).

In this case I'd like to address this in slices, doing -C3 first because it's what should be testable on the images we have now. Does the C2Rust macro translation issue occur there as well, or is that limited to the -S2 case? (Depending on how fast you want this to progress, it might be good to have your colleague CC'd as well, or ask them right away if they have any suggestions on what to add to the riotdocker repository -- when opening issues related to Rust there, please tag me).

Remmirad commented 2023年02月22日 11:38:28 +01:00 (Migrated from github.com)

The macro issue occurred on both -S2 and -C3. Also all rust-riot-sys specific things seemed to affect both -S2 and -C3 equally. So hopefully "fixing"(Not so much to be fixed here) -C3 might fix -S2 issues related to this crate simultaneously.

The macro issue occurred on both -S2 and -C3. Also all `rust-riot-sys` specific things seemed to affect both -S2 and -C3 equally. So hopefully "fixing"(Not so much to be fixed here) -C3 might fix -S2 issues related to this crate simultaneously.

chrysn commented 2023年02月24日 14:47:31 +01:00 (Migrated from github.com)

Sounds good, I'll look into what's open.

On a side note, if we're lucky with timing, and the patches keep coming fast, the issue with needing a forked LLVM seems like it's about to be resolved.

Sounds good, I'll look into what's open. On a side note, if we're lucky with timing, and the patches keep coming fast, [the issue with needing a forked LLVM seems like it's about to be resolved](https://mabez.dev/blog/posts/esp-rust-24-02-2023/).

chrysn commented 2023年03月04日 21:52:13 +01:00 (Migrated from github.com)

Have you collected the changes you had to make on the RIOT side while testing this?

I've started, and wound up with

diff --git a/.cargo/config.toml b/.cargo/config.toml
index 03fd973948..a50f522beb 100644
--- a/.cargo/config.toml
+++ b/.cargo/config.toml
@@ -8 +8,2 @@
-riot-sys = { git = "https://github.com/RIOT-OS/rust-riot-sys" }
+#riot-sys = { git = "https://github.com/RIOT-OS/rust-riot-sys" }
+riot-sys = { git = "https://github.com/Remmirad/rust-riot-sys", branch = "compile-esp32" }
diff --git a/cpu/esp_common/Makefile.features b/cpu/esp_common/Makefile.features
index d36e47dcee..83767e7a3c 100644
--- a/cpu/esp_common/Makefile.features
+++ b/cpu/esp_common/Makefile.features
@@ -30,0 +31 @@ ifeq (rv32,$(CPU_ARCH))
+ FEATURES_PROVIDED += rust_target
diff --git a/cpu/esp_common/Makefile.include b/cpu/esp_common/Makefile.include
index 4b4d3e3de3..c8965468c2 100644
--- a/cpu/esp_common/Makefile.include
+++ b/cpu/esp_common/Makefile.include
@@ -40,0 +41,7 @@ endif
+ifeq (rv32,$(CPU_ARCH))
+ RUST_TARGET = riscv32i-unknown-none-elf
+ # we don't have it in the riotbuild images yet
+ CARGO_OPTIONS += -Zbuild-std=core
+ CARGO_CHANNEL = $(CARGO_CHANNEL_NIGHTLY)
+endif
+

but it's still not going through when running

$ make -C examples/rust-hello-world all BOARD=hip-badge BUILD_IN_DOCKER=1 DOCKER=podman

(although maybe I'm seeing just the issues you pointed out to be open, didn't check yet)

Have you collected the changes you had to make on the RIOT side while testing this? I've started, and wound up with ```patch diff --git a/.cargo/config.toml b/.cargo/config.toml index 03fd973948..a50f522beb 100644 --- a/.cargo/config.toml +++ b/.cargo/config.toml @@ -8 +8,2 @@ -riot-sys = { git = "https://github.com/RIOT-OS/rust-riot-sys" } +#riot-sys = { git = "https://github.com/RIOT-OS/rust-riot-sys" } +riot-sys = { git = "https://github.com/Remmirad/rust-riot-sys", branch = "compile-esp32" } diff --git a/cpu/esp_common/Makefile.features b/cpu/esp_common/Makefile.features index d36e47dcee..83767e7a3c 100644 --- a/cpu/esp_common/Makefile.features +++ b/cpu/esp_common/Makefile.features @@ -30,0 +31 @@ ifeq (rv32,$(CPU_ARCH)) + FEATURES_PROVIDED += rust_target diff --git a/cpu/esp_common/Makefile.include b/cpu/esp_common/Makefile.include index 4b4d3e3de3..c8965468c2 100644 --- a/cpu/esp_common/Makefile.include +++ b/cpu/esp_common/Makefile.include @@ -40,0 +41,7 @@ endif +ifeq (rv32,$(CPU_ARCH)) + RUST_TARGET = riscv32i-unknown-none-elf + # we don't have it in the riotbuild images yet + CARGO_OPTIONS += -Zbuild-std=core + CARGO_CHANNEL = $(CARGO_CHANNEL_NIGHTLY) +endif + ``` but it's still not going through when running ``` $ make -C examples/rust-hello-world all BOARD=hip-badge BUILD_IN_DOCKER=1 DOCKER=podman ``` (although maybe I'm seeing just the issues you pointed out to be open, didn't check yet)

Remmirad commented 2023年03月06日 12:06:19 +01:00 (Migrated from github.com)

Oh yes thanks for the reminder. We did something there, but did not yet open a PR for RIOT I think. (But we did not place the cargo flags here and instead put them in the Makefile, might be a nice idea to put them here as you suggested)

What we're currently using looks like this in RIOT/cpu/esp32/Makefile.features

Note: we had to use riscv32imac-unknown-none-elf since we needed the atomics, if I remember correctly

ifeq (esp32,$(CPU_FAM))
 CPU_ARCH = xtensa
 CPU_CORE = xtensa-lx6
 RUST_TARGET = xtensa-esp32-none-elf
else ifneq (,$(filter esp32c3,$(CPU_FAM)))
 CPU_ARCH = rv32
 CPU_CORE = rv32imc
 RUST_TARGET = riscv32imac-unknown-none-elf
else ifneq (,$(filter esp32s2,$(CPU_FAM)))
 CPU_ARCH = xtensa
 CPU_CORE = xtensa-lx7
 RUST_TARGET = xtensa-esp32s2-none-elf
else ifneq (,$(filter esp32s3,$(CPU_FAM)))
 CPU_ARCH = xtensa
 CPU_CORE = xtensa-lx7
 RUST_TARGET = xtensa-esp32s3-none-elf
else
 $(error Unkwnown ESP32x SoC variant (family))
endif
ifneq (,$(RUST_TARGET))
 FEATURES_PROVIDED += rust_target
endif

Oh yes thanks for the reminder. We did something there, but did not yet open a PR for RIOT I think. (But we did not place the cargo flags here and instead put them in the Makefile, might be a nice idea to put them here as you suggested) What we're currently using looks like this in `RIOT/cpu/esp32/Makefile.features` Note: we had to use `riscv32imac-unknown-none-elf` since we needed the atomics, if I remember correctly ```Makefile ifeq (esp32,$(CPU_FAM)) CPU_ARCH = xtensa CPU_CORE = xtensa-lx6 RUST_TARGET = xtensa-esp32-none-elf else ifneq (,$(filter esp32c3,$(CPU_FAM))) CPU_ARCH = rv32 CPU_CORE = rv32imc RUST_TARGET = riscv32imac-unknown-none-elf else ifneq (,$(filter esp32s2,$(CPU_FAM))) CPU_ARCH = xtensa CPU_CORE = xtensa-lx7 RUST_TARGET = xtensa-esp32s2-none-elf else ifneq (,$(filter esp32s3,$(CPU_FAM))) CPU_ARCH = xtensa CPU_CORE = xtensa-lx7 RUST_TARGET = xtensa-esp32s3-none-elf else $(error Unkwnown ESP32x SoC variant (family)) endif ifneq (,$(RUST_TARGET)) FEATURES_PROVIDED += rust_target endif ```

chrysn commented 2023年03月06日 23:57:38 +01:00 (Migrated from github.com)

Note: we had to use riscv32imac-unknown-none-elf since we needed the atomics, if I remember correctly

Just setting up the compiler for that target won't make the machine process them -- the ESP32-C3 is advertised to be a RV32IMC, and hitting atomic instruction will probably just trigger a fault. I know that some of the (esp. new) Rust code does rely on atomics, but that'll need to be switched to the polyfill (which will effectively just disable interrupts, run stuff non-atomically and enable interrupts again) to work on anything that doesn't have atomics (also necessary for the smaller ARMs).

> Note: we had to use riscv32imac-unknown-none-elf since we needed the atomics, if I remember correctly Just setting up the compiler for that target won't make the machine process them -- the ESP32-C3 is advertised to be a RV32IMC, and hitting atomic instruction will probably just trigger a fault. I know that some of the (esp. new) Rust code does rely on atomics, but that'll need to be switched to the polyfill (which will effectively just disable interrupts, run stuff non-atomically and enable interrupts again) to work on anything that doesn't have atomics (also necessary for the smaller ARMs).

Remmirad commented 2023年03月07日 09:37:09 +01:00 (Migrated from github.com)

Hm you're right there, interesting that it still worked in our tests, but should definitely be changed. If I remember correctly we had the problem that something rust-riot-sys complained about not having atomics available.

Hm you're right there, interesting that it still worked in our tests, but should definitely be changed. If I remember correctly we had the problem that something `rust-riot-sys` complained about not having atomics available.

chrysn commented 2023年03月07日 10:05:14 +01:00 (Migrated from github.com)

It's not too surprising: I've assumed the presence of atomics pretty generously, and rust-riot-sys processes a lot of code that's not necessarily ever run. There are two possible reasons why things could have worked (the first one is likely, the second is pure speculation):

• The atomics were just not hit -- and code that's never executed doesn't create faults.
• Less likely, but possible: There is some atomic support in the chips, but it's so broken / unreliable the vendor could not afford to advertise it as RV32IMAC, and chose to advertise it as RF32IMC instead.

It's not too surprising: I've assumed the presence of atomics pretty generously, and rust-riot-sys processes a lot of code that's not necessarily ever run. There are two possible reasons why things could have worked (the first one is likely, the second is pure speculation): * The atomics were just not hit -- and code that's never executed doesn't create faults. * Less likely, but possible: There is *some* atomic support in the chips, but it's so broken / unreliable the vendor could not afford to advertise it as RV32IMAC, and chose to advertise it as RF32IMC instead.

Remmirad commented 2023年03月07日 11:02:59 +01:00 (Migrated from github.com)

I would definitely go with the first one too :)

O.k. so to make this right one would use atomic_polyfill and provide it with a critical-section implementation via. RIOTs interfaces (e.g. disable interrupts)?

And where would one put these things? Inside this crate and enable disable critical-section implementations based on the platform? (Maybe not necessary when done via RIOT)

I would definitely go with the first one too :) O.k. so to make this right one would use [`atomic_polyfill`](https://github.com/embassy-rs/atomic-polyfill) and provide it with a [`critical-section`](https://github.com/rust-embedded/critical-section) implementation via. RIOTs interfaces (e.g. disable [interrupts](https://doc.riot-os.org/group__core__irq.html))? And where would one put these things? Inside this crate and enable disable `critical-section` implementations based on the platform? (Maybe not necessary when done via RIOT)

Flole998 commented 2023年03月09日 01:53:41 +01:00 (Migrated from github.com)

I'd like to point out that the heapless-library which we are using is considering moving away from atomic-polyfill in favor for portable-atomic.

See https://github.com/japaric/heapless/pull/328 for the reasons, so maybe portable-atomic might be better suited for this project aswell.

I'd like to point out that the heapless-library which we are using is considering moving away from atomic-polyfill in favor for portable-atomic. See https://github.com/japaric/heapless/pull/328 for the reasons, so maybe portable-atomic might be better suited for this project aswell.

Remmirad commented 2023年03月24日 09:45:14 +01:00 (Migrated from github.com)

In https://github.com/RIOT-OS/rust-riot-sys/pull/20/commits/c7df4f1f491b9bc289782e3fdcdc2980ed7381e3 I added a first draft how to use these portable atomics. It provides a critical section implementation that uses RIOTs interrupt interface to just disable and enable interrupts in a critical section. (Thus only working for single core cpus atm)
Maybe we even want to introduce this in a separate PR because this is not wholly esp related...
It at least solves the atomic problem occurring when compiling for esp32c3.

In https://github.com/RIOT-OS/rust-riot-sys/pull/20/commits/c7df4f1f491b9bc289782e3fdcdc2980ed7381e3 I added a first draft how to use these portable atomics. It provides a critical section implementation that uses RIOTs [interrupt interface](https://doc.riot-os.org/group__core__irq.html) to just disable and enable interrupts in a critical section. (Thus only working for single core cpus atm) Maybe we even want to introduce this in a separate PR because this is not wholly esp related... It at least solves the atomic problem occurring when compiling for esp32c3.

This pull request has changes conflicting with the target branch.