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.

Sign up for GitHub

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

Update Windows builds to use ClangCL #549

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

Update Windows builds to use ClangCL #549

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
16c9a50
Update Windows builds to use ClangCL
zanieb Mar 4, 2025
2636c55
debug: Try setting some clang compatibility flags?
zanieb Mar 18, 2025
4033e0d
debug: explicitly set `-m64`?
zanieb Mar 18, 2025
0935643
debug: try `"/p:PreferredToolArchitecture=x64"`
zanieb Mar 18, 2025
10a69b0
Drop "exp" file
zanieb Mar 18, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions cpython-windows/build.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -653,6 +653,10 @@ def run_msbuild(
# This can also work around known incompatibilities with the Windows 11
# SDK as of at least CPython 3.9.7.
f"/property:DefaultWindowsSDKVersion={windows_sdk_version}",
# Use ClangCL for better build and runtime performance
# https://github.com/python/cpython/issues/130090
"/p:PlatformToolset=ClangCL",
Copy link

@Fidget-Spinner Fidget-Spinner Mar 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Probably should guard only on 3.14. (It's the first version with Clang PGO, that will guarantee better performance).

Copy link

Choose a reason for hiding this comment

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

Also, requires python/cpython#130040 to pass on CI. For some reason there's a one line patch required.

Copy link

Choose a reason for hiding this comment

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

If you are using clang versions below 19, this will furthermore need a fix for python/cpython#130213, due to the problems with intrinsics.

Copy link
Member Author

@zanieb zanieb Mar 9, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

We're using LLVM 19 here (and presumably 20 soon — #553) actually, realized we're only setting up a LLVM toolchain on Unix right now. I will look into the version we're using on Windows.

Copy link
Collaborator

Choose a reason for hiding this comment

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

We should teach the build system to pull a pre-built LLVM toolchain from somewhere. The official releases from e.g. https://github.com/llvm/llvm-project/releases/tag/llvmorg-20.1.0 should be fine.

If we use the toolchain built into the GitHub Actions Runner, it will randomly be updated out from under us. Given some sensitivity we've seen around LLVM versions lately, this spooky-action-at-a-distance will likely randomly cause CI to fail.

Best to make CI as deterministic and reproducible as possible.

"/p:PreferredToolArchitecture=%s" % platform,
]

if freethreaded:
Expand Down Expand Up @@ -1069,6 +1073,13 @@ def find_additional_dependencies(project: pathlib.Path):
exts = ("lib", "exp")

for ext in exts:
# On 3.14+, we're building with ClangCL which does not produce `.exp`
# files.
if ext == "exp" and python_majmin == "314":
# TODO(zanieb): Scope this specifically to use of ClangCL instead of
# MSVC / determine if it's a bug that this file is missing.
continue

source = outputs_path / ("python%s%s.%s" % (python_majmin, lib_suffix, ext))
dest = core_dir / ("python%s%s.%s" % (python_majmin, lib_suffix, ext))
log("copying %s" % source)
Expand Down