New cargo target detection regress *-esp-idf target builds.
Opened this issue · 11 comments
Regression introduced in: cc-rs 1.1.32
.
Problem: The target is misidentified as riscv32imc_zicsr_zifencei-esp-espidf
. While this would be a "correctly" formed target for a riscv target with respect to riscv ISA version 2.1, this is not a correct target description for any esp-idf target introduced on ISA 2.0.
The esp-idf targets where added while the underlying compiler still supported the riscv ISA 2.0 that did not include any zicsr_zifencei
description. E.g the official names are listed https://doc.rust-lang.org/rustc/platform-support/esp-idf.html#requirements there.
Hello, what cargo/rustc version are you using?
cc try to use cargo env var passed to the build-script first before falling back to using pre-generated targets.
I checked our pre-generated targets and they are fine, so I think it's due to the rustc/cargo version you use.
This is on current rustc nightly
rustc 1.84.0-nightly (705cfe0e9 2024-11-01)
binary: rustc
commit-hash: 705cfe0e966399e061d64dd3661bfbc57553ed87
commit-date: 2024-11-01
host: x86_64-unknown-linux-gnu
release: 1.84.0-nightly
LLVM version: 19.1.1
The same setup is working with setting cc-rs
to 1.1.31. The build is invoked within the following env vars:
CARGO=/home/.../.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/bin/cargo
CARGO_CFG_ESPIDF_TIME64=''
CARGO_CFG_FMT_DEBUG=full
CARGO_CFG_OVERFLOW_CHECKS=''
CARGO_CFG_PANIC=abort
CARGO_CFG_RELOCATION_MODEL=static
CARGO_CFG_TARGET_ABI=''
CARGO_CFG_TARGET_ARCH=riscv32
CARGO_CFG_TARGET_ENDIAN=little
CARGO_CFG_TARGET_ENV=newlib
CARGO_CFG_TARGET_FAMILY=unix
CARGO_CFG_TARGET_FEATURE=c,m
CARGO_CFG_TARGET_HAS_ATOMIC=16,32,8,ptr
CARGO_CFG_TARGET_HAS_ATOMIC_EQUAL_ALIGNMENT=16,32,8,ptr
CARGO_CFG_TARGET_HAS_ATOMIC_LOAD_STORE=16,32,8,ptr
CARGO_CFG_TARGET_OS=espidf
CARGO_CFG_TARGET_POINTER_WIDTH=32
CARGO_CFG_TARGET_VENDOR=espressif
CARGO_CFG_UB_CHECKS=''
CARGO_CFG_UNIX=''
CARGO_ENCODED_RUSTFLAGS='--cfgespidf_time64'
CARGO_FEATURE_BINSTART=1
CARGO_FEATURE_NATIVE=1
CARGO_FEATURE_STD=1
CARGO_MANIFEST_DIR=/home/.../.cargo/registry/src/index.crates.io-6f17d22bba15001f/esp-idf-sys-0.35.0
CARGO_MANIFEST_LINKS=esp_idf
CARGO_MANIFEST_PATH=/home/.../.cargo/registry/src/index.crates.io-6f17d22bba15001f/esp-idf-sys-0.35.0/Cargo.toml
CARGO_PKG_NAME=esp-idf-sys
CARGO_PKG_README=README.md
CARGO_PKG_REPOSITORY='https://github.com/esp-rs/esp-idf-sys'
CARGO_PKG_RUST_VERSION=1.66
CARGO_PKG_VERSION=0.35.0
CARGO_PKG_VERSION_MAJOR=0
CARGO_PKG_VERSION_MINOR=35
CARGO_PKG_VERSION_PATCH=0
CARGO_PKG_VERSION_PRE=''
CRATE_CC_NO_DEFAULTS=1
DEBUG=true
DEP_COMPILER_RT_COMPILER_RT=/home/.../.cargo/registry/src/index.crates.io-6f17d22bba15001f/compiler_builtins-0.1.136/compiler-rt
ESP_IDF_VERSION=v5.2.2
HOST=x86_64-unknown-linux-gnu
LD_LIBRARY_PATH='/home/.../rust/cc-regression-test/target/debug/deps:/home/.../rust/cc-regression-test/target/debug:/home/.../.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/lib'
MCU=esp32c3
NUM_JOBS=12
OPT_LEVEL=z
OUT_DIR=/home/.../rust/cc-regression-test/target/riscv32imc-esp-espidf/debug/build/esp-idf-sys-122dd241fb831de6/out
PROFILE=debug
RUSTC=/home/.../.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/bin/rustc
RUSTC_LINKER=ldproxy
RUSTDOC=/home/.../.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/bin/rustdoc
TARGET=riscv32imc-esp-espidf
The two notable env vars here are CRATE_CC_NO_DEFAULTS=1
that we always set and well the TARGET=riscv32imc-esp-espidf
one. Still it will go into morphing that into riscv32imc_zicsr_zifencei-esp-espidf
That's strange I can't find anything in target/generated.rs
Can you show me where the riscv32imc_zicsr_zifencei-esp-espidf is used? Is it in the args passed to the compiler?
Also cc @madsmtm
In the past it was the case that rust / llvm where following the ISA spec 2.0, In there words the i2p0
were equivalent to i2p1_zicsr2p0_zifencei2p0
. In the idf toolchain based on gcc12+ ISA spec 2.1 is used, making a breaking change out of it.
We then needed to somehow not break the ecosystem and bridge the gap between the rust/llvm world and gcc toolchain version that were introduced into newer esp-idf targets.
This lead us to manually injecting a "new target" into cc-rs via cmake config in code like this:
455 let mut cmake_config = cmake::Config::new(&out_dir);
473 if target == "riscv32imc-esp-espidf" {
474 cmake_config.target("riscv32imc_zicsr_zifencei-esp-espidf");
475 } else if target == "riscv32imac-esp-espidf" {
476 cmake_config.target("riscv32imac_zicsr_zifencei-esp-espidf");
477 } else if target == "riscv32imafc-esp-espidf" {
478 cmake_config.target("riscv32imafc_zicsr_zifencei-esp-espidf");
This would end up creating the right invocation for the newer gcc version's. I think this is what is now broken. Have to look closer tomorrow at this.
Thanks, so gcc start using newer schema and we need some mapping here?
I'm not against adding custom code for handling these targets, preferably in the gen-target-info
crate.
In a perfect world cc-rs would have info about the gcc version in the Tool struct or something. Either knowing its gcc12+ or a older version. And then if it sees riscv32imc it would emit -march=rv32imc for the older compiler versions and --march=rv32imc_zicsr_zifencei, but i guess we don't have this luxury. At least in the default case.
That is one of the points why we currently still always emitting the CC_CRATES_NO_DEFAULT
env var so that code path is not triggered for builds that are using the newer toolchain.
Still have to test into how cmake crate handels the stuff when we would change it directly.
Either knowing its gcc12+ or a older version.
I think that might be doable?
Our compiler_detection.c
already uses bunches of macros, and IIRC there're also macros for gcc major/minor version
I'm not against adding custom code for handling these targets, preferably in the
gen-target-info
crate.
IMO it is not "just" about the ESP IDF targets (rsicv32im(af)c-esp-espidf
).
I think currently, every single RISCV target is affected, so this future custom code that emits this extra _zicsr_zifencei
string appended to the GCC target for GCC 12+ - I think - needs to be triggered for all RISCV targets, as soon as GCC 12+ is detected.
It is a RISCV ISA 2.0 vs 2.1 confusion, in that Rust+LLVM still follow ISA 2.0 (right?), while GCC >= 12 follows ISA 2.1.
Related: rust-lang/cmake-rs#225
Related: rust-lang/cmake-rs#225
Since it might not be clear how this ^^^ is related:
- In our own cross-compiling scenario, I think it is best to completely turn off the
cc-rs
build flags' derivation logic, as we have all the flags we need "apriori" and without turning off thecc-rs
flags generation, we end up with duplicated flags. Including the-march
flag duplicated wrongly, as per above.
The reason why we can't just switch off the flags' generation logic of cc-rs
is simply because cmake-rs
(which we actually use) does not expose the no_default_flags
builder-setter of cc-rs
, hence the above PR to cmake-rs
.
But I think regardless, the "zicsr" and "zifencei" problem needs to be solved anyway, if not for us, then for those folks that might want cc-rs
to generate the -march
flag for them. I'm a bit surprised no-one had complained so far?
Perhaps the reason is the combination of the facts that at least RISCV32 is still new-ish, and then these targets - when used from Rust - tend to be used baremetal-only (i.e. no C code around to compile with GCC as these are usually MCU targets). With the notable exception of the -espidf
targets of course.