GeezerVison

I've reviewed the three header files added to PC and they seem generic. It's a shame they aren't part of the sources, but figured it's easier/safer to just copy them into our source tree so that the cpython-source-deps repo is a straight import.

🤖 New build scheduled with the buildbot fleet by @zooba for commit 08eecb1 🤖

Results will be shown at:

https://buildbot.python.org/all/#/grid?branch=refs%2Fpull%2F131438%2Fmerge

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

The two failing buildbots appear unrelated, and everything else passed. All I changed in the last commit was docs, so I'm not going to rerun the buildbots unless someone thinks it's necessary or worthwhile.

I'm kicking off a pyperformance benchmarking run with this -- we have a non-zero usage of zlib in that suite.

The results for this on pyperformance are basically within the noise / no change. Not all that surprising, given that there are no benchmarks specifically aimed at zlib. https://github.com/faster-cpython/benchmarking-public/tree/main/results/bm-20250319-3.14.0a6+-548daa7

This has broken the tailcall builds on Windows: https://github.com/python/cpython/actions/runs/13967899073/job/39102464260

D:\a\cpython\cpython\externals\\zlib-ng-2.2.4\\functable.c(79,26): error : use of undeclared identifier 'slide_hash_sse2'; did you mean 'slide_hash_c'? [D:\a\cpython\cpython\PCbuild\zlib-ng.vcxproj]
D:\a\cpython\cpython\externals\\zlib-ng-2.2.4\\functable.c(123,26): error : use of undeclared identifier 'slide_hash_avx2'; did you mean 'slide_hash_c'? [D:\a\cpython\cpython\PCbuild\zlib-ng.vcxproj]

Probably needs some fixed preprocessor checks (hopefully only in what's in the PC directory) to better handle the intrinsics being present/absent.

Though I notice that build warns NOTE: Visual Studio not detected. LLVM does not provide a C/C++ standard library and may be unable to locate MSVC headers., which might also be relevant.

I think, this is because here
https://github.com/zlib-ng/zlib-ng/blob/860e4cff7917d93f54f5d7f0bc1d0e8b1a3cb988/fallback_builtins.h#L4-L25
in line 4 __clang__ gets excluded and hence #define HAVE_BUILTIN_CTZ is not set (but in the MSVC case it is).

Then, in
https://github.com/zlib-ng/zlib-ng/blob/860e4cff7917d93f54f5d7f0bc1d0e8b1a3cb988/arch/x86/x86_functions.h#L13-L20
we dont get the prototype for slide_hash_sse2 in line 17, hence the error.

Sorry, I cannot get the permalinks to work here the usual way I am doing it :(

Though I notice that build warns NOTE: Visual Studio not detected. LLVM does not provide a C/C++ standard library and may be unable to locate MSVC headers., which might also be relevant.

FWIW, when building with the bundled clang-cl of VS I do not have these warnings, but get the same error.

Most likely zlib-ng would generate different versions of the header files I checked into the PC directory for Clang. Figuring out what differences belong in there and detecting it dynamically rather than using CMake is the way forward.

This should get a new issue (because clang-cl isn't a supported configuration, so we don't have to revert this change until it's working). Tag me on it and we can continue there.

Hi @zooba, a heads-up on zlib-ng 2.2.4, in case it's useful to anyone: PGO builds made with Visual Studio 17.11 (MSVC 14.41) decompress deflate data wrongly on CPUs with AVX-512 (test_zlib: "incorrect data check", test_gzip: "CRC check failed"). It's the known zlib-ng bug zlib-ng/zlib-ng#1883 (MSVC before 17.13 miscompiles the AVX-512 GET_CHUNK_MAG()), worked around in zlib-ng 2.2.5 by zlib-ng/zlib-ng#1884. On a Ryzen 9 7950X, adding just that pragma to 2.2.4 fixes the build, and removing it brings the wrong output back.

It doesn't affect CI or the python.org builds, which use newer compilers, but it does affect building from source with an older Visual Studio, which is how I stumbled upon it. Is there any chance of updating the Windows build to zlib-ng 2.2.5? main, 3.15 and 3.14 still bundle 2.2.4.

@maikschulze Yes, that sounds like an update we'd make. Can you file a new issue?

Hi, thanks for the quick response! I will file an issue and along with a pull request tomorrow.

I think we keep the zlib-ng sources in a separate repo, so just file the issue without a PR as we'll need a core developer to update that. We don't accept PRs, as it's too hard to validate that the sources are unmodified (but if you've got a link to the right download page, that will be appreciated)