search

rust-cross / cargo-zigbuild

Compile Cargo project with zig as linker
MIT License
1.35k stars 50 forks source link

Allow customizing CPU features #241

Closed breezewish closed 2 months ago

breezewish commented 3 months ago

For example, currently for x86_64 targets we set -mcpu=pentium4. However it is usually not an ideal one because of lacking modern instructions like CRC32, SSE4.2, AVX, AVX2. AVX2 is shipped in 2013 so that it is actually not that "modern" and should have a broad adoption.

On the other hand, some users might indeed want the compiled binary to be as much as portable, so setting these cpu features by default might not be a good choice.

My idea is to allow users specifying CPU features when calling zigbuild, like --target-cpu-features=.... When this is specified, we no longer provide default CPU features.

I came up two ways how these CPU features could be passed from cargo-zigbuild zigbuild to cargo-zigbuild zig cc/c++. Not sure whether there are better ways.

Solution 1:

Users specify the target CPU feature via an environment variable, so that in cargo-zigbuild zig cc/c++ command we receives it transparently.

Solution 2:

Users specify the target CPU feature via cargo-zigbuild zigbuild --target-cpu-features. The CPU features are then passed to the cache file with a hashed suffix (like zigcxx-x86_64-unknown-linux-gnu.2.17-<some_hash>.sh). In this way, multiple concurrent zigbuilds will not cause conflicts with each other, and we can keep the file untouched when the cpu features are unchanged (for compatible with https://github.com/rust-cross/cargo-zigbuild/issues/239). Pass by file could be possibly better, as it is more reproduciable than environment variables.

messense commented 3 months ago

currently for x86_64 targets we set -mcpu=pentium4.

This isn't true? We are only using pentium4 for i686 Linux target.

https://github.com/rust-cross/cargo-zigbuild/blob/402ddac9e6a186e986d6478d9bca8c379d80faaa/src/zig.rs#L1017-L1022

I think we should just go with solution 1: parse RUSTFLAGS to get target-cpu and target-features, then translate them to zig -mcpu flag.

breezewish commented 3 months ago

currently for x86_64 targets we set -mcpu=pentium4.

This isn't true? We are only using pentium4 for i686 Linux target.

https://github.com/rust-cross/cargo-zigbuild/blob/402ddac9e6a186e986d6478d9bca8c379d80faaa/src/zig.rs#L1017-L1022

I think we should just go with solution 1: parse RUSTFLAGS to get target-cpu and target-features, then translate them to zig -mcpu flag.

Good idea! In this way Rust land and C land uses the same CPU features, looks better.

breezewish commented 3 months ago

After some investigation I'm now worrying about one case:

cargo-zigbuild zig cc as a compiler wrapper, now changes behavior when there are environmental changes (e.g. RUSTFLAGS). This actually conflicts with other compiler wrappers such as ccache, where they expect the compilers (and compiler wrappers) to produce exactly the same output using the same input files.

TL;DR, what will happen is that, for some crates that tries to invoke ccache internally (like rust-rocksdb), when user changes RUSTFLAGS, rebuild will not happen due to ccache not invalidated.

As an alternative, we could parse the RUSTFLAGS (from config files and env variables) when invoking cargo-zigbuild zigbuild, and then passing it explicitly as part of the compiler wrapper. This may be friendly with these compiler wrappers.