Skip to content

Commit

Permalink
build: add option to enable clang-cl on Windows
Browse files Browse the repository at this point in the history
Most changes are gated by the `clang==1` condition to avoid breaking
MSVC builds.

Select C/C++ language standard with ClCompile options.
This avoids passing the `-std:c++20` flag while compiling C code.
Do it only under clang option to avoid breaking addons until node-gyp
supports the new LanguageStandard options.

Disable precompiled header configuration for now as it doesn't seem to
work with clang-cl.

Disable C++20 warnings emitted by the Visual Studio C++ STL.
They're very noisy and not our responsibility to fix.

Co-authored-by: Daniel Lemire <[email protected]>
Co-authored-by: Stefan Stojanovic <[email protected]>
PR-URL: #52870
Reviewed-By: Daniel Lemire <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
  • Loading branch information
3 people committed May 15, 2024
1 parent 609d90b commit a97ff75
Show file tree
Hide file tree
Showing 8 changed files with 120 additions and 53 deletions.
32 changes: 26 additions & 6 deletions common.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,9 @@
'cflags': [ '-fPIC' ],
'ldflags': [ '-fPIC' ]
}],
['clang==1', {
'msbuild_toolset': 'ClangCL',
}],
],
'msvs_settings': {
'VCCLCompilerTool': {
Expand Down Expand Up @@ -240,6 +243,9 @@
'cflags': [ '-fPIC', '-I<(android_ndk_path)/sources/android/cpufeatures' ],
'ldflags': [ '-fPIC' ]
}],
['clang==1', {
'msbuild_toolset': 'ClangCL',
}],
],
'msvs_settings': {
'VCCLCompilerTool': {
Expand Down Expand Up @@ -282,12 +288,26 @@
],
'msvs_settings': {
'VCCLCompilerTool': {
'AdditionalOptions': [
'/Zc:__cplusplus',
# The following option enables c++20 on Windows. This is needed for V8 v12.4+
'-std:c++20',
# The following option reduces the "error C1060: compiler is out of heap space"
'/Zm2000',
# TODO(targos): Remove condition and always use LanguageStandard options
# once node-gyp supports them.
'conditions': [
['clang==1', {
'LanguageStandard': 'stdcpp20',
'LanguageStandard_C': 'stdc11',
'AdditionalOptions': [
'/Zc:__cplusplus',
# The following option reduces the "error C1060: compiler is out of heap space"
'/Zm2000',
],
}, {
'AdditionalOptions': [
'/Zc:__cplusplus',
# The following option enables c++20 on Windows. This is needed for V8 v12.4+
'-std:c++20',
# The following option reduces the "error C1060: compiler is out of heap space"
'/Zm2000',
],
}],
],
'BufferSecurityCheck': 'true',
'DebugInformationFormat': 1, # /Z7 embed info in .obj files
Expand Down
16 changes: 14 additions & 2 deletions configure.py
Original file line number Diff line number Diff line change
Expand Up @@ -946,6 +946,13 @@
default=None,
help=argparse.SUPPRESS)

parser.add_argument('--clang-cl',
action='store',
dest='clang_cl',
default=None,
help='Configure for clang-cl on Windows. This flag sets the GYP "clang" ' +
'variable to 1 and "llvm_version" to the specified value.')

(options, args) = parser.parse_known_args()

# Expand ~ in the install prefix now, it gets written to multiple files.
Expand Down Expand Up @@ -1122,8 +1129,13 @@ def get_gas_version(cc):
# quite prepared to go that far yet.
def check_compiler(o):
if sys.platform == 'win32':
o['variables']['clang'] = 0
o['variables']['llvm_version'] = '0.0'
if options.clang_cl:
o['variables']['clang'] = 1
o['variables']['llvm_version'] = options.clang_cl
else:
o['variables']['clang'] = 0
o['variables']['llvm_version'] = '0.0'

if not options.openssl_no_asm and options.dest_cpu in ('x86', 'x64'):
nasm_version = get_nasm_version('nasm')
o['variables']['nasm_version'] = nasm_version
Expand Down
5 changes: 5 additions & 0 deletions deps/zlib/zlib.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@
'xcode_settings': {
'OTHER_CFLAGS': [ '-mssse3' ],
},
'msvs_settings': {
'VCCLCompilerTool': {
'AdditionalOptions': [ '-mssse3' ],
},
},
}],
],
}],
Expand Down
4 changes: 2 additions & 2 deletions node.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -610,8 +610,8 @@
'msvs_settings': {
'VCLinkerTool': {
'AdditionalOptions': [
'/WHOLEARCHIVE:<(node_lib_target_name)<(STATIC_LIB_SUFFIX)',
'/WHOLEARCHIVE:<(STATIC_LIB_PREFIX)v8_base_without_compiler<(STATIC_LIB_SUFFIX)',
'/WHOLEARCHIVE:<(PRODUCT_DIR)/lib/<(node_lib_target_name)<(STATIC_LIB_SUFFIX)',
'/WHOLEARCHIVE:<(PRODUCT_DIR)/lib/<(STATIC_LIB_PREFIX)v8_base_without_compiler<(STATIC_LIB_SUFFIX)',
],
},
},
Expand Down
20 changes: 12 additions & 8 deletions node.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -65,11 +65,15 @@
'NODE_PLATFORM="win32"',
'_UNICODE=1',
],
'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)',
'conditions': [
['clang==0', {
'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)',
],
}],
],
}, { # POSIX
'defines': [ '__POSIX__' ],
Expand Down Expand Up @@ -148,7 +152,7 @@
'msvs_settings': {
'VCLinkerTool': {
'AdditionalOptions': [
'/WHOLEARCHIVE:zlib<(STATIC_LIB_SUFFIX)',
'/WHOLEARCHIVE:<(PRODUCT_DIR)/lib/zlib<(STATIC_LIB_SUFFIX)',
],
},
},
Expand Down Expand Up @@ -187,7 +191,7 @@
'msvs_settings': {
'VCLinkerTool': {
'AdditionalOptions': [
'/WHOLEARCHIVE:libuv<(STATIC_LIB_SUFFIX)',
'/WHOLEARCHIVE:<(PRODUCT_DIR)/lib/libuv<(STATIC_LIB_SUFFIX)',
],
},
},
Expand Down Expand Up @@ -370,7 +374,7 @@
'msvs_settings': {
'VCLinkerTool': {
'AdditionalOptions': [
'/WHOLEARCHIVE:<(openssl_product)',
'/WHOLEARCHIVE:<(PRODUCT_DIR)/lib/<(openssl_product)',
],
},
},
Expand Down
6 changes: 6 additions & 0 deletions tools/v8_gypfiles/toolchain.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,11 @@
# -Wno-invalid-offsetof
'GCC_WARN_ABOUT_INVALID_OFFSETOF_MACRO': 'NO',
},
'msvs_settings': {
'VCCLCompilerTool': {
'AdditionalOptions': ['-Wno-invalid-offsetof'],
},
},
}],
['v8_target_arch=="arm"', {
'defines': [
Expand Down Expand Up @@ -536,6 +541,7 @@
'WIN32',
'NOMINMAX', # Refs: https://chromium-review.googlesource.com/c/v8/v8/+/1456620
'_WIN32_WINNT=0x0602', # Windows 8
'_SILENCE_ALL_CXX20_DEPRECATION_WARNINGS',
],
# 4351: VS 2005 and later are warning us that they've fixed a bug
# present in VS 2003 and earlier.
Expand Down
84 changes: 50 additions & 34 deletions tools/v8_gypfiles/v8.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
'type': 'none',
'toolsets': ['host', 'target'],
'conditions': [
['OS=="win"', {
['OS=="win" and clang==0', {
'direct_dependent_settings': {
'msvs_precompiled_header': '<(V8_ROOT)/../../tools/msvs/pch/v8_pch.h',
'msvs_precompiled_source': '<(V8_ROOT)/../../tools/msvs/pch/v8_pch.cc',
Expand Down Expand Up @@ -381,38 +381,6 @@
'target_name': 'v8_snapshot',
'type': 'static_library',
'toolsets': ['target'],
'conditions': [
['want_separate_host_toolset', {
'conditions': [
['v8_target_arch=="arm64"', {
'msvs_enable_marmasm': 1,
}]
],
'dependencies': [
'generate_bytecode_builtins_list',
'run_torque',
'mksnapshot#host',
'v8_maybe_icu',
# [GYP] added explicitly, instead of inheriting from the other deps
'v8_base_without_compiler',
'v8_compiler_for_mksnapshot',
'v8_initializers',
'v8_libplatform',
]
}, {
'dependencies': [
'generate_bytecode_builtins_list',
'run_torque',
'mksnapshot',
'v8_maybe_icu',
# [GYP] added explicitly, instead of inheriting from the other deps
'v8_base_without_compiler',
'v8_compiler_for_mksnapshot',
'v8_initializers',
'v8_libplatform',
]
}],
],
'sources': [
'<(V8_ROOT)/src/init/setup-isolate-deserialize.cc',
],
Expand Down Expand Up @@ -488,6 +456,54 @@
],
},
],
'conditions': [
['want_separate_host_toolset', {
'dependencies': [
'generate_bytecode_builtins_list',
'run_torque',
'mksnapshot#host',
'v8_maybe_icu',
# [GYP] added explicitly, instead of inheriting from the other deps
'v8_base_without_compiler',
'v8_compiler_for_mksnapshot',
'v8_initializers',
'v8_libplatform',
]
}, {
'dependencies': [
'generate_bytecode_builtins_list',
'run_torque',
'mksnapshot',
'v8_maybe_icu',
# [GYP] added explicitly, instead of inheriting from the other deps
'v8_base_without_compiler',
'v8_compiler_for_mksnapshot',
'v8_initializers',
'v8_libplatform',
]
}],
['OS=="win" and clang==1', {
'actions': [
{
'action_name': 'asm_to_inline_asm',
'message': 'generating: >@(_outputs)',
'inputs': [
'<(INTERMEDIATE_DIR)/embedded.S',
],
'outputs': [
'<(INTERMEDIATE_DIR)/embedded.cc',
],
'process_outputs_as_sources': 1,
'action': [
'<(python)',
'<(V8_ROOT)/tools/snapshot/asm_to_inline_asm.py',
'<@(_inputs)',
'<(INTERMEDIATE_DIR)/embedded.cc',
],
},
],
}],
],
}, # v8_snapshot
{
'target_name': 'v8_version',
Expand Down Expand Up @@ -1928,7 +1944,7 @@
}],
]
}],
['OS=="win"', {
['OS=="win" and clang==0', {
'conditions': [
['_toolset == "host" and host_arch == "x64" or _toolset == "target" and target_arch=="x64"', {
'sources': [
Expand Down
6 changes: 5 additions & 1 deletion vcbuild.bat
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ set ltcg=
set target_env=
set noprojgen=
set projgen=
set clang_cl=
set nobuild=
set sign=
set nosnapshot=
Expand Down Expand Up @@ -87,6 +88,7 @@ if /i "%1"=="arm64" set target_arch=arm64&goto arg-ok
if /i "%1"=="vs2022" set target_env=vs2022&goto arg-ok
if /i "%1"=="noprojgen" set noprojgen=1&goto arg-ok
if /i "%1"=="projgen" set projgen=1&goto arg-ok
if /i "%1"=="clang-cl" set clang_cl=1&goto arg-ok
if /i "%1"=="nobuild" set nobuild=1&goto arg-ok
if /i "%1"=="nosign" set "sign="&echo Note: vcbuild no longer signs by default. "nosign" is redundant.&goto arg-ok
if /i "%1"=="sign" set sign=1&goto arg-ok
Expand Down Expand Up @@ -190,6 +192,8 @@ if defined nosnapshot set configure_flags=%configure_flags% --without-snap
if defined nonpm set configure_flags=%configure_flags% --without-npm
if defined nocorepack set configure_flags=%configure_flags% --without-corepack
if defined ltcg set configure_flags=%configure_flags% --with-ltcg
:: If clang-cl build is requested, set it to 17.0, which is the version shipped with VS 2022.
if defined clang_cl set configure_flags=%configure_flags% --clang-cl=17.0
if defined release_urlbase set configure_flags=%configure_flags% --release-urlbase=%release_urlbase%
if defined download_arg set configure_flags=%configure_flags% %download_arg%
if defined enable_vtune_arg set configure_flags=%configure_flags% --enable-vtune-profiling
Expand Down Expand Up @@ -750,7 +754,7 @@ set exit_code=1
goto exit

:help
echo vcbuild.bat [debug/release] [msi] [doc] [test/test-all/test-addons/test-doc/test-js-native-api/test-node-api/test-internet/test-tick-processor/test-known-issues/test-node-inspect/test-check-deopts/test-npm/test-v8/test-v8-intl/test-v8-benchmarks/test-v8-all] [ignore-flaky] [static/dll] [noprojgen] [projgen] [small-icu/full-icu/without-intl] [nobuild] [nosnapshot] [nonpm] [nocorepack] [ltcg] [licensetf] [sign] [ia32/x86/x64/arm64] [vs2022] [download-all] [enable-vtune] [lint/lint-ci/lint-js/lint-md] [lint-md-build] [format-md] [package] [build-release] [upload] [no-NODE-OPTIONS] [link-module path-to-module] [debug-http2] [debug-nghttp2] [clean] [cctest] [no-cctest] [openssl-no-asm]
echo vcbuild.bat [debug/release] [msi] [doc] [test/test-all/test-addons/test-doc/test-js-native-api/test-node-api/test-internet/test-tick-processor/test-known-issues/test-node-inspect/test-check-deopts/test-npm/test-v8/test-v8-intl/test-v8-benchmarks/test-v8-all] [ignore-flaky] [static/dll] [noprojgen] [projgen] [clang-cl] [small-icu/full-icu/without-intl] [nobuild] [nosnapshot] [nonpm] [nocorepack] [ltcg] [licensetf] [sign] [ia32/x86/x64/arm64] [vs2022] [download-all] [enable-vtune] [lint/lint-ci/lint-js/lint-md] [lint-md-build] [format-md] [package] [build-release] [upload] [no-NODE-OPTIONS] [link-module path-to-module] [debug-http2] [debug-nghttp2] [clean] [cctest] [no-cctest] [openssl-no-asm]
echo Examples:
echo vcbuild.bat : builds release build
echo vcbuild.bat debug : builds debug build
Expand Down

0 comments on commit a97ff75

Please sign in to comment.