From cf6098db30195b937ea044d1ab5fdbd443fe7368 Mon Sep 17 00:00:00 2001 From: justdan6 Date: Fri, 28 Jul 2023 15:55:56 -0600 Subject: [PATCH 1/5] Attempting to work out gh-106608 --- Include/internal/pycore_uops.h | 13 ++++++++++++- Python/executor.c | 33 +++++++++++++++++++++++++++++++++ Python/optimizer.c | 12 +++++------- 3 files changed, 50 insertions(+), 8 deletions(-) diff --git a/Include/internal/pycore_uops.h b/Include/internal/pycore_uops.h index edb141cc79f752b..a6a67bfdcb935b0 100644 --- a/Include/internal/pycore_uops.h +++ b/Include/internal/pycore_uops.h @@ -17,8 +17,9 @@ typedef struct { } _PyUOpInstruction; typedef struct { + PyObject_VAR_HEAD _PyExecutorObject base; - _PyUOpInstruction trace[_Py_UOP_MAX_TRACE_LENGTH]; // TODO: variable length + _PyUOpInstruction trace[1]; } _PyUOpExecutorObject; _PyInterpreterFrame *_PyUopExecute( @@ -26,6 +27,16 @@ _PyInterpreterFrame *_PyUopExecute( _PyInterpreterFrame *frame, PyObject **stack_pointer); +/* Cast argument to _PyUOpExecutorObject* type. */ +#define _PyUOpExecutor_CAST(op) \ + _Py_CAST(_PyUOpExecutorObject*, (op)) + +static inline Py_ssize_t PyUOpExecutor_GET_SIZE(PyObject *op) { + _PyUOpExecutorObject *executor = _PyUOpExecutor_CAST(op); + return Py_SIZE(executor); +} +#define PyUOpExecutor_GET_SIZE(op) PyUOpExecutor_GET_SIZE(_PyObject_CAST(op)) + #ifdef __cplusplus } #endif diff --git a/Python/executor.c b/Python/executor.c index 57525df202d861a..9adeda065f21eec 100644 --- a/Python/executor.c +++ b/Python/executor.c @@ -31,6 +31,39 @@ #define ENABLE_SPECIALIZATION 0 +// TODO: continue editing this from tupleobject.c to fit _PyUOpExecutorObject +static _PyUOpExecutorObject * +tuple_alloc(Py_ssize_t size) +{ + _PyUOpExecutorObject *op = maybe_freelist_pop(size); + if (op == NULL) { + /* Check for overflow */ + if ((size_t)size > ((size_t)PY_SSIZE_T_MAX - (sizeof(PyTupleObject) - + sizeof(PyObject *))) / sizeof(PyObject *)) { + return (_PyUOpExecutorObject *)PyErr_NoMemory(); + } + op = PyObject_GC_NewVar(_PyUOpExecutorObject, &PyTuple_Type, size); + if (op == NULL) + return NULL; + } + return op; +} + +PyObject * +PyUOpExecutor_New(_PyUOpInstruction trace[], Py_ssize_t size) +{ + _PyUOpExecutorObject *op; + op = tuple_alloc(size); + if (op == NULL) { + return NULL; + } + for (Py_ssize_t i = 0; i < size; i++) { + op->trace[i] = trace[i]; + } + _PyObject_GC_TRACK(op); + return (PyObject *) op; +} + _PyInterpreterFrame * _PyUopExecute(_PyExecutorObject *executor, _PyInterpreterFrame *frame, PyObject **stack_pointer) { diff --git a/Python/optimizer.c b/Python/optimizer.c index 09120c33d130cae..52b7021922609bd 100644 --- a/Python/optimizer.c +++ b/Python/optimizer.c @@ -320,13 +320,7 @@ uop_name(int index) { static Py_ssize_t uop_len(_PyUOpExecutorObject *self) { - int count = 0; - for (; count < _Py_UOP_MAX_TRACE_LENGTH; count++) { - if (self->trace[count].opcode == 0) { - break; - } - } - return count; + return PyUOpExecutor_GET_SIZE(self); } static PyObject * @@ -703,6 +697,10 @@ uop_optimize( return -1; } executor->base.execute = _PyUopExecute; + /* + This should replace PyObject_New and memcpy + _PyUOpExecutorObject *executor = PyUOpExecutor_New(trace, trace_length); + */ memcpy(executor->trace, trace, trace_length * sizeof(_PyUOpInstruction)); if (trace_length < _Py_UOP_MAX_TRACE_LENGTH) { executor->trace[trace_length].opcode = 0; // Sentinel From 8e0114b43e3d5e4a386678df0c97b372f7c18bae Mon Sep 17 00:00:00 2001 From: justdan6 Date: Sat, 29 Jul 2023 21:01:48 -0600 Subject: [PATCH 2/5] Building locally but PyUOpExecutor_Type is mostly stubbed out --- Include/internal/pycore_uops.h | 2 ++ Python/executor.c | 60 +++++++++++++++++++++++++++------- 2 files changed, 50 insertions(+), 12 deletions(-) diff --git a/Include/internal/pycore_uops.h b/Include/internal/pycore_uops.h index a6a67bfdcb935b0..3ef583f277ae9de 100644 --- a/Include/internal/pycore_uops.h +++ b/Include/internal/pycore_uops.h @@ -10,6 +10,8 @@ extern "C" { #define _Py_UOP_MAX_TRACE_LENGTH 32 +PyAPI_DATA(PyTypeObject) PyUOpExecutor_Type; + typedef struct { uint32_t opcode; uint32_t oparg; diff --git a/Python/executor.c b/Python/executor.c index 9adeda065f21eec..bad5c58052b990c 100644 --- a/Python/executor.c +++ b/Python/executor.c @@ -30,21 +30,57 @@ #undef ENABLE_SPECIALIZATION #define ENABLE_SPECIALIZATION 0 +// Unsure what's needed here and what's unnecessary. Set everything to 0 for now. +PyTypeObject PyUOpExecutor_Type = { + PyVarObject_HEAD_INIT(&PyType_Type, 0) + "PyUOpExecutor", + sizeof(_PyUOpExecutorObject) - sizeof(PyObject *), + sizeof(PyObject *), + 0, /* tp_dealloc */ + 0, /* tp_vectorcall_offset */ + 0, /* tp_getattr */ + 0, /* tp_setattr */ + 0, /* tp_as_async */ + 0, /* tp_repr */ + 0, /* tp_as_number */ + 0, /* tp_as_sequence */ + 0, /* tp_as_mapping */ + 0, /* tp_hash */ + 0, /* tp_call */ + 0, /* tp_str */ + PyObject_GenericGetAttr, /* tp_getattro */ + 0, /* tp_setattro */ + 0, /* tp_as_buffer */ + Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC | + Py_TPFLAGS_BASETYPE | Py_TPFLAGS_TUPLE_SUBCLASS | + _Py_TPFLAGS_MATCH_SELF | Py_TPFLAGS_SEQUENCE, /* tp_flags */ + 0, /* tp_doc */ + 0, /* tp_traverse */ + 0, /* tp_clear */ + 0, /* tp_richcompare */ + 0, /* tp_weaklistoffset */ + 0, /* tp_iter */ + 0, /* tp_iternext */ + 0, /* tp_methods */ + 0, /* tp_members */ + 0, /* tp_getset */ + 0, /* tp_base */ + 0, /* tp_dict */ + 0, /* tp_descr_get */ + 0, /* tp_descr_set */ + 0, /* tp_dictoffset */ + 0, /* tp_init */ + 0, /* tp_alloc */ + 0, /* tp_new */ + PyObject_GC_Del, /* tp_free */ +}; -// TODO: continue editing this from tupleobject.c to fit _PyUOpExecutorObject static _PyUOpExecutorObject * -tuple_alloc(Py_ssize_t size) +PyUOpExecutor_alloc(Py_ssize_t size) { - _PyUOpExecutorObject *op = maybe_freelist_pop(size); + _PyUOpExecutorObject *op = PyObject_GC_NewVar(_PyUOpExecutorObject, &PyUOpExecutor_Type, size); if (op == NULL) { - /* Check for overflow */ - if ((size_t)size > ((size_t)PY_SSIZE_T_MAX - (sizeof(PyTupleObject) - - sizeof(PyObject *))) / sizeof(PyObject *)) { - return (_PyUOpExecutorObject *)PyErr_NoMemory(); - } - op = PyObject_GC_NewVar(_PyUOpExecutorObject, &PyTuple_Type, size); - if (op == NULL) - return NULL; + return NULL; } return op; } @@ -53,7 +89,7 @@ PyObject * PyUOpExecutor_New(_PyUOpInstruction trace[], Py_ssize_t size) { _PyUOpExecutorObject *op; - op = tuple_alloc(size); + op = PyUOpExecutor_alloc(size); if (op == NULL) { return NULL; } From 3ebe927b168c6a7e09f4821a71a713f5a40bfe07 Mon Sep 17 00:00:00 2001 From: justdan6 Date: Sat, 29 Jul 2023 21:23:02 -0600 Subject: [PATCH 3/5] Use UOpExecutor_Type. Replaced PyObject_New with PyUOpExecutor_New --- Include/internal/pycore_uops.h | 3 ++- Python/executor.c | 47 +--------------------------------- Python/optimizer.c | 14 ++-------- 3 files changed, 5 insertions(+), 59 deletions(-) diff --git a/Include/internal/pycore_uops.h b/Include/internal/pycore_uops.h index 3ef583f277ae9de..20da3bbf6a64050 100644 --- a/Include/internal/pycore_uops.h +++ b/Include/internal/pycore_uops.h @@ -10,7 +10,7 @@ extern "C" { #define _Py_UOP_MAX_TRACE_LENGTH 32 -PyAPI_DATA(PyTypeObject) PyUOpExecutor_Type; +PyAPI_DATA(PyTypeObject) UOpExecutor_Type; typedef struct { uint32_t opcode; @@ -38,6 +38,7 @@ static inline Py_ssize_t PyUOpExecutor_GET_SIZE(PyObject *op) { return Py_SIZE(executor); } #define PyUOpExecutor_GET_SIZE(op) PyUOpExecutor_GET_SIZE(_PyObject_CAST(op)) +PyAPI_FUNC(PyObject *) PyUOpExecutor_New(_PyUOpInstruction trace[], Py_ssize_t size); #ifdef __cplusplus } diff --git a/Python/executor.c b/Python/executor.c index bad5c58052b990c..38320e49bc7cf4e 100644 --- a/Python/executor.c +++ b/Python/executor.c @@ -30,55 +30,10 @@ #undef ENABLE_SPECIALIZATION #define ENABLE_SPECIALIZATION 0 -// Unsure what's needed here and what's unnecessary. Set everything to 0 for now. -PyTypeObject PyUOpExecutor_Type = { - PyVarObject_HEAD_INIT(&PyType_Type, 0) - "PyUOpExecutor", - sizeof(_PyUOpExecutorObject) - sizeof(PyObject *), - sizeof(PyObject *), - 0, /* tp_dealloc */ - 0, /* tp_vectorcall_offset */ - 0, /* tp_getattr */ - 0, /* tp_setattr */ - 0, /* tp_as_async */ - 0, /* tp_repr */ - 0, /* tp_as_number */ - 0, /* tp_as_sequence */ - 0, /* tp_as_mapping */ - 0, /* tp_hash */ - 0, /* tp_call */ - 0, /* tp_str */ - PyObject_GenericGetAttr, /* tp_getattro */ - 0, /* tp_setattro */ - 0, /* tp_as_buffer */ - Py_TPFLAGS_DEFAULT | Py_TPFLAGS_HAVE_GC | - Py_TPFLAGS_BASETYPE | Py_TPFLAGS_TUPLE_SUBCLASS | - _Py_TPFLAGS_MATCH_SELF | Py_TPFLAGS_SEQUENCE, /* tp_flags */ - 0, /* tp_doc */ - 0, /* tp_traverse */ - 0, /* tp_clear */ - 0, /* tp_richcompare */ - 0, /* tp_weaklistoffset */ - 0, /* tp_iter */ - 0, /* tp_iternext */ - 0, /* tp_methods */ - 0, /* tp_members */ - 0, /* tp_getset */ - 0, /* tp_base */ - 0, /* tp_dict */ - 0, /* tp_descr_get */ - 0, /* tp_descr_set */ - 0, /* tp_dictoffset */ - 0, /* tp_init */ - 0, /* tp_alloc */ - 0, /* tp_new */ - PyObject_GC_Del, /* tp_free */ -}; - static _PyUOpExecutorObject * PyUOpExecutor_alloc(Py_ssize_t size) { - _PyUOpExecutorObject *op = PyObject_GC_NewVar(_PyUOpExecutorObject, &PyUOpExecutor_Type, size); + _PyUOpExecutorObject *op = PyObject_GC_NewVar(_PyUOpExecutorObject, &UOpExecutor_Type, size); if (op == NULL) { return NULL; } diff --git a/Python/optimizer.c b/Python/optimizer.c index 52b7021922609bd..cc11b8e5151995a 100644 --- a/Python/optimizer.c +++ b/Python/optimizer.c @@ -359,7 +359,7 @@ PySequenceMethods uop_as_sequence = { .sq_item = (ssizeargfunc)uop_item, }; -static PyTypeObject UOpExecutor_Type = { +PyTypeObject UOpExecutor_Type = { PyVarObject_HEAD_INIT(&PyType_Type, 0) .tp_name = "uop_executor", .tp_basicsize = sizeof(_PyUOpExecutorObject), @@ -692,19 +692,9 @@ uop_optimize( return trace_length; } OBJECT_STAT_INC(optimization_traces_created); - _PyUOpExecutorObject *executor = PyObject_New(_PyUOpExecutorObject, &UOpExecutor_Type); - if (executor == NULL) { - return -1; - } - executor->base.execute = _PyUopExecute; - /* - This should replace PyObject_New and memcpy _PyUOpExecutorObject *executor = PyUOpExecutor_New(trace, trace_length); - */ + executor->base.execute = _PyUopExecute; memcpy(executor->trace, trace, trace_length * sizeof(_PyUOpInstruction)); - if (trace_length < _Py_UOP_MAX_TRACE_LENGTH) { - executor->trace[trace_length].opcode = 0; // Sentinel - } *exec_ptr = (_PyExecutorObject *)executor; return 1; } From cdbd49d379c477e75bfddf40788b37dd863f4ca4 Mon Sep 17 00:00:00 2001 From: justdan6 Date: Sat, 29 Jul 2023 21:28:16 -0600 Subject: [PATCH 4/5] Remove memcpy. There's also a warning to fix --- Python/optimizer.c | 1 - 1 file changed, 1 deletion(-) diff --git a/Python/optimizer.c b/Python/optimizer.c index cc11b8e5151995a..d91857480d13216 100644 --- a/Python/optimizer.c +++ b/Python/optimizer.c @@ -694,7 +694,6 @@ uop_optimize( OBJECT_STAT_INC(optimization_traces_created); _PyUOpExecutorObject *executor = PyUOpExecutor_New(trace, trace_length); executor->base.execute = _PyUopExecute; - memcpy(executor->trace, trace, trace_length * sizeof(_PyUOpInstruction)); *exec_ptr = (_PyExecutorObject *)executor; return 1; } From 5b1a09b4ce910c10217f0e1bd2b21487e6224cf5 Mon Sep 17 00:00:00 2001 From: justdan6 Date: Sun, 30 Jul 2023 18:18:17 -0600 Subject: [PATCH 5/5] Try casting return value from PyUOpExecutor_New --- Python/optimizer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Python/optimizer.c b/Python/optimizer.c index d91857480d13216..8c7e0dd112cdc53 100644 --- a/Python/optimizer.c +++ b/Python/optimizer.c @@ -692,7 +692,7 @@ uop_optimize( return trace_length; } OBJECT_STAT_INC(optimization_traces_created); - _PyUOpExecutorObject *executor = PyUOpExecutor_New(trace, trace_length); + _PyUOpExecutorObject *executor = (_PyUOpExecutorObject *) PyUOpExecutor_New(trace, trace_length); executor->base.execute = _PyUopExecute; *exec_ptr = (_PyExecutorObject *)executor; return 1;