From 7a09150761159ff00cb69c4864c7282a2e18d51c Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Sat, 3 May 2025 00:38:59 +0200 Subject: [PATCH 1/2] Keep instruction definitions in bytecodes.c and optimizer_bytecodes.c in sync --- Lib/test/test_generated_cases.py | 139 ++++++++++++++++ Python/optimizer_bytecodes.c | 120 +++++++------- Python/optimizer_cases.c.h | 161 ++++++++++--------- Tools/cases_generator/optimizer_generator.py | 28 +++- 4 files changed, 313 insertions(+), 135 deletions(-) diff --git a/Lib/test/test_generated_cases.py b/Lib/test/test_generated_cases.py index d481cb07f757d8..ae3fa17872f5e6 100644 --- a/Lib/test/test_generated_cases.py +++ b/Lib/test/test_generated_cases.py @@ -2069,6 +2069,145 @@ def test_missing_override_failure(self): with self.assertRaisesRegex(AssertionError, "All abstract uops"): self.run_cases_test(input, input2, output) + def test_validate_uop_input_length_mismatch(self): + input = """ + op(OP, (arg1 -- out)) { + SPAM(); + } + """ + input2 = """ + op(OP, (arg1, arg2 -- out)) { + } + """ + output = """ + """ + with self.assertRaisesRegex(SyntaxError, + "Must have the same number of inputs"): + self.run_cases_test(input, input2, output) + + def test_validate_uop_output_length_mismatch(self): + input = """ + op(OP, (arg1 -- out)) { + SPAM(); + } + """ + input2 = """ + op(OP, (arg1 -- out1, out2)) { + } + """ + output = """ + """ + with self.assertRaisesRegex(SyntaxError, + "Must have the same number of outputs"): + self.run_cases_test(input, input2, output) + + def test_validate_uop_input_name_mismatch(self): + input = """ + op(OP, (foo -- out)) { + SPAM(); + } + """ + input2 = """ + op(OP, (bar -- out)) { + } + """ + output = """ + """ + with self.assertRaisesRegex(SyntaxError, + "Inputs must have equal names"): + self.run_cases_test(input, input2, output) + + def test_validate_uop_output_name_mismatch(self): + input = """ + op(OP, (arg1 -- foo)) { + SPAM(); + } + """ + input2 = """ + op(OP, (arg1 -- bar)) { + } + """ + output = """ + """ + with self.assertRaisesRegex(SyntaxError, + "Outputs must have equal names"): + self.run_cases_test(input, input2, output) + + def test_validate_uop_unused_input(self): + input = """ + op(OP, (unused -- )) { + } + """ + input2 = """ + op(OP, (foo -- )) { + } + """ + output = """ + case OP: { + stack_pointer += -1; + assert(WITHIN_STACK_BOUNDS()); + break; + } + """ + self.run_cases_test(input, input2, output) + + input = """ + op(OP, (foo -- )) { + } + """ + input2 = """ + op(OP, (unused -- )) { + } + """ + output = """ + case OP: { + stack_pointer += -1; + assert(WITHIN_STACK_BOUNDS()); + break; + } + """ + self.run_cases_test(input, input2, output) + + def test_validate_uop_unused_override(self): + input = """ + op(OP, ( -- unused)) { + } + """ + input2 = """ + op(OP, ( -- foo)) { + foo = NULL; + } + """ + output = """ + case OP: { + JitOptSymbol *foo; + foo = NULL; + stack_pointer[0] = foo; + stack_pointer += 1; + assert(WITHIN_STACK_BOUNDS()); + break; + } + """ + self.run_cases_test(input, input2, output) + + input = """ + op(OP, ( -- foo)) { + foo = NULL; + } + """ + input2 = """ + op(OP, ( -- unused)) { + } + """ + output = """ + case OP: { + stack_pointer += 1; + assert(WITHIN_STACK_BOUNDS()); + break; + } + """ + self.run_cases_test(input, input2, output) + if __name__ == "__main__": unittest.main() diff --git a/Python/optimizer_bytecodes.c b/Python/optimizer_bytecodes.c index 567caad22554ea..41fa6a64367731 100644 --- a/Python/optimizer_bytecodes.c +++ b/Python/optimizer_bytecodes.c @@ -104,18 +104,18 @@ dummy_func(void) { res = sym_new_null(ctx); } - op(_GUARD_TOS_INT, (tos -- tos)) { - if (sym_matches_type(tos, &PyLong_Type)) { + op(_GUARD_TOS_INT, (value -- value)) { + if (sym_matches_type(value, &PyLong_Type)) { REPLACE_OP(this_instr, _NOP, 0, 0); } - sym_set_type(tos, &PyLong_Type); + sym_set_type(value, &PyLong_Type); } - op(_GUARD_NOS_INT, (nos, unused -- nos, unused)) { - if (sym_matches_type(nos, &PyLong_Type)) { + op(_GUARD_NOS_INT, (left, unused -- left, unused)) { + if (sym_matches_type(left, &PyLong_Type)) { REPLACE_OP(this_instr, _NOP, 0, 0); } - sym_set_type(nos, &PyLong_Type); + sym_set_type(left, &PyLong_Type); } op(_GUARD_TYPE_VERSION, (type_version/2, owner -- owner)) { @@ -141,25 +141,25 @@ dummy_func(void) { } } - op(_GUARD_TOS_FLOAT, (tos -- tos)) { - if (sym_matches_type(tos, &PyFloat_Type)) { + op(_GUARD_TOS_FLOAT, (value -- value)) { + if (sym_matches_type(value, &PyFloat_Type)) { REPLACE_OP(this_instr, _NOP, 0, 0); } - sym_set_type(tos, &PyFloat_Type); + sym_set_type(value, &PyFloat_Type); } - op(_GUARD_NOS_FLOAT, (nos, unused -- nos, unused)) { - if (sym_matches_type(nos, &PyFloat_Type)) { + op(_GUARD_NOS_FLOAT, (left, unused -- left, unused)) { + if (sym_matches_type(left, &PyFloat_Type)) { REPLACE_OP(this_instr, _NOP, 0, 0); } - sym_set_type(nos, &PyFloat_Type); + sym_set_type(left, &PyFloat_Type); } - op(_BINARY_OP, (left, right -- res)) { - bool lhs_int = sym_matches_type(left, &PyLong_Type); - bool rhs_int = sym_matches_type(right, &PyLong_Type); - bool lhs_float = sym_matches_type(left, &PyFloat_Type); - bool rhs_float = sym_matches_type(right, &PyFloat_Type); + op(_BINARY_OP, (lhs, rhs -- res)) { + bool lhs_int = sym_matches_type(lhs, &PyLong_Type); + bool rhs_int = sym_matches_type(rhs, &PyLong_Type); + bool lhs_float = sym_matches_type(lhs, &PyFloat_Type); + bool rhs_float = sym_matches_type(rhs, &PyFloat_Type); if (!((lhs_int || lhs_float) && (rhs_int || rhs_float))) { // There's something other than an int or float involved: res = sym_new_unknown(ctx); @@ -185,11 +185,11 @@ dummy_func(void) { // Case C: res = sym_new_type(ctx, &PyFloat_Type); } - else if (!sym_is_const(ctx, right)) { + else if (!sym_is_const(ctx, rhs)) { // Case A or B... can't know without the sign of the RHS: res = sym_new_unknown(ctx); } - else if (_PyLong_IsNegative((PyLongObject *)sym_get_const(ctx, right))) { + else if (_PyLong_IsNegative((PyLongObject *)sym_get_const(ctx, rhs))) { // Case B: res = sym_new_type(ctx, &PyFloat_Type); } @@ -366,7 +366,7 @@ dummy_func(void) { ctx->done = true; } - op(_BINARY_OP_SUBSCR_STR_INT, (left, right -- res)) { + op(_BINARY_OP_SUBSCR_STR_INT, (str_st, sub_st -- res)) { res = sym_new_type(ctx, &PyUnicode_Type); } @@ -398,11 +398,11 @@ dummy_func(void) { } } - op(_TO_BOOL_BOOL, (value -- res)) { - int already_bool = optimize_to_bool(this_instr, ctx, value, &res); + op(_TO_BOOL_BOOL, (value -- value)) { + int already_bool = optimize_to_bool(this_instr, ctx, value, &value); if (!already_bool) { sym_set_type(value, &PyBool_Type); - res = sym_new_truthiness(ctx, value, true); + value = sym_new_truthiness(ctx, value, true); } } @@ -493,20 +493,20 @@ dummy_func(void) { res = sym_new_type(ctx, &PyBool_Type); } - op(_IS_OP, (left, right -- res)) { - res = sym_new_type(ctx, &PyBool_Type); + op(_IS_OP, (left, right -- b)) { + b = sym_new_type(ctx, &PyBool_Type); } - op(_CONTAINS_OP, (left, right -- res)) { - res = sym_new_type(ctx, &PyBool_Type); + op(_CONTAINS_OP, (left, right -- b)) { + b = sym_new_type(ctx, &PyBool_Type); } - op(_CONTAINS_OP_SET, (left, right -- res)) { - res = sym_new_type(ctx, &PyBool_Type); + op(_CONTAINS_OP_SET, (left, right -- b)) { + b = sym_new_type(ctx, &PyBool_Type); } - op(_CONTAINS_OP_DICT, (left, right -- res)) { - res = sym_new_type(ctx, &PyBool_Type); + op(_CONTAINS_OP_DICT, (left, right -- b)) { + b = sym_new_type(ctx, &PyBool_Type); } op(_LOAD_CONST, (-- value)) { @@ -707,10 +707,10 @@ dummy_func(void) { } } - op(_MAYBE_EXPAND_METHOD, (callable, self_or_null, args[oparg] -- func, maybe_self, args[oparg])) { + op(_MAYBE_EXPAND_METHOD, (callable, self_or_null, args[oparg] -- callable, self_or_null, args[oparg])) { (void)args; - func = sym_new_not_null(ctx); - maybe_self = sym_new_not_null(ctx); + callable = sym_new_not_null(ctx); + self_or_null = sym_new_not_null(ctx); } op(_PY_FRAME_GENERAL, (callable, self_or_null, args[oparg] -- new_frame: _Py_UOpsAbstractFrame *)) { @@ -730,14 +730,14 @@ dummy_func(void) { ctx->done = true; } - op(_CHECK_AND_ALLOCATE_OBJECT, (type_version/2, callable, null, args[oparg] -- self, init, args[oparg])) { + op(_CHECK_AND_ALLOCATE_OBJECT, (type_version/2, callable, self_or_null, args[oparg] -- callable, self_or_null, args[oparg])) { (void)type_version; (void)args; - self = sym_new_not_null(ctx); - init = sym_new_not_null(ctx); + callable = sym_new_not_null(ctx); + self_or_null = sym_new_not_null(ctx); } - op(_CREATE_INIT_FRAME, (self, init, args[oparg] -- init_frame: _Py_UOpsAbstractFrame *)) { + op(_CREATE_INIT_FRAME, (init, self, args[oparg] -- init_frame: _Py_UOpsAbstractFrame *)) { init_frame = NULL; ctx->done = true; } @@ -789,21 +789,23 @@ dummy_func(void) { } } - op(_YIELD_VALUE, (unused -- res)) { - res = sym_new_unknown(ctx); + op(_YIELD_VALUE, (unused -- value)) { + value = sym_new_unknown(ctx); } - op(_FOR_ITER_GEN_FRAME, ( -- )) { + op(_FOR_ITER_GEN_FRAME, (unused -- unused, gen_frame: _Py_UOpsAbstractFrame*)) { + gen_frame = NULL; /* We are about to hit the end of the trace */ ctx->done = true; } - op(_SEND_GEN_FRAME, ( -- )) { + op(_SEND_GEN_FRAME, (unused, unused -- unused, gen_frame: _Py_UOpsAbstractFrame *)) { + gen_frame = NULL; // We are about to hit the end of the trace: ctx->done = true; } - op(_CHECK_STACK_SPACE, ( --)) { + op(_CHECK_STACK_SPACE, (unused, unused, unused[oparg] -- unused, unused, unused[oparg])) { assert(corresponding_check_stack == NULL); corresponding_check_stack = this_instr; } @@ -848,14 +850,16 @@ dummy_func(void) { corresponding_check_stack = NULL; } - op(_UNPACK_SEQUENCE, (seq -- values[oparg])) { + op(_UNPACK_SEQUENCE, (seq -- values[oparg], top[0])) { + (void)top; /* This has to be done manually */ for (int i = 0; i < oparg; i++) { values[i] = sym_new_unknown(ctx); } } - op(_UNPACK_EX, (seq -- values[oparg & 0xFF], unused, unused[oparg >> 8])) { + op(_UNPACK_EX, (seq -- values[oparg & 0xFF], unused, unused[oparg >> 8], top[0])) { + (void)top; /* This has to be done manually */ int totalargs = (oparg & 0xFF) + (oparg >> 8) + 1; for (int i = 0; i < totalargs; i++) { @@ -904,27 +908,27 @@ dummy_func(void) { sym_set_const(flag, Py_False); } - op(_GUARD_IS_NONE_POP, (flag -- )) { - if (sym_is_const(ctx, flag)) { - PyObject *value = sym_get_const(ctx, flag); + op(_GUARD_IS_NONE_POP, (val -- )) { + if (sym_is_const(ctx, val)) { + PyObject *value = sym_get_const(ctx, val); assert(value != NULL); eliminate_pop_guard(this_instr, !Py_IsNone(value)); } - else if (sym_has_type(flag)) { - assert(!sym_matches_type(flag, &_PyNone_Type)); + else if (sym_has_type(val)) { + assert(!sym_matches_type(val, &_PyNone_Type)); eliminate_pop_guard(this_instr, true); } - sym_set_const(flag, Py_None); + sym_set_const(val, Py_None); } - op(_GUARD_IS_NOT_NONE_POP, (flag -- )) { - if (sym_is_const(ctx, flag)) { - PyObject *value = sym_get_const(ctx, flag); + op(_GUARD_IS_NOT_NONE_POP, (val -- )) { + if (sym_is_const(ctx, val)) { + PyObject *value = sym_get_const(ctx, val); assert(value != NULL); eliminate_pop_guard(this_instr, Py_IsNone(value)); } - else if (sym_has_type(flag)) { - assert(!sym_matches_type(flag, &_PyNone_Type)); + else if (sym_has_type(val)) { + assert(!sym_matches_type(val, &_PyNone_Type)); eliminate_pop_guard(this_instr, false); } } @@ -969,7 +973,7 @@ dummy_func(void) { list = sym_new_type(ctx, &PyList_Type); } - op(_BUILD_SLICE, (values[oparg] -- slice)) { + op(_BUILD_SLICE, (args[oparg] -- slice)) { slice = sym_new_type(ctx, &PySlice_Type); } @@ -977,7 +981,7 @@ dummy_func(void) { map = sym_new_type(ctx, &PyDict_Type); } - op(_BUILD_STRING, (values[oparg] -- str)) { + op(_BUILD_STRING, (pieces[oparg] -- str)) { str = sym_new_type(ctx, &PyUnicode_Type); } diff --git a/Python/optimizer_cases.c.h b/Python/optimizer_cases.c.h index 622c0b5eb65dd0..ae2b34ab5cfcf5 100644 --- a/Python/optimizer_cases.c.h +++ b/Python/optimizer_cases.c.h @@ -171,14 +171,13 @@ case _TO_BOOL_BOOL: { JitOptSymbol *value; - JitOptSymbol *res; value = stack_pointer[-1]; - int already_bool = optimize_to_bool(this_instr, ctx, value, &res); + int already_bool = optimize_to_bool(this_instr, ctx, value, &value); if (!already_bool) { sym_set_type(value, &PyBool_Type); - res = sym_new_truthiness(ctx, value, true); + value = sym_new_truthiness(ctx, value, true); } - stack_pointer[-1] = res; + stack_pointer[-1] = value; break; } @@ -292,22 +291,22 @@ } case _GUARD_NOS_INT: { - JitOptSymbol *nos; - nos = stack_pointer[-2]; - if (sym_matches_type(nos, &PyLong_Type)) { + JitOptSymbol *left; + left = stack_pointer[-2]; + if (sym_matches_type(left, &PyLong_Type)) { REPLACE_OP(this_instr, _NOP, 0, 0); } - sym_set_type(nos, &PyLong_Type); + sym_set_type(left, &PyLong_Type); break; } case _GUARD_TOS_INT: { - JitOptSymbol *tos; - tos = stack_pointer[-1]; - if (sym_matches_type(tos, &PyLong_Type)) { + JitOptSymbol *value; + value = stack_pointer[-1]; + if (sym_matches_type(value, &PyLong_Type)) { REPLACE_OP(this_instr, _NOP, 0, 0); } - sym_set_type(tos, &PyLong_Type); + sym_set_type(value, &PyLong_Type); break; } @@ -396,22 +395,22 @@ } case _GUARD_NOS_FLOAT: { - JitOptSymbol *nos; - nos = stack_pointer[-2]; - if (sym_matches_type(nos, &PyFloat_Type)) { + JitOptSymbol *left; + left = stack_pointer[-2]; + if (sym_matches_type(left, &PyFloat_Type)) { REPLACE_OP(this_instr, _NOP, 0, 0); } - sym_set_type(nos, &PyFloat_Type); + sym_set_type(left, &PyFloat_Type); break; } case _GUARD_TOS_FLOAT: { - JitOptSymbol *tos; - tos = stack_pointer[-1]; - if (sym_matches_type(tos, &PyFloat_Type)) { + JitOptSymbol *value; + value = stack_pointer[-1]; + if (sym_matches_type(value, &PyFloat_Type)) { REPLACE_OP(this_instr, _NOP, 0, 0); } - sym_set_type(tos, &PyFloat_Type); + sym_set_type(value, &PyFloat_Type); break; } @@ -811,14 +810,17 @@ /* _SEND is not a viable micro-op for tier 2 */ case _SEND_GEN_FRAME: { + _Py_UOpsAbstractFrame *gen_frame; + gen_frame = NULL; ctx->done = true; + stack_pointer[-1] = (JitOptSymbol *)gen_frame; break; } case _YIELD_VALUE: { - JitOptSymbol *res; - res = sym_new_unknown(ctx); - stack_pointer[-1] = res; + JitOptSymbol *value; + value = sym_new_unknown(ctx); + stack_pointer[-1] = value; break; } @@ -858,7 +860,10 @@ case _UNPACK_SEQUENCE: { JitOptSymbol **values; + JitOptSymbol **top; values = &stack_pointer[-1]; + top = &stack_pointer[-1 + oparg]; + (void)top; for (int i = 0; i < oparg; i++) { values[i] = sym_new_unknown(ctx); } @@ -907,7 +912,10 @@ case _UNPACK_EX: { JitOptSymbol **values; + JitOptSymbol **top; values = &stack_pointer[-1]; + top = &stack_pointer[(oparg & 0xFF) + (oparg >> 8)]; + (void)top; int totalargs = (oparg & 0xFF) + (oparg >> 8) + 1; for (int i = 0; i < totalargs; i++) { values[i] = sym_new_unknown(ctx); @@ -1376,18 +1384,18 @@ } case _IS_OP: { - JitOptSymbol *res; - res = sym_new_type(ctx, &PyBool_Type); - stack_pointer[-2] = res; + JitOptSymbol *b; + b = sym_new_type(ctx, &PyBool_Type); + stack_pointer[-2] = b; stack_pointer += -1; assert(WITHIN_STACK_BOUNDS()); break; } case _CONTAINS_OP: { - JitOptSymbol *res; - res = sym_new_type(ctx, &PyBool_Type); - stack_pointer[-2] = res; + JitOptSymbol *b; + b = sym_new_type(ctx, &PyBool_Type); + stack_pointer[-2] = b; stack_pointer += -1; assert(WITHIN_STACK_BOUNDS()); break; @@ -1405,18 +1413,18 @@ } case _CONTAINS_OP_SET: { - JitOptSymbol *res; - res = sym_new_type(ctx, &PyBool_Type); - stack_pointer[-2] = res; + JitOptSymbol *b; + b = sym_new_type(ctx, &PyBool_Type); + stack_pointer[-2] = b; stack_pointer += -1; assert(WITHIN_STACK_BOUNDS()); break; } case _CONTAINS_OP_DICT: { - JitOptSymbol *res; - res = sym_new_type(ctx, &PyBool_Type); - stack_pointer[-2] = res; + JitOptSymbol *b; + b = sym_new_type(ctx, &PyBool_Type); + stack_pointer[-2] = b; stack_pointer += -1; assert(WITHIN_STACK_BOUNDS()); break; @@ -1600,7 +1608,12 @@ } case _FOR_ITER_GEN_FRAME: { + _Py_UOpsAbstractFrame *gen_frame; + gen_frame = NULL; ctx->done = true; + stack_pointer[0] = (JitOptSymbol *)gen_frame; + stack_pointer += 1; + assert(WITHIN_STACK_BOUNDS()); break; } @@ -1721,15 +1734,16 @@ case _MAYBE_EXPAND_METHOD: { JitOptSymbol **args; - JitOptSymbol *func; - JitOptSymbol *maybe_self; - args = &stack_pointer[-oparg]; + JitOptSymbol *self_or_null; + JitOptSymbol *callable; args = &stack_pointer[-oparg]; + self_or_null = stack_pointer[-1 - oparg]; + callable = stack_pointer[-2 - oparg]; (void)args; - func = sym_new_not_null(ctx); - maybe_self = sym_new_not_null(ctx); - stack_pointer[-2 - oparg] = func; - stack_pointer[-1 - oparg] = maybe_self; + callable = sym_new_not_null(ctx); + self_or_null = sym_new_not_null(ctx); + stack_pointer[-2 - oparg] = callable; + stack_pointer[-1 - oparg] = self_or_null; break; } @@ -1997,17 +2011,18 @@ case _CHECK_AND_ALLOCATE_OBJECT: { JitOptSymbol **args; - JitOptSymbol *self; - JitOptSymbol *init; - args = &stack_pointer[-oparg]; + JitOptSymbol *self_or_null; + JitOptSymbol *callable; args = &stack_pointer[-oparg]; + self_or_null = stack_pointer[-1 - oparg]; + callable = stack_pointer[-2 - oparg]; uint32_t type_version = (uint32_t)this_instr->operand0; (void)type_version; (void)args; - self = sym_new_not_null(ctx); - init = sym_new_not_null(ctx); - stack_pointer[-2 - oparg] = self; - stack_pointer[-1 - oparg] = init; + callable = sym_new_not_null(ctx); + self_or_null = sym_new_not_null(ctx); + stack_pointer[-2 - oparg] = callable; + stack_pointer[-1 - oparg] = self_or_null; break; } @@ -2255,15 +2270,15 @@ } case _BINARY_OP: { - JitOptSymbol *right; - JitOptSymbol *left; - JitOptSymbol *res; - right = stack_pointer[-1]; - left = stack_pointer[-2]; - bool lhs_int = sym_matches_type(left, &PyLong_Type); - bool rhs_int = sym_matches_type(right, &PyLong_Type); - bool lhs_float = sym_matches_type(left, &PyFloat_Type); - bool rhs_float = sym_matches_type(right, &PyFloat_Type); + JitOptSymbol *rhs; + JitOptSymbol *lhs; + JitOptSymbol *res; + rhs = stack_pointer[-1]; + lhs = stack_pointer[-2]; + bool lhs_int = sym_matches_type(lhs, &PyLong_Type); + bool rhs_int = sym_matches_type(rhs, &PyLong_Type); + bool lhs_float = sym_matches_type(lhs, &PyFloat_Type); + bool rhs_float = sym_matches_type(rhs, &PyFloat_Type); if (!((lhs_int || lhs_float) && (rhs_int || rhs_float))) { res = sym_new_unknown(ctx); } @@ -2274,10 +2289,10 @@ else if (lhs_float) { res = sym_new_type(ctx, &PyFloat_Type); } - else if (!sym_is_const(ctx, right)) { + else if (!sym_is_const(ctx, rhs)) { res = sym_new_unknown(ctx); } - else if (_PyLong_IsNegative((PyLongObject *)sym_get_const(ctx, right))) { + else if (_PyLong_IsNegative((PyLongObject *)sym_get_const(ctx, rhs))) { res = sym_new_type(ctx, &PyFloat_Type); } else { @@ -2360,33 +2375,33 @@ } case _GUARD_IS_NONE_POP: { - JitOptSymbol *flag; - flag = stack_pointer[-1]; - if (sym_is_const(ctx, flag)) { - PyObject *value = sym_get_const(ctx, flag); + JitOptSymbol *val; + val = stack_pointer[-1]; + if (sym_is_const(ctx, val)) { + PyObject *value = sym_get_const(ctx, val); assert(value != NULL); eliminate_pop_guard(this_instr, !Py_IsNone(value)); } - else if (sym_has_type(flag)) { - assert(!sym_matches_type(flag, &_PyNone_Type)); + else if (sym_has_type(val)) { + assert(!sym_matches_type(val, &_PyNone_Type)); eliminate_pop_guard(this_instr, true); } - sym_set_const(flag, Py_None); + sym_set_const(val, Py_None); stack_pointer += -1; assert(WITHIN_STACK_BOUNDS()); break; } case _GUARD_IS_NOT_NONE_POP: { - JitOptSymbol *flag; - flag = stack_pointer[-1]; - if (sym_is_const(ctx, flag)) { - PyObject *value = sym_get_const(ctx, flag); + JitOptSymbol *val; + val = stack_pointer[-1]; + if (sym_is_const(ctx, val)) { + PyObject *value = sym_get_const(ctx, val); assert(value != NULL); eliminate_pop_guard(this_instr, Py_IsNone(value)); } - else if (sym_has_type(flag)) { - assert(!sym_matches_type(flag, &_PyNone_Type)); + else if (sym_has_type(val)) { + assert(!sym_matches_type(val, &_PyNone_Type)); eliminate_pop_guard(this_instr, false); } stack_pointer += -1; diff --git a/Tools/cases_generator/optimizer_generator.py b/Tools/cases_generator/optimizer_generator.py index 7a32275347e896..76fd074812097e 100644 --- a/Tools/cases_generator/optimizer_generator.py +++ b/Tools/cases_generator/optimizer_generator.py @@ -30,16 +30,36 @@ def validate_uop(override: Uop, uop: Uop) -> None: - # To do - pass + for stack_effect in ('inputs', 'outputs'): + orig_effects = getattr(uop.stack, stack_effect) + new_effects = getattr(override.stack, stack_effect) + + if len(orig_effects) != len(new_effects): + msg = ( + f"{uop.name}: Must have the same number of {stack_effect} " + "in bytecodes.c and optimizer_bytecodes.c " + f"({len(orig_effects)} != {len(new_effects)})" + ) + raise analysis_error(msg, override.body.open) + + for orig, new in zip(orig_effects, new_effects, strict=True): + if orig.name == 'unused' or new.name == 'unused': + continue + if orig.name != new.name: + msg = ( + f"{uop.name}: {stack_effect.capitalize()} must have " + "equal names in bytecodes.c and optimizer_bytecodes.c " + f"({orig.name} != {new.name})" + ) + raise analysis_error(msg, override.body.open) def type_name(var: StackItem) -> str: if var.is_array(): - return f"JitOptSymbol **" + return "JitOptSymbol **" if var.type: return var.type - return f"JitOptSymbol *" + return "JitOptSymbol *" def declare_variables(uop: Uop, out: CWriter, skip_inputs: bool) -> None: From 8e5e19bccc837db9c191ff6013c2e1c504232cc2 Mon Sep 17 00:00:00 2001 From: Tomas Roun Date: Sat, 3 May 2025 11:14:34 +0200 Subject: [PATCH 2/2] Also validate sizes --- Lib/test/test_generated_cases.py | 46 +++++++++++++++++++- Python/optimizer_bytecodes.c | 2 +- Tools/cases_generator/optimizer_generator.py | 22 ++++++++-- 3 files changed, 65 insertions(+), 5 deletions(-) diff --git a/Lib/test/test_generated_cases.py b/Lib/test/test_generated_cases.py index ae3fa17872f5e6..a71ddc01d1c045 100644 --- a/Lib/test/test_generated_cases.py +++ b/Lib/test/test_generated_cases.py @@ -2168,7 +2168,7 @@ def test_validate_uop_unused_input(self): """ self.run_cases_test(input, input2, output) - def test_validate_uop_unused_override(self): + def test_validate_uop_unused_output(self): input = """ op(OP, ( -- unused)) { } @@ -2208,6 +2208,50 @@ def test_validate_uop_unused_override(self): """ self.run_cases_test(input, input2, output) + def test_validate_uop_input_size_mismatch(self): + input = """ + op(OP, (arg1[2] -- )) { + } + """ + input2 = """ + op(OP, (arg1[4] -- )) { + } + """ + output = """ + """ + with self.assertRaisesRegex(SyntaxError, + "Inputs must have equal sizes"): + self.run_cases_test(input, input2, output) + + def test_validate_uop_output_size_mismatch(self): + input = """ + op(OP, ( -- out[2])) { + } + """ + input2 = """ + op(OP, ( -- out[4])) { + } + """ + output = """ + """ + with self.assertRaisesRegex(SyntaxError, + "Outputs must have equal sizes"): + self.run_cases_test(input, input2, output) + + def test_validate_uop_unused_size_mismatch(self): + input = """ + op(OP, (foo[2] -- )) { + } + """ + input2 = """ + op(OP, (unused[4] -- )) { + } + """ + output = """ + """ + with self.assertRaisesRegex(SyntaxError, + "Inputs must have equal sizes"): + self.run_cases_test(input, input2, output) if __name__ == "__main__": unittest.main() diff --git a/Python/optimizer_bytecodes.c b/Python/optimizer_bytecodes.c index 41fa6a64367731..98cf26bac4e0db 100644 --- a/Python/optimizer_bytecodes.c +++ b/Python/optimizer_bytecodes.c @@ -1088,7 +1088,7 @@ dummy_func(void) { sym_set_const(callable, (PyObject *)&PyUnicode_Type); } - op(_CALL_LEN, (callable[1], self_or_null[1], args[oparg] -- res)) { + op(_CALL_LEN, (callable, self_or_null, args[oparg] -- res)) { res = sym_new_type(ctx, &PyLong_Type); } diff --git a/Tools/cases_generator/optimizer_generator.py b/Tools/cases_generator/optimizer_generator.py index 76fd074812097e..fda022a44e59cc 100644 --- a/Tools/cases_generator/optimizer_generator.py +++ b/Tools/cases_generator/optimizer_generator.py @@ -30,6 +30,16 @@ def validate_uop(override: Uop, uop: Uop) -> None: + """ + Check that the overridden uop (defined in 'optimizer_bytecodes.c') + has the same stack effects as the original uop (defined in 'bytecodes.c'). + + Ensure that: + - The number of inputs and outputs is the same. + - The names of the inputs and outputs are the same + (except for 'unused' which is ignored). + - The sizes of the inputs and outputs are the same. + """ for stack_effect in ('inputs', 'outputs'): orig_effects = getattr(uop.stack, stack_effect) new_effects = getattr(override.stack, stack_effect) @@ -43,9 +53,7 @@ def validate_uop(override: Uop, uop: Uop) -> None: raise analysis_error(msg, override.body.open) for orig, new in zip(orig_effects, new_effects, strict=True): - if orig.name == 'unused' or new.name == 'unused': - continue - if orig.name != new.name: + if orig.name != new.name and orig.name != "unused" and new.name != "unused": msg = ( f"{uop.name}: {stack_effect.capitalize()} must have " "equal names in bytecodes.c and optimizer_bytecodes.c " @@ -53,6 +61,14 @@ def validate_uop(override: Uop, uop: Uop) -> None: ) raise analysis_error(msg, override.body.open) + if orig.size != new.size: + msg = ( + f"{uop.name}: {stack_effect.capitalize()} must have " + "equal sizes in bytecodes.c and optimizer_bytecodes.c " + f"({orig.size!r} != {new.size!r})" + ) + raise analysis_error(msg, override.body.open) + def type_name(var: StackItem) -> str: if var.is_array():