FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

GH-98831: Remove all remaining DISPATCH() calls from bytecodes.c by gvanrossum · Pull Request #99271 · python/cpython · GitHub

/ cpython Public

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
Conversation 13 Commits 8 Checks 0 Files changed
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 .c  (1) .h  (1) All 2 file types selected
Viewed files
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Unified
Split
Hide whitespace
Diff view
Unified
Split
Hide whitespace
182 changes: 88 additions & 94 deletions Python/bytecodes.c
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff 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 Down Expand 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 Down Expand 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;

markshannon Nov 10, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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

gvanrossum Nov 10, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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.

brandtbucher Nov 10, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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 Down Expand 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 Down Expand 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 Down Expand Up @@ -982,7 +976,6 @@ dummy_func(
PyException_SetContext(error, exc);
Py_DECREF(message);
}
DISPATCH();
}


Expand Down Expand 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 Down Expand 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 Down Expand 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 Down Expand 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 Down Expand 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 Down Expand 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

brandtbucher Nov 10, 2022

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

:(

gvanrossum Nov 10, 2022

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

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
Back | FazBrowse Home | New Git URL