build: enable Clang-cl Windows builds #35433

targos · Sep 30, 2020

This is very hacky at the moment. I'm trying to find out what needs to be done to enable it and where.
I'm opening a PR to discuss the changes and seek some help, because it doesn't work yet!

To try it: .\vcbuild.bat noetw

targos · Sep 30, 2020

deps/icu-small/source/tools/toolutil/pkg_genc.cpp

 #   else
-        *pCPU = IMAGE_FILE_MACHINE_UNKNOWN;
+        *pCPU = IMAGE_FILE_MACHINE_AMD64;


This is for #34201

targos · Sep 30, 2020

deps/zlib/crc_folding.c

+#include <smmintrin.h>
+#include <tmmintrin.h>


I opened https://chromium-review.googlesource.com/c/chromium/src/+/2281008 for this

targos · Sep 30, 2020

node.gypi

+      # 'msvs_precompiled_header': 'tools/msvs/pch/node_pch.h',
+      # 'msvs_precompiled_source': 'tools/msvs/pch/node_pch.cc',
+      # 'sources': [
+      #   '<(_msvs_precompiled_header)',
+      #   '<(_msvs_precompiled_source)',
+      # ],


I get an error about non-portable includes if precompiled headers are enabled.

targos · Sep 30, 2020

tools/gyp/pylib/gyp/generator/msvs.py

