Resurrect Windows-2022 with LLVM 16 #1092

ohodson · 2023-08-30T09:16:37Z

This is required due to actions/runner-images#8125

An assortment of changes to enable building with the LLVM16 toolchain on Windows:

add Windows-2022 back to the test strategy matrix in test.yml.
add an LLVM upgrade step to test.yml to install LLVM 16
add command to test.yml to fix the include path for LLVM includes
(Bazel issue Wrong include path to Clang 16 on Windows bazelbuild/bazel#17863).
add a macro definition to .bazelrc to prefer __builtin_offsetof
(the offsetof macro without this is broken under LLVM16).
add enforced include path override to files including emmintrin.h.
clang-cl.exe in LLVM15 picks up the LLVM version of this file, but
LLVM16 was picking up the MSFT version and preventing the compiler
from replacing intrinsics. It looks like there is a change in
precedence between LLVM15 and LLVM16 for the include path setup by
Bazel in the INCLUDE environment variable.

Test: Install LLVM16, fix the LLVM include directory location for bazel, then
      run `bazel test //...`

ohodson · 2023-08-30T10:17:13Z

This PR likely requires a few rounds of iteration. It fails in CI and locally in the same way:

C:\Program Files\LLVM\bin\clang-cl.exe /nologo /DCOMPILER_MSVC /DNOMINMAX /D_WIN32_WINNT=0x0601 /D_CRT_SECURE_NO_DEPRECATE /D_CRT_SECURE_NO_WARNINGS /bigobj /Zm500 /EHsc /wd4351 /wd4291 /wd4250 /wd4996 /Iexternal/ssl /Ibazel-out/x64_windows-opt/bin/external/ssl /Iexternal/ssl/src/include /Ibazel-out/x64_windows-opt/bin/external/ssl/src/include /DBAZEL_CURRENT_REPOSITORY="ssl" /showIncludes /MD /O2 /Oy- /DNDEBUG /wd4117 -D__DATE__="redacted" -D__TIMESTAMP__="redacted" -D__TIME__="redacted" -Wno-builtin-macro-redefined -O0 /Od /INCREMENTAL:NO -DBORINGSSL_IMPLEMENTATION -DWIN32_LEAN_AND_MEAN -DOPENSSL_NO_ASM /Fobazel-out/x64_windows-opt/bin/external/ssl/_objs/crypto/hrss.obj /c external/ssl/src/crypto/hrss/hrss.c
# Configuration: cb3b06e277531bbfc54aa2173589df5a5a669a07dd29527bb79267982c66f4b4
# Execution platform: @local_config_platform//:host
clang-cl: warning: argument unused during compilation: '-O0' [-Wunused-command-line-argument]
external/ssl/src/crypto/hrss/hrss.c(104,8): error: invalid operands to binary expression ('vec_t' (aka 'union __m128i') and 'const __m128i' (aka 'const union __m128i'))
  v[1] |= carry0;
  ~~~~ ^  ~~~~~~
external/ssl/src/crypto/hrss/hrss.c(107,8): error: invalid operands to binary expression ('vec_t' (aka 'union __m128i') and 'const __m128i' (aka 'const union __m128i'))
  v[2] |= carry1;
  ~~~~ ^  ~~~~~~

These errors occur because the compilation is using a Windows SDK version of emmintrin.h rather than the clang version of emmintrin.h. Just before the compilation failure in the log we see:

  SET INCLUDE=C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.37.32822\include;C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.37.32822\ATLMFC\include;C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Auxiliary\VS\include;C:\Program Files (x86)\Windows Kits\10\include\10.0.22621.0\ucrt;C:\Program Files (x86)\Windows Kits\10\\include\10.0.22621.0\\um;C:\Program Files (x86)\Windows Kits\10\\include\10.0.22621.0\\shared;C:\Program Files (x86)\Windows Kits\10\\include\10.0.22621.0\\winrt;C:\Program Files (x86)\Windows Kits\10\\include\10.0.22621.0\\cppwinrt;C:\Program Files (x86)\Windows Kits\NETFXSDK\4.8\include\um;C:\Program Files\LLVM\lib\clang\16.0.6\include

Hopefully there is bazel option to say ignore Windows Kits and Visual Studio...

ohodson · 2023-08-30T13:08:13Z

Capturing the build output with --execution_log_json_file, there is no difference in the environment and commands used for compiling the file that's faililng, hrss.c for LLVM 15 and LLVM 16. Same includes, same paths, same everything up until the point where one compiless successfully and the other doesn not.

It looks like undefining OPENSSL_SSE2 in hrss.c allows the build to get almost to the end. There it is failing with some missing SSE2 intrinsics definitions (do not appear to be related to the tweak to hrss.c).

LLVM 16 release notes: https://releases.llvm.org/16.0.0/docs/ReleaseNotes.html

The INCLUDE env variable is defined as here for builds with either LLVM version:

Update: doh! I think we can probably specify per file headers for hrss.c that override the paths from the INCLUDE env var. It looks like LLVM 15 and LLVM 16 may treat the order there differently, e.g. LLVM 15 is picking up the LLVM includes (last in INCLUDE); LLVM 16 is picking up Windows Kits version and LLVM includes are still last in INCLUDE.

ohodson · 2023-08-30T16:33:00Z

As a temporary band-aid, this lets the build to get past the ssl compilation failure:

build:windows --per_file_copt='external/ssl/src/crypto/hrss.*/.*\.c@-imsvcC:\\Program Files\\LLVM\\lib\\clang\\16.0.6\\include'

but will require Windows devs to update the LLVM version and is super fragile.

https://clang.llvm.org/docs/UsersManual.html#clang-cl

The next failure after this appears to be relatively old intrinsics in V8 code not being replaced by the compiler. Adding /arch:AVX and /arch:AVX2 does not fix things. Intrinsics left as undefined symbols are:

_mm_castsi128_ps (SSE3), _mm_cmpeq_epi8 (SSE2), _mm_cmpeq_epi32 (SSE2), _mm_cmpeq_pd (SSE2),
_mm_loadu_si128 (SSE2), _mm_movemask_epi8 (SSE2), _mm_movemask_pd (SSE2), _mm_movemask_ps (SSE),
_mm_set1_epi8 (SSE2), _mm_set1_epi32 (SSE2), _mm_set1_pd (SSE2), _mm_sign_epi8 (SSE3)

Example error:

lld-link: error: undefined symbol: _mm_cmpeq_epi8
>>> referenced by C:\tmp\rp4ezqc4\execroot\workerd\external\v8\src\objects\swiss-hash-table-helpers.h:232
>>>               v8_libshared_icu.lo.lib(literal-objects.obj):(public: class v8::internal::swiss_table::BitMask<unsigned int, 16, 0> __cdecl v8::internal::swiss_table::GroupSse2Impl::Match(unsigned char) const)
>>> referenced by v8_libshared_icu.lo.lib(runtime-object.obj)

ohodson · 2023-08-30T20:20:15Z

Progress: manually setting V8_SWISS_TABLE_HAVE...{HOST,TABLE} to 0 in swiss-hash-table-helpers.h fixes the missing intrinsics when linking mksnapshot, but exposes similar intrinsics issues with external/ssl when linking workerd.

Kludging these out seems doable, but not desirable. Here's a couple of likely related discussions of intrinsics and clang-cl:

clang-cl cannot compile Windows source files when SIMD intrinsics are used llvm/llvm-project#63492
clang-cl can not support function targets llvm/llvm-project#53520

Next step, rebuild with LLVM 15 and disassemble the binaries to grok whether intrinsics were transformed appropriately, or something else (predefined macros) has changed, between 15 and 16.

ohodson · 2023-08-31T09:19:35Z

Next step, rebuild with LLVM 15 and disassemble the binaries to grok whether intrinsics were transformed appropriately, or something else (predefined macros) has changed, between 15 and 16.

The Intel Intrinsics Guide maps intrinsics to instructions that show up in the workerd disassembly.

dumpbin /disasm workerd.exe | find "pmovmskb" # _mm_movemask_epi8 (__m128i a) FOUND
dumpbin /disasm workerd.exe | find "pcmpeqb" # __m128i _mm_cmpeq_epi8 (__m128i a, __m128i b) FOUND

Two V8 object files that reported linkage errors for intrinsics (e.g. _mm_movemask_epi8 == pmovmskb) with LLVM 16 are runtime-object.obj and swiss-name-dictionary.obj, ditto literal-objects.obj and _mm_cmpeq_epi8. The LLVM15 binaries have the intrinsics replaced with corresponding instructions, e.g.

> dumpbin /disasm bazel-bin\external\v8\_objs\v8_libshared_icu\runtime-object.obj | find "pmov"
  00000000000001EF: 66 0F D7 D0        pmovmskb    edx,xmm0
> dumpbin /disasm bazel-bin\external\v8\_objs\v8_libshared_icu\swiss-name-dictionary.obj | find "pmov"
  00000000000001EF: 66 0F D7 D0        pmovmskb    edx,xmm0
> dumpbin /disasm bazel-bin\external\v8\_objs\v8_libshared_icu\literal-objects.obj | find "pcmp"
  00000000000001DF: 66 0F 74 C1        pcmpeqb     xmm0,xmm1

It looks like clang-cl.exe in LLVM 16 is not substituting intrinsics in these files and leaving functions to be linked.

ohodson · 2023-08-31T13:51:41Z

This passes locally and has passed on github, but the last build flaked out fetching dependencies on Linux...

jasnell

LGTM but let's keep the branch protection turned off for now while waiting for the official fix.

.github/workflows/test.yml

.bazelrc

This is required due to actions/runner-images#8125 An assortment of changes to enable building with the LLVM16 toolchain on Windows: - add Windows-2022 back to the test strategy matrix in test.yml. - add an LLVM upgrade step to test.yml to install LLVM 16 - add command to test.yml to fix the include path for LLVM includes (Bazel issue bazelbuild/bazel#17863). - add a macro definition to .bazelrc to prefer __builtin_offsetof (the offsetof macro without this is broken under LLVM16). - add enforced include path override to files including emmintrin.h. clang-cl.exe in LLVM15 picks up the LLVM version of this file, but LLVM16 was picking up the MSFT version and preventing the compiler from replacing intrinsics. It looks like there is a change in precedence between LLVM15 and LLVM16 for the include path setup by Bazel in the INCLUDE environment variable. Test: Install LLVM16, fix the LLVM include directory location for bazel, then run `bazel test //...`

ohodson · 2023-09-01T11:24:28Z

LGTM but let's keep the branch protection turned off for now while waiting for the official fix.

https://github.com/bazelbuild/bazel/pull/19391🤞

Sync with .bazelrc changes in d9338a6 (#1092) The script now installs LLVM 16.0.6 and moves the clang include dir to match the expectations in .bazelrc. Hopefully, the .bazelrc changes can be dropped soon, but in the interim ensure local setup and build continues to work.

ohodson mentioned this pull request Aug 30, 2023

Temporary update to LLVM 16 on Windows #1079

Closed

ohodson mentioned this pull request Aug 30, 2023

Bump compatibility date for release #1094

Merged

ohodson force-pushed the orion/Windows-LLVM-16 branch from e8e1514 to 5706ae1 Compare August 30, 2023 16:51

ohodson marked this pull request as draft August 31, 2023 08:50

ohodson force-pushed the orion/Windows-LLVM-16 branch 2 times, most recently from b066823 to 830ebad Compare August 31, 2023 12:27

ohodson marked this pull request as ready for review August 31, 2023 12:36

ohodson force-pushed the orion/Windows-LLVM-16 branch from 830ebad to ff7b832 Compare August 31, 2023 13:38

ohodson changed the title ~~Resurrect Windows-2022 with LLVM 16 [WIP]~~ Resurrect Windows-2022 with LLVM 16 Aug 31, 2023

ohodson requested review from fhanau and jasnell August 31, 2023 13:44

ohodson requested a review from penalosa August 31, 2023 14:39

jasnell approved these changes Aug 31, 2023

View reviewed changes

fhanau approved these changes Aug 31, 2023

View reviewed changes

penalosa reviewed Aug 31, 2023

View reviewed changes

.github/workflows/test.yml Show resolved Hide resolved

ohodson force-pushed the orion/Windows-LLVM-16 branch 2 times, most recently from 3ebdb84 to b54a83c Compare August 31, 2023 15:04

ohodson commented Aug 31, 2023

View reviewed changes

.bazelrc Outdated Show resolved Hide resolved

ohodson force-pushed the orion/Windows-LLVM-16 branch from cab2aa5 to 047ab30 Compare August 31, 2023 15:08

ohodson mentioned this pull request Aug 31, 2023

Wrong include path to Clang 16 on Windows bazelbuild/bazel#17863

Closed

ohodson force-pushed the orion/Windows-LLVM-16 branch from 047ab30 to 60d9cb7 Compare August 31, 2023 16:54

ohodson closed this Sep 1, 2023

ohodson reopened this Sep 1, 2023

ohodson force-pushed the orion/Windows-LLVM-16 branch from 4873b05 to d9338a6 Compare September 1, 2023 09:00

ohodson merged commit 40509ea into main Sep 1, 2023
7 checks passed

ohodson deleted the orion/Windows-LLVM-16 branch September 1, 2023 09:14

ohodson mentioned this pull request Sep 1, 2023

Update install-deps.bat to LLVM 16 #1112

Merged

fhanau mentioned this pull request Jan 10, 2024

Explore using LLVM17 on Windows runner #1528

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Resurrect Windows-2022 with LLVM 16 #1092

Resurrect Windows-2022 with LLVM 16 #1092

ohodson commented Aug 30, 2023 •

edited

Loading

ohodson commented Aug 30, 2023

ohodson commented Aug 30, 2023 •

edited

Loading

ohodson commented Aug 30, 2023 •

edited

Loading

ohodson commented Aug 30, 2023 •

edited

Loading

ohodson commented Aug 31, 2023 •

edited

Loading

ohodson commented Aug 31, 2023

jasnell left a comment

ohodson commented Sep 1, 2023

Resurrect Windows-2022 with LLVM 16 #1092

Resurrect Windows-2022 with LLVM 16 #1092

Conversation

ohodson commented Aug 30, 2023 • edited Loading

ohodson commented Aug 30, 2023

ohodson commented Aug 30, 2023 • edited Loading

ohodson commented Aug 30, 2023 • edited Loading

ohodson commented Aug 30, 2023 • edited Loading

ohodson commented Aug 31, 2023 • edited Loading

ohodson commented Aug 31, 2023

jasnell left a comment

Choose a reason for hiding this comment

ohodson commented Sep 1, 2023

ohodson commented Aug 30, 2023 •

edited

Loading

ohodson commented Aug 30, 2023 •

edited

Loading

ohodson commented Aug 30, 2023 •

edited

Loading

ohodson commented Aug 30, 2023 •

edited

Loading

ohodson commented Aug 31, 2023 •

edited

Loading