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.

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>

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.

…h2S.rst Co-authored-by: Savannah Ostrowski <savannah@python.org>

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

savannahostrowski · 2025-10-30T02:49:18Z

Tools/jit/_llvm.py

+) -> str | None:
+    # Explicitly defined LLVM installation location
+    if llvm_tools_install_dir:
+        path = os.path.join(llvm_tools_install_dir, tool)


For consistency, I think this should point to the installation prefix where the tools are in bin/, like we have on lines 96 and 102, right?

Suggested change

path = os.path.join(llvm_tools_install_dir, tool)

path = os.path.join(llvm_tools_install_dir, "bin", tool)

zooba · 2025-10-30T20:46:02Z

configure.ac

          [AS_VAR_APPEND([CFLAGS_NODIST], [" $jit_flags"])
           AS_VAR_SET([REGEN_JIT_COMMAND],
-                      ["\$(PYTHON_FOR_REGEN) \$(srcdir)/Tools/jit/build.py ${ARCH_TRIPLES:-$host} --output-dir . --pyconfig-dir . --cflags=\"$CFLAGS_JIT\" --llvm-version=\"$LLVM_VERSION\""])
+                      ["\$(PYTHON_FOR_REGEN) \$(srcdir)/Tools/jit/build.py ${ARCH_TRIPLES:-$host} --output-dir . --pyconfig-dir . --cflags=\"$CFLAGS_JIT\" --llvm-version=\"$LLVM_VERSION\" --llvm-tools-install-dir=\"$LLVM_TOOLS_INSTALL_DIR\""])


Does this change need to be replicated under PCbuild somewhere?

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 and others added 6 commits October 26, 2025 11:18

Support custom LLVM installation path

0d7195d

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.

fb6867f

Use LLVM_TOOLS_INSTALL_DIR to search for LLVM tools

92616cf

Don't join on empty values.

8a2d921

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

Fix syntax error

593d5a4

Document LLVM_TOOLS_INSTALL_DIR

be6b5b2

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

Add LLVM_TOOLS_INSTALL_DIR to configure

634210b

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

Update Misc/NEWS.d/next/Build/2025-09-03-14-55-59.gh-issue-138451.-Qz…

5023625

…h2S.rst Co-authored-by: Savannah Ostrowski <savannah@python.org>

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

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

	path = os.path.join(llvm_tools_install_dir, tool)
	path = os.path.join(llvm_tools_install_dir, "bin", tool)

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

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

xhochy commented Sep 3, 2025 •

edited by bedevere-app bot

Loading

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

edited

Loading