search

rust-cross / cargo-zigbuild

Compile Cargo project with zig as linker
MIT License
1.5k stars 55 forks source link

Respect target-cpu in RUSTFLAGS #243

Closed breezewish closed 5 months ago

breezewish commented 5 months ago

Partially resolve https://github.com/rust-cross/cargo-zigbuild/issues/241

  1. in zigbuild command, target-cpu in the RUSTFLAGS (or local Cargo config files) is parsed and appended to the compiler wrapper.

    For example, RUSTFLAGS="-Ctarget-cpu=x86-64-v3" will cause compiler wrapper to be cargo-zigbuild zig cc -- -g -mcpu=x86_64_v3 ...

  2. -Ctarget-features is currently not respected in this PR. However it is parsed well.

  3. Add a test using https://github.com/breezewish/x86-instruction-set-analyzer to check whether desired instruction set presents in the compiled binary. This helps us make sure that RUSTFLAGS are correctly passed.

    Note: As we check instruction sets for the entire binary, it is possible that the instruction set (for example AVX2) comes from the Rust part, not the C++ part. I tested manually that, when RUSTFLAGS="-C target_cpu=x86-64-v4" is set, the Rust part will not produce AVX2 instructions for tests/target-cpu. If we discovered AVX2 instruction, it must comes from the C++ part.

    Here is the instruction check result with RUSTFLAGS="-C target_cpu=x86-64-v4" without using this PR:

    Format: Elf Little-endian 64-bit
Kind: Dynamic
Architecture: X86_64

Instruction set usage (total 144541):
X64: 93623 (64.77%)
INTEL386: 31545 (21.82%)
INTEL8086: 11774 (8.15%)
SSE: 2660 (1.84%)
SSE2: 1547 (1.07%)
CMOV: 1395 (0.97%)
MULTIBYTENOP: 1280 (0.89%)
BMI2: 528 (0.37%)
INTEL286: 46 (0.03%)
INTEL186: 31 (0.02%)
BMI1: 31 (0.02%)
LZCNT: 30 (0.02%)
INTEL486: 28 (0.02%)
AVX: 17 (0.01%)
PAUSE: 4 (0.00%)
CPUID: 2 (0.00%)
breezewish commented 5 months ago

Label CI-no-fail-fast may be needed in order to verify the newly added test case.

Oh, looks like the newly introduced dependency rustflags required rustc 1.74. Ref https://github.com/dtolnay/rustflags/pull/5

As a tool, a high rustc version requirement might be fine, as users can just install latest rustc and then install this tool.

breezewish commented 5 months ago

CI is now passed, except for zig-master branch :)

messense commented 5 months ago

Oh, looks like the newly introduced dependency rustflags required rustc 1.74. Ref dtolnay/rustflags#5

Not a big issue, I'll cut a new release from main before merging this.