build: enable sse4.2 and ssse3 in zlib #36693

RaisinTen · 2020-12-30T14:47:18Z

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
commit message follows commit guidelines

targos · 2020-12-30T14:55:18Z

This is a vendored dependency and we should avoid patching it directly. That said, I'm not sure there's anything to do on our side. In #36678 (comment), compilation is using unsupported GCC 4.8.5 headers, unless I'm misundestanding the directory structure.

RaisinTen · 2020-12-30T15:09:39Z

It's a temporary fix, just like #35679 because #33044 has not landed yet. Should I add a TODO comment to later add -msse4.1 to the build file as a command line option? It sure does look like GCC 4.8.5 to me too, but if the patch passes CI, I think it'll be okay.

mscdex · 2020-12-30T15:35:11Z

Should I add a TODO comment to later add -msse4.1 to the build file as a command line option?

That sort of thing (-msse4.1) was actually removed from the gyp file awhile back, so I don't think we should re-add it.

addaleax · 2020-12-30T23:27:14Z

Should I add a TODO comment to later add -msse4.1 to the build file as a command line option?

That sort of thing (-msse4.1) was actually removed from the gyp file awhile back, so I don't think we should re-add it.

@mscdex This is not “that sort of thing”.

addaleax · 2020-12-30T23:28:28Z

Should I add a TODO comment to later add -msse4.1 to the build file as a command line option?

Yeah, I think that might be good, as @targos mentioned we don’t want to keep floating patches unless we can avoid it.

Fixes: nodejs#36678

RaisinTen · 2020-12-31T15:25:28Z

@d-mozulyov can you confirm whether this patch works on your system?

jasnell · 2020-12-31T16:02:04Z

deps/zlib/contrib/optimizations/insert_string.h

@@ -16,6 +16,9 @@
 // clang-format off
 #if defined(CRC32_SIMD_SSE42_PCLMUL)
  /* Required to make MSVC bot build pass. */
+  // TODO(raisinten): When https://github.com/nodejs/node/pull/33044 lands,
+  // remove the next line and add `-msse4.2` to the command line options.


Assuming this lands before #33044, a commit should be added to that PR that resolves this TODO.

CC @richardlau
Please consider adding these options in your PR if you haven't added them already. :)

d-mozulyov · 2020-12-31T18:40:32Z

Hello everyone

@RaisinTen,
I tried this patch: RaisinTen@23873bf

In file included from ../deps/zlib/adler32_simd.c:57:0:
/usr/lib/gcc/x86_64-redhat-linux/4.8.5/include/tmmintrin.h:31:3: ошибка: #error "SSSE3 instruction set not enabled"
 # error "SSSE3 instruction set not enabled"

Fixes: nodejs#36678

Refs: nodejs#35679

RaisinTen · 2021-01-01T14:40:09Z

@d-mozulyov I enabled ssse3. Does it work now?

d-mozulyov · 2021-01-02T08:58:40Z

@RaisinTen

../deps/zlib/crc32_simd.c:24:0:
/usr/lib/gcc/x86_64-redhat-linux/4.8.5/include/smmintrin.h:31:3: ошибка: #error "SSE4.1 instruction set not enabled"

RaisinTen · 2021-01-02T09:43:46Z

@d-mozulyov how about now?

d-mozulyov · 2021-01-02T13:08:07Z

@RaisinTen

In file included from ../deps/zlib/crc_folding.c:24:0:
/usr/lib/gcc/x86_64-redhat-linux/4.8.5/include/wmmintrin.h:34:3: ошибка: #error "AES/PCLMUL instructions not enabled"

Maybe there are some automated tests so that I don't start the build manually?

RaisinTen · 2021-01-02T13:15:02Z

I don't think there is any way to automatically test it out currently because there is no official support for x86 anymore. :/

RaisinTen · 2021-01-02T13:21:12Z

I added the change for this file, does it build now?

d-mozulyov · 2021-01-02T15:51:46Z

I would like to clarify a few points

Judging by the logs, version 4.8.5 header files are used. Maybe I installed the GCC incorrectly, but the terminal displays the following:

[osboxes@osboxes node]$ gcc --version
gcc (GCC) 9.2.0
Copyright (C) 2019 Free Software Foundation, Inc.

