From cdfbacb5992f355276e65a1d95f9765966891e2e Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Tue, 31 Jan 2023 23:44:46 +0000 Subject: [PATCH 1/8] POP_JUMP_IF_NOT_NONE, POP_JUMP_IF_NONE --- Python/bytecodes.c | 13 ++++++------- Python/generated_cases.c.h | 11 ++++++++--- Python/opcode_metadata.h | 8 ++++---- 3 files changed, 18 insertions(+), 14 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 336088e08197de..58383619ca7a84 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -1978,18 +1978,17 @@ dummy_func( } } - // 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); diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index d70d64ecbdd5c1..0103f174816817 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -2380,16 +2380,20 @@ } 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); @@ -2397,6 +2401,7 @@ else { Py_DECREF(value); } + STACK_SHRINK(1); DISPATCH(); } diff --git a/Python/opcode_metadata.h b/Python/opcode_metadata.h index 171fed363d3be0..c1fa05c8e77fa1 100644 --- a/Python/opcode_metadata.h +++ b/Python/opcode_metadata.h @@ -237,9 +237,9 @@ _PyOpcode_num_popped(int opcode, int oparg) { case POP_JUMP_IF_TRUE: 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; case JUMP_IF_TRUE_OR_POP: @@ -583,9 +583,9 @@ _PyOpcode_num_pushed(int opcode, int oparg) { case POP_JUMP_IF_TRUE: return -1; 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; case JUMP_IF_TRUE_OR_POP: From 121bc553e8335296c879785c2fe92c6ab2c9c641 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Tue, 31 Jan 2023 23:56:45 +0000 Subject: [PATCH 2/8] POP_JUMP_IF_FALSE, POP_JUMP_IF_TRUE --- Python/bytecodes.c | 24 +++++++++--------------- Python/generated_cases.c.h | 22 +++++++++++----------- Python/opcode_metadata.h | 8 ++++---- 3 files changed, 24 insertions(+), 30 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 58383619ca7a84..6247952118c0b2 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -1932,9 +1932,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); } @@ -1945,19 +1943,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); } @@ -1971,10 +1966,9 @@ dummy_func( if (err > 0) { JUMPBY(oparg); } - else if (err == 0) - ; - else - goto error; + else { + ERROR_IF(err < 0, error); + } } } diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 0103f174816817..975a8034f796d9 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -2334,7 +2334,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); } @@ -2345,19 +2345,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); } @@ -2371,11 +2371,11 @@ if (err > 0) { JUMPBY(oparg); } - else if (err == 0) - ; - else - goto error; + else { + if (err < 0) goto pop_1_error; + } } + STACK_SHRINK(1); DISPATCH(); } diff --git a/Python/opcode_metadata.h b/Python/opcode_metadata.h index c1fa05c8e77fa1..c57897911ac567 100644 --- a/Python/opcode_metadata.h +++ b/Python/opcode_metadata.h @@ -233,9 +233,9 @@ _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; case POP_JUMP_IF_NONE: @@ -579,9 +579,9 @@ _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 0; case POP_JUMP_IF_NONE: From a052d10e818b6de863c5854e8c067ac256c37657 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Wed, 1 Feb 2023 00:11:28 +0000 Subject: [PATCH 3/8] PUSH_EXC_INFO --- Python/bytecodes.c | 16 ++++++---------- Python/generated_cases.c.h | 18 ++++++++++-------- Python/opcode_metadata.h | 4 ++-- 3 files changed, 18 insertions(+), 20 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 6247952118c0b2..542aa1faba3909 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -2349,21 +2349,17 @@ dummy_func( } // 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)); + Py_INCREF(new_exc); + exc_info->exc_value = new_exc; } // error: LOAD_ATTR has irregular stack effect diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 975a8034f796d9..7325e2879eae78 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -2819,19 +2819,21 @@ } 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)); + Py_INCREF(new_exc); + exc_info->exc_value = 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 c57897911ac567..eae81707e9745b 100644 --- a/Python/opcode_metadata.h +++ b/Python/opcode_metadata.h @@ -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: @@ -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 -1; case LOAD_ATTR_METHOD_NO_DICT: From 253ec9b95b098e6b225cd62707f1c434e4a4089f Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Wed, 1 Feb 2023 15:39:00 +0000 Subject: [PATCH 4/8] JUMP_IF_FALSE_OR_POP --- Python/bytecodes.c | 9 ++++----- Python/compile.c | 24 +++++++++++++----------- Python/generated_cases.c.h | 9 ++++++--- Python/opcode_metadata.h | 8 ++++---- Tools/cases_generator/generate_cases.py | 2 +- 5 files changed, 28 insertions(+), 24 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 542aa1faba3909..60e8de2b28350a 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -1992,25 +1992,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; diff --git a/Python/compile.c b/Python/compile.c index a11bcc79a6dd10..d9ec68958972b5 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 7325e2879eae78..bfc97797011fa8 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -2406,28 +2406,31 @@ } 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(); } diff --git a/Python/opcode_metadata.h b/Python/opcode_metadata.h index eae81707e9745b..ac77bea71faffa 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; @@ -241,7 +241,7 @@ _PyOpcode_num_popped(int opcode, int oparg) { case POP_JUMP_IF_NONE: return 1; case JUMP_IF_FALSE_OR_POP: - return -1; + return 1; case JUMP_IF_TRUE_OR_POP: return -1; case JUMP_BACKWARD_NO_INTERRUPT: @@ -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; @@ -587,7 +587,7 @@ _PyOpcode_num_pushed(int opcode, int oparg) { case POP_JUMP_IF_NONE: return 0; case JUMP_IF_FALSE_OR_POP: - return -1; + return (jump ? 1 : 0); case JUMP_IF_TRUE_OR_POP: return -1; case JUMP_BACKWARD_NO_INTERRUPT: diff --git a/Tools/cases_generator/generate_cases.py b/Tools/cases_generator/generate_cases.py index 43685450cc0dfe..3925583b40e728 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}:") From 78e20c4c32e52c391a2a7eb6e2fb492e3466ec6d Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Wed, 1 Feb 2023 15:51:00 +0000 Subject: [PATCH 5/8] JUMP_IF_TRUE_OR_POP --- Python/bytecodes.c | 9 ++++----- Python/generated_cases.c.h | 9 ++++++--- Python/opcode_metadata.h | 4 ++-- 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 60e8de2b28350a..73c26eb11ecac0 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -2017,24 +2017,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 { diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index bfc97797011fa8..9a8aea0c641b05 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -2435,28 +2435,31 @@ } 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(); } diff --git a/Python/opcode_metadata.h b/Python/opcode_metadata.h index ac77bea71faffa..58687821799ee4 100644 --- a/Python/opcode_metadata.h +++ b/Python/opcode_metadata.h @@ -243,7 +243,7 @@ _PyOpcode_num_popped(int opcode, int oparg, bool jump) { case JUMP_IF_FALSE_OR_POP: return 1; case JUMP_IF_TRUE_OR_POP: - return -1; + return 1; case JUMP_BACKWARD_NO_INTERRUPT: return 0; case GET_LEN: @@ -589,7 +589,7 @@ _PyOpcode_num_pushed(int opcode, int oparg, bool jump) { case JUMP_IF_FALSE_OR_POP: 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: From 9b2fb90cf951661ea67adac15a4b65ddb2ec4bd6 Mon Sep 17 00:00:00 2001 From: Irit Katriel <1055913+iritkatriel@users.noreply.github.com> Date: Wed, 1 Feb 2023 18:58:06 +0000 Subject: [PATCH 6/8] use Py_NewRef for new_exc. Co-authored-by: Guido van Rossum --- Python/bytecodes.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 73c26eb11ecac0..2e80a731fa6a0a 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -2356,8 +2356,7 @@ dummy_func( prev_exc = Py_NewRef(Py_None); } assert(PyExceptionInstance_Check(new_exc)); - Py_INCREF(new_exc); - exc_info->exc_value = new_exc; + exc_info->exc_value = Py_NewRef(new_exc); } // error: LOAD_ATTR has irregular stack effect From 09b39b011d6c458bd0dc04552a16eafe251fa1f8 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Wed, 1 Feb 2023 19:00:05 +0000 Subject: [PATCH 7/8] remove obsolete comment --- Python/bytecodes.c | 1 - 1 file changed, 1 deletion(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 2e80a731fa6a0a..9c9934e1d1cc3d 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -2346,7 +2346,6 @@ dummy_func( ERROR_IF(res == NULL, error); } - // stack effect: ( -- __0) inst(PUSH_EXC_INFO, (new_exc -- prev_exc, new_exc)) { _PyErr_StackItem *exc_info = tstate->exc_info; if (exc_info->exc_value != NULL) { From 7a932bc24dc543dc05f3c6d7e15a5b3959764d9d Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Wed, 1 Feb 2023 19:11:51 +0000 Subject: [PATCH 8/8] make regen --- Python/generated_cases.c.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 95ff943861ef48..a02d8d79c60d37 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -2849,8 +2849,7 @@ prev_exc = Py_NewRef(Py_None); } assert(PyExceptionInstance_Check(new_exc)); - Py_INCREF(new_exc); - exc_info->exc_value = new_exc; + exc_info->exc_value = Py_NewRef(new_exc); STACK_GROW(1); POKE(1, new_exc); POKE(2, prev_exc);