Skip to content

gh-130090: Support PGO for clang-cl #129907

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

Merged
merged 24 commits into from
Mar 4, 2025
Merged

gh-130090: Support PGO for clang-cl #129907

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
859c89f
minimal changes needed for PGO with clang-cl
chris-eibl Feb 9, 2025
a529c39
like for MSVC, don't use link time optimization
chris-eibl Feb 9, 2025
26fb51f
Do not build _freeze_module twice in case of PGO
chris-eibl Feb 9, 2025
6e2cb69
remove /arch:AVX for blake2module.c again
chris-eibl Feb 10, 2025
7b46aeb
Revert "Do not build _freeze_module twice in case of PGO"
chris-eibl Feb 10, 2025
ced66bd
always clean the profile directory when doing a new clang PGO build
chris-eibl Feb 10, 2025
1aae65d
Merge remote-tracking branch 'origin/main' into clang-pgo
chris-eibl Feb 11, 2025
52fd5ab
Merge branch 'main' into clang-pgo
chris-eibl Feb 13, 2025
c2ecb57
blurb it
chris-eibl Feb 13, 2025
ad72df5
Adding myself to ACKS
chris-eibl Feb 13, 2025
74ec74e
extract pyproject-clangcl.props
chris-eibl Feb 13, 2025
79ce418
Merge branch 'python:main' into clang-pgo
chris-eibl Feb 16, 2025
8c8aa79
move MergeClangProfileData to pyproject-clangcl.props
chris-eibl Feb 16, 2025
2a9da27
rename RequirePGCFiles to RequireProfileData
chris-eibl Feb 16, 2025
ea4de96
Apply suggestions from code review
chris-eibl Feb 18, 2025
3346b9d
Use -Wno-profile-instr-unprofiled in the PGUpdate case
chris-eibl Feb 18, 2025
9db1a29
introduce CLANG_PROFILE_PATH
chris-eibl Feb 19, 2025
4ad2365
update readme.txt
chris-eibl Feb 21, 2025
1977953
Apply suggestions from code review
chris-eibl Feb 24, 2025
ad2dfce
Apply suggestions from code review
chris-eibl Feb 24, 2025
6edbe07
credit zooba
chris-eibl Feb 24, 2025
81b4c1d
address review comments regarding PGO
chris-eibl Feb 24, 2025
263870d
Explicitely set the architecture based on $(Platform)
chris-eibl Feb 24, 2025
db208ae
Update PCbuild/readme.txt
chris-eibl Feb 24, 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
1 change: 1 addition & 0 deletions Misc/ACKS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -504,6 +504,7 @@ Grant Edwards
Vlad Efanov
Zvi Effron
John Ehresman
Chris Eibl
Tal Einat
Eric Eisner
Andrew Eland
Expand Down
2 changes: 2 additions & 0 deletions Misc/NEWS.d/next/Build/2025-02-13-19-21-41.gh-issue-130090.3ngJaV.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Building with ``PlatformToolset=ClangCL`` on Windows now supports PGO
(profile guided optimization). Patch by Chris Eibl with invaluable support from Steve Dover.
1 change: 1 addition & 0 deletions PCbuild/_freeze_module.vcxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@
<AdditionalIncludeDirectories>$(IntDir);%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
<Optimization>Disabled</Optimization>
<WholeProgramOptimization>false</WholeProgramOptimization>
<AdditionalOptions Condition="$(PlatformToolset) == 'ClangCL'">%(AdditionalOptions) -fno-lto</AdditionalOptions>
</ClCompile>
<Link>
<SubSystem>Console</SubSystem>
Expand Down
50 changes: 50 additions & 0 deletions PCbuild/pyproject-clangcl.props
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
<?xml version="1.0" encoding="utf-8"?>
<Project ToolsVersion="15.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
<PropertyGroup Label="Globals">
<__PyprojectClangCl_Props_Imported>true</__PyprojectClangCl_Props_Imported>
</PropertyGroup>

<PropertyGroup>
<!-- CLANG_PROFILE_PATH is configurable for "remote PGO builds"
For convenience, we also accept paths without trailing slashes.
-->
<CLANG_PROFILE_PATH Condition="'$(CLANG_PROFILE_PATH)' == ''">$(OutDir)</CLANG_PROFILE_PATH>
<_CLANG_PROFILE_PATH>$(CLANG_PROFILE_PATH)</_CLANG_PROFILE_PATH>
<_CLANG_PROFILE_PATH Condition="!HasTrailingSlash($(_CLANG_PROFILE_PATH))">$(_CLANG_PROFILE_PATH)\</_CLANG_PROFILE_PATH>
</PropertyGroup>

<ItemGroup>
<_profrawFiles Include="$(OutDir)instrumented\$(TargetName)_*.profraw" />
</ItemGroup>

<Target Name="EnsureClangProfileData" BeforeTargets="PrepareForBuild"
Condition="'$(SupportPGO)' and $(Configuration) == 'PGUpdate'">
<Error Text="PGO run did not succeed (no $(TargetName)_*.profraw files) and there is no data to merge"
Condition="$(RequireProfileData) == 'true' and @(_profrawFiles) == ''" />
</Target>

<Target Name="MergeClangProfileData" BeforeTargets="PrepareForBuild"
Condition="'$(SupportPGO)' and $(Configuration) == 'PGUpdate'"
Inputs="@(_profrawFiles)"
Outputs="$(OutDir)instrumented\profdata.profdata">
<Exec
Command='"$(LLVMInstallDir)\bin\llvm-profdata.exe" merge -output="$(OutDir)instrumented\profdata.profdata" "$(OutDir)instrumented\*_*.profraw"' />
</Target>

<Target Name="CleanClangProfileData" BeforeTargets="Clean">
<Delete Files="@(_profrawFiles)" TreatErrorsAsWarnings="true" />
<Delete Files="$(OutDir)instrumented\profdata.profdata" TreatErrorsAsWarnings="true" />
</Target>

<ItemDefinitionGroup>
<ClCompile>
<AdditionalOptions>-Wno-deprecated-non-prototype -Wno-unused-label -Wno-pointer-sign -Wno-incompatible-pointer-types-discards-qualifiers -Wno-unused-function %(AdditionalOptions)</AdditionalOptions>
<AdditionalOptions Condition="'$(Platform)' == 'Win32'">-m32 %(AdditionalOptions)</AdditionalOptions>
<AdditionalOptions Condition="'$(Platform)' == 'x64'">-m64 %(AdditionalOptions)</AdditionalOptions>
Copy link
Member

Choose a reason for hiding this comment

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

Is there an ARM64 option as well?

Copy link
Member

Choose a reason for hiding this comment

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

I'm glad you found this option, though! Much neater way to do it (and a bit embarrassing that the built-in targets don't do it...).

Copy link
Member Author

Choose a reason for hiding this comment

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

Is there an ARM64 option as well?

I think so, yes. I see some hits in the net, and did a clang-cl /?, which e.g. reveals option /arm64EC.

But I do not know what to actually write here in case of '$(Platform)'=='ARM64' - and more importantly how to test it, since I have no arm based device.

I think we should leave that for another PR of someone who does?

Copy link
Member

Choose a reason for hiding this comment

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

Sure. @brandtbucher might be that someone. But at the conditions here will just not set an option on that platform and so it should work the same as today.

<AdditionalOptions Condition="$(Configuration) != 'Debug'">-flto %(AdditionalOptions)</AdditionalOptions>
<AdditionalOptions Condition="$(SupportPGO) and $(Configuration) == 'PGInstrument'">-fprofile-instr-generate=$(_CLANG_PROFILE_PATH)$(TargetName)_%m.profraw %(AdditionalOptions)</AdditionalOptions>
<AdditionalOptions Condition="$(SupportPGO) and $(Configuration) == 'PGUpdate'">-fprofile-instr-use=$(OutDir)instrumented\profdata.profdata -Wno-profile-instr-unprofiled %(AdditionalOptions)</AdditionalOptions>
</ClCompile>
</ItemDefinitionGroup>

</Project>
9 changes: 5 additions & 4 deletions PCbuild/pyproject.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@
<LinkIncremental Condition="$(Configuration) != 'Debug'">false</LinkIncremental>
</PropertyGroup>

<!-- We need the above overridden OutDir, so this must be imported after PropertyGroup -->
<Import Project="pyproject-clangcl.props" Condition="$(PlatformToolset) == 'ClangCL' and $(__PyprojectClangCl_Props_Imported) != 'true'" />

