forward the crc target feature on aarch64#270
Conversation
Byron
left a comment
There was a problem hiding this comment.
Thanks a lot! This PR seems like something I should have merged before making a new release, but here we are 😅.
As now there is no rush, maybe you can take a look at this Codex-generated comment. It looks like CI wouldn't detect the issue. Maybe this is also something this PR could add, maybe even first, to reproduce the issue (please see the second block).
I am setting the PR to changes requested, just because I don't have other means of labeling it. All it needs is your feedback/investigation.
[P2] Guard CRC march flag for MSVC compilers — /Users/byron/dev/github.com/rust-lang/libz-sys/build.rs:146-147
When stock zlib is built for an AArch64 MSVC target with -C target-feature=+crc, this raw GCC/Clang -march=armv8-a+crc flag is passed to the C compiler. cl.exe does not accept that option, so aarch64-pc-windows-msvc builds that enable CRC will fail in the build script; gate this on the compiler family or use a supported/probed flag path.
No, this issue should not show up in the current CI configuration.
The review comment looks valid as a latent bug: MSVC targets always build bundled zlib via build.rs, and the CRC path unconditionally passes GCC/Clang syntax at build.rs.
But CI does not cover the failing combination:
- Windows CI only tests
x86_64/i686MSVC/GNU targets, notaarch64-pc-windows-msvc: .github/workflows/ci.yml - Linux CI tests AArch64, but only Linux targets: .github/workflows/ci.yml
- I found no workflow/config setting
RUSTFLAGS=-Ctarget-feature=+crcor similar. rustc --print cfg --target aarch64-pc-windows-msvcreportstarget_feature="neon"by default, notcrc.
So CI would only catch this if it added an aarch64-pc-windows-msvc build with CRC enabled, for example via RUSTFLAGS="-Ctarget-feature=+crc".
708e6ed to
0c09327
Compare
0c09327 to
bb7eaeb
Compare
|
Later commits
|
When statically linking
zlib(i.e. when building it from source), forward thecrctarget feature.zlibdoes not dynamically detect it, and apparently it is not part of the baseline feature set in C.