gh-138451: Support custom LLVM installation path #138452

xhochy · 2025-09-03T14:53:29Z

Add support for explicitly defined LLVM installation location.

This is necessary as MSVC now comes with its own LLVM installation and activating MSVC via vcvars.bat will put LLVM tools on the PATH before the local ones.

You can see this in usage in conda-forge's latest Python 3.13 build: conda-forge/python-feedstock#807

Issue: Wrong LLVM tools picked up if using MSVC activation and custom LLVM build #138451

python-cla-bot · 2025-09-03T14:53:34Z

All commit authors signed the Contributor License Agreement.

bedevere-app · 2025-09-03T14:53:36Z

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

Fidget-Spinner · 2025-09-04T17:24:29Z

Tools/jit/_llvm.py

 async def _find_tool(tool: str, *, echo: bool = False) -> str | None:
+    # Explicitly defined LLVM installation location
+    if (llvm_root := os.getenv("LLVM_ROOT")) is not None:
+        path = os.path.join(llvm_root, "bin", tool)


Are all LLVM installation folders guaranteed to have the tool located in the $LLVM_ROOT/bin directory? I think it's somewhat risky to assume that.

Actually there is a LLVM_TOOLS_INSTALL_DIR in the CMakeLists to customize this: https://github.com/llvm/llvm-project/blob/74ec38fad0a1289f936e5388fa8bbe74653c55d9/llvm/CMakeLists.txt#L494

I should probably use that instead?

Yeah that seems better

Is LLVM_TOOLS_INSTALL_DIR (or LLVM_ROOT for that matter) documented anywhere?

It seems like an env var used for building/installing LLVM itself, not necessarily something meant to be used after-the-fact by people trying to find LLVM. Or am I misunderstanding?

zooba · 2025-09-05T15:09:00Z

Tools/jit/_llvm.py

 @_async_cache
 async def _find_tool(tool: str, *, echo: bool = False) -> str | None:
+    # Explicitly defined LLVM installation location
+    if (llvm_tools_dir := os.getenv("LLVM_TOOLS_INSTALL_DIR")) is not None:


Should we document this somewhere? If it gets a NEWS entry, then we probably have (or need) a list of environment variables that may need to be set when building.

I'd expect the list to be in the devguide, really, which is a different repo. But do we have one in the main repo?

Also:

Suggested change

if (llvm_tools_dir := os.getenv("LLVM_TOOLS_INSTALL_DIR")) is not None:

if llvm_tools_dir := os.getenv("LLVM_TOOLS_INSTALL_DIR")):

We shouldn't join to an empty value either (since that's interchangeable with "not set"). And possibly we should make sure it's an absolute path as well, though that may not matter so much.

Considering the JIT is still experimental, I don't think we should document this in the CPython docs. I agree that it should be in the devguide.

As I did not know about the devguide before this review: Where should I add something? On first sight there was no section obvious to me.

I assume it's somewhere on this page, but to be honest I didn't read the whole thing: https://github.com/python/devguide/blob/main/getting-started/setup-building.rst

Possibly we need a new section here anyway for building the JIT? @savannahostrowski @brandtbucher are there JIT-specific build docs somewhere to document a relevant environment variable?

We have docs for building in https://github.com/python/cpython/blob/main/Tools/jit/README.md#building. We'd probably want to update this for now.

xhochy · 2025-10-26T14:55:44Z

Rebased and added documentation to the mentioned README. Is there anything more I can do here to get it merged?

Misc/NEWS.d/next/Build/2025-09-03-14-55-59.gh-issue-138451.-Qzh2S.rst

savannahostrowski

I'm wondering if we should make this a configure flag instead, like we did with #138498. That way, you can pass the flag into configure once (e.g., ./configure --with-llvm-tools-dir=/opt/homebrew/opt/llvm...) and then just run make without remembering to pass LLVM_TOOLS_INSTALL_DIR each time. That'd make it more consistent with other build options.

bedevere-app · 2025-10-26T16:35:30Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

xhochy · 2025-10-27T10:51:33Z

I have made the requested changes; please review again

bedevere-app · 2025-10-27T10:51:38Z

Thanks for making the requested changes!

@savannahostrowski: please review the changes made to this pull request.

Tools/jit/README.md

Tools/jit/_llvm.py

configure.ac

Add support for explicitly defined LLVM installation location. This is necessary as MSVC now comes with its own LLVM installation and activating MSVC via vcvars.bat will put LLVM tools on the `PATH` before the local ones.

Co-authored-by: Steve Dower <steve.dower@microsoft.com>

