Skip to content

GH-133171: Prevent combinations of --disable-gil and --enable-experimental-jit (for now)#133179

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
brandtbucher merged 4 commits into from
May 2, 2025
Merged

GH-133171: Prevent combinations of --disable-gil and --enable-experimental-jit (for now) #133179

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1076f10
--disable-gil doesn't support --enable-experimental-jit
brandtbucher Apr 29, 2025
9b9ff5d
blurb add
brandtbucher Apr 29, 2025
976cdb9
Remove JIT/FT CI job
brandtbucher Apr 30, 2025
fd55937
Merge branch 'main' into nogil-means-nojit
brandtbucher May 2, 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
53 changes: 27 additions & 26 deletions .github/workflows/jit.yml
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -126,29 +126,30 @@ jobs:
make all --jobs 4
./python -m test --multiprocess 0 --timeout 4500 --verbose2 --verbose3

jit-with-disabled-gil:
name: Free-Threaded (Debug)
needs: interpreter
runs-on: ubuntu-24.04
timeout-minutes: 90
strategy:
fail-fast: false
matrix:
llvm:
- 19
steps:
- uses: actions/checkout@v4
with:
persist-credentials: false
- uses: actions/setup-python@v5
with:
python-version: '3.11'
- name: Build with JIT enabled and GIL disabled
run: |
sudo bash -c "$(wget -O - https://apt.llvm.org/llvm.sh)" ./llvm.sh ${{matrix.llvm }}
export PATH="$(llvm-config-${{matrix.llvm }} --bindir):$PATH"
./configure --enable-experimental-jit --with-pydebug --disable-gil
make all --jobs 4
- name: Run tests
run: |
./python -m test --multiprocess 0 --timeout 4500 --verbose2 --verbose3
# XXX: GH-133171
# jit-with-disabled-gil:
# name: Free-Threaded (Debug)
# needs: interpreter
# runs-on: ubuntu-24.04
# timeout-minutes: 90
# strategy:
# fail-fast: false
# matrix:
# llvm:
# - 19
# steps:
# - uses: actions/checkout@v4
# with:
# persist-credentials: false
# - uses: actions/setup-python@v5
# with:
# python-version: '3.11'
# - name: Build with JIT enabled and GIL disabled
# run: |
# sudo bash -c "$(wget -O - https://apt.llvm.org/llvm.sh)" ./llvm.sh ${{matrix.llvm }}
# export PATH="$(llvm-config-${{matrix.llvm }} --bindir):$PATH"
# ./configure --enable-experimental-jit --with-pydebug --disable-gil
# make all --jobs 4
# - name: Run tests
# run: |
# ./python -m test --multiprocess 0 --timeout 4500 --verbose2 --verbose3
2 changes: 2 additions & 0 deletions Misc/NEWS.d/next/Build/2025-04-29-15-29-11.gh-issue-133171.YbwbwP.rst
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
Since free-threaded builds do not support the experimental JIT compiler,
prevent these configurations from being combined.
7 changes: 7 additions & 0 deletions PCbuild/build.bat
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -123,6 +123,13 @@ if "%do_pgo%" EQU "true" if "%platf%" EQU "x64" (
)
)

if "%UseDisableGil%" EQU "true" if "%UseTIER2%" NEQ "" (
Copy link
Member

@zoobazoobaApr 30, 2025

Choose a reason for hiding this comment

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

Strictly speaking this would be better as target in pythoncore.vcxproj (see the existing _WarnAboutZlib for example), since there's no requirement to use the batch file, and it's very easy to bypass it. (The same goes for the PGO warning above, I don't remember when that was added.)

But it's not a big deal. It's relatively temporary code anyway

Copy link
MemberAuthor

@brandtbucherbrandtbucherMay 2, 2025

Choose a reason for hiding this comment

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

Yeah, this will only be around for 3.14. I could just put an #error in the C source or whatever, but I figure failing early is best.

Besides, it's not like there's anything wrong with enabling both features. It just doesn't work the way you'd expect.

rem GH-133171: This configuration builds the JIT but never actually uses it,
rem which is surprising (and strictly worse than not building it at all):
echo.ERROR: --experimental-jit cannot be used with --disable-gil.
exit /b 1
)

if not exist "%GIT%" where git > "%TEMP%\git.loc" 2> nul && set /P GIT= < "%TEMP%\git.loc" & del "%TEMP%\git.loc"
if exist "%GIT%" set GITProperty=/p:GIT="%GIT%"
if not exist "%GIT%" echo Cannot find Git on PATH & set GITProperty=
Expand Down
6 changes: 6 additions & 0 deletions configure
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 6 additions & 0 deletions configure.ac
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -2786,6 +2786,12 @@ AC_SUBST([REGEN_JIT_COMMAND])
AC_SUBST([JIT_STENCILS_H])
AC_MSG_RESULT([$tier2_flags $jit_flags])

if test "$disable_gil" = "yes" -a "$enable_experimental_jit" != "no" then
# GH-133171: This configuration builds the JIT but never actually uses it,
# which is surprising (and strictly worse than not building it at all):
AC_MSG_ERROR([--enable-experimental-jit cannot be used with --disable-gil.])
fi

case "$ac_cv_cc_name" in
mpicc)
CFLAGS_NODIST="$CFLAGS_NODIST"
Expand Down
Loading