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

deps: update zlib to upstream 8603eee #34183

Closed
wants to merge 2 commits into from
Closed

Conversation

targos
Copy link
Member

@targos targos commented Jul 3, 2020

Updated as described in doc/guides/maintaining-zlib.md.

I need help to fix a compiler error:

In file included from ../../deps/zlib/adler32_simd.c:54:
../../deps/zlib/adler32_simd.c: In function ‘adler32_simd_’:
/usr/lib/gcc/x86_64-redhat-linux/8/include/tmmintrin.h:112:1: error: inlining failed in call to always_inline ‘_mm_maddubs_epi16’: target specific option mismatch
 _mm_maddubs_epi16 (__m128i __X, __m128i __Y)
 ^~~~~~~~~~~~~~~~~
../../deps/zlib/adler32_simd.c:120:34: note: called from here
             const __m128i mad2 = _mm_maddubs_epi16(bytes2, tap2);
                                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Updated as described in doc/guides/maintaining-zlib.md.
@nodejs-github-bot nodejs-github-bot added the zlib Issues and PRs related to the zlib subsystem. label Jul 3, 2020
@targos targos marked this pull request as draft July 3, 2020 15:06
@targos targos added the help wanted Issues that need assistance from volunteers or PRs that need help to proceed. label Jul 3, 2020
@richardlau
Copy link
Member

FWIW there's #33044 that attempted to rework the zlib.gyp file to more closely follow the structure of the gn file (e.g. targets, defines, etc.) in a similar way to how we handle V8. It reportedly fixes the unofficial 32-bit x86 builds (#33019 (comment)) but I'm close to abandoning it as I don't really have time to investigate the drop in benchmarks.

deps/zlib/adler32_simd.c Outdated Show resolved Hide resolved
Fix the compile flags so that zlib can run on CPUs that do
not have SSSE3/SSE4.2/etc. Do not compile zlib with flags that
indicate that those features are available, and instead enable
them selectively for functions that use them.

There are probably better way to do this, e.g. through gyp file
modifications as suggested in the issue. However, this patch
should do just fine until that happens.

Fixes: nodejs#32553

PR-URL: nodejs#32627
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
@targos
Copy link
Member Author

targos commented Jul 4, 2020

The build breaks on Windows. It seems to conflict with the zlib internal to V8:

v8_zlib.lib(cpu_features.obj) : error LNK2005: cpu_check_features already defined in zlib.lib(cpu_features.obj) [D:\a\node\node\node_mksnapshot.vcxproj]
v8_zlib.lib(cpu_features.obj) : error LNK2005: cpu_check_features already defined in zlib.lib(cpu_features.obj) [D:\a\node\node\mkcodecache.vcxproj]

@targos
Copy link
Member Author

targos commented Jul 14, 2020

any idea?

@targos
Copy link
Member Author

targos commented Jan 29, 2022

Replaced with #41745

@targos targos closed this Jan 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Issues that need assistance from volunteers or PRs that need help to proceed. zlib Issues and PRs related to the zlib subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants