[Python-checkins] gh-98831: rewrite PUSH_EXC_INFO and conditional jumps in the instruction definition DSL (#101481)
iritkatriel
webhook-mailer at python.org
Wed Feb 1 14:38:13 EST 2023
https://github.com/python/cpython/commit/b91b42d236c81bd7cbe402b322c82bfcd0d883a1
commit: b91b42d236c81bd7cbe402b322c82bfcd0d883a1
branch: main
author: Irit Katriel <1055913+iritkatriel at users.noreply.github.com>
committer: iritkatriel <1055913+iritkatriel at users.noreply.github.com>
date: 2023-02-01T19:38:06Z
summary:
gh-98831: rewrite PUSH_EXC_INFO and conditional jumps in the instruction definition DSL (#101481)
files:
M Python/bytecodes.c
M Python/compile.c
M Python/generated_cases.c.h
M Python/opcode_metadata.h
M Tools/cases_generator/generate_cases.py
diff --git a/Python/bytecodes.c b/Python/bytecodes.c
index bb1deaf3fbeb..105bd95be0fd 100644
--- a/Python/bytecodes.c
+++ b/Python/bytecodes.c
@@ -1898,9 +1898,7 @@ dummy_func(
CHECK_EVAL_BREAKER();
}
- // stack effect: (__0 -- )
- inst(POP_JUMP_IF_FALSE) {
- PyObject *cond = POP();
+ inst(POP_JUMP_IF_FALSE, (cond -- )) {
if (Py_IsTrue(cond)) {
_Py_DECREF_NO_DEALLOC(cond);
}
@@ -1911,19 +1909,16 @@ dummy_func(
else {
int err = PyObject_IsTrue(cond);
Py_DECREF(cond);
- if (err > 0)
- ;
- else if (err == 0) {
+ if (err == 0) {
JUMPBY(oparg);
}
- else
- goto error;
+ else {
+ ERROR_IF(err < 0, error);
+ }
}
}
- // stack effect: (__0 -- )
- inst(POP_JUMP_IF_TRUE) {
- PyObject *cond = POP();
+ inst(POP_JUMP_IF_TRUE, (cond -- )) {
if (Py_IsFalse(cond)) {
_Py_DECREF_NO_DEALLOC(cond);
}
@@ -1937,25 +1932,23 @@ dummy_func(
if (err > 0) {
JUMPBY(oparg);
}
- else if (err == 0)
- ;
- else
- goto error;
+ else {
+ ERROR_IF(err < 0, error);
+ }
}
}
- // stack effect: (__0 -- )
- inst(POP_JUMP_IF_NOT_NONE) {
- PyObject *value = POP();
+ inst(POP_JUMP_IF_NOT_NONE, (value -- )) {
if (!Py_IsNone(value)) {
+ Py_DECREF(value);
JUMPBY(oparg);
}
- Py_DECREF(value);
+ else {
+ _Py_DECREF_NO_DEALLOC(value);
+ }
}
- // stack effect: (__0 -- )
- inst(POP_JUMP_IF_NONE) {
- PyObject *value = POP();
+ inst(POP_JUMP_IF_NONE, (value -- )) {
if (Py_IsNone(value)) {
_Py_DECREF_NO_DEALLOC(value);
JUMPBY(oparg);
@@ -1965,25 +1958,24 @@ dummy_func(
}
}
- // error: JUMP_IF_FALSE_OR_POP stack effect depends on jump flag
- inst(JUMP_IF_FALSE_OR_POP) {
- PyObject *cond = TOP();
+ inst(JUMP_IF_FALSE_OR_POP, (cond -- cond if (jump))) {
+ bool jump = false;
int err;
if (Py_IsTrue(cond)) {
- STACK_SHRINK(1);
_Py_DECREF_NO_DEALLOC(cond);
}
else if (Py_IsFalse(cond)) {
JUMPBY(oparg);
+ jump = true;
}
else {
err = PyObject_IsTrue(cond);
if (err > 0) {
- STACK_SHRINK(1);
Py_DECREF(cond);
}
else if (err == 0) {
JUMPBY(oparg);
+ jump = true;
}
else {
goto error;
@@ -1991,24 +1983,23 @@ dummy_func(
}
}
- // error: JUMP_IF_TRUE_OR_POP stack effect depends on jump flag
- inst(JUMP_IF_TRUE_OR_POP) {
- PyObject *cond = TOP();
+ inst(JUMP_IF_TRUE_OR_POP, (cond -- cond if (jump))) {
+ bool jump = false;
int err;
if (Py_IsFalse(cond)) {
- STACK_SHRINK(1);
_Py_DECREF_NO_DEALLOC(cond);
}
else if (Py_IsTrue(cond)) {
JUMPBY(oparg);
+ jump = true;
}
else {
err = PyObject_IsTrue(cond);
if (err > 0) {
JUMPBY(oparg);
+ jump = true;
}
else if (err == 0) {
- STACK_SHRINK(1);
Py_DECREF(cond);
}
else {
@@ -2321,22 +2312,16 @@ dummy_func(
ERROR_IF(res == NULL, error);
}
- // stack effect: ( -- __0)
- inst(PUSH_EXC_INFO) {
- PyObject *value = TOP();
-
+ inst(PUSH_EXC_INFO, (new_exc -- prev_exc, new_exc)) {
_PyErr_StackItem *exc_info = tstate->exc_info;
if (exc_info->exc_value != NULL) {
- SET_TOP(exc_info->exc_value);
+ prev_exc = exc_info->exc_value;
}
else {
- SET_TOP(Py_NewRef(Py_None));
+ prev_exc = Py_NewRef(Py_None);
}
-
- PUSH(Py_NewRef(value));
- assert(PyExceptionInstance_Check(value));
- exc_info->exc_value = value;
-
+ assert(PyExceptionInstance_Check(new_exc));
+ exc_info->exc_value = Py_NewRef(new_exc);
}
inst(LOAD_ATTR_METHOD_WITH_VALUES, (unused/1, type_version/2, keys_version/2, descr/4, self -- res2 if (oparg & 1), res)) {
diff --git a/Python/compile.c b/Python/compile.c
index a11bcc79a6dd..d9ec68958972 100644
--- a/Python/compile.c
+++ b/Python/compile.c
@@ -8630,17 +8630,19 @@ opcode_metadata_is_sane(cfg_builder *g) {
int opcode = instr->i_opcode;
int oparg = instr->i_oparg;
assert(opcode <= MAX_REAL_OPCODE);
- int popped = _PyOpcode_num_popped(opcode, oparg);
- int pushed = _PyOpcode_num_pushed(opcode, oparg);
- assert((pushed < 0) == (popped < 0));
- if (pushed >= 0) {
- assert(_PyOpcode_opcode_metadata[opcode].valid_entry);
- int effect = stack_effect(opcode, instr->i_oparg, -1);
- if (effect != pushed - popped) {
- fprintf(stderr,
- "op=%d: stack_effect (%d) != pushed (%d) - popped (%d)\n",
- opcode, effect, pushed, popped);
- result = false;
+ for (int jump = 0; jump <= 1; jump++) {
+ int popped = _PyOpcode_num_popped(opcode, oparg, jump ? true : false);
+ int pushed = _PyOpcode_num_pushed(opcode, oparg, jump ? true : false);
+ assert((pushed < 0) == (popped < 0));
+ if (pushed >= 0) {
+ assert(_PyOpcode_opcode_metadata[opcode].valid_entry);
+ int effect = stack_effect(opcode, instr->i_oparg, jump);
+ if (effect != pushed - popped) {
+ fprintf(stderr,
+ "op=%d arg=%d jump=%d: stack_effect (%d) != pushed (%d) - popped (%d)\n",
+ opcode, oparg, jump, effect, pushed, popped);
+ result = false;
+ }
}
}
}
diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h
index e5c5c7e557a3..a02d8d79c60d 100644
--- a/Python/generated_cases.c.h
+++ b/Python/generated_cases.c.h
@@ -2348,7 +2348,7 @@
TARGET(POP_JUMP_IF_FALSE) {
PREDICTED(POP_JUMP_IF_FALSE);
- PyObject *cond = POP();
+ PyObject *cond = PEEK(1);
if (Py_IsTrue(cond)) {
_Py_DECREF_NO_DEALLOC(cond);
}
@@ -2359,19 +2359,19 @@
else {
int err = PyObject_IsTrue(cond);
Py_DECREF(cond);
- if (err > 0)
- ;
- else if (err == 0) {
+ if (err == 0) {
JUMPBY(oparg);
}
- else
- goto error;
+ else {
+ if (err < 0) goto pop_1_error;
+ }
}
+ STACK_SHRINK(1);
DISPATCH();
}
TARGET(POP_JUMP_IF_TRUE) {
- PyObject *cond = POP();
+ PyObject *cond = PEEK(1);
if (Py_IsFalse(cond)) {
_Py_DECREF_NO_DEALLOC(cond);
}
@@ -2385,25 +2385,29 @@
if (err > 0) {
JUMPBY(oparg);
}
- else if (err == 0)
- ;
- else
- goto error;
+ else {
+ if (err < 0) goto pop_1_error;
+ }
}
+ STACK_SHRINK(1);
DISPATCH();
}
TARGET(POP_JUMP_IF_NOT_NONE) {
- PyObject *value = POP();
+ PyObject *value = PEEK(1);
if (!Py_IsNone(value)) {
+ Py_DECREF(value);
JUMPBY(oparg);
}
- Py_DECREF(value);
+ else {
+ _Py_DECREF_NO_DEALLOC(value);
+ }
+ STACK_SHRINK(1);
DISPATCH();
}
TARGET(POP_JUMP_IF_NONE) {
- PyObject *value = POP();
+ PyObject *value = PEEK(1);
if (Py_IsNone(value)) {
_Py_DECREF_NO_DEALLOC(value);
JUMPBY(oparg);
@@ -2411,58 +2415,65 @@
else {
Py_DECREF(value);
}
+ STACK_SHRINK(1);
DISPATCH();
}
TARGET(JUMP_IF_FALSE_OR_POP) {
- PyObject *cond = TOP();
+ PyObject *cond = PEEK(1);
+ bool jump = false;
int err;
if (Py_IsTrue(cond)) {
- STACK_SHRINK(1);
_Py_DECREF_NO_DEALLOC(cond);
}
else if (Py_IsFalse(cond)) {
JUMPBY(oparg);
+ jump = true;
}
else {
err = PyObject_IsTrue(cond);
if (err > 0) {
- STACK_SHRINK(1);
Py_DECREF(cond);
}
else if (err == 0) {
JUMPBY(oparg);
+ jump = true;
}
else {
goto error;
}
}
+ STACK_SHRINK(1);
+ STACK_GROW((jump ? 1 : 0));
DISPATCH();
}
TARGET(JUMP_IF_TRUE_OR_POP) {
- PyObject *cond = TOP();
+ PyObject *cond = PEEK(1);
+ bool jump = false;
int err;
if (Py_IsFalse(cond)) {
- STACK_SHRINK(1);
_Py_DECREF_NO_DEALLOC(cond);
}
else if (Py_IsTrue(cond)) {
JUMPBY(oparg);
+ jump = true;
}
else {
err = PyObject_IsTrue(cond);
if (err > 0) {
JUMPBY(oparg);
+ jump = true;
}
else if (err == 0) {
- STACK_SHRINK(1);
Py_DECREF(cond);
}
else {
goto error;
}
}
+ STACK_SHRINK(1);
+ STACK_GROW((jump ? 1 : 0));
DISPATCH();
}
@@ -2828,19 +2839,20 @@
}
TARGET(PUSH_EXC_INFO) {
- PyObject *value = TOP();
-
+ PyObject *new_exc = PEEK(1);
+ PyObject *prev_exc;
_PyErr_StackItem *exc_info = tstate->exc_info;
if (exc_info->exc_value != NULL) {
- SET_TOP(exc_info->exc_value);
+ prev_exc = exc_info->exc_value;
}
else {
- SET_TOP(Py_NewRef(Py_None));
+ prev_exc = Py_NewRef(Py_None);
}
-
- PUSH(Py_NewRef(value));
- assert(PyExceptionInstance_Check(value));
- exc_info->exc_value = value;
+ assert(PyExceptionInstance_Check(new_exc));
+ exc_info->exc_value = Py_NewRef(new_exc);
+ STACK_GROW(1);
+ POKE(1, new_exc);
+ POKE(2, prev_exc);
DISPATCH();
}
diff --git a/Python/opcode_metadata.h b/Python/opcode_metadata.h
index 96e57be8cf5d..c9f9759e16b3 100644
--- a/Python/opcode_metadata.h
+++ b/Python/opcode_metadata.h
@@ -4,7 +4,7 @@
#ifndef NDEBUG
static int
-_PyOpcode_num_popped(int opcode, int oparg) {
+_PyOpcode_num_popped(int opcode, int oparg, bool jump) {
switch(opcode) {
case NOP:
return 0;
@@ -233,17 +233,17 @@ _PyOpcode_num_popped(int opcode, int oparg) {
case JUMP_BACKWARD:
return 0;
case POP_JUMP_IF_FALSE:
- return -1;
+ return 1;
case POP_JUMP_IF_TRUE:
- return -1;
+ return 1;
case POP_JUMP_IF_NOT_NONE:
- return -1;
+ return 1;
case POP_JUMP_IF_NONE:
- return -1;
+ return 1;
case JUMP_IF_FALSE_OR_POP:
- return -1;
+ return 1;
case JUMP_IF_TRUE_OR_POP:
- return -1;
+ return 1;
case JUMP_BACKWARD_NO_INTERRUPT:
return 0;
case GET_LEN:
@@ -277,7 +277,7 @@ _PyOpcode_num_popped(int opcode, int oparg) {
case WITH_EXCEPT_START:
return 4;
case PUSH_EXC_INFO:
- return -1;
+ return 1;
case LOAD_ATTR_METHOD_WITH_VALUES:
return 1;
case LOAD_ATTR_METHOD_NO_DICT:
@@ -350,7 +350,7 @@ _PyOpcode_num_popped(int opcode, int oparg) {
#ifndef NDEBUG
static int
-_PyOpcode_num_pushed(int opcode, int oparg) {
+_PyOpcode_num_pushed(int opcode, int oparg, bool jump) {
switch(opcode) {
case NOP:
return 0;
@@ -579,17 +579,17 @@ _PyOpcode_num_pushed(int opcode, int oparg) {
case JUMP_BACKWARD:
return 0;
case POP_JUMP_IF_FALSE:
- return -1;
+ return 0;
case POP_JUMP_IF_TRUE:
- return -1;
+ return 0;
case POP_JUMP_IF_NOT_NONE:
- return -1;
+ return 0;
case POP_JUMP_IF_NONE:
- return -1;
+ return 0;
case JUMP_IF_FALSE_OR_POP:
- return -1;
+ return (jump ? 1 : 0);
case JUMP_IF_TRUE_OR_POP:
- return -1;
+ return (jump ? 1 : 0);
case JUMP_BACKWARD_NO_INTERRUPT:
return 0;
case GET_LEN:
@@ -623,7 +623,7 @@ _PyOpcode_num_pushed(int opcode, int oparg) {
case WITH_EXCEPT_START:
return 5;
case PUSH_EXC_INFO:
- return -1;
+ return 2;
case LOAD_ATTR_METHOD_WITH_VALUES:
return ((oparg & 1) ? 1 : 0) + 1;
case LOAD_ATTR_METHOD_NO_DICT:
diff --git a/Tools/cases_generator/generate_cases.py b/Tools/cases_generator/generate_cases.py
index 43685450cc0d..3925583b40e7 100644
--- a/Tools/cases_generator/generate_cases.py
+++ b/Tools/cases_generator/generate_cases.py
@@ -868,7 +868,7 @@ def write_function(
) -> None:
self.out.emit("\n#ifndef NDEBUG")
self.out.emit("static int")
- self.out.emit(f"_PyOpcode_num_{direction}(int opcode, int oparg) {{")
+ self.out.emit(f"_PyOpcode_num_{direction}(int opcode, int oparg, bool jump) {{")
self.out.emit(" switch(opcode) {")
for instr, effect in data:
self.out.emit(f" case {instr.name}:")
More information about the Python-checkins
mailing list