From 7fe160297b56726fb4b3248e49c0055c162e2651 Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Tue, 24 Jan 2023 23:59:38 +0000 Subject: [PATCH 1/5] gh-98831: rewrite RAISE_VARARGS in the instruction definition DSL --- Python/bytecodes.c | 11 +++++------ Python/generated_cases.c.h | 11 +++++++---- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index d3e242b81e608d..b203954efcc62c 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -505,19 +505,18 @@ dummy_func( ERROR_IF(res == NULL, error); } - // This should remain a legacy instruction. - inst(RAISE_VARARGS) { + inst(RAISE_VARARGS, (args[oparg] -- )) { PyObject *cause = NULL, *exc = NULL; switch (oparg) { case 2: - cause = POP(); /* cause */ + cause = args[1]; /* fall through */ case 1: - exc = POP(); /* exc */ + exc = args[0]; /* fall through */ case 0: if (do_raise(tstate, exc, cause)) { - goto exception_unwind; + ERROR_IF(true, exception_unwind); } break; default: @@ -525,7 +524,7 @@ dummy_func( "bad RAISE_VARARGS oparg"); break; } - goto error; + ERROR_IF(true, error); } inst(INTERPRETER_EXIT, (retval --)) { diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 7d3396ad6bdec3..50e15a3d41020e 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -689,17 +689,18 @@ } TARGET(RAISE_VARARGS) { + PyObject **args = &PEEK(oparg); PyObject *cause = NULL, *exc = NULL; switch (oparg) { case 2: - cause = POP(); /* cause */ + cause = args[1]; /* fall through */ case 1: - exc = POP(); /* exc */ + exc = args[0]; /* fall through */ case 0: if (do_raise(tstate, exc, cause)) { - goto exception_unwind; + if (true) { STACK_SHRINK(oparg); goto exception_unwind; } } break; default: @@ -707,7 +708,9 @@ "bad RAISE_VARARGS oparg"); break; } - goto error; + if (true) { STACK_SHRINK(oparg); goto error; } + STACK_SHRINK(oparg); + DISPATCH(); } TARGET(INTERPRETER_EXIT) { From 60715034330d3c16800cac5e2c38ec218e2e99ff Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Wed, 25 Jan 2023 21:15:03 +0000 Subject: [PATCH 2/5] avoid emitting dead code --- Python/generated_cases.c.h | 2 -- Tools/cases_generator/generate_cases.py | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 50e15a3d41020e..76e26a4a2f8bb1 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -709,8 +709,6 @@ break; } if (true) { STACK_SHRINK(oparg); goto error; } - STACK_SHRINK(oparg); - DISPATCH(); } TARGET(INTERPRETER_EXIT) { diff --git a/Tools/cases_generator/generate_cases.py b/Tools/cases_generator/generate_cases.py index 429c9d34d60601..bdaa3a4748c2eb 100644 --- a/Tools/cases_generator/generate_cases.py +++ b/Tools/cases_generator/generate_cases.py @@ -981,7 +981,7 @@ def always_exits(lines: list[str]) -> bool: return False line = line[12:] return line.startswith( - ("goto ", "return ", "DISPATCH", "GO_TO_", "Py_UNREACHABLE()") + ("goto ", "return ", "DISPATCH", "GO_TO_", "Py_UNREACHABLE()", "ERROR_IF(true, ") ) From 2daaa2e516520193cf4d46da3d3d6ea876aa856b Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Wed, 25 Jan 2023 21:03:01 +0000 Subject: [PATCH 3/5] apply Guido's suggestion --- Python/bytecodes.c | 4 +--- Python/generated_cases.c.h | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index b203954efcc62c..e5769f61fc28d0 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -515,9 +515,7 @@ dummy_func( exc = args[0]; /* fall through */ case 0: - if (do_raise(tstate, exc, cause)) { - ERROR_IF(true, exception_unwind); - } + ERROR_IF(do_raise(tstate, exc, cause), exception_unwind); break; default: _PyErr_SetString(tstate, PyExc_SystemError, diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 76e26a4a2f8bb1..287a1f1f042089 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -699,9 +699,7 @@ exc = args[0]; /* fall through */ case 0: - if (do_raise(tstate, exc, cause)) { - if (true) { STACK_SHRINK(oparg); goto exception_unwind; } - } + if (do_raise(tstate, exc, cause)) { STACK_SHRINK(oparg); goto exception_unwind; } break; default: _PyErr_SetString(tstate, PyExc_SystemError, From 4df3e6b25e5aecc4a3503ce30aed806cccd0790a Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Wed, 25 Jan 2023 21:24:21 +0000 Subject: [PATCH 4/5] regen --- Python/opcode_metadata.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Python/opcode_metadata.h b/Python/opcode_metadata.h index 46fd9673e8fb64..8195fc6178857c 100644 --- a/Python/opcode_metadata.h +++ b/Python/opcode_metadata.h @@ -86,7 +86,7 @@ _PyOpcode_num_popped(int opcode, int oparg) { case CALL_INTRINSIC_1: return 1; case RAISE_VARARGS: - return -1; + return oparg; case INTERPRETER_EXIT: return 1; case RETURN_VALUE: @@ -430,7 +430,7 @@ _PyOpcode_num_pushed(int opcode, int oparg) { case CALL_INTRINSIC_1: return 1; case RAISE_VARARGS: - return -1; + return 0; case INTERPRETER_EXIT: return 0; case RETURN_VALUE: From 06ac7dd1ff687236dfd519dc1e54fac965ca0d8a Mon Sep 17 00:00:00 2001 From: Irit Katriel Date: Wed, 25 Jan 2023 21:58:17 +0000 Subject: [PATCH 5/5] avoid 'unused function' warnings --- Python/opcode_metadata.h | 4 ++++ Tools/cases_generator/generate_cases.py | 4 +++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/Python/opcode_metadata.h b/Python/opcode_metadata.h index 8195fc6178857c..cca86629e48d16 100644 --- a/Python/opcode_metadata.h +++ b/Python/opcode_metadata.h @@ -2,6 +2,7 @@ // from Python/bytecodes.c // Do not edit! +#ifndef NDEBUG static int _PyOpcode_num_popped(int opcode, int oparg) { switch(opcode) { @@ -345,7 +346,9 @@ _PyOpcode_num_popped(int opcode, int oparg) { Py_UNREACHABLE(); } } +#endif +#ifndef NDEBUG static int _PyOpcode_num_pushed(int opcode, int oparg) { switch(opcode) { @@ -689,6 +692,7 @@ _PyOpcode_num_pushed(int opcode, int oparg) { Py_UNREACHABLE(); } } +#endif enum Direction { DIR_NONE, DIR_READ, DIR_WRITE }; enum InstructionFormat { INSTR_FMT_IB, INSTR_FMT_IBC, INSTR_FMT_IBC0, INSTR_FMT_IBC000, INSTR_FMT_IBIB, INSTR_FMT_IX, INSTR_FMT_IXC, INSTR_FMT_IXC000 }; struct opcode_metadata { diff --git a/Tools/cases_generator/generate_cases.py b/Tools/cases_generator/generate_cases.py index 710e155ae9e82a..b7942410c82fc3 100644 --- a/Tools/cases_generator/generate_cases.py +++ b/Tools/cases_generator/generate_cases.py @@ -774,7 +774,8 @@ def write_stack_effect_functions(self) -> None: pushed_data.append( (instr, pushed) ) def write_function(direction: str, data: list[tuple[Instruction, str]]) -> None: - self.out.emit("\nstatic int"); + 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(" switch(opcode) {"); for instr, effect in data: @@ -784,6 +785,7 @@ def write_function(direction: str, data: list[tuple[Instruction, str]]) -> None: self.out.emit(" Py_UNREACHABLE();") self.out.emit(" }") self.out.emit("}") + self.out.emit("#endif"); write_function('popped', popped_data) write_function('pushed', pushed_data)