Skip to content
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
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
2 changes: 1 addition & 1 deletion Include/internal/pycore_ceval.h
Original file line number Diff line number Diff line change
Expand Up @@ -271,7 +271,7 @@ PyAPI_FUNC(PyObject **) _PyObjectArray_FromStackRefArray(_PyStackRef *input, Py_
PyAPI_FUNC(void) _PyObjectArray_Free(PyObject **array, PyObject **scratch);

PyAPI_FUNC(PyObject *) _PyEval_GetANext(PyObject *aiter);
PyAPI_FUNC(PyObject *) _PyEval_LoadGlobal(PyObject *globals, PyObject *builtins, PyObject *name);
PyAPI_FUNC(void) _PyEval_LoadGlobalStackRef(PyObject *globals, PyObject *builtins, PyObject *name, _PyStackRef *writeto);
PyAPI_FUNC(PyObject *) _PyEval_GetAwaitable(PyObject *iterable, int oparg);
PyAPI_FUNC(PyObject *) _PyEval_LoadName(PyThreadState *tstate, _PyInterpreterFrame *frame, PyObject *name);

Expand Down
3 changes: 3 additions & 0 deletions Include/internal/pycore_dict.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ extern "C" {

#include "pycore_object.h" // PyManagedDictPointer
#include "pycore_pyatomic_ft_wrappers.h" // FT_ATOMIC_LOAD_SSIZE_ACQUIRE
#include "pycore_stackref.h" // _PyStackRef

// Unsafe flavor of PyDict_GetItemWithError(): no error checking
extern PyObject* _PyDict_GetItemWithError(PyObject *dp, PyObject *key);
Expand Down Expand Up @@ -100,10 +101,12 @@ extern void _PyDictKeys_DecRef(PyDictKeysObject *keys);
*/
extern Py_ssize_t _Py_dict_lookup(PyDictObject *mp, PyObject *key, Py_hash_t hash, PyObject **value_addr);
extern Py_ssize_t _Py_dict_lookup_threadsafe(PyDictObject *mp, PyObject *key, Py_hash_t hash, PyObject **value_addr);
extern Py_ssize_t _Py_dict_lookup_threadsafe_stackref(PyDictObject *mp, PyObject *key, Py_hash_t hash, _PyStackRef *value_addr);

extern Py_ssize_t _PyDict_LookupIndex(PyDictObject *, PyObject *);
extern Py_ssize_t _PyDictKeys_StringLookup(PyDictKeysObject* dictkeys, PyObject *key);
PyAPI_FUNC(PyObject *)_PyDict_LoadGlobal(PyDictObject *, PyDictObject *, PyObject *);
PyAPI_FUNC(void) _PyDict_LoadGlobalStackRef(PyDictObject *, PyDictObject *, PyObject *, _PyStackRef *);

/* Consumes references to key and value */
PyAPI_FUNC(int) _PyDict_SetItem_Take2(PyDictObject *op, PyObject *key, PyObject *value);
Expand Down
74 changes: 74 additions & 0 deletions Objects/dictobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -1496,6 +1496,45 @@ _Py_dict_lookup_threadsafe(PyDictObject *mp, PyObject *key, Py_hash_t hash, PyOb
return ix;
}

Py_ssize_t
_Py_dict_lookup_threadsafe_stackref(PyDictObject *mp, PyObject *key, Py_hash_t hash, _PyStackRef *value_addr)
{
PyDictKeysObject *dk = _Py_atomic_load_ptr(&mp->ma_keys);
if (dk->dk_kind == DICT_KEYS_UNICODE && PyUnicode_CheckExact(key)) {
Py_ssize_t ix = unicodekeys_lookup_unicode_threadsafe(dk, key, hash);
if (ix == DKIX_EMPTY) {
*value_addr = PyStackRef_NULL;
return ix;
}
else if (ix >= 0) {
PyObject **addr_of_value = &DK_UNICODE_ENTRIES(dk)[ix].me_value;
PyObject *value = _Py_atomic_load_ptr(addr_of_value);
if (value == NULL) {
*value_addr = PyStackRef_NULL;
return DKIX_EMPTY;
}
if (_Py_IsImmortal(value) || _PyObject_HasDeferredRefcount(value)) {
*value_addr = (_PyStackRef){ .bits = (uintptr_t)value | Py_TAG_DEFERRED };
return ix;
}
if (_Py_TryIncrefCompare(addr_of_value, value)) {
*value_addr = PyStackRef_FromPyObjectSteal(value);
return ix;
}
}
}

PyObject *obj;
Py_ssize_t ix = _Py_dict_lookup_threadsafe(mp, key, hash, &obj);
if (ix >= 0 && obj != NULL) {
*value_addr = PyStackRef_FromPyObjectSteal(obj);
}
else {
*value_addr = PyStackRef_NULL;
}
return ix;
}

#else // Py_GIL_DISABLED

Py_ssize_t
Expand All @@ -1506,6 +1545,15 @@ _Py_dict_lookup_threadsafe(PyDictObject *mp, PyObject *key, Py_hash_t hash, PyOb
return ix;
}

Py_ssize_t
_Py_dict_lookup_threadsafe_stackref(PyDictObject *mp, PyObject *key, Py_hash_t hash, _PyStackRef *value_addr)
{
PyObject *val;
Py_ssize_t ix = _Py_dict_lookup(mp, key, hash, &val);
*value_addr = val == NULL ? PyStackRef_NULL : PyStackRef_FromPyObjectNew(val);
return ix;
}

#endif

int
Expand Down Expand Up @@ -2418,6 +2466,32 @@ _PyDict_LoadGlobal(PyDictObject *globals, PyDictObject *builtins, PyObject *key)
return value;
}

void
_PyDict_LoadGlobalStackRef(PyDictObject *globals, PyDictObject *builtins, PyObject *key, _PyStackRef *res)
{
Py_ssize_t ix;
Py_hash_t hash;

hash = _PyObject_HashFast(key);
if (hash == -1) {
*res = PyStackRef_NULL;
return;
}

/* namespace 1: globals */
ix = _Py_dict_lookup_threadsafe_stackref(globals, key, hash, res);
if (ix == DKIX_ERROR) {
*res = PyStackRef_NULL;
}
if (ix != DKIX_EMPTY && !PyStackRef_IsNull(*res)) {
return;
}

/* namespace 2: builtins */
ix = _Py_dict_lookup_threadsafe_stackref(builtins, key, hash, res);
assert(ix >= 0 || PyStackRef_IsNull(*res));
}

/* Consumes references to key and value */
static int
setitem_take2_lock_held(PyDictObject *mp, PyObject *key, PyObject *value)
Expand Down
9 changes: 4 additions & 5 deletions Python/bytecodes.c
Original file line number Diff line number Diff line change
Expand Up @@ -1448,8 +1448,8 @@ dummy_func(
&& PyDict_CheckExact(BUILTINS()))
{
v_o = _PyDict_LoadGlobal((PyDictObject *)GLOBALS(),
(PyDictObject *)BUILTINS(),
name);
(PyDictObject *)BUILTINS(),
name);
if (v_o == NULL) {
if (!_PyErr_Occurred(tstate)) {
/* _PyDict_LoadGlobal() returns NULL without raising
Expand Down Expand Up @@ -1507,10 +1507,9 @@ dummy_func(

op(_LOAD_GLOBAL, ( -- res, null if (oparg & 1))) {
PyObject *name = GETITEM(FRAME_CO_NAMES, oparg>>1);
PyObject *res_o = _PyEval_LoadGlobal(GLOBALS(), BUILTINS(), name);
ERROR_IF(res_o == NULL, error);
_PyEval_LoadGlobalStackRef(GLOBALS(), BUILTINS(), name, &STACK_ENTRY(res));
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
_PyEval_LoadGlobalStackRef(GLOBALS(), BUILTINS(), name, &STACK_ENTRY(res));
_PyEval_LoadGlobalStackRef(GLOBALS(), BUILTINS(), name, &res[0]));

Can we go back to the earlier design of making res an array?
Please also add a comment explaining why we res needs to be trated specially (we need pass a pointer to _PyEval_LoadGlobalStackRef).

The code generator already understands arrays, and it avoids confusion as to where it should insert stack spills for fully deferred reference counting.

You'll able to remove def stack_entry below.

Sorry for the back and forth on this.

ERROR_IF(PyStackRef_IsNull(res), error);
null = PyStackRef_NULL;
res = PyStackRef_FromPyObjectSteal(res_o);
}

macro(LOAD_GLOBAL) =
Expand Down
20 changes: 11 additions & 9 deletions Python/ceval.c
Original file line number Diff line number Diff line change
Expand Up @@ -3072,15 +3072,14 @@ _PyEval_GetANext(PyObject *aiter)
return awaitable;
}

PyObject *
_PyEval_LoadGlobal(PyObject *globals, PyObject *builtins, PyObject *name)
void
_PyEval_LoadGlobalStackRef(PyObject *globals, PyObject *builtins, PyObject *name, _PyStackRef *writeto)
{
PyObject *res;
if (PyDict_CheckExact(globals) && PyDict_CheckExact(builtins)) {
res = _PyDict_LoadGlobal((PyDictObject *)globals,
_PyDict_LoadGlobalStackRef((PyDictObject *)globals,
(PyDictObject *)builtins,
name);
if (res == NULL && !PyErr_Occurred()) {
name, writeto);
if (PyStackRef_IsNull(*writeto) && !PyErr_Occurred()) {
/* _PyDict_LoadGlobal() returns NULL without raising
* an exception if the key doesn't exist */
_PyEval_FormatExcCheckArg(PyThreadState_GET(), PyExc_NameError,
Expand All @@ -3090,22 +3089,25 @@ _PyEval_LoadGlobal(PyObject *globals, PyObject *builtins, PyObject *name)
else {
/* Slow-path if globals or builtins is not a dict */
/* namespace 1: globals */
PyObject *res;
if (PyMapping_GetOptionalItem(globals, name, &res) < 0) {
return NULL;
*writeto = PyStackRef_NULL;
return;
}
if (res == NULL) {
/* namespace 2: builtins */
if (PyMapping_GetOptionalItem(builtins, name, &res) < 0) {
return NULL;
*writeto = PyStackRef_NULL;
return;
}
if (res == NULL) {
_PyEval_FormatExcCheckArg(
PyThreadState_GET(), PyExc_NameError,
NAME_ERROR_MSG, name);
}
}
*writeto = PyStackRef_FromPyObjectSteal(res);
}
return res;
}

PyObject *
Expand Down
6 changes: 3 additions & 3 deletions Python/executor_cases.c.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions Python/generated_cases.c.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

32 changes: 32 additions & 0 deletions Tools/cases_generator/generators_common.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ def __init__(self, out: CWriter):
"DECREF_INPUTS": self.decref_inputs,
"SYNC_SP": self.sync_sp,
"PyStackRef_FromPyObjectNew": self.py_stack_ref_from_py_object_new,
"STACK_ENTRY": self.stack_entry,
}
self.out = out

Expand Down Expand Up @@ -211,6 +212,37 @@ def py_stack_ref_from_py_object_new(
# unused portions of the stack to NULL.
stack.flush_single_var(self.out, target, uop.stack.outputs)

def stack_entry(
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure that the stack calculations are correct here, and I don't think it will be able to handle code that writes to multiple results, like UNPACK....

If we go back to using arrays, we can remove this.

self,
tkn: Token,
tkn_iter: Iterator[Token],
uop: Uop,
stack: Stack,
inst: Instruction | None,
) -> None:
emit_to(self.out, tkn_iter, "LPAREN")
target = next(tkn_iter)
size = "0"
for output in uop.stack.inputs:
size += f" - {output.size or 1}"
for output in uop.stack.outputs:
if output.name == target.text:
self.out.emit(f"stack_pointer[{size}]")
break
size += f" + {output.size or 1}"
else:
raise analysis_error("STACK_ENTRY operand is not a stack output", target)

next(tkn_iter) # Consume )
# Emit all the way to SEMI
for tkn in tkn_iter:
self.out.emit(tkn)
if tkn.kind == "SEMI":
break
self.emit("\n")
# Update the variable
self.out.emit(f"{target.text} = stack_pointer[{size}];\n")

def emit_tokens(
self,
uop: Uop,
Expand Down
Loading