Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Temporary update to LLVM 16 on Windows #1079

Closed
wants to merge 2 commits into from
Closed

Conversation

ohodson
Copy link
Contributor

@ohodson ohodson commented Aug 29, 2023

No description provided.

Copy link
Member

@jasnell jasnell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming the experiment works, this LGTM

@ohodson
Copy link
Contributor Author

ohodson commented Aug 29, 2023 via email

@ohodson
Copy link
Contributor Author

ohodson commented Aug 29, 2023

Github action language...my PR here breaks Linux / Mac which then fail before the Windows build can get going...would like to specialize the execution flow for Windows vs Unix...it's going to be trivial once you know the runes...

@ohodson ohodson force-pushed the orion/runner-experiment branch 2 times, most recently from c42b1b7 to 12a23e5 Compare August 29, 2023 15:20
@fhanau
Copy link
Collaborator

fhanau commented Aug 29, 2023

Issue is actions/runner-images#8125 (comment)

@ohodson ohodson force-pushed the orion/runner-experiment branch 2 times, most recently from 58aa182 to acaf3fb Compare August 29, 2023 16:05
@ohodson ohodson changed the title Experiment adding bazel-env.bat to Windows bazel build steps Temporary update to LLVM 16 on Windows Aug 29, 2023
@ohodson
Copy link
Contributor Author

ohodson commented Aug 29, 2023

Still failing with LLVM 16:

ERROR: C:/users/runneradmin/_bazel_runneradmin/hs7imhxj/external/capnp-cpp/src/kj/BUILD.bazel:5:11: Compiling src/kj/units.c++ failed: undeclared inclusion(s) in rule '@capnp-cpp//src/kj:kj':
this rule is missing dependency declarations for the following files included by 'src/kj/units.c++':
  'C:/Program Files/LLVM/lib/clang/16/include/stddef.h'
  'C:/Program Files/LLVM/lib/clang/16/include/__stddef_max_align_t.h'
  'C:/Program Files/LLVM/lib/clang/16/include/vadefs.h'
  'C:/Program Files/LLVM/lib/clang/16/include/stddef.h'
  'C:/Program Files/LLVM/lib/clang/16/include/intrin.h'
  'C:/Program Files/LLVM/lib/clang/16/include/x86intrin.h'
  'C:/Program Files/LLVM/lib/clang/16/include/ia32intrin.h'
  'C:/Program Files/LLVM/lib/clang/16/include/immintrin.h'
  'C:/Program Files/LLVM/lib/clang/16/include/x86gprintrin.h'

@ohodson
Copy link
Contributor Author

ohodson commented Aug 29, 2023

Compiling src/kj/units.c++ failed: undeclared inclusion(s) in rule '@capnp-cpp//src/kj:kj':
this rule is missing dependency declarations for the following files included by

Looks similar to #41, adding test.yml to the files hashed for the cache to invalidate it.

@ohodson ohodson force-pushed the orion/runner-experiment branch 2 times, most recently from e375991 to 0de0d83 Compare August 29, 2023 17:09
Recent github runner config changes for Windows breaks use of LLVM 15
and does not install LLVM 16 in the image.

This change adds an LLVM upgrade step that should pull in LLVM 16. This
follows scikit-image/scikit-image#7109.

Bug: actions/runner-images#812
@ohodson
Copy link
Contributor Author

ohodson commented Aug 29, 2023

From experimenting locally at home, this issue repros perfectly after upgrading to LLVM 16 (winget upgrade LLVM).

The underlying problem with LLVM 16 has been reported here:
bazelbuild/bazel#17863

Renaming C:\Program Files\LLVM\lib\clang\16 to C:\Program Files\LLVM\lib\clang\16.0.6 unblocks the errors we've seen in our runner when trying to move to LLVM16, but then we have new static assertion failures. Without looking into this any further today, it may be that a later Windows Kit does not have this issue.

external/v8/src/base/platform/platform-win32.cc(52,15): error: static assertion expression is not an integral constant expression
static_assert(offsetof(V8_CRITICAL_SECTION, DebugInfo) ==
              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
C:\Program Files (x86)\Windows Kits\10\include\10.0.22000.0\ucrt\stddef.h(47,31): note: expanded from macro 'offsetof'      
        #define offsetof(s,m) ((::size_t)&reinterpret_cast<char const volatile&>((((s*)0)->m)))
                              ^
external/v8/src/base/platform/platform-win32.cc(52,15): note: cast that performs the conversions of a reinterpret_cast is not allowed in a constant expression
C:\Program Files (x86)\Windows Kits\10\include\10.0.22000.0\ucrt\stddef.h(47,32): note: expanded from macro 'offsetof'      
        #define offsetof(s,m) ((::size_t)&reinterpret_cast<char const volatile&>((((s*)0)->m)))
                               ^
external/v8/src/base/platform/platform-win32.cc(54,15): error: static assertion expression is not an integral constant expression
static_assert(offsetof(V8_CRITICAL_SECTION, LockCount) ==
              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
C:\Program Files (x86)\Windows Kits\10\include\10.0.22000.0\ucrt\stddef.h(47,31): note: expanded from macro 'offsetof'      
        #define offsetof(s,m) ((::size_t)&reinterpret_cast<char const volatile&>((((s*)0)->m)))
                              ^
external/v8/src/base/platform/platform-win32.cc(54,15): note: cast that performs the conversions of a reinterpret_cast is not allowed in a constant expression
C:\Program Files (x86)\Windows Kits\10\include\10.0.22000.0\ucrt\stddef.h(47,32): note: expanded from macro 'offsetof'      
        #define offsetof(s,m) ((::size_t)&reinterpret_cast<char const volatile&>((((s*)0)->m)))

@ohodson
Copy link
Contributor Author

ohodson commented Aug 30, 2023

Dropping this in favour of #1092

@ohodson ohodson closed this Aug 30, 2023
@ohodson ohodson deleted the orion/runner-experiment branch September 8, 2023 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants