Skip to content

Commit c87b5b2

Browse files
authored
GH-135379: Remove types from stack items in code generator. (GH-135384)
* Make casts explicit in the instruction definitions
1 parent 49d7236 commit c87b5b2

File tree

13 files changed

+257
-255
lines changed

13 files changed

+257
-255
lines changed

Include/internal/pycore_stackref.h

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,32 @@ PyStackRef_IsNullOrInt(_PyStackRef ref);
264264

265265
static const _PyStackRef PyStackRef_ERROR = { .bits = Py_TAG_INVALID };
266266

267+
/* Wrap a pointer in a stack ref.
268+
* The resulting stack reference is not safe and should only be used
269+
* in the interpreter to pass values from one uop to another.
270+
* The GC should never see one of these stack refs. */
271+
static inline _PyStackRef
272+
PyStackRef_Wrap(void *ptr)
273+
{
274+
assert(ptr != NULL);
275+
#ifdef Py_DEBUG
276+
return (_PyStackRef){ .bits = ((uintptr_t)ptr) | Py_TAG_INVALID };
277+
#else
278+
return (_PyStackRef){ .bits = (uintptr_t)ptr };
279+
#endif
280+
}
281+
282+
static inline void *
283+
PyStackRef_Unwrap(_PyStackRef ref)
284+
{
285+
#ifdef Py_DEBUG
286+
assert ((ref.bits & Py_TAG_BITS) == Py_TAG_INVALID);
287+
return (void *)(ref.bits & ~Py_TAG_BITS);
288+
#else
289+
return (void *)(ref.bits);
290+
#endif
291+
}
292+
267293
static inline bool
268294
PyStackRef_IsError(_PyStackRef ref)
269295
{

Lib/test/test_generated_cases.py

Lines changed: 6 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -56,14 +56,14 @@ class TestEffects(unittest.TestCase):
5656
def test_effect_sizes(self):
5757
stack = Stack()
5858
inputs = [
59-
x := StackItem("x", None, "1"),
60-
y := StackItem("y", None, "oparg"),
61-
z := StackItem("z", None, "oparg*2"),
59+
x := StackItem("x", "1"),
60+
y := StackItem("y", "oparg"),
61+
z := StackItem("z", "oparg*2"),
6262
]
6363
outputs = [
64-
StackItem("x", None, "1"),
65-
StackItem("b", None, "oparg*4"),
66-
StackItem("c", None, "1"),
64+
StackItem("x", "1"),
65+
StackItem("b", "oparg*4"),
66+
StackItem("c", "1"),
6767
]
6868
null = CWriter.null()
6969
stack.pop(z, null)
@@ -1103,32 +1103,6 @@ def test_array_of_one(self):
11031103
"""
11041104
self.run_cases_test(input, output)
11051105

1106-
def test_pointer_to_stackref(self):
1107-
input = """
1108-
inst(OP, (arg: _PyStackRef * -- out)) {
1109-
out = *arg;
1110-
DEAD(arg);
1111-
}
1112-
"""
1113-
output = """
1114-
TARGET(OP) {
1115-
#if Py_TAIL_CALL_INTERP
1116-
int opcode = OP;
1117-
(void)(opcode);
1118-
#endif
1119-
frame->instr_ptr = next_instr;
1120-
next_instr += 1;
1121-
INSTRUCTION_STATS(OP);
1122-
_PyStackRef *arg;
1123-
_PyStackRef out;
1124-
arg = (_PyStackRef *)stack_pointer[-1].bits;
1125-
out = *arg;
1126-
stack_pointer[-1] = out;
1127-
DISPATCH();
1128-
}
1129-
"""
1130-
self.run_cases_test(input, output)
1131-
11321106
def test_unused_cached_value(self):
11331107
input = """
11341108
op(FIRST, (arg1 -- out)) {
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
The cases generator no longer accepts type annotations on stack items.
2+
Conversions to non-default types are now done explictly in bytecodes.c and
3+
optimizer_bytecodes.c. This will simplify code generation for top-of-stack
4+
caching and other future features.

Python/bytecodes.c

Lines changed: 33 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -985,12 +985,13 @@ dummy_func(
985985
STAT_INC(BINARY_OP, hit);
986986
}
987987

988-
op(_BINARY_OP_SUBSCR_INIT_CALL, (container, sub, getitem -- new_frame: _PyInterpreterFrame* )) {
989-
new_frame = _PyFrame_PushUnchecked(tstate, getitem, 2, frame);
990-
new_frame->localsplus[0] = container;
991-
new_frame->localsplus[1] = sub;
988+
op(_BINARY_OP_SUBSCR_INIT_CALL, (container, sub, getitem -- new_frame)) {
989+
_PyInterpreterFrame* pushed_frame = _PyFrame_PushUnchecked(tstate, getitem, 2, frame);
990+
pushed_frame->localsplus[0] = container;
991+
pushed_frame->localsplus[1] = sub;
992992
INPUTS_DEAD();
993993
frame->return_offset = INSTRUCTION_SIZE;
994+
new_frame = PyStackRef_Wrap(pushed_frame);
994995
}
995996

996997
macro(BINARY_OP_SUBSCR_GETITEM) =
@@ -1296,20 +1297,21 @@ dummy_func(
12961297

12971298
macro(SEND) = _SPECIALIZE_SEND + _SEND;
12981299

1299-
op(_SEND_GEN_FRAME, (receiver, v -- receiver, gen_frame: _PyInterpreterFrame *)) {
1300+
op(_SEND_GEN_FRAME, (receiver, v -- receiver, gen_frame)) {
13001301
PyGenObject *gen = (PyGenObject *)PyStackRef_AsPyObjectBorrow(receiver);
13011302
DEOPT_IF(Py_TYPE(gen) != &PyGen_Type && Py_TYPE(gen) != &PyCoro_Type);
13021303
DEOPT_IF(gen->gi_frame_state >= FRAME_EXECUTING);
13031304
STAT_INC(SEND, hit);
1304-
gen_frame = &gen->gi_iframe;
1305-
_PyFrame_StackPush(gen_frame, PyStackRef_MakeHeapSafe(v));
1305+
_PyInterpreterFrame *pushed_frame = &gen->gi_iframe;
1306+
_PyFrame_StackPush(pushed_frame, PyStackRef_MakeHeapSafe(v));
13061307
DEAD(v);
13071308
gen->gi_frame_state = FRAME_EXECUTING;
13081309
gen->gi_exc_state.previous_item = tstate->exc_info;
13091310
tstate->exc_info = &gen->gi_exc_state;
13101311
assert(INSTRUCTION_SIZE + oparg <= UINT16_MAX);
13111312
frame->return_offset = (uint16_t)(INSTRUCTION_SIZE + oparg);
1312-
gen_frame->previous = frame;
1313+
pushed_frame->previous = frame;
1314+
gen_frame = PyStackRef_Wrap(pushed_frame);
13131315
}
13141316

13151317
macro(SEND_GEN) =
@@ -2463,7 +2465,7 @@ dummy_func(
24632465
_LOAD_ATTR_CLASS +
24642466
_PUSH_NULL_CONDITIONAL;
24652467

2466-
op(_LOAD_ATTR_PROPERTY_FRAME, (fget/4, owner -- new_frame: _PyInterpreterFrame *)) {
2468+
op(_LOAD_ATTR_PROPERTY_FRAME, (fget/4, owner -- new_frame)) {
24672469
assert((oparg & 1) == 0);
24682470
assert(Py_IS_TYPE(fget, &PyFunction_Type));
24692471
PyFunctionObject *f = (PyFunctionObject *)fget;
@@ -2473,9 +2475,10 @@ dummy_func(
24732475
DEOPT_IF(code->co_argcount != 1);
24742476
DEOPT_IF(!_PyThreadState_HasStackSpace(tstate, code->co_framesize));
24752477
STAT_INC(LOAD_ATTR, hit);
2476-
new_frame = _PyFrame_PushUnchecked(tstate, PyStackRef_FromPyObjectNew(fget), 1, frame);
2477-
new_frame->localsplus[0] = owner;
2478+
_PyInterpreterFrame *pushed_frame = _PyFrame_PushUnchecked(tstate, PyStackRef_FromPyObjectNew(fget), 1, frame);
2479+
pushed_frame->localsplus[0] = owner;
24782480
DEAD(owner);
2481+
new_frame = PyStackRef_Wrap(pushed_frame);
24792482
}
24802483

24812484
macro(LOAD_ATTR_PROPERTY) =
@@ -3344,7 +3347,7 @@ dummy_func(
33443347
_ITER_JUMP_RANGE +
33453348
_ITER_NEXT_RANGE;
33463349

3347-
op(_FOR_ITER_GEN_FRAME, (iter, null -- iter, null, gen_frame: _PyInterpreterFrame*)) {
3350+
op(_FOR_ITER_GEN_FRAME, (iter, null -- iter, null, gen_frame)) {
33483351
PyGenObject *gen = (PyGenObject *)PyStackRef_AsPyObjectBorrow(iter);
33493352
DEOPT_IF(Py_TYPE(gen) != &PyGen_Type);
33503353
#ifdef Py_GIL_DISABLED
@@ -3356,14 +3359,15 @@ dummy_func(
33563359
#endif
33573360
DEOPT_IF(gen->gi_frame_state >= FRAME_EXECUTING);
33583361
STAT_INC(FOR_ITER, hit);
3359-
gen_frame = &gen->gi_iframe;
3360-
_PyFrame_StackPush(gen_frame, PyStackRef_None);
3362+
_PyInterpreterFrame *pushed_frame = &gen->gi_iframe;
3363+
_PyFrame_StackPush(pushed_frame, PyStackRef_None);
33613364
gen->gi_frame_state = FRAME_EXECUTING;
33623365
gen->gi_exc_state.previous_item = tstate->exc_info;
33633366
tstate->exc_info = &gen->gi_exc_state;
3364-
gen_frame->previous = frame;
3367+
pushed_frame->previous = frame;
33653368
// oparg is the return offset from the next instruction.
33663369
frame->return_offset = (uint16_t)(INSTRUCTION_SIZE + oparg);
3370+
gen_frame = PyStackRef_Wrap(pushed_frame);
33673371
}
33683372

33693373
macro(FOR_ITER_GEN) =
@@ -3715,7 +3719,7 @@ dummy_func(
37153719
macro(CALL) = _SPECIALIZE_CALL + unused/2 + _MAYBE_EXPAND_METHOD + _DO_CALL + _CHECK_PERIODIC;
37163720
macro(INSTRUMENTED_CALL) = unused/3 + _MAYBE_EXPAND_METHOD + _MONITOR_CALL + _DO_CALL + _CHECK_PERIODIC;
37173721

3718-
op(_PY_FRAME_GENERAL, (callable, self_or_null, args[oparg] -- new_frame: _PyInterpreterFrame*)) {
3722+
op(_PY_FRAME_GENERAL, (callable, self_or_null, args[oparg] -- new_frame)) {
37193723
PyObject *callable_o = PyStackRef_AsPyObjectBorrow(callable);
37203724

37213725
// oparg counts all of the args, but *not* self:
@@ -3737,7 +3741,7 @@ dummy_func(
37373741
if (temp == NULL) {
37383742
ERROR_NO_POP();
37393743
}
3740-
new_frame = temp;
3744+
new_frame = PyStackRef_Wrap(temp);
37413745
}
37423746

37433747
op(_CHECK_FUNCTION_VERSION, (func_version/2, callable, unused, unused[oparg] -- callable, unused, unused[oparg])) {
@@ -3874,27 +3878,26 @@ dummy_func(
38743878
DEOPT_IF(tstate->py_recursion_remaining <= 1);
38753879
}
38763880

3877-
replicate(5) pure op(_INIT_CALL_PY_EXACT_ARGS, (callable, self_or_null, args[oparg] -- new_frame: _PyInterpreterFrame*)) {
3881+
replicate(5) pure op(_INIT_CALL_PY_EXACT_ARGS, (callable, self_or_null, args[oparg] -- new_frame)) {
38783882
int has_self = !PyStackRef_IsNull(self_or_null);
38793883
STAT_INC(CALL, hit);
3880-
new_frame = _PyFrame_PushUnchecked(tstate, callable, oparg + has_self, frame);
3881-
_PyStackRef *first_non_self_local = new_frame->localsplus + has_self;
3882-
new_frame->localsplus[0] = self_or_null;
3884+
_PyInterpreterFrame *pushed_frame = _PyFrame_PushUnchecked(tstate, callable, oparg + has_self, frame);
3885+
_PyStackRef *first_non_self_local = pushed_frame->localsplus + has_self;
3886+
pushed_frame->localsplus[0] = self_or_null;
38833887
for (int i = 0; i < oparg; i++) {
38843888
first_non_self_local[i] = args[i];
38853889
}
38863890
INPUTS_DEAD();
3891+
new_frame = PyStackRef_Wrap(pushed_frame);
38873892
}
38883893

3889-
op(_PUSH_FRAME, (new_frame: _PyInterpreterFrame* -- )) {
3890-
// Write it out explicitly because it's subtly different.
3891-
// Eventually this should be the only occurrence of this code.
3894+
op(_PUSH_FRAME, (new_frame -- )) {
38923895
assert(tstate->interp->eval_frame == NULL);
3893-
_PyInterpreterFrame *temp = new_frame;
3896+
_PyInterpreterFrame *temp = PyStackRef_Unwrap(new_frame);
38943897
DEAD(new_frame);
38953898
SYNC_SP();
38963899
_PyFrame_SetStackPointer(frame, stack_pointer);
3897-
assert(new_frame->previous == frame || new_frame->previous->previous == frame);
3900+
assert(temp->previous == frame || temp->previous->previous == frame);
38983901
CALL_STAT_INC(inlined_py_calls);
38993902
frame = tstate->current_frame = temp;
39003903
tstate->py_recursion_remaining--;
@@ -4046,7 +4049,7 @@ dummy_func(
40464049
PyStackRef_CLOSE(temp);
40474050
}
40484051

4049-
op(_CREATE_INIT_FRAME, (init, self, args[oparg] -- init_frame: _PyInterpreterFrame *)) {
4052+
op(_CREATE_INIT_FRAME, (init, self, args[oparg] -- init_frame)) {
40504053
_PyInterpreterFrame *shim = _PyFrame_PushTrampolineUnchecked(
40514054
tstate, (PyCodeObject *)&_Py_InitCleanup, 1, frame);
40524055
assert(_PyFrame_GetBytecode(shim)[0].op.code == EXIT_INIT_CHECK);
@@ -4063,12 +4066,12 @@ dummy_func(
40634066
_PyEval_FrameClearAndPop(tstate, shim);
40644067
ERROR_NO_POP();
40654068
}
4066-
init_frame = temp;
40674069
frame->return_offset = 1 + INLINE_CACHE_ENTRIES_CALL;
40684070
/* Account for pushing the extra frame.
40694071
* We don't check recursion depth here,
40704072
* as it will be checked after start_frame */
40714073
tstate->py_recursion_remaining--;
4074+
init_frame = PyStackRef_Wrap(temp);
40724075
}
40734076

40744077
macro(CALL_ALLOC_AND_ENTER_INIT) =
@@ -4594,7 +4597,7 @@ dummy_func(
45944597
res = PyStackRef_FromPyObjectSteal(res_o);
45954598
}
45964599

4597-
op(_PY_FRAME_KW, (callable, self_or_null, args[oparg], kwnames -- new_frame: _PyInterpreterFrame*)) {
4600+
op(_PY_FRAME_KW, (callable, self_or_null, args[oparg], kwnames -- new_frame)) {
45984601
PyObject *callable_o = PyStackRef_AsPyObjectBorrow(callable);
45994602

46004603
// oparg counts all of the args, but *not* self:
@@ -4621,7 +4624,7 @@ dummy_func(
46214624
DEAD(callable);
46224625
SYNC_SP();
46234626
ERROR_IF(temp == NULL);
4624-
new_frame = temp;
4627+
new_frame = PyStackRef_Wrap(temp);
46254628
}
46264629

46274630
op(_CHECK_FUNCTION_VERSION_KW, (func_version/2, callable, unused, unused[oparg], unused -- callable, unused, unused[oparg], unused)) {

0 commit comments

Comments
 (0)