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-109094: replace frame->prev_instr by frame->instr_ptr #109095

Merged
merged 102 commits into from
Oct 26, 2023
Merged
Show file tree
Hide file tree
Changes from 97 commits
Commits
Show all changes
102 commits
Select commit Hold shift + click to select a range
637b5da
add instr_ptr and assert it's 1 more than prev_instr in SET_LOCALS_FR…
iritkatriel Aug 29, 2023
ffb980c
fix YIELD_VALUE
iritkatriel Aug 30, 2023
82a5b92
fix test_sys
iritkatriel Aug 30, 2023
1fc0df1
update instr_ptr in _PyOptimizer_BackEdge
iritkatriel Sep 1, 2023
6bbb1c2
All tests pass except test_jump_from_yield (which is disabled)
iritkatriel Sep 4, 2023
be6d428
assert always
iritkatriel Sep 5, 2023
bbe8e2c
remove unnecessary stuff
iritkatriel Sep 5, 2023
50a1f3a
fix test_sys expected sizes
iritkatriel Sep 5, 2023
4beb50b
add assert
iritkatriel Sep 5, 2023
8e4a310
use instr_ptr instead of prev_instr in genobject.c
iritkatriel Sep 5, 2023
58d3bc5
update executor.c
iritkatriel Sep 6, 2023
909de93
remove unecessary setting of prev_instr in _Py_call_instrumentation_jump
iritkatriel Sep 7, 2023
0a06b75
replace assignment by assertion in _Py_call_instrumentation_line
iritkatriel Sep 7, 2023
da6c97b
revert change to signature of _PyFrame_PushTrampolineUnchecked
iritkatriel Sep 7, 2023
6488d69
prev_instr --> previous_instr to avoid confusion
iritkatriel Sep 7, 2023
6c684b0
revert change to JUMPBY
iritkatriel Sep 7, 2023
b6a8b0c
use instr_ptr in _PyFrame_OpAlreadyRan
iritkatriel Sep 8, 2023
1ceeba1
use instr_ptr in _PyFrame_IsIncomplete
iritkatriel Sep 8, 2023
371fa66
use instr_ptr in LOAD_IP
iritkatriel Sep 8, 2023
557f36f
use intr_ptr in INSTRUMENTED_RESUME
iritkatriel Sep 10, 2023
cb22365
Python/optimizer.c
iritkatriel Sep 11, 2023
9875704
Merge remote-tracking branch 'upstream/main' into instr_ptr
iritkatriel Sep 11, 2023
aea21a6
tweaks
iritkatriel Sep 14, 2023
b2221d1
Merge remote-tracking branch 'upstream/main' into instr_ptr
iritkatriel Sep 14, 2023
7078fe5
skip test_long_loop for now
iritkatriel Sep 14, 2023
3ac7cc6
added harness for LASTI migration, but no functional change yet
iritkatriel Sep 15, 2023
5a672a4
WIP
iritkatriel Sep 17, 2023
4fd470e
WIP
iritkatriel Sep 18, 2023
d676f9c
WIP
iritkatriel Sep 18, 2023
1afba52
WIP
iritkatriel Sep 19, 2023
8b1643a
WIP
iritkatriel Sep 19, 2023
a386dee
fix bug
iritkatriel Sep 20, 2023
32b2cfd
add dump_frame
iritkatriel Sep 26, 2023
add2321
add yield_offset
iritkatriel Sep 26, 2023
218228b
give the POP_TOP after a RETURN_GENERATOR the same line number
iritkatriel Sep 26, 2023
0d0c25d
fix getattr overridden
iritkatriel Sep 27, 2023
4418fc4
add more data to debug output
iritkatriel Sep 27, 2023
e07b415
disable Uops tests for now
iritkatriel Sep 27, 2023
cb96aa0
always assert
iritkatriel Sep 28, 2023
729944f
remove some debug prints
iritkatriel Sep 28, 2023
44e37ee
use new lasti macro
iritkatriel Sep 28, 2023
b3b55ee
rename local
iritkatriel Sep 28, 2023
6f0fe7b
use prev_traced_instr instead of prev_instr in line tracing
iritkatriel Sep 29, 2023
2cf7e94
Merge remote-tracking branch 'upstream/main' into instr_ptr
iritkatriel Sep 29, 2023
66005ce
remove some debug prints
iritkatriel Sep 29, 2023
c9a9601
don't use prev_instr in jump instrumentation
iritkatriel Sep 29, 2023
764bd37
remove prev_instr and return_offset from frame
iritkatriel Sep 30, 2023
30e74de
skip failing tests
iritkatriel Sep 30, 2023
b63c8da
fix test_sys_settrace tests
iritkatriel Sep 30, 2023
8bade18
re-enable some tests
iritkatriel Sep 30, 2023
8b14bde
remove prev_traced_instr
iritkatriel Oct 11, 2023
d1387ed
unskip tests which are now passing
iritkatriel Oct 11, 2023
9c10b8d
one more test
iritkatriel Oct 11, 2023
287c1a5
more tests
iritkatriel Oct 11, 2023
3639796
Merge remote-tracking branch 'upstream/main' into instr_ptr
iritkatriel Oct 11, 2023
41bba62
rename new_return_offset to next_instr_offset
iritkatriel Oct 11, 2023
d363fc3
remove debug prints
iritkatriel Oct 11, 2023
efa8eab
tidy up
iritkatriel Oct 11, 2023
86cf45f
use constants
iritkatriel Oct 11, 2023
19da0c0
Merge remote-tracking branch 'upstream/main' into instr_ptr
iritkatriel Oct 12, 2023
cbee4f8
add and use constant NEXT_INSTR_OFFSET_FOR_YIELD
iritkatriel Oct 13, 2023
3a9a029
remove yield_offset from the frame
iritkatriel Oct 13, 2023
a44f4db
re-enable test_sys
iritkatriel Oct 13, 2023
62ca8ce
adjust shim frame
iritkatriel Oct 13, 2023
3fa0423
tweak assertions
iritkatriel Oct 13, 2023
488e5e7
tweaks
iritkatriel Oct 13, 2023
85f08bc
cleanup
iritkatriel Oct 13, 2023
012f911
add news
iritkatriel Oct 13, 2023
3fcfc47
fix most uop tests
iritkatriel Oct 17, 2023
d796107
_PUSH_FRAME doesn't need to update next_instr_offset
iritkatriel Oct 17, 2023
76474e3
Apply suggestions from code review
iritkatriel Oct 18, 2023
3fcbc3a
fix ()
iritkatriel Oct 18, 2023
2043c83
assert YIELD has no caches
iritkatriel Oct 18, 2023
3511bef
apply review feedback
iritkatriel Oct 18, 2023
6a85588
remove obsolete comment
iritkatriel Oct 18, 2023
8ccbe6e
remove frame->instr_ptr--; in executor
iritkatriel Oct 20, 2023
d55f048
Merge remote-tracking branch 'upstream/main' into instr_ptr
iritkatriel Oct 21, 2023
b7f86a7
Add uops support to instr_ptr branch (#55)
gvanrossum Oct 22, 2023
c6d69da
address some of Mark's comments
iritkatriel Oct 23, 2023
0da356b
address some of Mark's comments
iritkatriel Oct 23, 2023
0089b30
apply next_instr_offset on return/yield
iritkatriel Oct 24, 2023
e1ab25d
remove debug stuff
iritkatriel Oct 24, 2023
3e0b86d
prev can't be NULL
iritkatriel Oct 24, 2023
d0b2469
yield doesn't need to use next_instr_offset
iritkatriel Oct 25, 2023
7d15260
apply next_instr_offset to next_instr instead of instr_ptr
iritkatriel Oct 25, 2023
2986b9c
rename next_instr_offset to return_offset
iritkatriel Oct 25, 2023
91eb2a4
Merge remote-tracking branch 'upstream/main' into instr_ptr
iritkatriel Oct 25, 2023
1bd0be3
disable a few tier2 tests
iritkatriel Oct 25, 2023
91dddc0
revert some unnecessary changes
iritkatriel Oct 25, 2023
f3c25b5
fix ENTER_EXECUTOR and re-enable the tests
iritkatriel Oct 25, 2023
6617d60
cleanup
iritkatriel Oct 25, 2023
661dad0
_SAVE_CURRENT_IP --> _SAVE_RETURN_OFFSET
iritkatriel Oct 25, 2023
5a5103b
assignment to assertion
iritkatriel Oct 25, 2023
0b68e54
address (most of) Guido's comments
iritkatriel Oct 25, 2023
96a996c
LOAD_IP takes the offset
iritkatriel Oct 25, 2023
8574469
no need to zero return_offset
iritkatriel Oct 25, 2023
855513a
remove OPARG_SET_IP
iritkatriel Oct 25, 2023
0cf9ec9
fix test_gdb?
iritkatriel Oct 26, 2023
82f9d9d
move explanations to frame_layout.md
iritkatriel Oct 26, 2023
95b63df
Mark's comments
iritkatriel Oct 26, 2023
f2d149b
same tweak to INSTRUMENTED_YIELD_VALUE as well
iritkatriel Oct 26, 2023
578e266
Merge branch 'main' into instr_ptr
iritkatriel Oct 26, 2023
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
29 changes: 15 additions & 14 deletions Include/internal/pycore_frame.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,26 +58,27 @@ typedef struct _PyInterpreterFrame {
PyObject *f_builtins; /* Borrowed reference. Only valid if not on C stack */
PyObject *f_locals; /* Strong reference, may be NULL. Only valid if not on C stack */
PyFrameObject *frame_obj; /* Strong reference, may be NULL. Only valid if not on C stack */
// NOTE: This is not necessarily the last instruction started in the given
// frame. Rather, it is the code unit *prior to* the *next* instruction. For
// example, it may be an inline CACHE entry, an instruction we just jumped
// over, or (in the case of a newly-created frame) a totally invalid value:
_Py_CODEUNIT *prev_instr;
/* When a frame is executing, instr_ptr points to the instruction currently executing.
* During a call, it points to the call instruction.
* In a suspended frame, it points to the instruction that would execute
* if the frame were to resume.
iritkatriel marked this conversation as resolved.
Show resolved Hide resolved
*/
_Py_CODEUNIT *instr_ptr;
int stacktop; /* Offset of TOS from localsplus */
/* The return_offset determines where a `RETURN` should go in the caller,
* relative to `prev_instr`.
* It is only meaningful to the callee,
* so it needs to be set in any CALL (to a Python function)
* or SEND (to a coroutine or generator).
* If there is no callee, then it is meaningless. */
* relative to `instr_ptr`.
* It is only meaningful to the callee, so it needs to be set in any
* instruction that implements a call (to a Python function), including CALL,
* SEND and BINARY_SUBSCR_GETITEM, among others.
* If there is no callee, then return_offset is meaningless. */
iritkatriel marked this conversation as resolved.
Show resolved Hide resolved
uint16_t return_offset;
char owner;
/* Locals and stack */
PyObject *localsplus[1];
} _PyInterpreterFrame;

#define _PyInterpreterFrame_LASTI(IF) \
((int)((IF)->prev_instr - _PyCode_CODE(_PyFrame_GetCode(IF))))
((int)((IF)->instr_ptr - _PyCode_CODE(_PyFrame_GetCode(IF))))

static inline PyCodeObject *_PyFrame_GetCode(_PyInterpreterFrame *f) {
assert(PyCode_Check(f->f_executable));
Expand Down Expand Up @@ -134,7 +135,7 @@ _PyFrame_Initialize(
frame->f_locals = locals;
frame->stacktop = code->co_nlocalsplus;
frame->frame_obj = NULL;
frame->prev_instr = _PyCode_CODE(code) - 1;
frame->instr_ptr = _PyCode_CODE(code);
frame->return_offset = 0;
iritkatriel marked this conversation as resolved.
Show resolved Hide resolved
frame->owner = FRAME_OWNED_BY_THREAD;

Expand Down Expand Up @@ -185,7 +186,7 @@ _PyFrame_IsIncomplete(_PyInterpreterFrame *frame)
return true;
}
return frame->owner != FRAME_OWNED_BY_GENERATOR &&
frame->prev_instr < _PyCode_CODE(_PyFrame_GetCode(frame)) + _PyFrame_GetCode(frame)->_co_firsttraceable;
frame->instr_ptr < _PyCode_CODE(_PyFrame_GetCode(frame)) + _PyFrame_GetCode(frame)->_co_firsttraceable;
iritkatriel marked this conversation as resolved.
Show resolved Hide resolved
}

static inline _PyInterpreterFrame *
Expand Down Expand Up @@ -297,7 +298,7 @@ _PyFrame_PushTrampolineUnchecked(PyThreadState *tstate, PyCodeObject *code, int
frame->f_locals = NULL;
frame->stacktop = code->co_nlocalsplus + stackdepth;
frame->frame_obj = NULL;
frame->prev_instr = _PyCode_CODE(code);
frame->instr_ptr = _PyCode_CODE(code);
frame->owner = FRAME_OWNED_BY_THREAD;
frame->return_offset = 0;
return frame;
Expand Down
16 changes: 8 additions & 8 deletions Include/internal/pycore_opcode_metadata.h

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

2 changes: 1 addition & 1 deletion Include/internal/pycore_runtime.h
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ typedef struct _Py_DebugOffsets {
struct _interpreter_frame {
off_t previous;
off_t executable;
off_t prev_instr;
off_t instr_ptr;
off_t localsplus;
off_t owner;
} interpreter_frame;
Expand Down
2 changes: 1 addition & 1 deletion Include/internal/pycore_runtime_init.h
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ extern PyTypeObject _PyExc_MemoryError;
.interpreter_frame = { \
.previous = offsetof(_PyInterpreterFrame, previous), \
.executable = offsetof(_PyInterpreterFrame, f_executable), \
.prev_instr = offsetof(_PyInterpreterFrame, prev_instr), \
.instr_ptr = offsetof(_PyInterpreterFrame, instr_ptr), \
.localsplus = offsetof(_PyInterpreterFrame, localsplus), \
.owner = offsetof(_PyInterpreterFrame, owner), \
}, \
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Replace ``prev_instr`` on the interpreter frame by ``instr_ptr`` which
points to the beginning of the instruction that is currently executing (or
will execute once the frame resumes).
8 changes: 4 additions & 4 deletions Objects/frameobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -821,7 +821,7 @@ frame_setlineno(PyFrameObject *f, PyObject* p_new_lineno, void *Py_UNUSED(ignore
}
/* Finally set the new lasti and return OK. */
f->f_lineno = 0;
f->f_frame->prev_instr = _PyCode_CODE(code) + best_addr;
f->f_frame->instr_ptr = _PyCode_CODE(code) + best_addr;
return 0;
}

Expand Down Expand Up @@ -1079,7 +1079,7 @@ PyFrame_New(PyThreadState *tstate, PyCodeObject *code,
f->f_frame = (_PyInterpreterFrame *)f->_f_frame_data;
f->f_frame->owner = FRAME_OWNED_BY_FRAME_OBJECT;
// This frame needs to be "complete", so pretend that the first RESUME ran:
f->f_frame->prev_instr = _PyCode_CODE(code) + code->_co_firsttraceable;
f->f_frame->instr_ptr = _PyCode_CODE(code) + code->_co_firsttraceable + 1;
assert(!_PyFrame_IsIncomplete(f->f_frame));
Py_DECREF(func);
_PyObject_GC_TRACK(f);
Expand All @@ -1093,7 +1093,7 @@ _PyFrame_OpAlreadyRan(_PyInterpreterFrame *frame, int opcode, int oparg)
assert(_PyOpcode_Deopt[opcode] == opcode);
int check_oparg = 0;
for (_Py_CODEUNIT *instruction = _PyCode_CODE(_PyFrame_GetCode(frame));
instruction < frame->prev_instr; instruction++)
instruction < frame->instr_ptr; instruction++)
{
int check_opcode = _PyOpcode_Deopt[instruction->op.code];
check_oparg |= instruction->op.arg;
Expand Down Expand Up @@ -1135,7 +1135,7 @@ frame_init_get_vars(_PyInterpreterFrame *frame)
frame->localsplus[offset + i] = Py_NewRef(o);
}
// COPY_FREE_VARS doesn't have inline CACHEs, either:
frame->prev_instr = _PyCode_CODE(_PyFrame_GetCode(frame));
frame->instr_ptr = _PyCode_CODE(_PyFrame_GetCode(frame));
}


Expand Down
13 changes: 8 additions & 5 deletions Objects/genobject.c
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "pycore_genobject.h" // struct _Py_async_gen_state
#include "pycore_modsupport.h" // _PyArg_CheckPositional()
#include "pycore_object.h" // _PyObject_GC_UNTRACK()
#include "pycore_opcode_metadata.h" // _PyOpcode_Caches
#include "pycore_pyerrors.h" // _PyErr_ClearExcState()
#include "pycore_pystate.h" // _PyThreadState_GET()

Expand Down Expand Up @@ -362,8 +363,7 @@ _PyGen_yf(PyGenObject *gen)
assert(_PyCode_CODE(_PyGen_GetCode(gen))[0].op.code != SEND);
return NULL;
}
_Py_CODEUNIT next = frame->prev_instr[1];
if (!is_resume(&next) || next.op.arg < 2)
if (!is_resume(frame->instr_ptr) || frame->instr_ptr->op.arg < 2)
{
/* Not in a yield from */
return NULL;
Expand Down Expand Up @@ -398,9 +398,12 @@ gen_close(PyGenObject *gen, PyObject *args)
_PyInterpreterFrame *frame = (_PyInterpreterFrame *)gen->gi_iframe;
/* It is possible for the previous instruction to not be a
* YIELD_VALUE if the debugger has changed the lineno. */
if (err == 0 && is_yield(frame->prev_instr)) {
assert(is_resume(frame->prev_instr + 1));
int exception_handler_depth = frame->prev_instr[0].op.arg;
assert(_PyOpcode_Caches[YIELD_VALUE] == 0);
assert(_PyOpcode_Caches[INSTRUMENTED_YIELD_VALUE] == 0);
if (err == 0 && is_yield(frame->instr_ptr - 1)) {
iritkatriel marked this conversation as resolved.
Show resolved Hide resolved
iritkatriel marked this conversation as resolved.
Show resolved Hide resolved
_Py_CODEUNIT *yield_instr = frame->instr_ptr - 1;
assert(is_resume(frame->instr_ptr));
int exception_handler_depth = yield_instr->op.arg;
assert(exception_handler_depth > 0);
/* We can safely ignore the outermost try block
* as it automatically generated to handle
Expand Down
4 changes: 4 additions & 0 deletions Python/abstract_interp_cases.c.h

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

Loading
Loading