savannahostrowski

One comment but I think this looks good.

Misc/NEWS.d/next/Build/2025-09-03-14-55-59.gh-issue-138451.-Qzh2S.rst

chris-eibl · 2025-12-02T22:04:10Z

activating MSVC via vcvars.bat will put LLVM tools on the PATH

I think this is the same problem I've faced in #141967 (comment), so thanks for doing this!

I've noticed there, that e.g. running

build.bat --experimental-jit --tail-call-interp "/p:PlatformToolset=ClangCL" "/p:LLVMInstallDir=<my-path-to>\llvm-21.1.4.0" "/p:LLVMToolsVersion=21"

will smuggle in the include path of the clang specified on the command line in front of the environment variable

INCLUDE=<my-path-to>\llvm-21.1.4.0\lib\clang\21\include;<other include paths>

which the hard coded (and maybe different) clang version used in the jit builds doesn't like depending on the version mismatch ...

chris-eibl · 2025-12-02T22:20:47Z

LLVMInstallDir is the variable documented here https://learn.microsoft.com/en-us/cpp/build/clang-support-msbuild?view=msvc-170 which we already use at various places like

cpython/PCbuild/python.props

Lines 23 to 31 in 8801c6d

    
               <!-- 
        
               This ensures, that in case of clang-cl bundled with VS 
        
               we find e.g. clang_rt.profile.lib in the correct bitness. 
        
               In case the user sets LLVMInstallDir and LLVMToolsVersion, 
        
               they are responsible to use the correct tool architecture. 
        
               For details please see GH-131473. 
        
               --> 
        
               <PreferredToolArchitecture Condition="$(LLVMInstallDir) == '' and $(PlatformToolset) == 'ClangCL' and $(Platform) == 'x64'">x64</PreferredToolArchitecture> 
        
               <PreferredToolArchitecture Condition="$(LLVMInstallDir) == '' and $(PlatformToolset) == 'ClangCL' and $(Platform) == 'Win32'">x86</PreferredToolArchitecture>

cpython/PCbuild/pyproject-clangcl.props

Line 31 in 8801c6d

    
           Command='"$(LLVMInstallDir)\bin\llvm-profdata.exe" merge -output="$(OutDir)instrumented\profdata.profdata" "$(OutDir)instrumented\*_*.profraw"' />

cpython/PCbuild/readme.txt

Lines 69 to 73 in 8801c6d

    
           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"

Does that mean that we have to specify the additional LLVM_TOOLS_INSTALL_DIR variable to let the jit build pick up the same clang that is used for building cPython?

zooba · 2025-12-03T14:37:35Z

Does that mean that we have to specify the additional LLVM_TOOLS_INSTALL_DIR variable to let the jit build pick up the same clang that is used for building cPython?

There's no strict requirement for make/configure variables to match pcbuild.proj variables, but if it's specified as an environment variable then we should probably detect if it's set and copy it over the actual variable. That's one line in python.props. I wouldn't bother updating all the references, just do a read in one place.

chris-eibl · 2025-12-03T16:21:12Z

IIUC, you mean setting LLVMInstallDir and LLVMToolsVersion based on LLVM_TOOLS_INSTALL_DIR and LLVM_VERSION?

I think it might be "too late", since AFAIR internally this is used by MS when preparing the environment according to "their" variable names, but we'll see.

What's definitely easy: just use the MS variable names here

    <Exec Command='$(PythonForBuild) "$(PySourcePath)Tools\jit\build.py" $(JITArgs) --output-dir "$(GeneratedJitStencilsDir)" --pyconfig-dir "$(PySourcePath)PC" --llvm-version="$(LLVM_VERSION)" --llvm-tools-install-dir="$(LLVM_TOOLS_INSTALL_DIR)"'/>

because, as you said

There's no strict requirement for make/configure variables

and just update the jit readme accordingly if we cannot map the variables easily "early" enough ...

Or we do it in build.bat. Let's see ...

zooba · 2025-12-03T16:50:26Z

Any file that sets PlatformToolset is loaded early enough to influence all the default settings in MSBuild, so it can certainly be set there. I believe that's only python.props, though it might be part/all of pyproject.props as well, but I think this is a better setting for python.props along with the other properties relating to external dependencies.

xhochy · 2025-12-16T12:53:57Z

@zooba @chris-eibl I'm a bit unsure what I need to do here to get this merged. Is there anything from your side that I should address in my code for windows builds?

chris-eibl · 2025-12-16T21:32:45Z

First I thought we have these choices:

