From 859c89f5e7874ff9212928200762ed108026a638 Mon Sep 17 00:00:00 2001 From: Chris Eibl <138194463+chris-eibl@users.noreply.github.com> Date: Sun, 9 Feb 2025 16:30:38 +0100 Subject: [PATCH 01/21] minimal changes needed for PGO with clang-cl --- PCbuild/pyproject.props | 6 +++++- PCbuild/pythoncore.vcxproj | 13 +++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/PCbuild/pyproject.props b/PCbuild/pyproject.props index dbdb6b743bea37..d945afade9eb47 100644 --- a/PCbuild/pyproject.props +++ b/PCbuild/pyproject.props @@ -10,6 +10,8 @@ $(MSBuildThisFileDirectory)obj\ $(Py_IntDir)\$(MajorVersionNumber)$(MinorVersionNumber)$(ArchName)_$(Configuration)\$(ProjectName)\ $(IntDir.Replace(`\\`, `\`)) + $(Py_IntDir)\$(MajorVersionNumber)$(MinorVersionNumber)$(ArchName)_PGInstrument\__clang_profiles\ + $(ClangProfileDir.Replace(`\\`, `\`)) $(Py_IntDir)\$(MajorVersionNumber)$(MinorVersionNumber)$(ArchName)_$(Configuration)\pythoncore\ $(Py_IntDir)\$(MajorVersionNumber)$(MinorVersionNumber)_frozen\ @@ -70,6 +72,8 @@ /utf-8 %(AdditionalOptions) -Wno-deprecated-non-prototype -Wno-unused-label -Wno-pointer-sign -Wno-incompatible-pointer-types-discards-qualifiers -Wno-unused-function %(AdditionalOptions) -flto %(AdditionalOptions) + -fprofile-instr-generate=$(ClangProfileDir)\default_%m.profraw %(AdditionalOptions) + -fprofile-instr-use=$(ClangProfileDir)\profdata.profdata %(AdditionalOptions) -d2pattern-opt-disable:-932189325 %(AdditionalOptions) -d2ssa-patterns-all- %(AdditionalOptions) /sourceDependencies "$(IntDir.Trim(`\`))" %(AdditionalOptions) @@ -185,7 +189,7 @@ public override bool Execute() { Targets="CleanAll" /> - + <_PGCFiles Include="$(OutDir)instrumented\$(TargetName)!*.pgc" /> <_PGDFile Include="$(OutDir)instrumented\$(TargetName).pgd" /> diff --git a/PCbuild/pythoncore.vcxproj b/PCbuild/pythoncore.vcxproj index 9ebf58ae8a9bc4..b05c20d638a694 100644 --- a/PCbuild/pythoncore.vcxproj +++ b/PCbuild/pythoncore.vcxproj @@ -420,6 +420,7 @@ HACL_CAN_COMPILE_SIMD128;%(PreprocessorDefinitions) HACL_CAN_COMPILE_SIMD256;%(PreprocessorDefinitions) + /arch:AVX @@ -716,6 +717,18 @@ + + + + + + + + + git From a529c3905a59453d6986c9b4d5cfcf1206f10067 Mon Sep 17 00:00:00 2001 From: Chris Eibl <138194463+chris-eibl@users.noreply.github.com> Date: Sun, 9 Feb 2025 16:32:36 +0100 Subject: [PATCH 02/21] like for MSVC, don't use link time optimization for _freeze_module in case of clang-cl to speed up the build --- PCbuild/_freeze_module.vcxproj | 1 + 1 file changed, 1 insertion(+) diff --git a/PCbuild/_freeze_module.vcxproj b/PCbuild/_freeze_module.vcxproj index 51b493f8a84c6f..8520f563d396fa 100644 --- a/PCbuild/_freeze_module.vcxproj +++ b/PCbuild/_freeze_module.vcxproj @@ -92,6 +92,7 @@ $(IntDir);%(AdditionalIncludeDirectories) Disabled false + %(AdditionalOptions) -fno-lto Console From 26fb51f81101fd361411b8e3ae53a3ce0d8bceb1 Mon Sep 17 00:00:00 2001 From: Chris Eibl <138194463+chris-eibl@users.noreply.github.com> Date: Sun, 9 Feb 2025 16:34:56 +0100 Subject: [PATCH 03/21] Do not build _freeze_module twice in case of PGO Speeds up both MSVC and clang-cl builds. Should most probably done in a separate PR and issue, though. --- PCbuild/pcbuild.proj | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/PCbuild/pcbuild.proj b/PCbuild/pcbuild.proj index a2a637a3044373..c7ddc1d23b301c 100644 --- a/PCbuild/pcbuild.proj +++ b/PCbuild/pcbuild.proj @@ -95,7 +95,8 @@ - Date: Mon, 10 Feb 2025 20:13:15 +0100 Subject: [PATCH 04/21] remove /arch:AVX for blake2module.c again I've previously gotten compile errors from clang, because the needed intrinsics were not available without that option. Cannot reproduce anymore. Most probably, because I've upgraded to Visual Studio 17.13.0 Preview 5.0, which now ships with clang 19.1.1 instead of 18.1.8 and they've done that for compatibility with MSVC? Anyway, let's keep the PR small :) --- PCbuild/pythoncore.vcxproj | 1 - 1 file changed, 1 deletion(-) diff --git a/PCbuild/pythoncore.vcxproj b/PCbuild/pythoncore.vcxproj index b05c20d638a694..3a7ec2f63b5d2a 100644 --- a/PCbuild/pythoncore.vcxproj +++ b/PCbuild/pythoncore.vcxproj @@ -420,7 +420,6 @@ HACL_CAN_COMPILE_SIMD128;%(PreprocessorDefinitions) HACL_CAN_COMPILE_SIMD256;%(PreprocessorDefinitions) - /arch:AVX From 7b46aeb149f785cfddf2c21bb7e88415f6431eae Mon Sep 17 00:00:00 2001 From: Chris Eibl <138194463+chris-eibl@users.noreply.github.com> Date: Mon, 10 Feb 2025 20:18:02 +0100 Subject: [PATCH 05/21] Revert "Do not build _freeze_module twice in case of PGO" This reverts commit 26fb51f81101fd361411b8e3ae53a3ce0d8bceb1. Shall be done in a separate PR. --- PCbuild/pcbuild.proj | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/PCbuild/pcbuild.proj b/PCbuild/pcbuild.proj index c7ddc1d23b301c..a2a637a3044373 100644 --- a/PCbuild/pcbuild.proj +++ b/PCbuild/pcbuild.proj @@ -95,8 +95,7 @@ - Date: Mon, 10 Feb 2025 21:26:18 +0100 Subject: [PATCH 06/21] always clean the profile directory when doing a new clang PGO build This better matches the behaviour of build.bat in case of MSVC PGO builds. --- PCbuild/pythoncore.vcxproj | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/PCbuild/pythoncore.vcxproj b/PCbuild/pythoncore.vcxproj index 3a7ec2f63b5d2a..75c07f3ffa5b4c 100644 --- a/PCbuild/pythoncore.vcxproj +++ b/PCbuild/pythoncore.vcxproj @@ -717,8 +717,16 @@ - + Condition="$(PlatformToolset) == 'ClangCL' and $(Configuration) == 'PGInstrument' and !Exists($(ClangProfileDir))"> + + + + + + + + Date: Thu, 13 Feb 2025 19:23:15 +0100 Subject: [PATCH 07/21] blurb it --- .../next/Build/2025-02-13-19-21-41.gh-issue-130090.3ngJaV.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 Misc/NEWS.d/next/Build/2025-02-13-19-21-41.gh-issue-130090.3ngJaV.rst diff --git a/Misc/NEWS.d/next/Build/2025-02-13-19-21-41.gh-issue-130090.3ngJaV.rst b/Misc/NEWS.d/next/Build/2025-02-13-19-21-41.gh-issue-130090.3ngJaV.rst new file mode 100644 index 00000000000000..9c87bcc68423db --- /dev/null +++ b/Misc/NEWS.d/next/Build/2025-02-13-19-21-41.gh-issue-130090.3ngJaV.rst @@ -0,0 +1,2 @@ +Building with ``PlatformToolset=ClangCL`` on Windows now supports PGO +(profile guided optimization). Patch by Chris Eibl. From ad72df5496aa44b5c2914551e5009be8c9f146ab Mon Sep 17 00:00:00 2001 From: Chris Eibl <138194463+chris-eibl@users.noreply.github.com> Date: Thu, 13 Feb 2025 19:30:53 +0100 Subject: [PATCH 08/21] Adding myself to ACKS --- Misc/ACKS | 1 + 1 file changed, 1 insertion(+) diff --git a/Misc/ACKS b/Misc/ACKS index 2a68b69f161041..7bf1d9c99ea24c 100644 --- a/Misc/ACKS +++ b/Misc/ACKS @@ -504,6 +504,7 @@ Grant Edwards Vlad Efanov Zvi Effron John Ehresman +Chris Eibl Tal Einat Eric Eisner Andrew Eland From 74ec74e889066e53ad0767dbd711828de89bc650 Mon Sep 17 00:00:00 2001 From: Chris Eibl <138194463+chris-eibl@users.noreply.github.com> Date: Thu, 13 Feb 2025 23:24:34 +0100 Subject: [PATCH 09/21] extract pyproject-clangcl.props --- PCbuild/pyproject-clangcl.props | 21 +++++++++++++++++++++ PCbuild/pyproject.props | 7 +------ 2 files changed, 22 insertions(+), 6 deletions(-) create mode 100644 PCbuild/pyproject-clangcl.props diff --git a/PCbuild/pyproject-clangcl.props b/PCbuild/pyproject-clangcl.props new file mode 100644 index 00000000000000..dbbf0e833f3be6 --- /dev/null +++ b/PCbuild/pyproject-clangcl.props @@ -0,0 +1,21 @@ + + + + <__PyprojectClangCl_Props_Imported>true + + + + $(MSBuildThisFileDirectory)obj\$(MajorVersionNumber)$(MinorVersionNumber)$(ArchName)_PGInstrument\__clang_profiles\ + $(ClangProfileDir.Replace(`\\`, `\`)) + + + + + -Wno-deprecated-non-prototype -Wno-unused-label -Wno-pointer-sign -Wno-incompatible-pointer-types-discards-qualifiers -Wno-unused-function %(AdditionalOptions) + -flto %(AdditionalOptions) + -fprofile-instr-generate=$(ClangProfileDir)\default_%m.profraw %(AdditionalOptions) + -fprofile-instr-use=$(ClangProfileDir)\profdata.profdata %(AdditionalOptions) + + + + diff --git a/PCbuild/pyproject.props b/PCbuild/pyproject.props index 20794201d91c52..95f4759cd289ea 100644 --- a/PCbuild/pyproject.props +++ b/PCbuild/pyproject.props @@ -1,6 +1,7 @@ + <__PyProject_Props_Imported>true <_ProjectFileVersion>10.0.30319.1 @@ -10,8 +11,6 @@ $(MSBuildThisFileDirectory)obj\ $(Py_IntDir)\$(MajorVersionNumber)$(MinorVersionNumber)$(ArchName)_$(Configuration)\$(ProjectName)\ $(IntDir.Replace(`\\`, `\`)) - $(Py_IntDir)\$(MajorVersionNumber)$(MinorVersionNumber)$(ArchName)_PGInstrument\__clang_profiles\ - $(ClangProfileDir.Replace(`\\`, `\`)) $(Py_IntDir)\$(MajorVersionNumber)$(MinorVersionNumber)$(ArchName)_$(Configuration)\pythoncore\ $(Py_IntDir)\$(MajorVersionNumber)$(MinorVersionNumber)_frozen\ @@ -71,10 +70,6 @@ $(EnableControlFlowGuard) true /utf-8 %(AdditionalOptions) - -Wno-deprecated-non-prototype -Wno-unused-label -Wno-pointer-sign -Wno-incompatible-pointer-types-discards-qualifiers -Wno-unused-function %(AdditionalOptions) - -flto %(AdditionalOptions) - -fprofile-instr-generate=$(ClangProfileDir)\default_%m.profraw %(AdditionalOptions) - -fprofile-instr-use=$(ClangProfileDir)\profdata.profdata %(AdditionalOptions) -d2pattern-opt-disable:-932189325 %(AdditionalOptions) -d2ssa-patterns-all- %(AdditionalOptions) /sourceDependencies "$(IntDir.Trim(`\`))" %(AdditionalOptions) From 8c8aa7934dcbd33d1f816fe71546f7ef8249e736 Mon Sep 17 00:00:00 2001 From: Chris Eibl <138194463+chris-eibl@users.noreply.github.com> Date: Sun, 16 Feb 2025 11:55:53 +0100 Subject: [PATCH 10/21] move MergeClangProfileData to pyproject-clangcl.props and make it a target with inputs and outputs --- PCbuild/pyproject-clangcl.props | 32 +++++++++++++++++++++++++------- PCbuild/pythoncore.vcxproj | 20 -------------------- 2 files changed, 25 insertions(+), 27 deletions(-) diff --git a/PCbuild/pyproject-clangcl.props b/PCbuild/pyproject-clangcl.props index dbbf0e833f3be6..cf62c46658c475 100644 --- a/PCbuild/pyproject-clangcl.props +++ b/PCbuild/pyproject-clangcl.props @@ -2,19 +2,37 @@ <__PyprojectClangCl_Props_Imported>true - - - - $(MSBuildThisFileDirectory)obj\$(MajorVersionNumber)$(MinorVersionNumber)$(ArchName)_PGInstrument\__clang_profiles\ - $(ClangProfileDir.Replace(`\\`, `\`)) + + <_profrawFiles Include="$(OutDir)instrumented\$(TargetName)_*.profraw" /> + + + + + + + + + + + + + + + -Wno-deprecated-non-prototype -Wno-unused-label -Wno-pointer-sign -Wno-incompatible-pointer-types-discards-qualifiers -Wno-unused-function %(AdditionalOptions) -flto %(AdditionalOptions) - -fprofile-instr-generate=$(ClangProfileDir)\default_%m.profraw %(AdditionalOptions) - -fprofile-instr-use=$(ClangProfileDir)\profdata.profdata %(AdditionalOptions) + -fprofile-instr-generate=$(OutDir)$(TargetName)_%m.profraw %(AdditionalOptions) + -fprofile-instr-use=$(OutDir)instrumented\profdata.profdata %(AdditionalOptions) diff --git a/PCbuild/pythoncore.vcxproj b/PCbuild/pythoncore.vcxproj index 75c07f3ffa5b4c..9ebf58ae8a9bc4 100644 --- a/PCbuild/pythoncore.vcxproj +++ b/PCbuild/pythoncore.vcxproj @@ -716,26 +716,6 @@ - - - - - - - - - - - - - - - - git From 2a9da272968c585559d38fc1b3ba729f8ec5daae Mon Sep 17 00:00:00 2001 From: Chris Eibl <138194463+chris-eibl@users.noreply.github.com> Date: Sun, 16 Feb 2025 11:58:33 +0100 Subject: [PATCH 11/21] rename RequirePGCFiles to RequireProfileData because the name is too MSVC specific --- PCbuild/pyproject-clangcl.props | 2 +- PCbuild/pyproject.props | 2 +- PCbuild/pythoncore.vcxproj | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/PCbuild/pyproject-clangcl.props b/PCbuild/pyproject-clangcl.props index cf62c46658c475..abef887a6a960a 100644 --- a/PCbuild/pyproject-clangcl.props +++ b/PCbuild/pyproject-clangcl.props @@ -11,7 +11,7 @@ + Condition="$(RequireProfileData) == 'true' and @(_profrawFiles) == ''" /> + Condition="$(RequireProfileData) == 'true' and @(_PGCFiles) == ''" /> true - true + true true false From ea4de96c40adc03ecd67a651c7a727f28bd6933f Mon Sep 17 00:00:00 2001 From: Chris Eibl <138194463+chris-eibl@users.noreply.github.com> Date: Tue, 18 Feb 2025 20:17:57 +0100 Subject: [PATCH 12/21] Apply suggestions from code review Co-authored-by: Steve Dower --- PCbuild/pyproject-clangcl.props | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/PCbuild/pyproject-clangcl.props b/PCbuild/pyproject-clangcl.props index abef887a6a960a..7a38bd4380da9f 100644 --- a/PCbuild/pyproject-clangcl.props +++ b/PCbuild/pyproject-clangcl.props @@ -17,13 +17,13 @@ + Outputs="$(OutDir)instrumented\profdata.profdata"> + Command='"$(LLVMInstallDir)\bin\llvm-profdata.exe" merge -output="$(OutDir)instrumented\profdata.profdata" "$(OutDir)instrumented\*_*.profraw"' /> - + From 3346b9d84673646dd295e8be22dd05a362b2a588 Mon Sep 17 00:00:00 2001 From: Chris Eibl <138194463+chris-eibl@users.noreply.github.com> Date: Tue, 18 Feb 2025 20:47:43 +0100 Subject: [PATCH 13/21] Use -Wno-profile-instr-unprofiled in the PGUpdate case --- PCbuild/pyproject-clangcl.props | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/PCbuild/pyproject-clangcl.props b/PCbuild/pyproject-clangcl.props index 7a38bd4380da9f..26273096010f35 100644 --- a/PCbuild/pyproject-clangcl.props +++ b/PCbuild/pyproject-clangcl.props @@ -32,7 +32,7 @@ -Wno-deprecated-non-prototype -Wno-unused-label -Wno-pointer-sign -Wno-incompatible-pointer-types-discards-qualifiers -Wno-unused-function %(AdditionalOptions) -flto %(AdditionalOptions) -fprofile-instr-generate=$(OutDir)$(TargetName)_%m.profraw %(AdditionalOptions) - -fprofile-instr-use=$(OutDir)instrumented\profdata.profdata %(AdditionalOptions) + -fprofile-instr-use=$(OutDir)instrumented\profdata.profdata -Wno-profile-instr-unprofiled %(AdditionalOptions) From 9db1a297d95574cc3114854c8c427736d9521917 Mon Sep 17 00:00:00 2001 From: Chris Eibl <138194463+chris-eibl@users.noreply.github.com> Date: Wed, 19 Feb 2025 06:40:46 +0100 Subject: [PATCH 14/21] introduce CLANG_PROFILE_PATH --- PCbuild/pyproject-clangcl.props | 11 ++++++++++- PCbuild/pyproject.props | 4 +++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/PCbuild/pyproject-clangcl.props b/PCbuild/pyproject-clangcl.props index 26273096010f35..21e591b7ca403d 100644 --- a/PCbuild/pyproject-clangcl.props +++ b/PCbuild/pyproject-clangcl.props @@ -4,6 +4,15 @@ <__PyprojectClangCl_Props_Imported>true + + + $(OutDir) + <_CLANG_PROFILE_PATH>$(CLANG_PROFILE_PATH) + <_CLANG_PROFILE_PATH Condition="!HasTrailingSlash($(_CLANG_PROFILE_PATH))">$(_CLANG_PROFILE_PATH)\ + + <_profrawFiles Include="$(OutDir)instrumented\$(TargetName)_*.profraw" /> @@ -31,7 +40,7 @@ -Wno-deprecated-non-prototype -Wno-unused-label -Wno-pointer-sign -Wno-incompatible-pointer-types-discards-qualifiers -Wno-unused-function %(AdditionalOptions) -flto %(AdditionalOptions) - -fprofile-instr-generate=$(OutDir)$(TargetName)_%m.profraw %(AdditionalOptions) + -fprofile-instr-generate=$(_CLANG_PROFILE_PATH)$(TargetName)_%m.profraw %(AdditionalOptions) -fprofile-instr-use=$(OutDir)instrumented\profdata.profdata -Wno-profile-instr-unprofiled %(AdditionalOptions) diff --git a/PCbuild/pyproject.props b/PCbuild/pyproject.props index 3fe358f7968c4f..2681e2d42e8996 100644 --- a/PCbuild/pyproject.props +++ b/PCbuild/pyproject.props @@ -1,7 +1,6 @@ - <__PyProject_Props_Imported>true <_ProjectFileVersion>10.0.30319.1 @@ -25,6 +24,9 @@ false + + + $(TargetName)$(TargetExt) <_TargetNameSep>$(TargetNameExt.LastIndexOf(`.`)) From 4ad2365a032b7f36fad72e1c0ffac2ab2b5111e8 Mon Sep 17 00:00:00 2001 From: Chris Eibl <138194463+chris-eibl@users.noreply.github.com> Date: Fri, 21 Feb 2025 13:58:47 +0100 Subject: [PATCH 15/21] update readme.txt --- PCbuild/readme.txt | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/PCbuild/readme.txt b/PCbuild/readme.txt index 693fcee5f90ce2..036b39f2664bf5 100644 --- a/PCbuild/readme.txt +++ b/PCbuild/readme.txt @@ -52,6 +52,42 @@ Release settings, though without PGO. +Building Python using Clang/LLVM +-------------------------------- + +See https://learn.microsoft.com/en-us/cpp/build/clang-support-msbuild?view=msvc-170 +how to install and use clang-cl bundled with Microsoft Visual Studio. +You can use the IDE to switch to clang-cl for local development, +but because this alters the *.vcxproj files, the recommended way is +to use build.bat: + +build.bat "/p:PlatformToolset=ClangCL" "/p:PreferredToolArchitecture=x64" + +All other build.bat options remain to work as with MSVC, so this +will create a 64bit release binary. PreferredToolArchitecture is needed, +because msbuild by default selects the 32bit architecture of the toolset, +which uses -m32 as the default target architecture. + +You can also use a specific version of clang-cl downloaded from +https://github.com/llvm/llvm-project/releases, e.g. +clang+llvm-18.1.8-x86_64-pc-windows-msvc.tar.xz. +Given you have extracted that to , you can use it like so +build.bat --pgo "/p:PlatformToolset=ClangCL" "/p:LLVMInstallDir= "/p:LLVMToolsVersion=18" + +Here, PreferredToolArchitecture is not needed, because this is a 64bit +platform toolset, but LLVMToolsVersion has to be set accordingly. +Setting the major version is enough, although you can be specific +and use 18.1.8 in the above example, too. + +Even --pgo works out of the box, except you do want to run the PGO task +on a different host than the build host. In this case you must pass +"/p:CLANG_PROFILE_PATH=" +in the PGInstrument step to make sure the profile data is generated +into the instrumented directory when running the PGO task. +Like in the MSVC case, after fetching (or manually copying) the instrumented +folder back into your build tree, you can continue with the PGUpdate +step with no further parameters. + Building Python using the build.bat script ---------------------------------------------- From 1977953f2beea21a49edfbb68d994c21efa523e0 Mon Sep 17 00:00:00 2001 From: Chris Eibl <138194463+chris-eibl@users.noreply.github.com> Date: Mon, 24 Feb 2025 08:30:34 +0100 Subject: [PATCH 16/21] Apply suggestions from code review Co-authored-by: Ken Jin --- PCbuild/readme.txt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/PCbuild/readme.txt b/PCbuild/readme.txt index 036b39f2664bf5..df59ff2797ebb1 100644 --- a/PCbuild/readme.txt +++ b/PCbuild/readme.txt @@ -56,14 +56,14 @@ Building Python using Clang/LLVM -------------------------------- See https://learn.microsoft.com/en-us/cpp/build/clang-support-msbuild?view=msvc-170 -how to install and use clang-cl bundled with Microsoft Visual Studio. +for how to install and use clang-cl bundled with Microsoft Visual Studio. You can use the IDE to switch to clang-cl for local development, but because this alters the *.vcxproj files, the recommended way is to use build.bat: build.bat "/p:PlatformToolset=ClangCL" "/p:PreferredToolArchitecture=x64" -All other build.bat options remain to work as with MSVC, so this +All other build.bat options continue to work as with MSVC, so this will create a 64bit release binary. PreferredToolArchitecture is needed, because msbuild by default selects the 32bit architecture of the toolset, which uses -m32 as the default target architecture. @@ -79,8 +79,8 @@ platform toolset, but LLVMToolsVersion has to be set accordingly. Setting the major version is enough, although you can be specific and use 18.1.8 in the above example, too. -Even --pgo works out of the box, except you do want to run the PGO task -on a different host than the build host. In this case you must pass +Even --pgo works out of the box. However, if you want to run the PGO task +on a different host than the build host, you must pass "/p:CLANG_PROFILE_PATH=" in the PGInstrument step to make sure the profile data is generated into the instrumented directory when running the PGO task. From ad2dfce550527871e0a0c30f26c4a880752f8503 Mon Sep 17 00:00:00 2001 From: Chris Eibl <138194463+chris-eibl@users.noreply.github.com> Date: Mon, 24 Feb 2025 19:30:47 +0100 Subject: [PATCH 17/21] Apply suggestions from code review Co-authored-by: Steve Dower --- PCbuild/readme.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/PCbuild/readme.txt b/PCbuild/readme.txt index df59ff2797ebb1..bd46086342c3eb 100644 --- a/PCbuild/readme.txt +++ b/PCbuild/readme.txt @@ -55,7 +55,7 @@ Release Building Python using Clang/LLVM -------------------------------- -See https://learn.microsoft.com/en-us/cpp/build/clang-support-msbuild?view=msvc-170 +See https://learn.microsoft.com/cpp/build/clang-support-msbuild for how to install and use clang-cl bundled with Microsoft Visual Studio. You can use the IDE to switch to clang-cl for local development, but because this alters the *.vcxproj files, the recommended way is From 6edbe079f7dace8e83b4cb80c7c2938da18409f0 Mon Sep 17 00:00:00 2001 From: Chris Eibl <138194463+chris-eibl@users.noreply.github.com> Date: Mon, 24 Feb 2025 19:38:09 +0100 Subject: [PATCH 18/21] credit zooba --- .../next/Build/2025-02-13-19-21-41.gh-issue-130090.3ngJaV.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Build/2025-02-13-19-21-41.gh-issue-130090.3ngJaV.rst b/Misc/NEWS.d/next/Build/2025-02-13-19-21-41.gh-issue-130090.3ngJaV.rst index 9c87bcc68423db..9ee7d5cdd8ae8a 100644 --- a/Misc/NEWS.d/next/Build/2025-02-13-19-21-41.gh-issue-130090.3ngJaV.rst +++ b/Misc/NEWS.d/next/Build/2025-02-13-19-21-41.gh-issue-130090.3ngJaV.rst @@ -1,2 +1,2 @@ Building with ``PlatformToolset=ClangCL`` on Windows now supports PGO -(profile guided optimization). Patch by Chris Eibl. +(profile guided optimization). Patch by Chris Eibl with invaluable support from Steve Dover. From 81b4c1d92947e4db60ca423d05bffc09f7dc05d4 Mon Sep 17 00:00:00 2001 From: Chris Eibl <138194463+chris-eibl@users.noreply.github.com> Date: Mon, 24 Feb 2025 20:35:03 +0100 Subject: [PATCH 19/21] address review comments regarding PGO --- PCbuild/readme.txt | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/PCbuild/readme.txt b/PCbuild/readme.txt index bd46086342c3eb..1277ea38fa7048 100644 --- a/PCbuild/readme.txt +++ b/PCbuild/readme.txt @@ -79,11 +79,18 @@ platform toolset, but LLVMToolsVersion has to be set accordingly. Setting the major version is enough, although you can be specific and use 18.1.8 in the above example, too. -Even --pgo works out of the box. However, if you want to run the PGO task +Use the --pgo option to build with PGO (Profile Guided Optimization). + +However, if you want to run the PGO task on a different host than the build host, you must pass "/p:CLANG_PROFILE_PATH=" in the PGInstrument step to make sure the profile data is generated into the instrumented directory when running the PGO task. +E.g., if you place the instrumented binaries into the folder +"workdir/instrumented" and then run the PGO task using "workdir" +as the current working directory, the usage is +"/p:CLANG_PROFILE_PATH=workdir/instrumented" + Like in the MSVC case, after fetching (or manually copying) the instrumented folder back into your build tree, you can continue with the PGUpdate step with no further parameters. From 263870dd319e79170f42fc5e05beb3879effff7b Mon Sep 17 00:00:00 2001 From: Chris Eibl <138194463+chris-eibl@users.noreply.github.com> Date: Mon, 24 Feb 2025 20:59:49 +0100 Subject: [PATCH 20/21] Explicitely set the architecture based on $(Platform) to be independent from the selected architecture or the PlatformToolset --- PCbuild/pyproject-clangcl.props | 2 ++ PCbuild/readme.txt | 12 ++++-------- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/PCbuild/pyproject-clangcl.props b/PCbuild/pyproject-clangcl.props index 21e591b7ca403d..30db6824f3caf5 100644 --- a/PCbuild/pyproject-clangcl.props +++ b/PCbuild/pyproject-clangcl.props @@ -39,6 +39,8 @@ -Wno-deprecated-non-prototype -Wno-unused-label -Wno-pointer-sign -Wno-incompatible-pointer-types-discards-qualifiers -Wno-unused-function %(AdditionalOptions) + -m32 %(AdditionalOptions) + -m64 %(AdditionalOptions) -flto %(AdditionalOptions) -fprofile-instr-generate=$(_CLANG_PROFILE_PATH)$(TargetName)_%m.profraw %(AdditionalOptions) -fprofile-instr-use=$(OutDir)instrumented\profdata.profdata -Wno-profile-instr-unprofiled %(AdditionalOptions) diff --git a/PCbuild/readme.txt b/PCbuild/readme.txt index 1277ea38fa7048..53d0e3e6914ecc 100644 --- a/PCbuild/readme.txt +++ b/PCbuild/readme.txt @@ -61,12 +61,10 @@ You can use the IDE to switch to clang-cl for local development, but because this alters the *.vcxproj files, the recommended way is to use build.bat: -build.bat "/p:PlatformToolset=ClangCL" "/p:PreferredToolArchitecture=x64" +build.bat "/p:PlatformToolset=ClangCL" All other build.bat options continue to work as with MSVC, so this -will create a 64bit release binary. PreferredToolArchitecture is needed, -because msbuild by default selects the 32bit architecture of the toolset, -which uses -m32 as the default target architecture. +will create a 64bit release binary. You can also use a specific version of clang-cl downloaded from https://github.com/llvm/llvm-project/releases, e.g. @@ -74,10 +72,8 @@ clang+llvm-18.1.8-x86_64-pc-windows-msvc.tar.xz. Given you have extracted that to , you can use it like so build.bat --pgo "/p:PlatformToolset=ClangCL" "/p:LLVMInstallDir= "/p:LLVMToolsVersion=18" -Here, PreferredToolArchitecture is not needed, because this is a 64bit -platform toolset, but LLVMToolsVersion has to be set accordingly. -Setting the major version is enough, although you can be specific -and use 18.1.8 in the above example, too. +Setting LLVMToolsVersion to the major version is enough, although you +can be specific and use 18.1.8 in the above example, too. Use the --pgo option to build with PGO (Profile Guided Optimization). From db208aef85aad85c99a8d3cf0a96c3e7186fa32e Mon Sep 17 00:00:00 2001 From: Chris Eibl <138194463+chris-eibl@users.noreply.github.com> Date: Mon, 24 Feb 2025 21:45:05 +0100 Subject: [PATCH 21/21] Update PCbuild/readme.txt Co-authored-by: Steve Dower --- PCbuild/readme.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/PCbuild/readme.txt b/PCbuild/readme.txt index 53d0e3e6914ecc..33952d31681cbc 100644 --- a/PCbuild/readme.txt +++ b/PCbuild/readme.txt @@ -85,7 +85,7 @@ into the instrumented directory when running the PGO task. E.g., if you place the instrumented binaries into the folder "workdir/instrumented" and then run the PGO task using "workdir" as the current working directory, the usage is -"/p:CLANG_PROFILE_PATH=workdir/instrumented" +"/p:CLANG_PROFILE_PATH=instrumented" Like in the MSVC case, after fetching (or manually copying) the instrumented folder back into your build tree, you can continue with the PGUpdate