Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Update Windows builds to use ClangCL#549

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Draft
zanieb wants to merge5 commits intomain
base:main
Choose a base branch
Loading
fromzb/clangcl
Draft

Conversation

@zanieb
Copy link
Member

@zaniebzanieb commentedMar 4, 2025•
edited
Loading

Adding the flag as described inpython/cpython#129907

ClangCL looks to provide a 20% runtime performance and build times were purportedly better as well.

This also would let us turn on the tail-calling interpreter for another 10% performance boost.

chris-eibl and YukiNagat0 reacted with hooray emojisamypr100 and Fidget-Spinner reacted with heart emoji
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-SpinnerFidget-SpinnerMar 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).

zanieb and chris-eibl reacted with thumbs up emoji

Choose a reason for hiding this comment

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

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

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 forpython/cpython#130213, due to the problems with intrinsics.

zanieb reacted with thumbs up emoji
Copy link
MemberAuthor

@zaniebzaniebMar 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.

chris-eibl reacted with thumbs up emoji
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.

zanieb reacted with thumbs up emoji
@zanieb
Copy link
MemberAuthor

The latest benchmark is atpython/cpython#129907 (comment) which shows some regressions, but an average 19% improvement.

@zaniebzanieb added platform:windowsSpecific to the Windows platform python:3.14 and removed ci:skip labelsMar 17, 2025
@zanieb
Copy link
MemberAuthor

Interesting, it failed with

cpython>        (ClCompile target) ->cpython>          C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.43.34808\include\vcruntime.h(188,30): error : typedef redefinition with different types ('unsigned long long' vs 'unsigned int') [C:\Users\RUNNER~1\AppData\Local\Temp\python-build-73gmp1o2\Python-3.14.0a6\PCbuild\pythoncore.vcxproj]cpython>          C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\Llvm\lib\clang\19\include\__stddef_ptrdiff_t.h(18,26): error : typedef redefinition with different types ('int' vs 'long long') [C:\Users\RUNNER~1\AppData\Local\Temp\python-build-73gmp1o2\Python-3.14.0a6\PCbuild\pythoncore.vcxproj]cpython>          C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.43.34808\include\vcruntime.h(188,30): error : typedef redefinition with different types ('unsigned long long' vs 'unsigned int') [C:\Users\RUNNER~1\AppData\Local\Temp\python-build-73gmp1o2\Python-3.14.0a6\PCbuild\pythoncore.vcxproj]cpython>          C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\Llvm\lib\clang\19\include\__stddef_ptrdiff_t.h(18,26): error : typedef redefinition with different types ('int' vs 'long long') [C:\Users\RUNNER~1\AppData\Local\Temp\python-build-73gmp1o2\Python-3.14.0a6\PCbuild\pythoncore.vcxproj]

@chris-eibl
Copy link

AFAIR this means it is compiling in 323bit mode. Try adding-m64.

Seepython/cpython@263870d.

Interesting, according topython/cpython@d8a1cf4 this should be part ofv3.14.0a6

zanieb reacted with eyes emoji

@chris-eibl
Copy link

But I see it is passed e.g. here:

-flto=thin -m64 -Wno-deprecated-non-prototype -Wno-unused-label -Wno-pointer-sign -Wno-incompatible-pointer-types-discards-qualifiers -Wno-unused-function  "..\Modules\_heapqmodule.c"

but not here

 /Z7 /nologo /W3 /WX- /diagnostics:column /O2 /Oi /D HACL_CAN_COMPILE_VEC256 /D _Py_HAVE_ZLIB /D _USRDLL /D Py_BUILD_CORE /D Py_BUILD_CORE_BUILTIN /D Py_ENABLE_SHARED /D "MS_DLL_ID=\"3.14\"" /D _Py_USING_PGO=1 /D WIN32 /D "PY3_DLLNAME=L\"python3\"" /D _WIN64 /D NDEBUG /D PyStats /D _WINDLL /GF /MD /GS /Gy /fp:precise /Fo"C:\Users\RUNNER~1\AppData\Local\Temp\python-build-2aa_wvqi\Python-3.14.0a6\PCbuild\obj\314amd64_PGInstrument\pythoncore\\" /Gd /TC /arch:AVX2 ..\Modules\_hacl\Hacl_Hash_Blake2b_Simd256.c

