8000 gh-130039: Tailcall for windows builds by Fidget-Spinner · Pull Request #130040 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

gh-130039: Tailcall for windows builds #130040

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 17 commits into from
Mar 11, 2025
Merged

gh-130039: Tailcall for windows builds #130040

Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
ab0a611
Tail-calling flag for Window
Fidget-Spinner Feb 12, 2025
d57c175
Update pythoncore.vcxproj
Fidget-Spinner Feb 12, 2025
3eb568f
Add Windows to the tailcall CI
Fidget-Spinner Feb 12, 2025
4dd19cf
Update tail-call.yml
Fidget-Spinner Feb 12, 2025
7f91920
Update tail-call.yml
Fidget-Spinner Feb 12, 2025
c3d656b
Update pycore_debug_offsets.h
Fidget-Spinner Feb 12, 2025
3638cda
Update pycore_debug_offsets.h
Fidget-Spinner Feb 12, 2025
b8b79ed
Update pycore_debug_offsets.h
Fidget-Spinner Feb 12, 2025
d3766d8
Merge branch 'main' into tailcall_for_windows
Fidget-Spinner Feb 12, 2025
c811b8a
Address review by using env vars
Fidget-Spinner Feb 13, 2025
904b7ee
Merge remote-tracking branch 'upstream/main' into tailcall_for_windows
Fidget-Spinner Feb 13, 2025
991b50f
Merge remote-tracking branch 'upstream/main' into tailcall_for_windows
Fidget-Spinner Feb 15, 2025
166187c
Update pythoncore.vcxproj
Fidget-Spinner Feb 15, 2025
5f7ad24
Update .github/workflows/tail-call.yml
Fidget-Spinner Mar 5, 2025
6d7b689
Update tail-call.yml
Fidget-Spinner Mar 5, 2025
aadfec4
Address review
Fidget-Spinner Mar 5, 2025
1d7c051
Partially revert "Address review"
Fidget-Spinner Mar 5, 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
30 changes: 20 additions & 10 deletions .github/workflows/tail-call.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,14 @@ name: Tail calling interpreter
on:
pull_request:
paths:
- '.github/workflows/tail-call.yml'
Copy link
Member

Choose a reason for hiding this comment

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

This feels wrong to me, but if it's needed, then sure. Hopefully someone else can sign off on this part of the change.

Copy link
Member

Choose a reason for hiding this comment

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

This is fine: when changing this file, it's good to run it as well to make sure we didn't break it -- which is rather too easy with YAML.

(And let's strip the trailing spaces.)

Suggested change
- '.github/workflows/tail-call.yml'
- '.github/workflows/tail-call.yml'

(We should add yaml to this list, feel free to include it in the PR if you like:)

cpython/.pre-commit-config.yaml

Line 49 in 2904ec2

types_or: [c, inc, python, rst]

- 'Python/bytecodes.c'
- 'Python/ceval.c'
- 'Python/ceval_macros.h'
- 'Python/generated_cases.c.h'
push:
paths:
- '.github/workflows/tail-call.yml'
- 'Python/bytecodes.c'
- 'Python/ceval.c'
- 'Python/ceval_macros.h'
Expand Down Expand Up @@ -35,7 +37,7 @@ jobs:
target:
# Un-comment as we add support for more platforms for tail-calling interpreters.
# - i686-pc-windows-msvc/msvc
# - x86_64-pc-windows-msvc/msvc
- x86_64-pc-windows-msvc/msvc
# - aarch64-pc-windows-msvc/msvc
- x86_64-apple-darwin/clang
- aarch64-apple-darwin/clang
Expand All @@ -48,9 +50,9 @@ jobs:
# - target: i686-pc-windows-msvc/msvc
# architecture: Win32
# runner: windows-latest
# - target: x86_64-pc-windows-msvc/msvc
# architecture: x64
# runner: windows-latest
- target: x86_64-pc-windows-msvc/msvc
architecture: x64
runner: windows-latest
# - target: aarch64-pc-windows-msvc/msvc
# architecture: ARM64
# runner: windows-latest
Expand Down Expand Up @@ -79,23 +81,31 @@ jobs:

- name: Native Windows (debug)
if: runner.os == 'Windows' && matrix.architecture != 'ARM64'
shell: cmd
run: |
choco install llvm --allow-downgrade --no-progress --version ${{ matrix.llvm }}.1.0
choco install llvm --allow-downgrade --no-progress --version ${{ matrix.llvm }}.1.5
set PlatformToolset=clangcl
set LLVMToolsVersion=19.1.5
set LLVMInstallDir=C:\Program Files\LLVM
./PCbuild/build.bat --tail-call-interp -d -p ${{ matrix.architecture }}
./PCbuild/rt.bat -d -p ${{ matrix.architecture }} -q --multiprocess 0 --timeout 4500 --verbose2 --verbose3

# No tests (yet):
- name: Emulated Windows (release)
if: runner.os == 'Windows' && matrix.architecture == 'ARM64'
shell: cmd
run: |
choco install llvm --allow-downgrade --no-progress --version ${{ matrix.llvm }}.1.0
choco install llvm --allow-downgrade --no-progress --version ${{ matrix.llvm }}.1.5
set PlatformToolset=clangcl
set LLVMToolsVersion=19.1.5
set LLVMInstallDir=C:\Program Files\LLVM
./PCbuild/build.bat --tail-call-interp -p ${{ matrix.architecture }}

# The `find` line is required as a result of https://github.com/actions/runner-images/issues/9966.
# This is a bug in the macOS runner image where the pre-installed Python is installed in the same
# directory as the Homebrew Python, which causes the build to fail for macos-13. This line removes
# the symlink to the pre-installed Python so that the Homebrew Python is used instead.
- name: Native macOS (debug)
- name: Native macOS (release)
if: runner.os == 'macOS'
run: |
brew update
Expand All @@ -104,16 +114,16 @@ jobs:
export SDKROOT="$(xcrun --show-sdk-path)"
export PATH="/opt/homebrew/opt/llvm/bin:$PATH"
export PATH="/usr/local/opt/llvm/bin:$PATH"
CC=clang-19 ./configure --with-tail-call-interp --with-pydebug
CC=clang-19 ./configure --with-tail-call-interp
make all --jobs 4
./python.exe -m test --multiprocess 0 --timeout 4500 --verbose2 --verbose3

- name: Native Linux (release)
- name: Native Linux (debug)
if: runner.os == 'Linux' && matrix.target != 'free-threading'
run: |
sudo bash -c "$(wget -O - https://apt.llvm.org/llvm.sh)" ./llvm.sh ${{ matrix.llvm }}
export PATH="$(llvm-config-${{ matrix.llvm }} --bindir):$PATH"
CC=clang-19 ./configure --with-tail-call-interp
CC=clang-19 ./configure --with-tail-call-interp --with-pydebug
make all --jobs 4
./python -m test --multiprocess 0 --timeout 4500 --verbose2 --verbose3

Expand Down
2 changes: 1 addition & 1 deletion Include/internal/pycore_debug_offsets.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ extern "C" {
declaration \
_GENERATE_DEBUG_SECTION_LINUX(name)

#if defined(MS_WINDOWS)
#if defined(MS_WINDOWS) && !defined(__clang__)
Copy link
Member

Choose a reason for hiding this comment

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

Does this need an alternate definition for Windows + clang?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, below there's a default that it uses is we're not one of the supported platforms. And that default works for Clang Windows.

#define _GENERATE_DEBUG_SECTION_WINDOWS(name) \
_Pragma(Py_STRINGIFY(section(Py_STRINGIFY(name), read, write))) \
__declspec(allocate(Py_STRINGIFY(name)))
Expand Down
3 changes: 3 additions & 0 deletions PCbuild/build.bat
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ echo. --experimental-jit Enable the experimental just-in-time compiler
echo. --experimental-jit-off Ditto but off by default (PYTHON_JIT=1 enables).
echo. --experimental-jit-interpreter Enable the experimental Tier 2 interpreter.
echo. --pystats Enable PyStats collection.
echo. --tail-call-interp Enable tail-calling interpreter.
echo.
echo.Available flags to avoid building certain modules.
echo.These flags have no effect if '-e' is not given:
Expand Down Expand Up @@ -95,6 +96,7 @@ if "%~1"=="--experimental-jit-off" (set UseJIT=true) & (set UseTIER2=3) & shift
if "%~1"=="--experimental-jit-interpreter" (set UseTIER2=4) & shift & goto CheckOpts
if "%~1"=="--experimental-jit-interpreter-off" (set UseTIER2=6) & shift & goto CheckOpts
if "%~1"=="--pystats" (set PyStats=1) & shift & goto CheckOpts
if "%~1"=="--tail-call-interp" (set UseTailCallInterp=true) & shift & goto CheckOpts
rem These use the actual property names used by MSBuild. We could just let
rem them in through the environment, but we specify them on the command line
rem anyway for visibility so set defaults after this
Expand Down Expand Up @@ -189,6 +191,7 @@ echo on
/p:UseJIT=%UseJIT%^
/p:UseTIER2=%UseTIER2%^
/p:PyStats=%PyStats%^
/p:UseTailCallInterp=%UseTailCallInterp%^
%1 %2 %3 %4 %5 %6 %7 %8 %9

@echo off
Expand Down
1 change: 1 addition & 0 deletions PCbuild/pythoncore.vcxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@
<PreprocessorDefinitions Condition="$(IncludeExternals)">_Py_HAVE_ZLIB;%(PreprocessorDefinitions)</PreprocessorDefinitions>
<PreprocessorDefinitions Condition="'$(UseJIT)' == 'true'">_Py_JIT;%(PreprocessorDefinitions)</PreprocessorDefinitions>
<PreprocessorDefinitions Condition="'$(UseTIER2)' != '' and '$(UseTIER2)' != '0'">_Py_TIER2=$(UseTIER2);%(PreprocessorDefinitions)</PreprocessorDefinitions>
<PreprocessorDefinitions Condition="'$(UseTailCallInterp)' == 'true'">Py_TAIL_CALL_INTERP=1;%(PreprocessorDefinitions)</PreprocessorDefinitions>
</ClCompile>
<Link>
<AdditionalDependencies>version.lib;ws2_32.lib;pathcch.lib;bcrypt.lib;%(AdditionalDependencies)</AdditionalDependencies>
Expand Down
Loading
0