Skip to content

GH-98831: Remove all remaining DISPATCH() calls from bytecodes.c#99271

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
gvanrossum merged 8 commits into from
Nov 10, 2022
Merged

GH-98831: Remove all remaining DISPATCH() calls from bytecodes.c #99271

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
d453fcc
Remove all remaining DISPATCH() calls from bytecodes.c
gvanrossum Nov 9, 2022
14d60fd
Merge commit 'c7f57087149968e9a72f6dee73514ff18fee78e8' into rm-dispatch
gvanrossum Nov 10, 2022
089b508
Merge commit '2e343fc465ed0206340cf139ba485f9004bbdd41' into rm-dispatch
gvanrossum Nov 10, 2022
3f57751
Merge remote-tracking branch 'origin/main' into rm-dispatch
gvanrossum Nov 10, 2022
f4ebe70
Merge branch 'main' into rm-dispatch
gvanrossum Nov 10, 2022
67200bf
Whitespace/braces from Brandt's review
gvanrossum Nov 10, 2022
a419b2b
Merge remote-tracking branch 'origin/main' into rm-dispatch
gvanrossum Nov 10, 2022
ed64309
Regen cases
gvanrossum Nov 10, 2022
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
182 changes: 88 additions & 94 deletions Python/bytecodes.c
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -68,7 +68,6 @@ do{\
#define JUMPBY(offset) ((void)0)
#define GO_TO_INSTRUCTION(instname) ((void)0)
#define DISPATCH_SAME_OPARG() ((void)0)
#define DISPATCH() ((void)0)

#define inst(name, ...) case name:
#define super(name) static int SUPER_##name
Expand DownExpand Up@@ -104,10 +103,6 @@ dummy_func(
{
switch (opcode){

/* BEWARE!
It is essential that any operation that fails must goto error
and that all operation that succeed call DISPATCH() ! */

// BEGIN BYTECODES //
inst(NOP, (--)){
}
Expand DownExpand Up@@ -152,16 +147,14 @@ dummy_func(
super(LOAD_FAST__LOAD_CONST) = LOAD_FAST + LOAD_CONST;
super(STORE_FAST__LOAD_FAST) = STORE_FAST + LOAD_FAST;
super(STORE_FAST__STORE_FAST) = STORE_FAST + STORE_FAST;
super(LOAD_CONST__LOAD_FAST) = LOAD_CONST + LOAD_FAST;
super(LOAD_CONST__LOAD_FAST) = LOAD_CONST + LOAD_FAST;

inst(POP_TOP, (value --)){
Py_DECREF(value);
}

// stack effect: ( -- __0)
inst(PUSH_NULL){
/* Use BASIC_PUSH as NULL is not a valid object pointer */
BASIC_PUSH(NULL);
inst(PUSH_NULL, (-- res)){
res = NULL;
Copy link
Member

@markshannonmarkshannonNov 10, 2022

Choose a reason for hiding this comment

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

I feel that this ought to fail, as we should have NULL checks in debug mode.

Copy link
MemberAuthor

@gvanrossumgvanrossumNov 10, 2022

Choose a reason for hiding this comment

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

Yeah, but we never had those. In debug mode we check for stack overflow/underflow but not for NULL-ness. (Of course the NULL will wreak havoc if it gets popped by the wrong opcode, bu that's also nothing new.) The code generator still checks for stack over/underflow by calling STACK_SHRINK()/GROW(), so nothing is really changed.

Copy link
Member

@brandtbucherbrandtbucherNov 10, 2022

Choose a reason for hiding this comment

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

Once we have typed stack effects, we can auto-generate those sorts of assertions. That would be really nice.

}

inst(END_FOR, (value1, value2 --)){
Expand DownExpand Up@@ -816,11 +809,12 @@ dummy_func(
Py_DECREF(receiver);
SET_TOP(retval);
JUMPBY(oparg);
DISPATCH();
}
assert(gen_status == PYGEN_NEXT);
assert(retval != NULL);
PUSH(retval);
else{
assert(gen_status == PYGEN_NEXT);
assert(retval != NULL);
PUSH(retval);
}
}

// stack effect: ( -- )
Expand DownExpand Up@@ -913,7 +907,6 @@ dummy_func(
if (PyErr_GivenExceptionMatches(val, PyExc_StopAsyncIteration)){
Py_DECREF(val);
Py_DECREF(POP());
DISPATCH();
}
else{
PyObject *exc = Py_NewRef(PyExceptionInstance_Class(val));
Expand All@@ -935,12 +928,13 @@ dummy_func(
Py_DECREF(POP()); // The last sent value.
Py_DECREF(POP()); // The delegated sub-iterator.
PUSH(value);
DISPATCH();
}
PyObject *exc_type = Py_NewRef(Py_TYPE(exc_value));
PyObject *exc_traceback = PyException_GetTraceback(exc_value);
_PyErr_Restore(tstate, exc_type, Py_NewRef(exc_value), exc_traceback);
goto exception_unwind;
else{
PyObject *exc_type = Py_NewRef(Py_TYPE(exc_value));
PyObject *exc_traceback = PyException_GetTraceback(exc_value);
_PyErr_Restore(tstate, exc_type, Py_NewRef(exc_value), exc_traceback);
goto exception_unwind;
}
}

inst(STOPITERATION_ERROR){
Expand DownExpand Up@@ -982,7 +976,6 @@ dummy_func(
PyException_SetContext(error, exc);
Py_DECREF(message);
}
DISPATCH();
}


Expand DownExpand Up@@ -1040,8 +1033,7 @@ dummy_func(
goto error;
}

// stack effect: ( -- )
inst(DELETE_NAME){
inst(DELETE_NAME, (--)){
PyObject *name = GETITEM(names, oparg);
PyObject *ns = LOCALS();
int err;
Expand All@@ -1051,6 +1043,7 @@ dummy_func(
goto error;
}
err = PyObject_DelItem(ns, name);
// Can't use ERROR_IF here.
if (err != 0){
format_exc_check_arg(tstate, PyExc_NameError,
NAME_ERROR_MSG,
Expand DownExpand Up@@ -1190,11 +1183,11 @@ dummy_func(
goto error;
}

// stack effect: ( -- )
inst(DELETE_GLOBAL){
inst(DELETE_GLOBAL, (--)){
PyObject *name = GETITEM(names, oparg);
int err;
err = PyDict_DelItem(GLOBALS(), name);
// Can't use ERROR_IF here.
if (err != 0){
if (_PyErr_ExceptionMatches(tstate, PyExc_KeyError)){
format_exc_check_arg(tstate, PyExc_NameError,
Expand DownExpand Up@@ -1374,18 +1367,13 @@ dummy_func(
SET_TOP(Py_NewRef(res));
}

// stack effect: ( -- )
inst(DELETE_FAST){
inst(DELETE_FAST, (--)){
PyObject *v = GETLOCAL(oparg);
if (v != NULL){
SETLOCAL(oparg, NULL);
DISPATCH();
}
goto unbound_local_error;
ERROR_IF(v == NULL, unbound_local_error);
SETLOCAL(oparg, NULL);
}

// stack effect: ( -- )
inst(MAKE_CELL){
inst(MAKE_CELL, (--)){
// "initial" is probably NULL but not if it's an arg (or set
// via PyFrame_LocalsToFast() before MAKE_CELL has run).
PyObject *initial = GETLOCAL(oparg);
Expand All@@ -1396,17 +1384,17 @@ dummy_func(
SETLOCAL(oparg, cell);
}

// stack effect: ( -- )
inst(DELETE_DEREF){
inst(DELETE_DEREF, (--)){
PyObject *cell = GETLOCAL(oparg);
PyObject *oldobj = PyCell_GET(cell);
if (oldobj != NULL){
PyCell_SET(cell, NULL);
Py_DECREF(oldobj);
DISPATCH();
// Can't use ERROR_IF here.
// Fortunately we don't need its superpower.
if (oldobj == NULL){
format_exc_unbound(tstate, frame->f_code, oparg);
goto error;
}
format_exc_unbound(tstate, frame->f_code, oparg);
goto error;
PyCell_SET(cell, NULL);
Py_DECREF(oldobj);
}

// stack effect: ( -- __0)
Expand DownExpand Up@@ -1769,15 +1757,15 @@ dummy_func(
Py_DECREF(owner);
PUSH(meth);
}
JUMPBY(INLINE_CACHE_ENTRIES_LOAD_ATTR);
DISPATCH();
}
PyObject *res = PyObject_GetAttr(owner, name);
if (res == NULL){
goto error;
else{
PyObject *res = PyObject_GetAttr(owner, name);
if (res == NULL){
goto error;
}
Py_DECREF(owner);
SET_TOP(res);
}
Py_DECREF(owner);
SET_TOP(res);
JUMPBY(INLINE_CACHE_ENTRIES_LOAD_ATTR);
}

Expand DownExpand Up@@ -2435,21 +2423,23 @@ dummy_func(
if (Py_IsTrue(cond)){
STACK_SHRINK(1);
_Py_DECREF_NO_DEALLOC(cond);
DISPATCH();
}
if (Py_IsFalse(cond)){
else if (Py_IsFalse(cond)){
JUMPBY(oparg);
DISPATCH();
}
err = PyObject_IsTrue(cond);
if (err > 0){
STACK_SHRINK(1);
Py_DECREF(cond);
else{
err = PyObject_IsTrue(cond);
if (err > 0){
STACK_SHRINK(1);
Py_DECREF(cond);
}
else if (err == 0){
JUMPBY(oparg);
}
else{
goto error;
}
}
else if (err == 0)
JUMPBY(oparg);
else
goto error;
}

// error: JUMP_IF_TRUE_OR_POP stack effect depends on jump flag
Expand All@@ -2459,22 +2449,23 @@ dummy_func(
if (Py_IsFalse(cond)){
STACK_SHRINK(1);
_Py_DECREF_NO_DEALLOC(cond);
DISPATCH();
}
if (Py_IsTrue(cond)){
JUMPBY(oparg);
DISPATCH();
}
err = PyObject_IsTrue(cond);
if (err > 0){
else if (Py_IsTrue(cond)){
JUMPBY(oparg);
}
else if (err == 0){
STACK_SHRINK(1);
Py_DECREF(cond);
else{
err = PyObject_IsTrue(cond);
if (err > 0){
JUMPBY(oparg);
}
else if (err == 0){
STACK_SHRINK(1);
Py_DECREF(cond);
}
else{
goto error;
}
}
else
goto error;
}

// stack effect: ( -- )
Expand DownExpand Up@@ -2615,23 +2606,24 @@ dummy_func(
if (next != NULL){
PUSH(next);
JUMPBY(INLINE_CACHE_ENTRIES_FOR_ITER);
DISPATCH();
}
if (_PyErr_Occurred(tstate)){
if (!_PyErr_ExceptionMatches(tstate, PyExc_StopIteration)){
goto error;
}
else if (tstate->c_tracefunc != NULL){
call_exc_trace(tstate->c_tracefunc, tstate->c_traceobj, tstate, frame);
else{
if (_PyErr_Occurred(tstate)){
if (!_PyErr_ExceptionMatches(tstate, PyExc_StopIteration)){
goto error;
}
else if (tstate->c_tracefunc != NULL){
call_exc_trace(tstate->c_tracefunc, tstate->c_traceobj, tstate, frame);
}
_PyErr_Clear(tstate);
}
_PyErr_Clear(tstate);
/* iterator ended normally */
assert(_Py_OPCODE(next_instr[INLINE_CACHE_ENTRIES_FOR_ITER + oparg]) == END_FOR);
STACK_SHRINK(1);
Py_DECREF(iter);
/* Skip END_FOR */
JUMPBY(INLINE_CACHE_ENTRIES_FOR_ITER + oparg + 1);
}
/* iterator ended normally */
assert(_Py_OPCODE(next_instr[INLINE_CACHE_ENTRIES_FOR_ITER + oparg]) == END_FOR);
STACK_SHRINK(1);
Py_DECREF(iter);
/* Skip END_FOR */
JUMPBY(INLINE_CACHE_ENTRIES_FOR_ITER + oparg + 1);
}

// stack effect: ( -- __0)
Expand All@@ -2646,14 +2638,15 @@ dummy_func(
PyObject *next = PyList_GET_ITEM(seq, it->it_index++);
PUSH(Py_NewRef(next));
JUMPBY(INLINE_CACHE_ENTRIES_FOR_ITER);
DISPATCH();
goto end_for_iter_list; // End of this instruction
Copy link
Member

@brandtbucherbrandtbucherNov 10, 2022

Choose a reason for hiding this comment

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

:(

Copy link
MemberAuthor

@gvanrossumgvanrossumNov 10, 2022

Choose a reason for hiding this comment

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

Yeah, I'm really counting on the compiler to optimize this for us.

}
it->it_seq = NULL;
Py_DECREF(seq);
}
STACK_SHRINK(1);
Py_DECREF(it);
JUMPBY(INLINE_CACHE_ENTRIES_FOR_ITER + oparg + 1);
end_for_iter_list:
}

// stack effect: ( -- __0)
Expand All@@ -2668,15 +2661,16 @@ dummy_func(
STACK_SHRINK(1);
Py_DECREF(r);
JUMPBY(INLINE_CACHE_ENTRIES_FOR_ITER + oparg + 1);
DISPATCH();
}
long value = (long)(r->start +
(unsigned long)(r->index++) * r->step);
if (_PyLong_AssignValue(&GETLOCAL(_Py_OPARG(next)), value) < 0){
goto error;
else{
long value = (long)(r->start +
(unsigned long)(r->index++) * r->step);
if (_PyLong_AssignValue(&GETLOCAL(_Py_OPARG(next)), value) < 0){
goto error;
}
// The STORE_FAST is already done.
JUMPBY(INLINE_CACHE_ENTRIES_FOR_ITER + 1);
}
// The STORE_FAST is already done.
JUMPBY(INLINE_CACHE_ENTRIES_FOR_ITER + 1);
}

inst(FOR_ITER_GEN){
Expand Down
Loading