Skip to content

gh-127022: Simplify PyStackRef_FromPyObjectSteal #127024

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Nov 22, 2024
Merged

gh-127022: Simplify PyStackRef_FromPyObjectSteal #127024

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5583ac0
gh-127022: Simplify `PyStackRef_FromPyObjectSteal`
colesbury Nov 18, 2024
16f7e7b
Simplify bytecodes.c logic a bit
colesbury Nov 21, 2024
3631451
Update Include/internal/pycore_stackref.h
colesbury Nov 21, 2024
06ab2ec
Add assert to PyStackRef_IsExactly
colesbury Nov 21, 2024
a9e4872
Use PyStackRef_IsFalse and PyStackRef_IsTrue
colesbury Nov 22, 2024
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
14 changes: 10 additions & 4 deletions Include/internal/pycore_stackref.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,7 @@ _PyStackRef_FromPyObjectSteal(PyObject *obj)
assert(obj != NULL);
// Make sure we don't take an already tagged value.
assert(((uintptr_t)obj & Py_TAG_BITS) == 0);
unsigned int tag = _Py_IsImmortal(obj) ? (Py_TAG_DEFERRED) : Py_TAG_PTR;
return ((_PyStackRef){.bits = ((uintptr_t)(obj)) | tag});
return (_PyStackRef){ .bits = (uintptr_t)obj };
}
# define PyStackRef_FromPyObjectSteal(obj) _PyStackRef_FromPyObjectSteal(_PyObject_CAST(obj))

Expand Down Expand Up @@ -190,9 +189,16 @@ static const _PyStackRef PyStackRef_NULL = { .bits = 0 };

#endif // Py_GIL_DISABLED

// Note: this is a macro because MSVC (Windows) has trouble inlining it.
// Check if a stackref is exactly the same as another stackref, including the
// the deferred bit. This can only be used safely if you know that the deferred
// bits of `a` and `b` match.
#define PyStackRef_IsExactly(a, b) \
(assert(((a).bits & Py_TAG_BITS) == ((b).bits & Py_TAG_BITS)), (a).bits == (b).bits)

#define PyStackRef_Is(a, b) ((a).bits == (b).bits)
// Checks that mask out the deferred bit in the free threading build.
#define PyStackRef_IsNone(ref) (PyStackRef_AsPyObjectBorrow(ref) == Py_None)
#define PyStackRef_IsTrue(ref) (PyStackRef_AsPyObjectBorrow(ref) == Py_True)
#define PyStackRef_IsFalse(ref) (PyStackRef_AsPyObjectBorrow(ref) == Py_False)

// Converts a PyStackRef back to a PyObject *, converting the
// stackref to a new reference.
Expand Down
42 changes: 17 additions & 25 deletions Python/bytecodes.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,7 @@ dummy_func(

pure inst(UNARY_NOT, (value -- res)) {
assert(PyStackRef_BoolCheck(value));
res = PyStackRef_Is(value, PyStackRef_False)
res = PyStackRef_IsFalse(value)
? PyStackRef_True : PyStackRef_False;
DEAD(value);
}
Expand Down Expand Up @@ -441,7 +441,7 @@ dummy_func(

inst(TO_BOOL_NONE, (unused/1, unused/2, value -- res)) {
// This one is a bit weird, because we expect *some* failures:
EXIT_IF(!PyStackRef_Is(value, PyStackRef_None));
EXIT_IF(!PyStackRef_IsNone(value));
DEAD(value);
STAT_INC(TO_BOOL, hit);
res = PyStackRef_False;
Expand Down Expand Up @@ -651,9 +651,7 @@ dummy_func(
// specializations, but there is no output.
// At the end we just skip over the STORE_FAST.
op(_BINARY_OP_INPLACE_ADD_UNICODE, (left, right --)) {
#ifndef NDEBUG
PyObject *left_o = PyStackRef_AsPyObjectBorrow(left);
#endif
PyObject *right_o = PyStackRef_AsPyObjectBorrow(right);

int next_oparg;
Expand All @@ -664,7 +662,7 @@ dummy_func(
next_oparg = CURRENT_OPERAND0();
#endif
_PyStackRef *target_local = &GETLOCAL(next_oparg);
DEOPT_IF(!PyStackRef_Is(*target_local, left));
DEOPT_IF(PyStackRef_AsPyObjectBorrow(*target_local) != left_o);
STAT_INC(BINARY_OP, hit);
/* Handle `left = left + right` or `left += right` for str.
*
Expand Down Expand Up @@ -1141,7 +1139,7 @@ dummy_func(
gen_frame->previous = frame;
DISPATCH_INLINED(gen_frame);
}
if (PyStackRef_Is(v, PyStackRef_None) && PyIter_Check(receiver_o)) {
if (PyStackRef_IsNone(v) && PyIter_Check(receiver_o)) {
retval_o = Py_TYPE(receiver_o)->tp_iternext(receiver_o);
}
else {
Expand Down Expand Up @@ -1249,7 +1247,7 @@ dummy_func(
inst(POP_EXCEPT, (exc_value -- )) {
_PyErr_StackItem *exc_info = tstate->exc_info;
Py_XSETREF(exc_info->exc_value,
PyStackRef_Is(exc_value, PyStackRef_None)
PyStackRef_IsNone(exc_value)
? NULL : PyStackRef_AsPyObjectSteal(exc_value));
}

Expand Down Expand Up @@ -2481,13 +2479,7 @@ dummy_func(
}

inst(IS_OP, (left, right -- b)) {
#ifdef Py_GIL_DISABLED
// On free-threaded builds, objects are conditionally immortalized.
// So their bits don't always compare equally.
int res = Py_Is(PyStackRef_AsPyObjectBorrow(left), PyStackRef_AsPyObjectBorrow(right)) ^ oparg;
#else
int res = PyStackRef_Is(left, right) ^ oparg;
#endif
DECREF_INPUTS();
b = res ? PyStackRef_True : PyStackRef_False;
}
Expand Down Expand Up @@ -2693,22 +2685,22 @@ dummy_func(

replaced op(_POP_JUMP_IF_FALSE, (cond -- )) {
assert(PyStackRef_BoolCheck(cond));
int flag = PyStackRef_Is(cond, PyStackRef_False);
int flag = PyStackRef_IsFalse(cond);
DEAD(cond);
RECORD_BRANCH_TAKEN(this_instr[1].cache, flag);
JUMPBY(oparg * flag);
}

replaced op(_POP_JUMP_IF_TRUE, (cond -- )) {
assert(PyStackRef_BoolCheck(cond));
int flag = PyStackRef_Is(cond, PyStackRef_True);
int flag = PyStackRef_IsTrue(cond);
DEAD(cond);
RECORD_BRANCH_TAKEN(this_instr[1].cache, flag);
JUMPBY(oparg * flag);
}

op(_IS_NONE, (value -- b)) {
if (PyStackRef_Is(value, PyStackRef_None)) {
if (PyStackRef_IsNone(value)) {
b = PyStackRef_True;
DEAD(value);
}
Expand Down Expand Up @@ -3752,7 +3744,7 @@ dummy_func(

inst(EXIT_INIT_CHECK, (should_be_none -- )) {
assert(STACK_LEVEL() == 2);
if (!PyStackRef_Is(should_be_none, PyStackRef_None)) {
if (!PyStackRef_IsNone(should_be_none)) {
PyErr_Format(PyExc_TypeError,
"__init__() should return None, not '%.200s'",
Py_TYPE(PyStackRef_AsPyObjectBorrow(should_be_none))->tp_name);
Expand Down Expand Up @@ -4712,7 +4704,7 @@ dummy_func(
inst(INSTRUMENTED_POP_JUMP_IF_TRUE, (unused/1 -- )) {
_PyStackRef cond = POP();
assert(PyStackRef_BoolCheck(cond));
int flag = PyStackRef_Is(cond, PyStackRef_True);
int flag = PyStackRef_IsTrue(cond);
int offset = flag * oparg;
RECORD_BRANCH_TAKEN(this_instr[1].cache, flag);
INSTRUMENTED_JUMP(this_instr, next_instr + offset, PY_MONITORING_EVENT_BRANCH);
Expand All @@ -4721,15 +4713,15 @@ dummy_func(
inst(INSTRUMENTED_POP_JUMP_IF_FALSE, (unused/1 -- )) {
_PyStackRef cond = POP();
assert(PyStackRef_BoolCheck(cond));
int flag = PyStackRef_Is(cond, PyStackRef_False);
int flag = PyStackRef_IsFalse(cond);
int offset = flag * oparg;
RECORD_BRANCH_TAKEN(this_instr[1].cache, flag);
INSTRUMENTED_JUMP(this_instr, next_instr + offset, PY_MONITORING_EVENT_BRANCH);
}

inst(INSTRUMENTED_POP_JUMP_IF_NONE, (unused/1 -- )) {
_PyStackRef value_stackref = POP();
int flag = PyStackRef_Is(value_stackref, PyStackRef_None);
int flag = PyStackRef_IsNone(value_stackref);
int offset;
if (flag) {
offset = oparg;
Expand All @@ -4745,7 +4737,7 @@ dummy_func(
inst(INSTRUMENTED_POP_JUMP_IF_NOT_NONE, (unused/1 -- )) {
_PyStackRef value_stackref = POP();
int offset;
int nflag = PyStackRef_Is(value_stackref, PyStackRef_None);
int nflag = PyStackRef_IsNone(value_stackref);
if (nflag) {
offset = 0;
}
Expand Down Expand Up @@ -4780,21 +4772,21 @@ dummy_func(
///////// Tier-2 only opcodes /////////

op (_GUARD_IS_TRUE_POP, (flag -- )) {
int is_true = PyStackRef_Is(flag, PyStackRef_True);
int is_true = PyStackRef_IsTrue(flag);
DEAD(flag);
SYNC_SP();
EXIT_IF(!is_true);
}

op (_GUARD_IS_FALSE_POP, (flag -- )) {
int is_false = PyStackRef_Is(flag, PyStackRef_False);
int is_false = PyStackRef_IsFalse(flag);
DEAD(flag);
SYNC_SP();
EXIT_IF(!is_false);
}

op (_GUARD_IS_NONE_POP, (val -- )) {
int is_none = PyStackRef_Is(val, PyStackRef_None);
int is_none = PyStackRef_IsNone(val);
if (!is_none) {
PyStackRef_CLOSE(val);
SYNC_SP();
Expand All @@ -4804,7 +4796,7 @@ dummy_func(
}

op (_GUARD_IS_NOT_NONE_POP, (val -- )) {
int is_none = PyStackRef_Is(val, PyStackRef_None);
int is_none = PyStackRef_IsNone(val);
PyStackRef_CLOSE(val);
SYNC_SP();
EXIT_IF(is_none);
Expand Down
28 changes: 10 additions & 18 deletions Python/executor_cases.c.h
View file
Open in desktop

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

Loading
Loading