I am using Linux 64 bit (x86-64 architecture), not x86
Are you sure that it is correct to add a set of supported extensions directly in the code via #pragma?

P.S. Build error:

/lib64/libstdc++.so.6: version `CXXABI_1.3.9' not found (required by /home/osboxes/Public/ultra-power/node/out/Release/icupkg)

RaisinTen · 2021-01-03T13:49:45Z

Hmm, that's interesting. What does cc --version produce? Was g++ being used in the last log when the error occurred?
The pragma thing did work for me and that's how I built node 15 on a 32-bit Linux.

d-mozulyov · 2021-01-03T14:39:06Z

What does cc --version produce?

[osboxes@osboxes node]$ cc --version
cc (GCC) 4.8.5 20150623 (Red Hat 4.8.5-44)
Copyright (C) 2015 Free Software Foundation, Inc.

Was g++ being used in the last log when the error occurred?

It looks like a rebuild is not being called. An error is displayed immediately:

[osboxes@osboxes node]$ make -j4
make -C out BUILDTYPE=Release V=0
make[1]: Вход в каталог `/home/osboxes/Public/ultra-power/node/out'
  LD_LIBRARY_PATH=/home/osboxes/Public/ultra-power/node/out/Release/lib.host:/home/osboxes/Public/ultra-power/node/out/Release/lib.target:$LD_LIBRARY_PATH; export LD_LIBRARY_PATH; cd ../tools/icu; mkdir -p /home/osboxes/Public/ultra-power/node/out/Release/obj/gen; "/home/osboxes/Public/ultra-power/node/out/Release/icupkg" -tl ../../deps/icu-tmp/icudt68l.dat "/home/osboxes/Public/ultra-power/node/out/Release/obj/gen/icudt68l.dat"
/home/osboxes/Public/ultra-power/node/out/Release/icupkg: /lib64/libstdc++.so.6: version `CXXABI_1.3.9' not found (required by /home/osboxes/Public/ultra-power/node/out/Release/icupkg)
make[1]: *** [/home/osboxes/Public/ultra-power/node/out/Release/obj/gen/icudt68l.dat] Ошибка 1
make[1]: Выход из каталога `/home/osboxes/Public/ultra-power/node/out'
make: *** [node] Ошибка 2

RaisinTen · 2021-01-03T14:46:12Z

cc (GCC) 4.8.5 20150623 (Red Hat 4.8.5-44)

That's where the problem is. Symlinking it to gcc (GCC) 9.2.0 might work.

It looks like a rebuild is not being called. An error is displayed immediately:

Perhaps run a make clean first?

d-mozulyov · 2021-01-04T10:17:52Z

@RaisinTen,

Thank you very much for helping to draw attention to the cc symlink.
Fixed

But now the following error is thrown (original master branch). I don't know yet what to do about it.

  LD_LIBRARY_PATH=/home/osboxes/Public/ultra-power/node/out/Release/lib.host:/home/osboxes/Public/ultra-power/node/out/Release/lib.target:$LD_LIBRARY_PATH; export LD_LIBRARY_PATH; cd ../tools/icu; mkdir -p /home/osboxes/Public/ultra-power/node/out/Release/obj/gen; "/home/osboxes/Public/ultra-power/node/out/Release/icupkg" -tl ../../deps/icu-tmp/icudt68l.dat "/home/osboxes/Public/ultra-power/node/out/Release/obj/gen/icudt68l.dat"
/home/osboxes/Public/ultra-power/node/out/Release/icupkg: /lib64/libstdc++.so.6: version `CXXABI_1.3.9' not found (required by /home/osboxes/Public/ultra-power/node/out/Release/icupkg)
make[1]: *** [/home/osboxes/Public/ultra-power/node/out/Release/obj/gen/icudt68l.dat] Ошибка 1
make[1]: *** Ожидание завершения заданий...
rm d7aaab5d72533b4e5b426649701051c55476756a.intermediate
make[1]: Выход из каталога `/home/osboxes/Public/ultra-power/node/out'
make: *** [node] Ошибка 2
[osboxes@osboxes node]$

