Sorry, something went wrong.
There was a problem hiding this comment.
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.
Sorry, something went wrong.
| conversion_func conv_fn; | ||
| assert(oparg >= FVC_STR && oparg <= FVC_ASCII); | ||
| conv_fn = CONVERSION_FUNCTIONS[oparg]; | ||
| conv_fn = _PyEval_ConversionFuncs[oparg]; |
There was a problem hiding this comment.
We could make inst(CONVERT_VALUE replicate(4) inst(CONVERT_VALUE and rely on clang removing the lookup.
Sorry, something went wrong.
There was a problem hiding this comment.
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?
Sorry, something went wrong.
There was a problem hiding this comment.
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.
Sorry, something went wrong.
| PyAPI_DATA(const binaryfunc) _PyEval_BinaryOps[]; | ||
| PyAPI_DATA(const conversion_func) _PyEval_ConversionFuncs[]; | ||
|
|
||
| PyAPI_FUNC(int) _PyEval_CheckExceptStarTypeValid(PyThreadState *tstate, PyObject* right); |
There was a problem hiding this comment.
Rather than exposing all these symbols, could we put the function pointers in a struct, and pass that to the JIT as an argument?
Sorry, something went wrong.
There was a problem hiding this comment.
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).
Sorry, something went wrong.
⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️Hi! The buildbot AMD64 FreeBSD14 3.x has failed when building commit f0df35e. What do you need to do:
You can take a look at the buildbot page here: https://buildbot.python.org/all/#builders/1232/builds/1515 Summary of the results of the build (if available): == Click to see traceback logsTraceback (most recent call last):
File "<frozen getpath>", line 352, in <module>
ValueError: embedded null byte
Warning -- Uncaught thread exception: RuntimeError
Exception in thread Thread-60 (task):
RuntimeError: error evaluating path
Traceback (most recent call last):
File "/home/buildbot/buildarea/3.x.opsec-fbsd14/build/Lib/threading.py", line 1086, in _bootstrap_inner
self.run()
~~~~~~~~^^
File "/home/buildbot/buildarea/3.x.opsec-fbsd14/build/Lib/threading.py", line 1023, in run
self._target(*self._args, **self._kwargs)
~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/buildbot/buildarea/3.x.opsec-fbsd14/build/Lib/test/test_interpreters/test_stress.py", line 29, in task
interp = interpreters.create()
~~~~~~~~~~~~~~~~~~~^^
File "/home/buildbot/buildarea/3.x.opsec-fbsd14/build/Lib/test/support/interpreters/__init__.py", line 76, in create
id = _interpreters.create(isolated=True)
~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^
RuntimeError: interpreter creation failed
k
|
Sorry, something went wrong.
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.