<PropertyGroup Condition="$(TargetExt) != ''">
<TargetNameExt>$(TargetName)$(TargetExt)</TargetNameExt>
<_TargetNameSep>$(TargetNameExt.LastIndexOf(`.`))</_TargetNameSep>
Expand Down Expand Up @@ -69,8 +72,6 @@
<ControlFlowGuard Condition="$(EnableControlFlowGuard) != ''">$(EnableControlFlowGuard)</ControlFlowGuard>
<MultiProcessorCompilation>true</MultiProcessorCompilation>
<AdditionalOptions>/utf-8 %(AdditionalOptions)</AdditionalOptions>
<AdditionalOptions Condition="$(PlatformToolset) == 'ClangCL'">-Wno-deprecated-non-prototype -Wno-unused-label -Wno-pointer-sign -Wno-incompatible-pointer-types-discards-qualifiers -Wno-unused-function %(AdditionalOptions)</AdditionalOptions>
<AdditionalOptions Condition="$(Configuration) != 'Debug' and $(PlatformToolset) == 'ClangCL'">-flto %(AdditionalOptions)</AdditionalOptions>
<AdditionalOptions Condition="$(MSVCHasBrokenARM64Clamping) == 'true' and $(Platform) == 'ARM64'">-d2pattern-opt-disable:-932189325 %(AdditionalOptions)</AdditionalOptions>
<AdditionalOptions Condition="$(MSVCHasBrokenARM64SignExtension) == 'true' and $(Platform) == 'ARM64'">-d2ssa-patterns-all- %(AdditionalOptions)</AdditionalOptions>
<AdditionalOptions Condition="$(GenerateSourceDependencies) == 'true'">/sourceDependencies "$(IntDir.Trim(`\`))" %(AdditionalOptions)</AdditionalOptions>
Expand Down Expand Up @@ -186,15 +187,15 @@ public override bool Execute() {
Targets="CleanAll" />
</Target>

<Target Name="CopyPGCFiles" BeforeTargets="PrepareForBuild" Condition="$(Configuration) == 'PGUpdate'">
<Target Name="CopyPGCFiles" BeforeTargets="PrepareForBuild" Condition="$(Configuration) == 'PGUpdate' and $(PlatformToolset) != 'ClangCL'">
<ItemGroup>
<_PGCFiles Include="$(OutDir)instrumented\$(TargetName)!*.pgc" />
<_PGDFile Include="$(OutDir)instrumented\$(TargetName).pgd" />
<_CopyFiles Include="@(_PGCFiles);@(_PGDFile)" Condition="Exists(%(FullPath))" />
</ItemGroup>
<Delete Files="@(_CopyFiles->'$(OutDir)%(Filename)%(Extension)')" />
<Error Text="PGO run did not succeed (no $(TargetName)!*.pgc files) and there is no data to merge"
Condition="$(RequirePGCFiles) == 'true' and @(_PGCFiles) == ''" />
Condition="$(RequireProfileData) == 'true' and @(_PGCFiles) == ''" />
<Copy SourceFiles="@(_CopyFiles)"
DestinationFolder="$(OutDir)"
UseHardLinksIfPossible="true"
Expand Down
2 changes: 1 addition & 1 deletion PCbuild/pythoncore.vcxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@
</ImportGroup>
<PropertyGroup>
<KillPython>true</KillPython>
<RequirePGCFiles>true</RequirePGCFiles>
<RequireProfileData>true</RequireProfileData>
<IncludeExternals Condition="$(IncludeExternals) == '' and Exists('$(zlibDir)\zlib.h')">true</IncludeExternals>
<IncludeExternals Condition="$(IncludeExternals) == ''">false</IncludeExternals>
</PropertyGroup>
Expand Down
39 changes: 39 additions & 0 deletions PCbuild/readme.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,45 @@ Release
settings, though without PGO.


Building Python using Clang/LLVM
--------------------------------

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
to use build.bat:

build.bat "/p:PlatformToolset=ClangCL"

All other build.bat options continue to work as with MSVC, so this
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.
clang+llvm-18.1.8-x86_64-pc-windows-msvc.tar.xz.
Given you have extracted that to <my-clang-dir>, you can use it like so
build.bat --pgo "/p:PlatformToolset=ClangCL" "/p:LLVMInstallDir=<my-clang-dir> "/p:LLVMToolsVersion=18"

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).

However, if you want to run the PGO task
on a different host than the build host, you must pass
"/p:CLANG_PROFILE_PATH=<relative-path-to-instrumented-dir-on-remote-host>"
in the PGInstrument step to make sure the profile data is generated
into the instrumented directory when running the PGO task.
Comment on lines +82 to +84
Copy link
Member

Choose a reason for hiding this comment

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

I think this needs a worked example. Just from this description, I couldn't confidently specify the parameter and trust that I got it right.

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=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.

Building Python using the build.bat script
----------------------------------------------

Expand Down
Loading