GH-115802: JIT "small" code for Windows by brandtbucher · Pull Request #115964 · python/cpython · GitHub
Skip to content

GH-115802: JIT "small" code for Windows - #115964

Merged
brandtbucher merged 18 commits into
python:mainfrom
brandtbucher:justin-relax-b
Feb 29, 2024
Merged

brandtbucher merged 18 commits into
python:mainfrom
brandtbucher:justin-relax-b

Conversation

@brandtbucher

@brandtbucher brandtbucher commented Feb 26, 2024

Copy link
Copy Markdown
Member

For 32-bit Windows, the small code model works fine out-of-the-box.

For 64-bit Windows, we don't have position-independent code with GOT relocations like Linux and macOS. Instead, we compile the stencils like a Py_BUILD_CORE_MODULE binary extension module, which creates a level of indirection similar to a GOT (__impl_Py_XXX holds the address of Py_XXX). We process this just like the GOT on other platforms, and everything just works.

This does require adding PyAPI_FUNC and PyAPI_DATA to some symbols in internal headers to get the correct visibility, which is why this PR touches so many files.

Looks like this makes the Windows JIT ~3-4% faster.

@brandtbucher brandtbucher added performance Performance or resource usage skip news interpreter-core (Objects, Python, Grammar, and Parser dirs) labels Feb 26, 2024
@brandtbucher brandtbucher self-assigned this Feb 26, 2024
@bedevere-app bedevere-app Bot mentioned this pull request Feb 26, 2024
13 tasks
@brandtbucher
brandtbucher marked this pull request as ready for review February 26, 2024 18:58

@gvanrossum gvanrossum left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm sorry, I realized halfway through the review that this touches upon several large blank spaces in my brain. So this is not a very useful review. I do think that _PyOptimize_Optimize() belongs in an internal header though.

Comment thread Include/cpython/optimizer.h Outdated
Comment thread Python/bytecodes.c
Comment thread Tools/jit/_targets.py Outdated
Comment thread Python/bytecodes.c

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We could make inst(CONVERT_VALUE replicate(4) inst(CONVERT_VALUE and rely on clang removing the lookup.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Is there a way to do that where a fifth generic version isn't generated too? Otherwise, it doesn't help, so we might as well stay with this?

Comment thread Python/jit.c Outdated

@markshannon markshannon left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good, I've a few comments.

As a general comment regarding JIT development:
Don't be afraid to make small changes to bytecodes.c and other interpreter files to enable the JIT to work more smoothly.

PyAPI_DATA(const binaryfunc) _PyEval_BinaryOps[];
PyAPI_DATA(const conversion_func) _PyEval_ConversionFuncs[];

PyAPI_FUNC(int) _PyEval_CheckExceptStarTypeValid(PyThreadState *tstate, PyObject* right);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Rather than exposing all these symbols, could we put the function pointers in a struct, and pass that to the JIT as an argument?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The issue isn't finding symbols when jitting the code (jit.c is linked into the main executable and can find everything just fine).

The purpose of adding PyAPI_FUNC(...) and PyAPI_DATA(...) to the declarations is so they are declared with __declspec(dllimport) when compiling the templates. This makes Clang emit indirect memory accesses on Windows (similar to position-independent code on other platforms).

@brandtbucher
brandtbucher merged commit f0df35e into python:main Feb 29, 2024
@bedevere-bot

Copy link
Copy Markdown

woodruffw pushed a commit to woodruffw-forks/cpython that referenced this pull request Mar 4, 2024
adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 25, 2024
diegorusso pushed a commit to diegorusso/cpython that referenced this pull request Apr 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

interpreter-core (Objects, Python, Grammar, and Parser dirs) performance Performance or resource usage skip news topic-JIT

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants