Skip to content
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.

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

gh-91049: Introduce set vectorcall field API for PyFunctionObject #92257

Merged
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
9cf96ec
Enable setting vectorcall field on PyFunctionObjects
May 3, 2022
282e3dc
check if vectorcall is nondefault before inlining call
May 3, 2022
edcdcf1
📜🤖 Added by blurb_it.
blurb-it[bot] May 3, 2022
debf5a9
Apply suggestions from code review
adphrost May 3, 2022
473d18d
addressed comments
May 3, 2022
76e8c9d
added to docs
May 3, 2022
3337459
Merge branch 'main' into pyfunctionobject-set-vectorcall-field
Jun 16, 2022
1543f44
updated doc with fix by itamaro
Jun 16, 2022
08082bd
formatting
Jun 16, 2022
ed93327
removed doc from 3.11
Jun 16, 2022
24ffd30
remove lines from 3.11
Jun 16, 2022
89cb4b2
Apply review feedback from vstinner and markshannon
itamaro Jun 21, 2022
3387d4a
Merge branch 'main' into gh-91049-set-vectorcall
itamaro Sep 2, 2022
b0cc28a
Merge branch 'main' into gh-91049-set-vectorcall
itamaro Sep 5, 2022
99e4085
Add deopt on func version in LOAD_ATTR_GETATTRIBUTE_OVERRIDDEN
itamaro Sep 6, 2022
24353c8
write func version to cache keys version when specializing LOAD_ATTR_…
itamaro Sep 6, 2022
60f7769
remove redundant argcount check in LOAD_ATTR_GETATTRIBUTE_OVERRIDDEN …
itamaro Sep 6, 2022
376ee75
Add missing periods in docs
itamaro Sep 6, 2022
56ffc70
move warning comment to the function C API docs
itamaro Sep 6, 2022
5340e87
Merge branch 'main' into pyfunctionobject-set-vectorcall-field
itamaro Sep 6, 2022
a5e9d13
fix race with GH-96519 (removed func_version in LOAD_ATTR_GETATTRIBUT…
itamaro Sep 6, 2022
12faf52
PEP-7 formatting
itamaro Sep 7, 2022
6623470
Address review feedback
itamaro Sep 7, 2022
9149f14
Add test for LOAD_ATTR specialization when overriding vectorcall of _…
itamaro Sep 8, 2022
e732d7e
Improve setvectorcall + specialization testing
itamaro Sep 8, 2022
84874fb
Merge branch 'main' into pyfunctionobject-set-vectorcall-field
itamaro Sep 15, 2022
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
5 changes: 5 additions & 0 deletions Doc/c-api/function.rst
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,11 @@ There are a few functions specific to Python functions.
Raises :exc:`SystemError` and returns ``-1`` on failure.


.. c:function:: void PyFunction_SetVectorcall(PyFunctionObject *func, vectorcallfunc vectorcall)

Set the vectorcall field of a given function object *func*
vstinner marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Set the vectorcall field of a given function object *func*
Set the vectorcall field of a given function object *func*.



.. c:function:: PyObject* PyFunction_GetClosure(PyObject *op)

Return the closure associated with the function object *op*. This can be ``NULL``
Expand Down
4 changes: 4 additions & 0 deletions Doc/whatsnew/3.12.rst
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,10 @@ New Features
an additional metaclass argument.
(Contributed by Wenzel Jakob in :gh:`93012`.)

* Add new function :c:func:`PyFunction_SetVectorcall` to the C API
which sets the vectorcall field of a given PyFunctionObject
Copy link
Member

Choose a reason for hiding this comment

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

You may explain the purpose of the function here, like mention Cinder JIT and Pyjion.

Copy link
Member

Choose a reason for hiding this comment

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

I'd rather not have a "purpose". We should state clearly what the function does, not bless some particular use case.

@adphrost
Could you add a warning that the new function pointer must have exactly the same behavior as the unaltered function.
I am a little concerned that C extensions will abuse this, and we will have deal with the bug reports.

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
which sets the vectorcall field of a given PyFunctionObject
which sets the vectorcall field of a given :c:type:`PyFunctionObject`.

(Contributed by Andrew Frost in :gh:`92257`.)

Porting to Python 3.12
----------------------

Expand Down
1 change: 1 addition & 0 deletions Include/cpython/funcobject.h
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ PyAPI_FUNC(PyObject *) PyFunction_GetGlobals(PyObject *);
PyAPI_FUNC(PyObject *) PyFunction_GetModule(PyObject *);
PyAPI_FUNC(PyObject *) PyFunction_GetDefaults(PyObject *);
PyAPI_FUNC(int) PyFunction_SetDefaults(PyObject *, PyObject *);
PyAPI_FUNC(void) PyFunction_SetVectorcall(PyFunctionObject *, vectorcallfunc);
PyAPI_FUNC(PyObject *) PyFunction_GetKwDefaults(PyObject *);
PyAPI_FUNC(int) PyFunction_SetKwDefaults(PyObject *, PyObject *);
PyAPI_FUNC(PyObject *) PyFunction_GetClosure(PyObject *);
Expand Down
9 changes: 9 additions & 0 deletions Lib/test/test_call.py
Original file line number Diff line number Diff line change
Expand Up @@ -665,6 +665,15 @@ def __call__(self, *args):
self.assertEqual(expected, meth(*args1, **kwargs))
self.assertEqual(expected, wrapped(*args, **kwargs))

def test_setvectorcall(self):
from _testcapi import function_setvectorcall
def f(num): return num + 1
num = 10
self.assertEqual(11, f(num))
function_setvectorcall(f)
# make sure specializer is triggered by running > 50 times
for _ in range(51):
Fidget-Spinner marked this conversation as resolved.
Show resolved Hide resolved
self.assertIsNone(f(num))

class A:
def method_two_args(self, x, y):
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
CPython extensions providing optimized execution of Python bytecode (like Cinder JIT and Pyjion)
can benefit from being able to modify the vectorcall field on instances of PyFunctionObject to allow calling the optimized path (e.g. JIT-compiled) directly.

This PR introduces an API call where vectorcall field can be modified like so:
Copy link
Member

Choose a reason for hiding this comment

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

This text lands into https://docs.python.org/dev/whatsnew/changelog.html#changelog which is a changlog, you should not mention "a PR" there.

I suggest to just copy/paste what you wrote in whatsnew.


`void PyFunction_SetVectorcall(PyFunctionObject *func, vectorcallfunc vectorcall);`
Copy link
Member

Choose a reason for hiding this comment

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

Please document the function in Doc/c-api/function.rst and in the NEWS entry, refer to use with:

:c:func:`PyFunction_SetVectorcall`

New public functions should also be documented in What's New in Python 3.11 > C API > New features: Doc/whatsnew/3.11.rst.

18 changes: 18 additions & 0 deletions Modules/_testcapimodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -5356,6 +5356,23 @@ test_pyobject_vectorcall(PyObject *self, PyObject *args)
}


static PyObject *
override_vectorcall(
PyObject *callable, PyObject *const *args, size_t nargsf, PyObject *kwnames) {
Py_RETURN_NONE;
Copy link
Member

Choose a reason for hiding this comment

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

Could you modify the test to return the string "overridden" instead of just returning None?

}

static PyObject *
function_setvectorcall(PyObject *self, PyObject *func)
{
if (!PyFunction_Check(func)) {
PyErr_BadInternalCall();
return NULL;
}
PyFunction_SetVectorcall((PyFunctionObject *)func, (vectorcallfunc)override_vectorcall);
Py_RETURN_NONE;
}

static PyObject *
test_pyvectorcall_call(PyObject *self, PyObject *args)
{
Expand Down Expand Up @@ -6340,6 +6357,7 @@ static PyMethodDef TestMethods[] = {
{"pyobject_fastcall", test_pyobject_fastcall, METH_VARARGS},
{"pyobject_fastcalldict", test_pyobject_fastcalldict, METH_VARARGS},
{"pyobject_vectorcall", test_pyobject_vectorcall, METH_VARARGS},
{"function_setvectorcall", function_setvectorcall, METH_O},
{"pyvectorcall_call", test_pyvectorcall_call, METH_VARARGS},
{"stack_pointer", stack_pointer, METH_NOARGS},
#ifdef W_STOPCODE
Expand Down
10 changes: 10 additions & 0 deletions Objects/funcobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,9 @@ uint32_t _PyFunction_GetVersionForCurrentState(PyFunctionObject *func)
if (func->func_version != 0) {
return func->func_version;
}
if (func->vectorcall != _PyFunction_Vectorcall) {
return 0;
markshannon marked this conversation as resolved.
Show resolved Hide resolved
}
if (next_func_version == 0) {
return 0;
}
Expand Down Expand Up @@ -209,6 +212,13 @@ PyFunction_SetDefaults(PyObject *op, PyObject *defaults)
return 0;
}

void
PyFunction_SetVectorcall(PyFunctionObject *func, vectorcallfunc vectorcall)
{
func->func_version = 0;
erlend-aasland marked this conversation as resolved.
Show resolved Hide resolved
func->vectorcall = vectorcall;
}

PyObject *
PyFunction_GetKwDefaults(PyObject *op)
{
Expand Down
4 changes: 3 additions & 1 deletion Python/ceval.c
Original file line number Diff line number Diff line change
Expand Up @@ -4686,7 +4686,9 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, _PyInterpreterFrame *frame, int
function = PEEK(total_args + 1);
int positional_args = total_args - KWNAMES_LEN();
// Check if the call can be inlined or not
if (Py_TYPE(function) == &PyFunction_Type && tstate->interp->eval_frame == NULL) {
if (Py_TYPE(function) == &PyFunction_Type &&
tstate->interp->eval_frame == NULL &&
((PyFunctionObject *)function)->vectorcall == _PyFunction_Vectorcall) {
markshannon marked this conversation as resolved.
Show resolved Hide resolved
markshannon marked this conversation as resolved.
Show resolved Hide resolved
int code_flags = ((PyCodeObject*)PyFunction_GET_CODE(function))->co_flags;
PyObject *locals = code_flags & CO_OPTIMIZED ? NULL : PyFunction_GET_GLOBALS(function);
STACK_SHRINK(total_args);
Expand Down