bytecodealliance / target-lexicon

Target "triple" support
Apache License 2.0
48 stars 40 forks source link

Add `xtensa-esp32-espidf` #76

Open 1c3t3a opened 2 years ago

1c3t3a commented 2 years ago

Would it be possible to add the xtensa-esp32-espidf target? It's the target for an esp32 chip with the standard library implemented in espressifs rust fork.

sunfishcode commented 2 years ago

Our main concern with adding targets that aren't in upstream Rust is that once we add a string, it's awkward to change its meaning or to remove it, if it later turns out that we want something different.

xtensa looks like an established architecture field already, so that part seems fine.

Is esp32 the Vendor or the OS? If it's the Vendor, the question is, do you need a defined vendor, or would it be sufficient to use a custom vendor?

Beyond that, I myself am unfamiliar with the xtensa ecosystem, so my question is, can we be reasonably confident that espidf (and esp32 if it's an OS or a non-custom Vendor) (a) won't change meaning in the future, and (b) won't change names should it ever become an official Rust target?

1c3t3a commented 2 years ago

Oke, I generally understand your point. I am relatively new to the field of embedded rust as well, so I might not be the perfect person to ask. But to still give some reasoning:

xtensa looks like an established architecture field already, so that part seems fine.

Yes, Xtensa is quite well established with both a well defined ISA (instruction set architecture here) and it's widely popularity.

Is esp32 the Vendor or the OS? If it's the Vendor, the question is, do you need a defined vendor, or would it be sufficient to use a custom vendor?

esp32 is the name of a specific chip. The company espressif builts widely used micro controllers that are very accessible (at least in Germany). They've recently increased their rust support with a compiler fork that supports the standard library and multiple other tooling in and for Rust. I don't know if it meets the criteria of a custom vendor, but it's at least "not specifically recognized by upstream Autotools, LLVM, Rust". There are also multiple other chip families built by espressif, current rust support is available for the chips xtensa-esp32s2-espidf and riscv32imc-esp-espidf. So when you decide to add this triple, it would make sense to think about the others as well.

The OS, or environment is called espidf. IDF is the interface designed by espressif for interacting with the chip (threading, allocating, etc). As mentioned above the idf was abstracted and embedded with the rust standard library on espressifs compiler fork.

Generally esp32 and espidf aren't words that change in the future as one is a already well established chip and the other an interface for all espressif chips.

sunfishcode commented 2 years ago

esp32 is the name of a specific chip. The company espressif builts widely used micro controllers that are very accessible (at least in Germany). They've recently increased their rust support with a compiler fork that supports the standard library and multiple other tooling in and for Rust. I don't know if it meets the criteria of a custom vendor, but it's at least "not specifically recognized by upstream Autotools, LLVM, Rust". There are also multiple other chip families built by espressif, current rust support is available for the chips xtensa-esp32s2-espidf and riscv32imc-esp-espidf. So when you decide to add this triple, it would make sense to think about the others as well.

Target strings aren't sequences of arbitrary identifiers; they're arch-vendor-os-env-binfmt, with some parts optional. target-lexicon is built around this structure, because it translates these into enums for Architecture, Vendor, OperatingSystem, Environment, and so on. As such, it's not clear how we should parse esp32, esp, or esp32s2, because there's no "chip name" field in that position. Typically, if a chip is distinct enough to be reflected in the target string, it's mangled into the architecture field, like armv7 or riscv64gc.

In the Rust toolchain, what are the values for target_arch, target_vendor, target_os, and target_env? That may be what target-lexicon can follow.

bjorn3 commented 2 years ago

https://github.com/esp-rs/rust/blob/6646175190e8e3305b7e989d55305f18df3fac7c/compiler/rustc_target/src/spec/xtensa_esp32_espidf.rs#L11-L15

target xtensa_esp32_espidf has: target_arch: xtensa target_cpu: esp32 target_os: espidf target_env: newlib target_vendor: espressif

https://github.com/esp-rs/rust/blob/6646175190e8e3305b7e989d55305f18df3fac7c/compiler/rustc_target/src/spec/riscv32imc_esp_espidf.rs#L13-L18

target riscv32imc_esp_espidf has: target_arch: riscv32 target_cpu: generic-rv32 target_os: espidf target_env: newlib target_vendor: espressif target_features: +m,+c

1c3t3a commented 2 years ago

Ah oke I see. But how should target lexicon treat this then? The 'real' target string for xtensa_esp32_espidf would be xtensa-espressif-espidf-newlib. Looking at the roundtrip test I assume that the target strings in this library are reflexive. One way of adding this could be to allow esp32 and esp as custom vendors and just add Xtensa as a target and espidf as an operating system.

I don't know if you would like to add this, but I think that as the availability of standard library usage on the espressif chips was enabled, more people could stumble accross this issue, just like me.

sunfishcode commented 2 years ago

It sounds like adding espidf as an OperatingSystem would make sense at least, and then you could treat esp and esp32 as custom "vendors" for now, so I'd accept a PR for that. Custom vendors are similar to private use areas, so ideally those would be changed before such targets are added to upstream Rust.

Alternatively, do you know which package is depending on target-lexicon, causing this to come up? A pattern that's come up a few times is code using target-lexicon to parse target strings just to have a type to hold them in. If the broken-out parts of the target string aren't needed, it may be better to just hold them in Strings, which automatically work on any target string, present or future.

1c3t3a commented 2 years ago

Oke, I am going to file a PR for that I think. The crate I would like to build is the wasmer WASM runtime. It currently also doesn't compile with a fix for target-lexicon for other reasons, but it's generally rather difficult to change wasmer's usage of target lexicon I think.