Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Respect target-cpu in RUSTFLAGS #243

Merged
merged 7 commits into from
Apr 24, 2024
Merged

Conversation

breezewish
Copy link
Contributor

@breezewish breezewish commented Apr 19, 2024

Partially resolve #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
Copy link
Contributor Author

breezewish commented Apr 19, 2024

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 dtolnay/rustflags#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
Copy link
Contributor Author

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

@messense messense self-requested a review April 21, 2024 13:27
.github/workflows/CI.yml Outdated Show resolved Hide resolved
@messense
Copy link
Member

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.

src/zig.rs Show resolved Hide resolved
Copy link
Member

@messense messense left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly LGTM, just a minor nit.

@messense messense merged commit 841df51 into rust-cross:main Apr 24, 2024
33 of 39 checks passed
@breezewish breezewish deleted the target-cpu branch April 25, 2024 04:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow customizing CPU features
2 participants