do nothing, i.e. allow people specifying LLVMInstallDir the "regular" way to determine the clang-cl used for compiling Python and LLVM_TOOLS_INSTALL_DIR to specify the clang used for the jit stencils. With the potential drawback, that those two might interfere like I've mentioned above. It doesn't always happen and depends on the "compatibility" of the clang versions, so I thought most probably we should prohibit that (and don't force users to specify the same args twice).
use just LLVMInstallDir and LLVMToolsVersion, i.e. different variables than on configure based builds
if LLVM_TOOLS_INSTALL_DIR is specified, override LLVMInstallDir. Likewise for LLVM_VERSION / LLVMToolsVersion.
if LLVMInstallDir is given, override LLVM_TOOLS_INSTALL_DIR. Likewise for LLVM_VERSION / LLVMToolsVersion.

chris-eibl · 2025-12-16T21:33:10Z

I've first tried option 4 via adding

<LLVM_TOOLS_INSTALL_DIR Condition="$(LLVMInstallDir) != '' and $(LLVM_TOOLS_INSTALL_DIR) == ''">$(LLVMInstallDir)</LLVM_TOOLS_INSTALL_DIR>
<LLVM_VERSION Condition="$(LLVMToolsVersion) != '' and $(LLVM_VERSION) == ''">$(LLVMToolsVersion)</LLVM_VERSION>

to

cpython/PCbuild/regen.targets

Lines 122 to 127 in 8c87bcd

    
           <PropertyGroup> 
        
             <JITArgs Condition="$(Platform) == 'ARM64'">aarch64-pc-windows-msvc</JITArgs> 
        
             <JITArgs Condition="$(Platform) == 'Win32'">i686-pc-windows-msvc</JITArgs> 
        
             <JITArgs Condition="$(Platform) == 'x64'">x86_64-pc-windows-msvc</JITArgs> 
        
             <JITArgs Condition="$(Configuration) == 'Debug'">$(JITArgs) --debug</JITArgs> 
        
           </PropertyGroup>

This would have been my preferred choice, because then we would not have to "teach something new" and could have the same variables on Windows / Linux.

However, if during installing of Visual Studio the bundled clang-cl is chosen to be installed, it turned out that LLVM_VERSION is always set to this bundled version, even if doing just a regular MSVC build. So option 2 and 4 are out ...

chris-eibl · 2025-12-16T21:44:10Z

Implementing option 3 is easy, too: just add

<LLVMInstallDir Condition="$(LLVM_TOOLS_INSTALL_DIR) != ''">$(LLVM_TOOLS_INSTALL_DIR)</LLVMInstallDir>
<LLVMToolsVersion Condition="$(LLVM_VERSION) != ''">$(LLVM_VERSION)</LLVMToolsVersion>

after

cpython/PCbuild/python.props

Lines 30 to 32 in 8c87bcd

    
           <PreferredToolArchitecture Condition="$(LLVMInstallDir) == '' and $(PlatformToolset) == 'ClangCL' and $(Platform) == 'x64'">x64</PreferredToolArchitecture> 
        
           <PreferredToolArchitecture Condition="$(LLVMInstallDir) == '' and $(PlatformToolset) == 'ClangCL' and $(Platform) == 'Win32'">x86</PreferredToolArchitecture>

Now we'd have to document that when overriding the clang used to build the jit stencils will chose that clang for building Python, too, if built with "/p:PlatformToolset=ClangCL".

So option 1 gives more flexibility, but needs (?) a warning and "specifying the same thing twice" might feel like a wart.
Option 3 needs even more documentation?

I am unsure what to do best, but considering the jit is maybe no longer experimental, we should care to find a good solution here?

Fidget-Spinner · 2025-12-16T22:00:30Z

I am unsure what to do best, but considering the jit is maybe no longer experimental, we should care to find a good solution here?

The JIT is still experimental, but Python build flags and in general build.bat flags are bound by backwards compatibility, so when adding things we have to treat it as non-experimental.

xhochy requested review from brandtbucher, diegorusso and savannahostrowski as code owners September 3, 2025 14:53

bedevere-app bot added the awaiting review label Sep 3, 2025

bedevere-app bot mentioned this pull request Sep 3, 2025

Wrong LLVM tools picked up if using MSVC activation and custom LLVM build #138451

Open

xhochy mentioned this pull request Sep 3, 2025

python v3.13.7 conda-forge/python-feedstock#807

Merged

3 tasks

Fidget-Spinner reviewed Sep 4, 2025

View reviewed changes

AA-Turner requested a review from a team September 4, 2025 20:12

zooba reviewed Sep 5, 2025

View reviewed changes

danigm mentioned this pull request Sep 29, 2025

Make LLVM_VERSION configurable with env variable #138497

Closed

xhochy force-pushed the gh-138451 branch from 788de07 to be6b5b2 Compare October 26, 2025 10:21

savannahostrowski reviewed Oct 26, 2025

View reviewed changes

Misc/NEWS.d/next/Build/2025-09-03-14-55-59.gh-issue-138451.-Qzh2S.rst Outdated Show resolved Hide resolved

savannahostrowski requested changes Oct 26, 2025

View reviewed changes

bedevere-app bot added awaiting changes and removed awaiting review labels Oct 26, 2025

xhochy requested review from AA-Turner, corona10, emmatyping and erlend-aasland as code owners October 27, 2025 08:41

bedevere-app bot added awaiting change review and removed awaiting changes labels Oct 27, 2025

bedevere-app bot requested a review from savannahostrowski October 27, 2025 10:51

savannahostrowski reviewed Oct 30, 2025

View reviewed changes

Tools/jit/README.md Outdated Show resolved Hide resolved

Tools/jit/_llvm.py Outdated Show resolved Hide resolved

zooba reviewed Oct 30, 2025

View reviewed changes

configure.ac Show resolved Hide resolved

xhochy and others added 10 commits December 2, 2025 14:19

Support custom LLVM installation path

3111c2d

Add support for explicitly defined LLVM installation location. This is necessary as MSVC now comes with its own LLVM installation and activating MSVC via vcvars.bat will put LLVM tools on the `PATH` before the local ones.

📜🤖 Added by blurb_it.

8321790

Use LLVM_TOOLS_INSTALL_DIR to search for LLVM tools

e6082d4

Don't join on empty values.

8bd8a98

Co-authored-by: Steve Dower <steve.dower@microsoft.com>

Fix syntax error

297fe1c

Document LLVM_TOOLS_INSTALL_DIR

e9c2cc3

Add LLVM_TOOLS_INSTALL_DIR to configure

a57427c

Fix wording in README

b90f60b

Expect tools in <toolsdir>/bin

06c9e51

Add environment variables to PCbuild

c801103

xhochy force-pushed the gh-138451 branch from 5023625 to c801103 Compare December 2, 2025 13:46

xhochy requested review from savannahostrowski and zooba December 2, 2025 14:36

savannahostrowski approved these changes Dec 2, 2025

View reviewed changes

Misc/NEWS.d/next/Build/2025-09-03-14-55-59.gh-issue-138451.-Qzh2S.rst Outdated Show resolved Hide resolved

bedevere-app bot added awaiting merge and removed awaiting change review labels Dec 2, 2025

Be more specific in NEWS entry

4801b37

chris-eibl mentioned this pull request Dec 11, 2025

GH-141362: Make get_externals handle fetching platform-specific release artifacts #142405

Merged

	if (llvm_tools_dir := os.getenv("LLVM_TOOLS_INSTALL_DIR")) is not None:
	if llvm_tools_dir := os.getenv("LLVM_TOOLS_INSTALL_DIR")):

