build: enable sse4.2 and ssse3 in zlib by RaisinTen · Pull Request #36693 · nodejs/node

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

 #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

Uh oh!

Conversation

RaisinTen commented Dec 30, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

targos commented Dec 30, 2020

Uh oh!

RaisinTen commented Dec 30, 2020

Uh oh!

mscdex commented Dec 30, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

addaleax commented Dec 30, 2020

Uh oh!

addaleax commented Dec 30, 2020

Uh oh!

RaisinTen commented Dec 31, 2020

Uh oh!

jasnell Dec 31, 2020

Choose a reason for hiding this comment

Uh oh!

RaisinTen Jan 1, 2021

Choose a reason for hiding this comment

Uh oh!

d-mozulyov commented Dec 31, 2020

Uh oh!

RaisinTen commented Jan 1, 2021

Uh oh!

d-mozulyov commented Jan 2, 2021

Uh oh!

RaisinTen commented Jan 2, 2021

Uh oh!

d-mozulyov commented Jan 2, 2021

Uh oh!

RaisinTen commented Jan 2, 2021

Uh oh!

RaisinTen commented Jan 2, 2021

Uh oh!

d-mozulyov commented Jan 2, 2021

Uh oh!

RaisinTen commented Jan 3, 2021

Uh oh!

d-mozulyov commented Jan 3, 2021

Uh oh!

RaisinTen commented Jan 3, 2021

Uh oh!

d-mozulyov commented Jan 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RaisinTen commented Jan 4, 2021

Uh oh!

d-mozulyov commented Jan 6, 2021

Uh oh!

RaisinTen commented Jan 6, 2021

Uh oh!

d-mozulyov commented Jan 6, 2021

Uh oh!

RaisinTen commented Jan 7, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

RaisinTen commented Jan 14, 2021

Uh oh!

d-mozulyov commented Feb 4, 2021

Uh oh!

RaisinTen commented Feb 4, 2021

Uh oh!

nodejs-github-bot commented Feb 4, 2021

Uh oh!

RaisinTen commented Feb 4, 2021

Uh oh!

targos commented Feb 5, 2021

Uh oh!

RaisinTen commented Feb 5, 2021

Uh oh!

d-mozulyov commented Feb 5, 2021

Uh oh!

RaisinTen commented Feb 6, 2021

Uh oh!

Reviewers

RaisinTen commented Dec 30, 2020 •

edited

Loading

mscdex commented Dec 30, 2020 •

edited

Loading

d-mozulyov commented Jan 4, 2021 •

edited

Loading

RaisinTen commented Jan 7, 2021 •

edited

Loading