But in the line above for\Hacl_Hash_Blake2s.c all the clang stuff is present like for_heapqmodule.c above :-o

@chris-eibl
Copy link

Try adding "/p:PreferredToolArchitecture=x64". Beforepython/cpython@263870d that was the trick that did it for me. Maybe it is still needed, although I back then did not need it anymore. That was a back and forth.

I will re-investigate ...

@zanieb
Copy link
MemberAuthor

We also do some non-standard things (like, we hack the project files), so it's possible it's a problem there.

@chris-eibl
Copy link

Interestingly, I can reproduce on cpython main and alpha6. I cannot build without"/p:PreferredToolArchitecture=x64" anymore, too.
Weird, but I have an idea.

Need some sleep though now :)

zanieb reacted with thumbs up emoji

@zanieb
Copy link
MemberAuthor

Cool that gets us to

cpython> copying C:\Users\RUNNER~1\AppData\Local\Temp\python-build-hctl79hu\Python-3.14.0a6\PCbuild\amd64\python314.expTraceback (most recent call last):  File "C:\a\python-build-standalone\python-build-standalone\cpython-windows\build.py", line 1816, in <module>    sys.exit(main())             ^^^^^^  File "C:\a\python-build-standalone\python-build-standalone\cpython-windows\build.py", line 1790, in main    tar_path = build_cpython(               ^^^^^^^^^^^^^^  File "C:\a\python-build-standalone\python-build-standalone\cpython-windows\build.py", line 1524, in build_cpython    build_info = collect_python_build_artifacts(                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^  File "C:\a\python-build-standalone\python-build-standalone\cpython-windows\build.py", line 1079, in collect_python_build_artifacts    shutil.copyfile(source, dest)  File "C:\hostedtoolcache\windows\Python\3.12.9\x64\Lib\shutil.py", line 260, in copyfile    with open(src, 'rb') as fsrc:         ^^^^^^^^^^^^^^^FileNotFoundError: [Errno 2] No such file or directory: 'C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\python-build-hctl79hu\\Python-3.14.0a6\\PCbuild\\amd64\\python314.exp'

which is from the copy at

exts= ("lib","exp")
forextinexts:
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)
shutil.copyfile(source,dest)

I presume this file is not produced upstream as well? I don't know if it's of consequence — I'm woefully lacking in knowledge of.exp files / Windows dynamic linking details.

@chris-eibl
Copy link

Ok, I've dug deeper this time - please seepython/cpython#131473 for details.

Regarding the*.exp file: most probably the gold linker does not write that, I did not find anything in the net, yet. Don't know whether this is a clang-cl compatibility issue or a wrong switch, etc, in the cPython build.

@zanieb
Copy link
MemberAuthor

A nice improvement in build times

Fidget-Spinner and chris-eibl reacted with laugh emoji

@Fidget-Spinner
Copy link

Tail calling for Windows clangcl has been mergedhttps://github.com/python/cpython/pull/130040/files#diff-4295039788386c2986b811f2a6b59cf088be59822b24b4f1243c9e9b184f65fbR109

chris-eibl and YukiNagat0 reacted with thumbs up emoji

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@indygregindygregindygreg left review comments

+2 more reviewers

@Fidget-SpinnerFidget-SpinnerFidget-Spinner left review comments

@chris-eiblchris-eiblchris-eibl left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

platform:windowsSpecific to the Windows platformpython:3.14

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@zanieb@chris-eibl@Fidget-Spinner@indygreg
[8]ページ先頭
©2009-2025 Movatter.jp