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.

savannahostrowski · 2025-10-30T02:30:20Z

Tools/jit/README.md

 The JIT compiler does not require end users to install any third-party dependencies, but part of it must be *built* using LLVM[^why-llvm]. You are *not* required to build the rest of CPython using LLVM, or even the same version of LLVM (in fact, this is uncommon).

-LLVM version 19 is the officially supported version. You can modify if needed using the `LLVM_VERSION` env var during configure. Both `clang` and `llvm-readobj` need to be installed and discoverable (version suffixes, like `clang-19`, are okay). It's highly recommended that you also have `llvm-objdump` available, since this allows the build script to dump human-readable assembly for the generated code.
+LLVM version 19 is the officially supported version. You can modify if needed using the `LLVM_VERSION` env var during configure. Both `clang` and `llvm-readobj` need to be installed and discoverable (version suffixes, like `clang-19`, are okay). It's highly recommended that you also have `llvm-objdump` available, since this allows the build script to dump human-readable assembly for the generated code. If you have multiple matching LLVM installations, you can use `LLVM_TOOLS_INSTALL_DIR` to point to the preferred installation prefix.


We should rephrase/restructure this section to say that you can configure LLVM_VERSION and LLVM_TOOLS_INSTALL_DIR during configure now that this PR has been refactored. Maybe something like:

You can customize the LLVM configuration using environment variables before running configure: - LLVM_VERSION: Specify a different LLVM version (default: 19) - LLVM_TOOLS_INSTALL_DIR: Point to a specific LLVM installation prefix when multiple installations exist

For additional clarity, it's probably worth specifying that LLVM_TOOLS_INSTALL_DIR should be the installation prefix (the directory containing bin/, not bin/ itself; related to my comment above).

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 LLVMToolsVersion 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 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

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

	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!

Choose a reason for hiding this comment

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

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