Uh oh!

gh-138451: Support custom LLVM installation path #138452

Are you sure you want to change the base?

gh-138451: Support custom LLVM installation path #138452

Uh oh!

Conversation

xhochy commented Sep 3, 2025 • edited by bedevere-app bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

python-cla-bot bot commented Sep 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bedevere-app bot commented Sep 3, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

xhochy commented Oct 26, 2025

Uh oh!

Uh oh!

savannahostrowski left a comment

Choose a reason for hiding this comment

Uh oh!

bedevere-app bot commented Oct 26, 2025

Uh oh!

xhochy commented Oct 27, 2025

Uh oh!

bedevere-app bot commented Oct 27, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

savannahostrowski left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

chris-eibl commented Dec 2, 2025

Uh oh!

chris-eibl commented Dec 2, 2025

Uh oh!

zooba commented Dec 3, 2025

Uh oh!

chris-eibl commented Dec 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zooba commented Dec 3, 2025

Uh oh!

xhochy commented Dec 16, 2025

Uh oh!

chris-eibl commented Dec 16, 2025

Uh oh!

chris-eibl commented Dec 16, 2025

Uh oh!

chris-eibl commented Dec 16, 2025

Uh oh!

Fidget-Spinner commented Dec 16, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

xhochy commented Sep 3, 2025 •

edited by bedevere-app bot

Loading

python-cla-bot bot commented Sep 3, 2025 •

edited

Loading

chris-eibl commented Dec 3, 2025 •

edited

Loading