RaisinTen · 2021-01-04T13:17:05Z

/lib64/libstdc++.so.6: version `CXXABI_1.3.9' not found

It can't find the shared standard C++ library.

What does g++ --version produce? I think that too needs to be symlinked to an appropriate version of g++.

d-mozulyov · 2021-01-06T08:48:07Z

@RaisinTen

[osboxes@osboxes ~]$ g++ --version
g++ (GCC) 9.2.0
Copyright (C) 2019 Free Software Foundation, Inc.

RaisinTen · 2021-01-06T14:16:31Z

ls -l $(g++ -print-file-name=libstdc++.so)

Does this print a symlink pointing to /lib64/libstdc++.so.6?

d-mozulyov · 2021-01-06T16:22:20Z

@RaisinTen

[osboxes@osboxes ~]$ ls -l $(g++ -print-file-name=libstdc++.so)
lrwxrwxrwx. 1 root root 19 дек 29 05:21 /usr/local/lib/gcc/x86_64-pc-linux-gnu/9.2.0/../../../../lib64/libstdc++.so -> libstdc++.so.6.0.27

RaisinTen · 2021-01-07T14:49:50Z

Did you add this earlier in your ~/.bashrc?

LD_LIBRARY_PATH=/usr/local/lib64/:$LD_LIBRARY_PATH
export LD_LIBRARY_PATH

The gcc documentation mentions it here: https://gcc.gnu.org/onlinedocs/libstdc++/faq.html#faq.how_to_set_paths

RaisinTen · 2021-01-14T09:56:44Z

@d-mozulyov is the problem solved?

d-mozulyov · 2021-02-04T05:50:03Z

@d-mozulyov is the problem solved?

Yep
Thank you so much :)

RaisinTen · 2021-02-04T14:22:51Z

@d-mozulyov awesome! 🎉

nodejs-github-bot · 2021-02-04T14:23:26Z

CI: https://ci.nodejs.org/job/node-test-pull-request/35939/

RaisinTen · 2021-02-04T14:25:12Z

cc @addaleax @jasnell
I think this could do another review because I had added changes after your last reviews. PTAL.

targos · 2021-02-05T06:14:40Z

I the problem is solved, we don't need this change, do we ?

RaisinTen · 2021-02-05T13:17:05Z

@targos I'm not really sure about that. @d-mozulyov could you please confirm that the master branch builds successfully on your machine or is my patch necessary for building Node.js on your tool chain?

d-mozulyov · 2021-02-05T22:55:25Z

@targos @RaisinTen
I confirm that the master branch is built without any Pull Requests. Thanks for explaining the problems on my side!

RaisinTen · 2021-02-06T14:01:53Z

@d-mozulyov Thanks for confirming. :)
Closing this as the issue is solved.

nodejs-github-bot added the zlib Issues and PRs related to the zlib subsystem. label Dec 30, 2020

RaisinTen added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 30, 2020

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 30, 2020

addaleax approved these changes Dec 30, 2020

View reviewed changes

addaleax added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 30, 2020

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 30, 2020

RaisinTen mentioned this pull request Dec 31, 2020

request-ci not working #36708

Closed

build: enable sse4.2 in zlib

23873bf

Fixes: nodejs#36678

RaisinTen force-pushed the build/enable-sse4.1 branch from 2f74dd5 to 23873bf Compare December 31, 2020 15:18

RaisinTen changed the title ~~build: enable sse4.1 in zlib~~ build: enable sse4.2 in zlib Dec 31, 2020

jasnell reviewed Dec 31, 2020

View reviewed changes

jasnell approved these changes Dec 31, 2020

View reviewed changes

RaisinTen added 2 commits January 1, 2021 19:47

build: enable ssse3 in zlib

c5e62b4

Fixes: nodejs#36678

build: add TODO comments to add -msse2 for zlib

b35ae3b

Refs: nodejs#35679

RaisinTen changed the title ~~build: enable sse4.2 in zlib~~ build: enable sse4.2 and ssse3 in zlib Jan 1, 2021

build: enable pclmul in zlib

4289461

build: enable sse4.2 and pclmul in zlib

bc657ec

RaisinTen added the review wanted PRs that need reviews. label Feb 4, 2021

RaisinTen closed this Feb 6, 2021

RaisinTen deleted the build/enable-sse4.1 branch February 6, 2021 14:01

RaisinTen mentioned this pull request Feb 6, 2021

GCC make: SSE4.1 instruction set not enabled #36678

Closed

RaisinTen removed the review wanted PRs that need reviews. label Feb 7, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build: enable sse4.2 and ssse3 in zlib #36693

build: enable sse4.2 and ssse3 in zlib #36693

RaisinTen commented Dec 30, 2020 •

edited

targos commented Dec 30, 2020

RaisinTen commented Dec 30, 2020

mscdex commented Dec 30, 2020 •

edited

addaleax commented Dec 30, 2020

addaleax commented Dec 30, 2020

RaisinTen commented Dec 31, 2020

jasnell Dec 31, 2020

RaisinTen Jan 1, 2021

d-mozulyov commented Dec 31, 2020

RaisinTen commented Jan 1, 2021

d-mozulyov commented Jan 2, 2021

RaisinTen commented Jan 2, 2021

d-mozulyov commented Jan 2, 2021

RaisinTen commented Jan 2, 2021

RaisinTen commented Jan 2, 2021

d-mozulyov commented Jan 2, 2021

RaisinTen commented Jan 3, 2021

d-mozulyov commented Jan 3, 2021

RaisinTen commented Jan 3, 2021

d-mozulyov commented Jan 4, 2021 •

edited

RaisinTen commented Jan 4, 2021

d-mozulyov commented Jan 6, 2021

RaisinTen commented Jan 6, 2021

d-mozulyov commented Jan 6, 2021

RaisinTen commented Jan 7, 2021 •

edited

RaisinTen commented Jan 14, 2021

d-mozulyov commented Feb 4, 2021

RaisinTen commented Feb 4, 2021

nodejs-github-bot commented Feb 4, 2021

RaisinTen commented Feb 4, 2021

targos commented Feb 5, 2021

RaisinTen commented Feb 5, 2021

d-mozulyov commented Feb 5, 2021

RaisinTen commented Feb 6, 2021

build: enable sse4.2 and ssse3 in zlib #36693

build: enable sse4.2 and ssse3 in zlib #36693

Conversation

RaisinTen commented Dec 30, 2020 • edited

Checklist

targos commented Dec 30, 2020

RaisinTen commented Dec 30, 2020

mscdex commented Dec 30, 2020 • edited

addaleax commented Dec 30, 2020

addaleax commented Dec 30, 2020

RaisinTen commented Dec 31, 2020

jasnell Dec 31, 2020

Choose a reason for hiding this comment

RaisinTen Jan 1, 2021

Choose a reason for hiding this comment

d-mozulyov commented Dec 31, 2020

RaisinTen commented Jan 1, 2021

d-mozulyov commented Jan 2, 2021

RaisinTen commented Jan 2, 2021

d-mozulyov commented Jan 2, 2021

RaisinTen commented Jan 2, 2021

RaisinTen commented Jan 2, 2021

d-mozulyov commented Jan 2, 2021

RaisinTen commented Jan 3, 2021

d-mozulyov commented Jan 3, 2021

RaisinTen commented Jan 3, 2021

d-mozulyov commented Jan 4, 2021 • edited

RaisinTen commented Jan 4, 2021

d-mozulyov commented Jan 6, 2021

RaisinTen commented Jan 6, 2021

d-mozulyov commented Jan 6, 2021

RaisinTen commented Jan 7, 2021 • edited

RaisinTen commented Jan 14, 2021

d-mozulyov commented Feb 4, 2021

RaisinTen commented Feb 4, 2021

nodejs-github-bot commented Feb 4, 2021

RaisinTen commented Feb 4, 2021

targos commented Feb 5, 2021

RaisinTen commented Feb 5, 2021

d-mozulyov commented Feb 5, 2021

RaisinTen commented Feb 6, 2021

RaisinTen commented Dec 30, 2020 •

edited

mscdex commented Dec 30, 2020 •

edited

d-mozulyov commented Jan 4, 2021 •

edited

RaisinTen commented Jan 7, 2021 •

edited