@@ -2799,7 +2799,7 @@ def _GetMSBuildLocalProperties(msbuild_toolset):
  if msbuild_toolset:
    properties = [
        ['PropertyGroup', {'Label': 'Locals'},
-          ['PlatformToolset', msbuild_toolset],
+          ['PlatformToolset', 'ClangCL'],


This obviously has to come from configuration in the end

targos · Sep 30, 2020

With all the above hacks, I am blocked on this error:

lld-link : error : undefined symbol: public: virtual __cdecl v8::internal::Relocatable::~Relocatable(void) [D:\Git\nodejs\node\tools\v8_gypfiles\mksnapshot.vcxproj]

Related warning in the build output:

..\..\deps\v8\src/objects/objects.h(818,18): warning : inline function 'v8::internal::Relocatable::~Relocatable' is not defined [-Wundefined-inline] [D:\a\node\node\tools\v8_gypfiles\v8_base_without_compiler.vcxproj]
..\..\deps\v8\src/objects/string.h(821,25): message : used here [D:\a\node\node\tools\v8_gypfiles\v8_base_without_compiler.vcxproj]

node/deps/v8/src/objects/objects.h

Lines 815 to 833 in 4a6005c

    
           class Relocatable { 
        
            public: 
        
             explicit inline Relocatable(Isolate* isolate); 
        
             inline virtual ~Relocatable(); 
        
             virtual void IterateInstance(RootVisitor* v) {} 
        
             virtual void PostGarbageCollection() {} 
        
             static void PostGarbageCollectionProcessing(Isolate* isolate); 
        
             static int ArchiveSpacePerThread(); 
        
             static char* ArchiveState(Isolate* isolate, char* to); 
        
             static char* RestoreState(Isolate* isolate, char* from); 
        
             static void Iterate(Isolate* isolate, RootVisitor* v); 
        
             static void Iterate(RootVisitor* v, Relocatable* top); 
        
             static char* Iterate(RootVisitor* v, char* t); 
        
            private: 
        
             Isolate* isolate_; 
        
             Relocatable* prev_; 
        
           };

node/deps/v8/src/objects/string.h

Lines 821 to 836 in 4a6005c

    
           class V8_EXPORT_PRIVATE FlatStringReader : public Relocatable { 
        
            public: 
        
             FlatStringReader(Isolate* isolate, Handle<String> str); 
        
             FlatStringReader(Isolate* isolate, Vector<const char> input); 
        
             void PostGarbageCollection() override; 
        
             inline uc32 Get(int index); 
        
             template <typename Char> 
        
             inline Char Get(int index); 
        
             int length() { return length_; } 
        
            private: 
        
             Address* str_; 
        
             bool is_one_byte_; 
        
             int length_; 
        
             const void* start_; 
        
           };

targos · Oct 2, 2020

/cc @nodejs/v8

gengjiawen · Oct 2, 2020

Another tricky thing is that we can't even test this on github action due to space limit https://github.com/nodejs/node/pull/35433/checks?check_run_id=1189469828 LIB : LLVM error : IO failure on output stream: no space on device.

dennisameling · Oct 3, 2020

Regarding the disk space issue: the D:\ drive on Windows VMs apparently has 14GB available - this is also described in the official docs. A workaround is to move things to the C:\ drive which has 80GB+ as described in actions/runner-images#1341 (comment)

gengjiawen · Oct 19, 2020

@targos Can you rebase this, so we can working on new V8 ?

gengjiawen · Oct 19, 2020

Regarding the disk space issue: the D:\ drive on Windows VMs apparently has 14GB available - this is also described in the official docs. A workaround is to move things to the C:\ drive which has 80GB+ as described in actions/virtual-environments#1341 (comment)

I investigate a little bit, looks like not an easy thing to do: actions/checkout#197.

Also, below won't work either on windows CI on github action.

    defaults:
      run:
        working-directory: C:/nodejs

Not sure why they default to drive D with such a limited disk space.

targos · Oct 19, 2020

Rebased on #35700

targos · Oct 19, 2020

I don't know if CI has clang-cl. Let's try: https://ci.nodejs.org/job/node-test-commit-windows-fanned/38888/

Edit: well, it doesn't 😄

08:23:59 C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\MSBuild\Microsoft\VC\v160\Microsoft.CppBuild.targets(411,5): error MSB8020: The build tools for ClangCL (Platform Toolset = 'ClangCL') cannot be found. To build using the ClangCL build tools, please install ClangCL build tools.  Alternatively, you may upgrade to the current Visual Studio tools by selecting the Project menu or right-click the solution, and then selecting "Retarget solution". [C:\workspace\node-compile-windows\node\node_etw.vcxproj]

targos · Oct 19, 2020

@nodejs/build-infra Would it be easy to install ClangCL build tools on the Windows machines?

gengjiawen · Oct 19, 2020

@nodejs/build-infra Would it be easy to install ClangCL build tools on the Windows machines?

Not sure this will help: https://github.com/appveyor/build-images/blob/27bde614bc60d7ef7a8bc46182f4d7582fa11b56/scripts/Windows/install_vs2019.ps1#L192.

targos · Oct 19, 2020

So I tried to build it on my local machine.

There's one warning that is very noisy (I think it's printed for every V8 source file:

..\..\deps\v8\src/base/safe_conversions_impl.h(158,46): warning : implicit conversion from 'long long' to 'double' changes value from 9223372036854775807 to 9223372036854775808 [-Wimplicit-int-float-conversion] [D:\Git\no 
dejs\node\tools\v8_gypfiles\v8_base_without_compiler.vcxproj]
..\..\deps\v8\src/base/safe_conversions_impl.h(213,52): message : in instantiation of member function 'v8::base::internal::DstRangeRelationToSrcRangeImpl<long long, double, v8::base::internal::INTEGER_REPRESENTATION_SIGNE 
D, v8::base::internal::INTEGER_REPRESENTATION_SIGNED, v8::base::internal::NUMERIC_RANGE_NOT_CONTAINED>::Check' requested here [D:\Git\nodejs\node\tools\v8_gypfiles\v8_base_without_compiler.vcxproj]
..\..\deps\v8\src/base/safe_conversions.h(44,21): message : in instantiation of function template specialization 'v8::base::internal::DstRangeRelationToSrcRange<long long, double>' requested here [D:\Git\nodejs\node\tools 
\v8_gypfiles\v8_base_without_compiler.vcxproj]
..\..\deps\v8\src/base/platform/time.h(228,20): message : in instantiation of function template specialization 'v8::base::saturated_cast<long long, double>' requested here [D:\Git\nodejs\node\tools\v8_gypfiles\v8_base_wit 
hout_compiler.vcxproj]

Then it fails with 3 errors (related warnings included):

lld-link : error : undefined symbol: public: virtual __cdecl v8::internal::Relocatable::~Relocatable(void) [D:\Git\nodejs\node\tools\v8_gypfiles\mksnapshot.vcxproj]
  >>> referenced by ..\..\out\Release\obj\mksnapshot\\deps\v8\src\snapshot\embedded\platform-embedded-file-writer-win.obj
  >>> referenced by ..\..\out\Release\obj\mksnapshot\\deps\v8\src\snapshot\mksnapshot.obj
  >>> referenced by v8_base_without_compiler.lib(stack-guard.obj)

..\..\deps\v8\src/objects/objects.h(818,18): warning : inline function 'v8::internal::Relocatable::~Relocatable' is not defined [-Wundefined-inline] [D:\Git\nodejs\node\tools\v8_gypfiles\mksnapshot.vcxproj]
..\..\deps\v8\src/objects/string.h(832,25): message : used here [D:\Git\nodejs\node\tools\v8_gypfiles\mksnapshot.vcxproj]

lld-link : error : undefined symbol: public: class v8::internal::Object __cdecl v8::internal::FixedArray::get(int) const [D:\Git\nodejs\node\tools\v8_gypfiles\mksnapshot.vcxproj]
  >>> referenced by ..\..\out\Release\obj\mksnapshot\\deps\v8\src\snapshot\embedded\platform-embedded-file-writer-win.obj
  >>> referenced by ..\..\out\Release\obj\mksnapshot\\deps\v8\src\snapshot\mksnapshot.obj
  >>> referenced by v8_base_without_compiler.lib(flush-instruction-cache.obj)

..\..\deps\v8\src/objects/fixed-array.h(102,17): warning : inline function 'v8::internal::FixedArray::get' is not defined [-Wundefined-inline] [D:\Git\nodejs\node\tools\v8_gypfiles\mksnapshot.vcxproj]
..\..\deps\v8\src/objects/ordered-hash-table.h(88,23): message : used here [D:\Git\nodejs\node\tools\v8_gypfiles\mksnapshot.vcxproj]

lld-link : error : undefined symbol: public: void __cdecl v8::internal::FixedArray::set(int, class v8::internal::Smi) [D:\Git\nodejs\node\tools\v8_gypfiles\mksnapshot.vcxproj]
  >>> referenced by ..\..\out\Release\obj\mksnapshot\\deps\v8\src\snapshot\embedded\platform-embedded-file-writer-win.obj
  >>> referenced by ..\..\out\Release\obj\mksnapshot\\deps\v8\src\snapshot\mksnapshot.obj
  >>> referenced by v8_base_without_compiler.lib(flush-instruction-cache.obj)

..\..\deps\v8\src/objects/fixed-array.h(134,15): warning : inline function 'v8::internal::FixedArray::set' is not defined [-Wundefined-inline] [D:\Git\nodejs\node\tools\v8_gypfiles\mksnapshot.vcxproj]
..\..\deps\v8\src/objects/ordered-hash-table.h(210,5): message : used here [D:\Git\nodejs\node\tools\v8_gypfiles\mksnapshot.vcxproj]

gengjiawen · Oct 19, 2020

Looks like all related to inline function on windows platform.

gengjiawen · Nov 9, 2020

Now stucks with

  ..\..\out\Release\obj\v8_libsampler\\deps\v8\src\libsampler\sampler.obj: no such file or directory
C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\MSBuild\Microsoft\VC\v160\Microsoft.CppCommon.targets(1309,5): error MSB6006: "llvm-lib.exe" exited with code 1. [D:\code\node\tools\v8_gypfiles\v8_libsampler.vcxproj]
  ..\..\out\Release\obj\v8_zlib\\deps\v8\third_party\zlib\adler32.obj: no such file or directory
C:\Program Files (x86)\Microsoft Visual Studio\2019\Community\MSBuild\Microsoft\VC\v160\Microsoft.CppCommon.targets(1309,5): error MSB6006: "llvm-lib.exe" exited with code 1. [D:\code\node\tools\v8_gypfiles\v8_zlib.vcxproj]

Maybe related: https://developercommunity.visualstudio.com/content/problem/1110835/clangcl-ltcg-is-passed-to-llvm-lib.html

This occurred when I update my visual studio to latest.

I also get some ideas to patch V8 to make the windows build works.

dennisameling · Nov 11, 2020

Just for the record, Visual Studio 16.8 was released yesterday and has the following mentioned in its release notes:

Support for ARM64 projects using clang-cl.

Thought I'd mention it here as it might help for this PR. I have a Surface Pro X based on the arm64 architecture, let me know if you'd like me to test something 👍

dennisameling · Nov 11, 2020

Just tried to build using .\vcbuild.bat noetw on x64 with Visual Studio 16.8, getting the same errors as @targos regarding mksnapshot, but there are actually quite some things that build correctly, so that's promising 🚀

@gengjiawen Does it make any difference if you update to Visual Studio 16.8, which was released yesterday?

gengjiawen · Nov 11, 2020

@gengjiawen Does it make any difference if you update to Visual Studio 16.8, which was released yesterday?

I forget to remove '/P'. I should try my build now.

I have a Surface Pro X based on the arm64 architecture

Have you tried the official arm64 build for windows. Will it run ? (I have no device to test it)

dennisameling · Nov 11, 2020

Have you tried the official arm64 build for windows. Will it run ? (I have no device to test it)

Yes (see nodejs/build#2450 (comment)), and it's blazing fast 🚀 I use it on a daily basis and to work on things like an arm64 build for GitHub Desktop: desktop/desktop#9691

gengjiawen · Dec 1, 2020

This is more likely a bug from what I test (refactor the inline implementation). Not sure this need to reported to clang team or msvc team.

mmomtchev · May 31, 2022

@targos, what is the current status of this?
FYI, Electron have made the decision to use only clang on Windows which means that currently it is not possible to use node-gyp to build a native module on Windows because of a V8 API (GetBackingStore) that returns a shared_ptr - these are not compatible between MSVC and the clang runtime.

electron/electron#29893

targos · May 31, 2022

Status hasn't changed. There are some issues and I don't have the knowledge or time to fix them.

TODO: upstream change to Chromium

targos · Oct 22, 2022

https://ci.nodejs.org/job/node-test-commit-windows-fanned/51194/

targos · Oct 22, 2022

Temporary ready for review to trigger GitHub CI

targos · Oct 22, 2022

clang-cl : warning : argument unused during compilation: '-std:c++17' [-Wunused-command-line-argument] [D:\a\node\node\deps\uvwasi\uvwasi.vcxproj]

Many errors in the new base64 dep:

base64\lib\arch\ssse3/enc_reshuffle.c(7,7): error : always_inline function '_mm_shuffle_epi8' requires target feature 'ssse3', but would be inlined into function 'enc_reshuffle' that is compiled without support for 'ssse3' [D:\a\node\node\deps\base64\base64_ssse3.vcxproj]
  In file included from base64\lib\arch\ssse3\codec.c:17:
base64\lib\arch\ssse3/enc_translate.c(32,26): error : always_inline function '_mm_shuffle_epi8' requires target feature 'ssse3', but would be inlined into function 'enc_translate' that is compiled without support for 'ssse3' [D:\a\node\node\deps\base64\base64_ssse3.vcxproj]
  In file included from base64\lib\arch\ssse3\codec.c:15:
base64\lib\arch\ssse3/dec_loop.c(91,29): error : always_inline function '_mm_shuffle_epi8' requires target feature 'ssse3', but would be inlined into function 'dec_loop_ssse3_inner' that is compiled without support for 'ssse3' [D:\a\node\node\deps\base64\base64_ssse3.vcxproj]
base64\lib\arch\ssse3/dec_loop.c(92,29): error : always_inline function '_mm_shuffle_epi8' requires target feature 'ssse3', but would be inlined into function 'dec_loop_ssse3_inner' that is compiled without support for 'ssse3' [D:\a\node\node\deps\base64\base64_ssse3.vcxproj]
base64\lib\arch\ssse3/dec_loop.c(101,24): error : always_inline function '_mm_shuffle_epi8' requires target feature 'ssse3', but would be inlined into function 'dec_loop_ssse3_inner' that is compiled without support for 'ssse3' [D:\a\node\node\deps\base64\base64_ssse3.vcxproj]
...

targos · Oct 22, 2022

@bnoordhuis fyi

bnoordhuis · Oct 22, 2022

You probably need to turn on -msse3 or -msse4.2 for clang-cl.

(_mm_shuffle_epi8 corresponds to PABSB and that's an SSE3 instruction but I expect it also uses newer instructions.)

targos · Oct 22, 2022

You probably need to turn on -msse3 or -msse4.2 for clang-cl.

I have no idea how to do that.

bnoordhuis · Oct 22, 2022

Just add it to cflags. If plain -msse3 doesn't work, use /clang:-msse3 (but I expect the former Just Works.)

targos · Oct 22, 2022

The errors are still here after 74f916b but I'm not sure cflags are actually used by GYP on Windows.

bnoordhuis · Oct 22, 2022

Oh! 🤦

Sorry about that, the way to do that with gyp is this:

'msvs_settings': {
  'VCCLCompilerTool': {
    'AdditionalOptions': ['-msse3'],
  },
},

targos · Oct 22, 2022

Next error is from V8:

  In file included from ..\..\deps\v8\src\init\setup-isolate-full.cc:8:
  In file included from ..\..\deps\v8\src/heap/heap-inl.h:23:
  In file included from ..\..\deps\v8\src/heap/concurrent-allocator-inl.h:13:
  In file included from ..\..\deps\v8\src/heap/incremental-marking.h:13:
  In file included from ..\..\deps\v8\src/heap/mark-compact.h:13:
  In file included from ..\..\deps\v8\src/heap/concurrent-marking.h:15:
  In file included from ..\..\deps\v8\src/heap/marking-visitor.h:9:
  In file included from ..\..\deps\v8\src/heap/marking-worklist.h:14:
  In file included from ..\..\deps\v8\src/heap/cppgc-js/cpp-marking-state.h:10:
  In file included from ..\..\deps\v8\src/heap/cppgc-js/cpp-heap.h:19:
..\..\deps\v8\src/heap/cppgc/heap-base.h(313,1): error : declaration of anonymous class must be a definition [D:\a\node\node\tools\v8_gypfiles\v8_init.vcxproj]
..\..\deps\v8\src/heap/cppgc/heap-base.h(313,1): warning : declaration does not declare anything [-Wmissing-declarations] [D:\a\node\node\tools\v8_gypfiles\v8_init.vcxproj]

targos · Oct 22, 2022

This warning appears thousands of times:

..\..\deps\v8\src/objects/fixed-array.h(105,17): warning : inline function 'v8::internal::FixedArray::get' is not defined [-Wundefined-inline] [D:\a\node\node\tools\v8_gypfiles\v8_compiler.vcxproj]

targos · Oct 22, 2022

The build ended up failing with "No space left on device" 😞

bnoordhuis · Oct 22, 2022

Hm, that's regrettable. Maybe it's possible to build on C:\ instead of D:?

GitHub's machines have two disks/mounts. The second one has about 10-15 GB free space, the first one is about twice the size when you remove a couple of big packages; .NET SDK + Android SDK + GHC is around 30 GB.

A full list is here: https://github.com/actions/runner-images/blob/main/images/win/Windows2019-Readme.md

nodejs-github-bot added build Issues and PRs related to build files or the CI. tools Issues and PRs related to the tools directory. zlib Issues and PRs related to the zlib subsystem. labels Sep 30, 2020

targos commented Sep 30, 2020

View changes

targos added the help wanted Issues that need assistance from volunteers or PRs that need help to proceed. label Sep 30, 2020

targos force-pushed the clang-cl branch from e0b3e1c to 647f0a6 Compare Oct 19, 2020

This comment has been minimized.

Sign in to view

gengjiawen mentioned this pull request Oct 23, 2020

tools: add msvc /P output to .gitignore #35735

Closed

gengjiawen mentioned this pull request Jan 5, 2021

Use Clang on Windows nodejs/node-gyp#2295

Open

targos mentioned this pull request Feb 24, 2022

Should we remove support for building with MSVC? nodejs/build-toolchain-next#12

Open

targos mentioned this pull request Oct 22, 2022

Reduce the number of patches we float on V8 #45118

Open

7 tasks

targos added 5 commits Oct 22, 2022

tools: hack gyp to generate clang-cl solutions

7d07c7c

deps: fix zlib to prevent redefining __attribute__

88be882

deps: add missing zlib includes

c9993a0

TODO: upstream change to Chromium

hack ICU

56e4bcc

hack: no pch

4596d8d

targos force-pushed the clang-cl branch from 185a78d to 4596d8d Compare Oct 22, 2022

targos marked this pull request as ready for review Oct 22, 2022

targos added 2 commits Oct 22, 2022

hack: sse on win

74f916b

disable some workflows

4724f72

additional options

bd8beb3

remove macros

e63aacd

targos mentioned this pull request Oct 25, 2022

deps: update zlib to upstream 926ac23 #44412

Closed

Trott force-pushed the main branch from 2d76238 to ca3ed36 Compare Nov 12, 2022

Oct	NOV	Dec
	14
2021	2022	2023

		#include <smmintrin.h>
		#include <tmmintrin.h>

build: enable Clang-cl Windows builds #35433

Are you sure you want to change the base?

build: enable Clang-cl Windows builds #35433

Conversation

targos commented Sep 30, 2020

targos Sep 30, 2020

Choose a reason for hiding this comment

targos Sep 30, 2020

Choose a reason for hiding this comment

targos Sep 30, 2020

Choose a reason for hiding this comment

targos Sep 30, 2020

Choose a reason for hiding this comment

targos commented Sep 30, 2020 • edited

targos commented Oct 2, 2020

gengjiawen commented Oct 2, 2020

dennisameling commented Oct 3, 2020

gengjiawen commented Oct 19, 2020

gengjiawen commented Oct 19, 2020 • edited

targos commented Oct 19, 2020

targos commented Oct 19, 2020 • edited

targos commented Oct 19, 2020

gengjiawen commented Oct 19, 2020

targos commented Oct 19, 2020

gengjiawen commented Oct 19, 2020

This comment has been minimized.

gengjiawen commented Nov 9, 2020 • edited

dennisameling commented Nov 11, 2020

dennisameling commented Nov 11, 2020

gengjiawen commented Nov 11, 2020

dennisameling commented Nov 11, 2020 • edited

gengjiawen commented Dec 1, 2020

mmomtchev commented May 31, 2022

targos commented May 31, 2022

targos commented Oct 22, 2022

targos commented Oct 22, 2022

targos commented Oct 22, 2022

targos commented Oct 22, 2022

bnoordhuis commented Oct 22, 2022

targos commented Oct 22, 2022

bnoordhuis commented Oct 22, 2022

targos commented Oct 22, 2022

bnoordhuis commented Oct 22, 2022

targos commented Oct 22, 2022

targos commented Oct 22, 2022

targos commented Oct 22, 2022

bnoordhuis commented Oct 22, 2022

targos commented Sep 30, 2020 •

edited

gengjiawen commented Oct 19, 2020 •

edited

targos commented Oct 19, 2020 •

edited

gengjiawen commented Nov 9, 2020 •

edited

dennisameling commented Nov 11, 2020 •

edited