From 7249cb7e968b186857029213490e077d155e7c8a Mon Sep 17 00:00:00 2001 From: Julia Date: Wed, 29 Mar 2023 01:47:57 +0800 Subject: [PATCH 1/8] Added EXTENDED_ARG to type prop --- Python/tier2_typepropagator.c.h | 2 -- Tools/cases_generator/generate_cases.py | 1 - 2 files changed, 3 deletions(-) diff --git a/Python/tier2_typepropagator.c.h b/Python/tier2_typepropagator.c.h index 96b514cd096b1e..357cc74c4e46c2 100644 --- a/Python/tier2_typepropagator.c.h +++ b/Python/tier2_typepropagator.c.h @@ -1054,8 +1054,6 @@ } TARGET(EXTENDED_ARG) { - fprintf(stderr, "Type propagation across `EXTENDED_ARG` shouldn't be handled statically!\n"); - Py_UNREACHABLE(); break; } diff --git a/Tools/cases_generator/generate_cases.py b/Tools/cases_generator/generate_cases.py index f299d234993e55..1ac6962e00cc20 100644 --- a/Tools/cases_generator/generate_cases.py +++ b/Tools/cases_generator/generate_cases.py @@ -70,7 +70,6 @@ "MATCH_MAPPING", "MATCH_SEQUENCE", "MATCH_KEYS", - "EXTENDED_ARG", "WITH_EXCEPT_START", # Type propagation across these instructions are forbidden # due to conditional effects that can't be determined statically From 6f43251755f0b8f57e9dac8f0da78cfee657eb77 Mon Sep 17 00:00:00 2001 From: Julia Date: Wed, 29 Mar 2023 01:56:53 +0800 Subject: [PATCH 2/8] Revert "Added EXTENDED_ARG to type prop" This reverts commit 7249cb7e968b186857029213490e077d155e7c8a. --- Python/tier2_typepropagator.c.h | 2 ++ Tools/cases_generator/generate_cases.py | 1 + 2 files changed, 3 insertions(+) diff --git a/Python/tier2_typepropagator.c.h b/Python/tier2_typepropagator.c.h index 357cc74c4e46c2..96b514cd096b1e 100644 --- a/Python/tier2_typepropagator.c.h +++ b/Python/tier2_typepropagator.c.h @@ -1054,6 +1054,8 @@ } TARGET(EXTENDED_ARG) { + fprintf(stderr, "Type propagation across `EXTENDED_ARG` shouldn't be handled statically!\n"); + Py_UNREACHABLE(); break; } diff --git a/Tools/cases_generator/generate_cases.py b/Tools/cases_generator/generate_cases.py index 1ac6962e00cc20..f299d234993e55 100644 --- a/Tools/cases_generator/generate_cases.py +++ b/Tools/cases_generator/generate_cases.py @@ -70,6 +70,7 @@ "MATCH_MAPPING", "MATCH_SEQUENCE", "MATCH_KEYS", + "EXTENDED_ARG", "WITH_EXCEPT_START", # Type propagation across these instructions are forbidden # due to conditional effects that can't be determined statically From b8416906b1946f5aa08964a49b24f30e0bf42932 Mon Sep 17 00:00:00 2001 From: Julia Date: Wed, 29 Mar 2023 23:46:33 +0800 Subject: [PATCH 3/8] Fix: Buggy type propagation across SWP --- Python/tier2.c | 79 ++++++++++++++++++ Python/tier2_typepropagator.c.h | 104 ------------------------ Tools/cases_generator/generate_cases.py | 22 ++--- 3 files changed, 84 insertions(+), 121 deletions(-) diff --git a/Python/tier2.c b/Python/tier2.c index 2bd8d756539259..92e020499e78b0 100644 --- a/Python/tier2.c +++ b/Python/tier2.c @@ -376,6 +376,74 @@ __type_propagate_TYPE_OVERWRITE( } } +// src and dst are assumed to already be within the type context +static void +__type_propagate_TYPE_SWAP( + _PyTier2TypeContext *type_context, + _Py_TYPENODE_t *src, _Py_TYPENODE_t *dst) +{ + // Check if they are the same tree + _Py_TYPENODE_t *srcrootref = src; + _Py_TYPENODE_t *dstrootref = dst; + uintptr_t dsttag = _Py_TYPENODE_GET_TAG(*dst); + uintptr_t srctag = _Py_TYPENODE_GET_TAG(*src); + switch (dsttag) { + case TYPE_REF: dstrootref = __typenode_get_rootptr(*dst); + case TYPE_ROOT: + switch (srctag) { + case TYPE_REF: srcrootref = __typenode_get_rootptr(*src); + case TYPE_ROOT: + if (srcrootref == dstrootref) { + // Same tree, no point swapping + return; + } + break; + default: + Py_UNREACHABLE(); + } + break; + default: + Py_UNREACHABLE(); + } + + // src and dst are different tree, + // Make all children of src be children of dst and vice versa + + _Py_TYPENODE_t src_child_test = _Py_TYPENODE_MAKE_REF( + _Py_TYPENODE_CLEAR_TAG((_Py_TYPENODE_t)src)); + _Py_TYPENODE_t dst_child_test = _Py_TYPENODE_MAKE_REF( + _Py_TYPENODE_CLEAR_TAG((_Py_TYPENODE_t)dst)); + + // Search locals for children + int nlocals = type_context->type_locals_len; + for (int i = 0; i < nlocals; i++) { + _Py_TYPENODE_t *node_ptr = &(type_context->type_locals[i]); + if (*node_ptr == src_child_test) { + *node_ptr = dst; + } + else if (*node_ptr == dst_child_test) { + *node_ptr = src; + } + } + + // Search stack for children + int nstack = type_context->type_stack_len; + for (int i = 0; i < nstack; i++) { + _Py_TYPENODE_t *node_ptr = &(type_context->type_stack[i]); + if (*node_ptr == src_child_test) { + *node_ptr = dst; + } + else if (*node_ptr == dst_child_test) { + *node_ptr = src; + } + } + + // Finally, actually swap the nodes + *src ^= *dst; + *dst ^= *src; + *src ^= *dst; +} + static void __type_stack_shrink(_Py_TYPENODE_t **type_stackptr, int idx) { @@ -489,6 +557,7 @@ type_propagate( #define TYPE_SET(src, dst, flag) __type_propagate_TYPE_SET((src), (dst), (flag)) #define TYPE_OVERWRITE(src, dst, flag) __type_propagate_TYPE_OVERWRITE(type_context, (src), (dst), (flag)) +#define TYPE_SWAP(src, dst) __type_propagate_TYPE_SWAP(type_context, (src), (dst)) #define STACK_GROW(idx) *type_stackptr += (idx) @@ -504,8 +573,18 @@ type_propagate( switch (opcode) { #include "tier2_typepropagator.c.h" + TARGET(SWAP) { + _Py_TYPENODE_t *top = TYPESTACK_PEEK(1); + _Py_TYPENODE_t * bottom = TYPESTACK_PEEK(2 + (oparg - 2)); + TYPE_SWAP(top, bottom); + break; + } default: +#ifdef Py_DEBUG + fprintf(stderr, "Unsupported opcode in type propagator: %s : %d\n", _PyOpcode_OpName[opcode], oparg); +#else fprintf(stderr, "Unsupported opcode in type propagator: %d\n", opcode); +#endif Py_UNREACHABLE(); } diff --git a/Python/tier2_typepropagator.c.h b/Python/tier2_typepropagator.c.h index 1f2cfddd0378b2..9c645df9007b0b 100644 --- a/Python/tier2_typepropagator.c.h +++ b/Python/tier2_typepropagator.c.h @@ -284,12 +284,6 @@ break; } - TARGET(RAISE_VARARGS) { - fprintf(stderr, "Type propagation across `RAISE_VARARGS` shouldn't be handled statically!\n"); - Py_UNREACHABLE(); - break; - } - TARGET(INTERPRETER_EXIT) { STACK_SHRINK(1); break; @@ -320,36 +314,6 @@ break; } - TARGET(SEND) { - fprintf(stderr, "Type propagation across `SEND` shouldn't be handled statically!\n"); - Py_UNREACHABLE(); - break; - } - - TARGET(SEND_GEN) { - fprintf(stderr, "Type propagation across `SEND_GEN` shouldn't be handled statically!\n"); - Py_UNREACHABLE(); - break; - } - - TARGET(YIELD_VALUE) { - fprintf(stderr, "Type propagation across `YIELD_VALUE` shouldn't be handled statically!\n"); - Py_UNREACHABLE(); - break; - } - - TARGET(POP_EXCEPT) { - fprintf(stderr, "Type propagation across `POP_EXCEPT` shouldn't be handled statically!\n"); - Py_UNREACHABLE(); - break; - } - - TARGET(RERAISE) { - fprintf(stderr, "Type propagation across `RERAISE` shouldn't be handled statically!\n"); - Py_UNREACHABLE(); - break; - } - TARGET(END_ASYNC_FOR) { STACK_SHRINK(2); break; @@ -464,18 +428,6 @@ break; } - TARGET(DELETE_FAST) { - fprintf(stderr, "Type propagation across `DELETE_FAST` shouldn't be handled statically!\n"); - Py_UNREACHABLE(); - break; - } - - TARGET(MAKE_CELL) { - fprintf(stderr, "Type propagation across `MAKE_CELL` shouldn't be handled statically!\n"); - Py_UNREACHABLE(); - break; - } - TARGET(DELETE_DEREF) { break; } @@ -486,12 +438,6 @@ break; } - TARGET(LOAD_DEREF) { - fprintf(stderr, "Type propagation across `LOAD_DEREF` shouldn't be handled statically!\n"); - Py_UNREACHABLE(); - break; - } - TARGET(STORE_DEREF) { STACK_SHRINK(1); break; @@ -766,24 +712,6 @@ break; } - TARGET(MATCH_MAPPING) { - fprintf(stderr, "Type propagation across `MATCH_MAPPING` shouldn't be handled statically!\n"); - Py_UNREACHABLE(); - break; - } - - TARGET(MATCH_SEQUENCE) { - fprintf(stderr, "Type propagation across `MATCH_SEQUENCE` shouldn't be handled statically!\n"); - Py_UNREACHABLE(); - break; - } - - TARGET(MATCH_KEYS) { - fprintf(stderr, "Type propagation across `MATCH_KEYS` shouldn't be handled statically!\n"); - Py_UNREACHABLE(); - break; - } - TARGET(GET_ITER) { TYPE_OVERWRITE((_Py_TYPENODE_t *)_Py_TYPENODE_NULLROOT, TYPESTACK_PEEK(1), true); break; @@ -794,12 +722,6 @@ break; } - TARGET(FOR_ITER) { - fprintf(stderr, "Type propagation across `FOR_ITER` shouldn't be handled statically!\n"); - Py_UNREACHABLE(); - break; - } - TARGET(BB_TEST_ITER) { STACK_GROW(1); TYPE_OVERWRITE((_Py_TYPENODE_t *)_Py_TYPENODE_NULLROOT, TYPESTACK_PEEK(1), true); @@ -843,18 +765,6 @@ break; } - TARGET(WITH_EXCEPT_START) { - fprintf(stderr, "Type propagation across `WITH_EXCEPT_START` shouldn't be handled statically!\n"); - Py_UNREACHABLE(); - break; - } - - TARGET(PUSH_EXC_INFO) { - fprintf(stderr, "Type propagation across `PUSH_EXC_INFO` shouldn't be handled statically!\n"); - Py_UNREACHABLE(); - break; - } - TARGET(LOAD_ATTR_METHOD_WITH_VALUES) { STACK_GROW(((oparg & 1) ? 1 : 0)); TYPE_OVERWRITE((_Py_TYPENODE_t *)_Py_TYPENODE_NULLROOT, TYPESTACK_PEEK(1), true); @@ -1045,20 +955,6 @@ break; } - TARGET(SWAP) { - _Py_TYPENODE_t *top = TYPESTACK_PEEK(1); - _Py_TYPENODE_t *bottom = TYPESTACK_PEEK(2 + (oparg-2)); - TYPE_OVERWRITE(bottom, TYPESTACK_PEEK(1), false); - TYPE_OVERWRITE(top, TYPESTACK_PEEK(2 + (oparg-2)), false); - break; - } - - TARGET(EXTENDED_ARG) { - fprintf(stderr, "Type propagation across `EXTENDED_ARG` shouldn't be handled statically!\n"); - Py_UNREACHABLE(); - break; - } - TARGET(CACHE) { break; } diff --git a/Tools/cases_generator/generate_cases.py b/Tools/cases_generator/generate_cases.py index 35207912ab2abb..1255ba463f5a8b 100644 --- a/Tools/cases_generator/generate_cases.py +++ b/Tools/cases_generator/generate_cases.py @@ -53,9 +53,9 @@ TYPE_PROPAGATOR_FORBIDDEN = [ # Type propagator shouldn't see these - "JUMP_IF_FALSE_OR_POP", - "JUMP_IF_TRUE_OR_POP", "FOR_ITER", + "SWAP", + # Not supported "SEND", "SEND_GEN", "YIELD_VALUE", @@ -71,12 +71,6 @@ "MATCH_KEYS", "EXTENDED_ARG", "WITH_EXCEPT_START", - # Type propagation across these instructions are forbidden - # due to conditional effects that can't be determined statically - # The handling of type propagation across these opcodes are handled elsewhere - # within tier2. - "BB_TEST_IF_FALSE_OR_POP", - "BB_TEST_IF_TRUE_OR_POP" # Type propagator handles this in BB_BRANCH ] arg_parser = argparse.ArgumentParser( @@ -344,10 +338,7 @@ def __init__(self, inst: parser.InstDef): def write_typeprop(self, out: Formatter) -> None: """Write one instruction's type propagation rules""" - if self.name in TYPE_PROPAGATOR_FORBIDDEN: - out.emit(f'fprintf(stderr, "Type propagation across `{self.name}` shouldn\'t be handled statically!\\n");') - out.emit("Py_UNREACHABLE();") - return + # TODO: Detect loops like in SWAP need_to_declare = [] # Stack input is used in local effect @@ -644,11 +635,6 @@ def write_body(self, out: Formatter, dedent: int, cache_adjust: int = 0) -> None def write_typeprop(self, out: Formatter) -> None: """Write one instruction's type propagation rules""" - if self.name in TYPE_PROPAGATOR_FORBIDDEN: - out.emit(f'fprintf(stderr, "Type propagation across `{self.name}` shouldn\'t be handled statically!\\n");') - out.emit("Py_UNREACHABLE();") - return - need_to_declare = [] # Stack input is used in local effect if self.local_effects and \ @@ -1328,6 +1314,8 @@ def write_typepropagator(self) -> None: self.out = Formatter(f, 8) for thing in self.everything: + if thing.name in TYPE_PROPAGATOR_FORBIDDEN: + continue match thing: case parser.InstDef(kind=kind, name=name): match kind: From ae904f195552857836394917b0e960f6c932772a Mon Sep 17 00:00:00 2001 From: Julia Date: Thu, 30 Mar 2023 00:05:23 +0800 Subject: [PATCH 4/8] Fix: Forgot to tag pointers in TYPE_SET --- Python/tier2.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Python/tier2.c b/Python/tier2.c index ecd27a4da85d60..07adcdfdca1b12 100644 --- a/Python/tier2.c +++ b/Python/tier2.c @@ -419,10 +419,10 @@ __type_propagate_TYPE_SWAP( for (int i = 0; i < nlocals; i++) { _Py_TYPENODE_t *node_ptr = &(type_context->type_locals[i]); if (*node_ptr == src_child_test) { - *node_ptr = dst; + *node_ptr = dst_child_test; } else if (*node_ptr == dst_child_test) { - *node_ptr = src; + *node_ptr = src_child_test; } } @@ -431,10 +431,10 @@ __type_propagate_TYPE_SWAP( for (int i = 0; i < nstack; i++) { _Py_TYPENODE_t *node_ptr = &(type_context->type_stack[i]); if (*node_ptr == src_child_test) { - *node_ptr = dst; + *node_ptr = dst_child_test; } else if (*node_ptr == dst_child_test) { - *node_ptr = src; + *node_ptr = src_child_test; } } From 5482f0faa522bc0794bfd5445ea718e4acdcd81a Mon Sep 17 00:00:00 2001 From: Julia Date: Thu, 30 Mar 2023 17:12:37 +0800 Subject: [PATCH 5/8] Fix: BB_TEST_ITER wrong type propagation --- Include/internal/pycore_code.h | 8 ++++---- Python/bytecodes.c | 2 +- Python/generated_cases.c.h | 2 +- Python/tier2.c | 6 ++++-- 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/Include/internal/pycore_code.h b/Include/internal/pycore_code.h index 863e6b383a4937..3bee8696ed5efb 100644 --- a/Include/internal/pycore_code.h +++ b/Include/internal/pycore_code.h @@ -261,15 +261,15 @@ extern int _PyStaticCode_Init(PyCodeObject *co); // gen_bb_is_successor: // true = successor // false = alternate -// gen_bb_requires_pop: +// gen_bb_requires_pop (maximum 7): // For tier2 type propagation, handling of jump instructions with // runtime-dependent stack effect. // This flag is used to determine if the type context of a new bb // requires a stack element to be popped. #define BB_TEST(gen_bb_is_successor, gen_bb_requires_pop) \ - (((gen_bb_is_successor) << 1) | (gen_bb_requires_pop)) -#define BB_TEST_IS_SUCCESSOR(bb_test) ((bb_test) >> 1) -#define BB_TEST_IS_REQUIRES_POP(bb_test) ((bb_test) & 1) + (((gen_bb_is_successor) << 4) | (gen_bb_requires_pop)) +#define BB_TEST_IS_SUCCESSOR(bb_test) ((bb_test) >> 4) +#define BB_TEST_GET_N_REQUIRES_POP(bb_test) ((bb_test) & 0b1111) extern _Py_CODEUNIT *_PyCode_Tier2Warmup(struct _PyInterpreterFrame *, _Py_CODEUNIT *); diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 93ab5df1b3f171..34ae3cbddd681c 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -2190,7 +2190,7 @@ dummy_func( /* iterator ended normally */ Py_DECREF(iter); STACK_SHRINK(1); - bb_test = BB_TEST(0, 1); + bb_test = BB_TEST(0, 2); JUMPBY(INLINE_CACHE_ENTRIES_FOR_ITER); DISPATCH(); } diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index f3eca0b9832732..0ef2014548b926 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -2824,7 +2824,7 @@ /* iterator ended normally */ Py_DECREF(iter); STACK_SHRINK(1); - bb_test = BB_TEST(0, 1); + bb_test = BB_TEST(0, 2); JUMPBY(INLINE_CACHE_ENTRIES_FOR_ITER); DISPATCH(); } diff --git a/Python/tier2.c b/Python/tier2.c index cc4974e047e4e8..2bb36794c131d3 100644 --- a/Python/tier2.c +++ b/Python/tier2.c @@ -1979,8 +1979,10 @@ _PyTier2_GenerateNextBBMetaWithTypeContext( // DEOPTIMIZE TO TIER 1? return NULL; } - if (BB_TEST_IS_REQUIRES_POP(bb_flag)) { - __type_stack_shrink(&(type_context_copy->type_stack_ptr), 1); + + int n_required_pop = BB_TEST_GET_N_REQUIRES_POP(bb_flag); + if (n_required_pop) { + __type_stack_shrink(&(type_context_copy->type_stack_ptr), n_required_pop); } // For type branches, they directly precede the bb branch instruction _Py_CODEUNIT *prev_type_guard = BB_IS_TYPE_BRANCH(bb_id_tagged) From 4a0683a8c12b7305bf48344482ee883d83cb2941 Mon Sep 17 00:00:00 2001 From: Julia Date: Fri, 31 Mar 2023 11:29:13 +0800 Subject: [PATCH 6/8] Fix: Bug in typeprop handling UNPACK_*, and bug in COPY_NO_INCREF --- Python/bytecodes.c | 5 +- Python/generated_cases.c.h | 5 +- Python/opcode_metadata.h | 2 +- Python/tier2_typepropagator.c.h | 10 +- Tools/cases_generator/generate_cases.py | 156 +++--------------------- Tools/cases_generator/parser.py | 1 + 6 files changed, 31 insertions(+), 148 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 7b3e11a7799373..993fb1523ceb42 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -978,7 +978,7 @@ dummy_func( UNPACK_SEQUENCE_LIST, }; - inst(UNPACK_SEQUENCE, (unused/1, seq -- unused[oparg])) { + inst(UNPACK_SEQUENCE, (unused/1, seq -- values[oparg])) { #if ENABLE_SPECIALIZATION _PyUnpackSequenceCache *cache = (_PyUnpackSequenceCache *)next_instr; if (ADAPTIVE_COUNTER_IS_ZERO(cache->counter)) { @@ -1028,7 +1028,7 @@ dummy_func( DECREF_INPUTS(); } - inst(UNPACK_EX, (seq -- unused[oparg & 0xFF], unused, unused[oparg >> 8])) { + inst(UNPACK_EX, (seq -- unused[oparg >> 8], unused, values[oparg & 0xFF])) { int totalargs = 1 + (oparg & 0xFF) + (oparg >> 8); PyObject **top = stack_pointer + totalargs - 1; int res = unpack_iterable(tstate, seq, oparg & 0xFF, oparg >> 8, top); @@ -3237,6 +3237,7 @@ dummy_func( inst(COPY_NO_INCREF, (bottom, unused[oparg - 1] -- bottom, unused[oparg - 1], top: *bottom)) { assert(oparg > 0); + top = bottom; } inst(BINARY_OP, (unused/1, lhs, rhs -- res)) { diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 455fb6ee3a2520..8ac8cfcf530627 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -1318,6 +1318,7 @@ PREDICTED(UNPACK_SEQUENCE); static_assert(INLINE_CACHE_ENTRIES_UNPACK_SEQUENCE == 1, "incorrect cache size"); PyObject *seq = stack_pointer[-1]; + PyObject **values = stack_pointer - (1); #if ENABLE_SPECIALIZATION _PyUnpackSequenceCache *cache = (_PyUnpackSequenceCache *)next_instr; if (ADAPTIVE_COUNTER_IS_ZERO(cache->counter)) { @@ -1391,12 +1392,13 @@ TARGET(UNPACK_EX) { PyObject *seq = stack_pointer[-1]; + PyObject **values = stack_pointer - (1) + 1 + (oparg >> 8); int totalargs = 1 + (oparg & 0xFF) + (oparg >> 8); PyObject **top = stack_pointer + totalargs - 1; int res = unpack_iterable(tstate, seq, oparg & 0xFF, oparg >> 8, top); Py_DECREF(seq); if (res == 0) goto pop_1_error; - STACK_GROW((oparg & 0xFF) + (oparg >> 8)); + STACK_GROW((oparg >> 8) + (oparg & 0xFF)); DISPATCH(); } @@ -4160,6 +4162,7 @@ PyObject *bottom = stack_pointer[-(1 + (oparg - 1))]; PyObject *top; assert(oparg > 0); + top = bottom; STACK_GROW(1); stack_pointer[-1] = top; DISPATCH(); diff --git a/Python/opcode_metadata.h b/Python/opcode_metadata.h index 7f7200503960f6..be21be496104fb 100644 --- a/Python/opcode_metadata.h +++ b/Python/opcode_metadata.h @@ -582,7 +582,7 @@ _PyOpcode_num_pushed(int opcode, int oparg, bool jump) { case UNPACK_SEQUENCE_LIST: return oparg; case UNPACK_EX: - return (oparg & 0xFF) + (oparg >> 8) + 1; + return (oparg >> 8) + (oparg & 0xFF) + 1; case STORE_ATTR: return 0; case DELETE_ATTR: diff --git a/Python/tier2_typepropagator.c.h b/Python/tier2_typepropagator.c.h index 9cdc9b062095c8..df008d7028e8ed 100644 --- a/Python/tier2_typepropagator.c.h +++ b/Python/tier2_typepropagator.c.h @@ -375,32 +375,34 @@ TARGET(UNPACK_SEQUENCE) { STACK_SHRINK(1); STACK_GROW(oparg); + for (int i = 0; i < (oparg); i++) {TYPE_OVERWRITE((_Py_TYPENODE_t *)_Py_TYPENODE_NULLROOT, TYPESTACK_PEEK(oparg - i), true);} break; } TARGET(UNPACK_SEQUENCE_TWO_TUPLE) { STACK_SHRINK(1); STACK_GROW(oparg); - TYPE_OVERWRITE((_Py_TYPENODE_t *)_Py_TYPENODE_NULLROOT, TYPESTACK_PEEK(oparg), true); + for (int i = 0; i < (oparg); i++) {TYPE_OVERWRITE((_Py_TYPENODE_t *)_Py_TYPENODE_NULLROOT, TYPESTACK_PEEK(oparg - i), true);} break; } TARGET(UNPACK_SEQUENCE_TUPLE) { STACK_SHRINK(1); STACK_GROW(oparg); - TYPE_OVERWRITE((_Py_TYPENODE_t *)_Py_TYPENODE_NULLROOT, TYPESTACK_PEEK(oparg), true); + for (int i = 0; i < (oparg); i++) {TYPE_OVERWRITE((_Py_TYPENODE_t *)_Py_TYPENODE_NULLROOT, TYPESTACK_PEEK(oparg - i), true);} break; } TARGET(UNPACK_SEQUENCE_LIST) { STACK_SHRINK(1); STACK_GROW(oparg); - TYPE_OVERWRITE((_Py_TYPENODE_t *)_Py_TYPENODE_NULLROOT, TYPESTACK_PEEK(oparg), true); + for (int i = 0; i < (oparg); i++) {TYPE_OVERWRITE((_Py_TYPENODE_t *)_Py_TYPENODE_NULLROOT, TYPESTACK_PEEK(oparg - i), true);} break; } TARGET(UNPACK_EX) { - STACK_GROW((oparg & 0xFF) + (oparg >> 8)); + STACK_GROW((oparg >> 8) + (oparg & 0xFF)); + for (int i = 0; i < (oparg & 0xFF); i++) {TYPE_OVERWRITE((_Py_TYPENODE_t *)_Py_TYPENODE_NULLROOT, TYPESTACK_PEEK((oparg & 0xFF) - i), true);} break; } diff --git a/Tools/cases_generator/generate_cases.py b/Tools/cases_generator/generate_cases.py index 1255ba463f5a8b..caa224493ae5d9 100644 --- a/Tools/cases_generator/generate_cases.py +++ b/Tools/cases_generator/generate_cases.py @@ -335,146 +335,6 @@ def __init__(self, inst: parser.InstDef): cache = "0" self.instr_fmt = fmt - def write_typeprop(self, out: Formatter) -> None: - """Write one instruction's type propagation rules""" - - # TODO: Detect loops like in SWAP - - need_to_declare = [] - # Stack input is used in local effect - if self.local_effects and \ - isinstance(val := self.local_effects.value, TypeSrcStackInput): - need_to_declare.append(val.name) - # Stack input is used in output effect - for oeffect in self.output_effects: - if not (typ := oeffect.type_annotation): continue - ops = typ.ops - for op in ops: - if not isinstance(src := op.src, TypeSrcStackInput): continue - if oeffect.name in self.unmoved_names and oeffect.name == src.name: - print( - f"Warn: {self.name} type annotation for {oeffect.name} will be ignored " - "as it is unmoved") - continue - need_to_declare.append(src.name) - - # Write input stack effect variable declarations and initializations - ieffects = list(reversed(self.input_effects)) - usable_for_local_effect = {} - all_input_effect_names = {} - for i, ieffect in enumerate(ieffects): - - if ieffect.name not in need_to_declare: continue - - isize = string_effect_size( - list_effect_size([ieff for ieff in ieffects[: i + 1]]) - ) - all_input_effect_names[ieffect.name] = (ieffect, i) - dst = StackEffect(ieffect.name, "_Py_TYPENODE_t *") - if ieffect.size: - # TODO: Support more cases as needed - raise Exception("Type propagation across sized input effect not implemented") - elif ieffect.cond: - src = StackEffect(f"({ieffect.cond}) ? TYPESTACK_PEEK({isize}) : NULL", "_Py_TYPENODE_t *") - else: - usable_for_local_effect[ieffect.name] = ieffect - src = StackEffect(f"TYPESTACK_PEEK({isize})", "_Py_TYPENODE_t *") - out.declare(dst, src) - - # Write localarr effect - if self.local_effects: - - idx = self.local_effects.index - val = self.local_effects.value - - typ_op = "TYPE_OVERWRITE" - dst = f"TYPELOCALS_GET({idx})" - match val: - case TypeSrcLiteral(name=valstr): - if valstr == "NULL": - src = "(_Py_TYPENODE_t *)_Py_TYPENODE_NULLROOT" - flag = "true" - else: - src = f"(_Py_TYPENODE_t *)_Py_TYPENODE_MAKE_ROOT((_Py_TYPENODE_t)&{valstr})" - flag = "true" - case TypeSrcStackInput(name=valstr): - assert valstr in usable_for_local_effect, \ - "`cond` and `size` stackvar not supported for localeffect" - src = valstr - flag = "false" - # TODO: Support more cases as needed - case TypeSrcConst(): - raise Exception("Not implemented") - case TypeSrcLocals(): - raise Exception("Not implemented") - case _: - typing.assert_never(val) - out.emit(f"{typ_op}({src}, {dst}, {flag});") - - # Update stack size - out.stack_adjust( - 0, - [ieff for ieff in self.input_effects], - [oeff for oeff in self.output_effects], - ) - - # Stack effect - oeffects = list(reversed(self.output_effects)) - for i, oeffect in enumerate(oeffects): - osize = string_effect_size( - list_effect_size([oeff for oeff in oeffects[: i + 1]]) - ) - dst = f"TYPESTACK_PEEK({osize})" - - # Check if it's even used - if oeffect.name == UNUSED: continue - - # Check if there's type info - if typ := oeffect.type_annotation: - for op in typ.ops: - match op.src: - case TypeSrcLiteral(literal=valstr): - if valstr == "NULL": - src = "(_Py_TYPENODE_t *)_Py_TYPENODE_NULLROOT" - flag = "true" - else: - src = f"(_Py_TYPENODE_t *)_Py_TYPENODE_MAKE_ROOT((_Py_TYPENODE_t)&{valstr})" - flag = "true" - case TypeSrcStackInput(name=valstr): - assert valstr in need_to_declare - assert oeffect.name not in self.unmoved_names - src = valstr - flag = "false" - case TypeSrcConst(index=idx): - src = f"(_Py_TYPENODE_t *)TYPECONST_GET({idx})" - flag = "true" - case TypeSrcLocals(index=idx): - src = f"TYPELOCALS_GET({idx})" - flag = "false" - case _: - typing.assert_never(op.src) - - opstr = f"{op.op}({src}, {dst}, {flag})" - if oeffect.cond: - out.emit(f"if ({oeffect.cond}) {{ {opstr}; }}") - else: - out.emit(f"{opstr};") - continue - - # Don't touch unmoved stack vars - if oeffect.name in self.unmoved_names: - continue - - # Just output null - typ_op = "TYPE_OVERWRITE" - src = "(_Py_TYPENODE_t *)_Py_TYPENODE_NULLROOT" - flag = "true" - opstr = f"{typ_op}({src}, {dst}, {flag})" - if oeffect.cond: - out.emit(f"if ({oeffect.cond}) {{ {opstr}; }}") - else: - out.emit(f"{opstr};") - def write(self, out: Formatter) -> None: """Write one instruction, sans prologue and epilogue.""" # Write a static assertion that a family's cache size is correct @@ -635,6 +495,8 @@ def write_body(self, out: Formatter, dedent: int, cache_adjust: int = 0) -> None def write_typeprop(self, out: Formatter) -> None: """Write one instruction's type propagation rules""" + # TODO: Add SWAP to DSL + need_to_declare = [] # Stack input is used in local effect if self.local_effects and \ @@ -724,6 +586,20 @@ def write_typeprop(self, out: Formatter) -> None: # Check if it's even used if oeffect.name == UNUSED: continue + # For now assume OVERWRITE with NULL + if oeffect.size: + op = "TYPE_OVERWRITE" + src = "(_Py_TYPENODE_t *)_Py_TYPENODE_NULLROOT" + flag = "true" + dst = f"TYPESTACK_PEEK({osize} - i)" + opstr = "".join([ + f"for (int i = 0; i < ({oeffect.size}); i++) {{" + f"{op}({src}, {dst}, {flag});" + f"}}" + ]) + out.emit(opstr) + continue + # Check if there's type info if typ := oeffect.type_annotation: for op in typ.ops: diff --git a/Tools/cases_generator/parser.py b/Tools/cases_generator/parser.py index 658b1b0d46c513..5c87a2b85a2dd6 100644 --- a/Tools/cases_generator/parser.py +++ b/Tools/cases_generator/parser.py @@ -343,6 +343,7 @@ def stack_effect(self) -> StackEffect | None: cond_text = cond.text.strip() size_text = "" if self.expect(lx.LBRACKET): + # TODO: Support type annotation for size output if has_type_annotation or cond_text: raise self.make_syntax_error("Unexpected [") if not (size := self.expression()): From 4b3997b004290491980076cf58c3965eb714165d Mon Sep 17 00:00:00 2001 From: Julia Date: Fri, 31 Mar 2023 11:51:33 +0800 Subject: [PATCH 7/8] Fix: Misc macro fix --- Python/tier2.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Python/tier2.c b/Python/tier2.c index dc66c756918de7..0e869925135523 100644 --- a/Python/tier2.c +++ b/Python/tier2.c @@ -121,8 +121,8 @@ _PyTier2TypeContext_Copy(const _PyTier2TypeContext *type_context) } // Is part of stack else { -#if TYPEPROP_DEBUG int offset_stack = (int)(parent - type_context->type_stack); +#if TYPEPROP_DEBUG assert(0 <= offset_stack && offset_stack < nstack); #endif type_locals[i] = _Py_TYPENODE_MAKE_REF((_Py_TYPENODE_t)(&type_stack[offset_stack])); @@ -152,8 +152,8 @@ _PyTier2TypeContext_Copy(const _PyTier2TypeContext *type_context) } // Is part of stack else { -#if TYPEPROP_DEBUG int offset_stack = (int)(parent - type_context->type_stack); +#if TYPEPROP_DEBUG assert(0 <= offset_stack && offset_stack < nstack); #endif type_stack[i] = _Py_TYPENODE_MAKE_REF((_Py_TYPENODE_t)(&type_stack[offset_stack])); From 6a322641da7cd907e3d1738a55a997a7adcdb65b Mon Sep 17 00:00:00 2001 From: Julia Date: Fri, 31 Mar 2023 21:17:07 +0800 Subject: [PATCH 8/8] Fix: Ref leak in unboxing checks --- Python/bytecodes.c | 23 ++++++++++++++--------- Python/generated_cases.c.h | 24 +++++++++++++++--------- 2 files changed, 29 insertions(+), 18 deletions(-) diff --git a/Python/bytecodes.c b/Python/bytecodes.c index 993fb1523ceb42..5d8143841c8add 100644 --- a/Python/bytecodes.c +++ b/Python/bytecodes.c @@ -318,21 +318,26 @@ dummy_func( char is_successor = PyFloat_CheckExact(left) && (Py_TYPE(left) == Py_TYPE(right)); bb_test = BB_TEST(is_successor, 0); - left_unboxed = (is_successor - ? *((PyObject **)(&(((PyFloatObject *)left)->ob_fval))) - : left); - right_unboxed = (is_successor - ? *((PyObject **)(&(((PyFloatObject *)right)->ob_fval))) - : right); + if (is_successor) { + left_unboxed = *((PyObject **)(&(((PyFloatObject *)left)->ob_fval))); + right_unboxed = *((PyObject **)(&(((PyFloatObject *)right)->ob_fval))); + DECREF_INPUTS(); + } else { + left_unboxed = left; + right_unboxed = right; + } } inst(UNARY_CHECK_FLOAT, (arg, unused[oparg] -- arg_unboxed : { <<= PyFloat_Type, PyRawFloat_Type}, unused[oparg])) { assert(cframe.use_tracing == 0); char is_successor = PyFloat_CheckExact(arg); bb_test = BB_TEST(is_successor, 0); - arg_unboxed = (is_successor - ? *((PyObject **)(&(((PyFloatObject *)arg)->ob_fval))) - : arg); + if (is_successor) { + arg_unboxed = *((PyObject **)(&(((PyFloatObject *)arg)->ob_fval))); + DECREF_INPUTS(); + } else { + arg_unboxed = arg; + } } inst(BINARY_OP_ADD_FLOAT_UNBOXED, (left, right -- sum : PyRawFloat_Type)) { diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h index 8ac8cfcf530627..500290fb32c108 100644 --- a/Python/generated_cases.c.h +++ b/Python/generated_cases.c.h @@ -474,12 +474,15 @@ char is_successor = PyFloat_CheckExact(left) && (Py_TYPE(left) == Py_TYPE(right)); bb_test = BB_TEST(is_successor, 0); - left_unboxed = (is_successor - ? *((PyObject **)(&(((PyFloatObject *)left)->ob_fval))) - : left); - right_unboxed = (is_successor - ? *((PyObject **)(&(((PyFloatObject *)right)->ob_fval))) - : right); + if (is_successor) { + left_unboxed = *((PyObject **)(&(((PyFloatObject *)left)->ob_fval))); + right_unboxed = *((PyObject **)(&(((PyFloatObject *)right)->ob_fval))); + Py_DECREF(left); + Py_DECREF(right); + } else { + left_unboxed = left; + right_unboxed = right; + } stack_pointer[-1] = right_unboxed; stack_pointer[-2] = left_unboxed; DISPATCH(); @@ -491,9 +494,12 @@ assert(cframe.use_tracing == 0); char is_successor = PyFloat_CheckExact(arg); bb_test = BB_TEST(is_successor, 0); - arg_unboxed = (is_successor - ? *((PyObject **)(&(((PyFloatObject *)arg)->ob_fval))) - : arg); + if (is_successor) { + arg_unboxed = *((PyObject **)(&(((PyFloatObject *)arg)->ob_fval))); + Py_DECREF(arg); + } else { + arg_unboxed = arg; + } stack_pointer[-(1 + oparg)] = arg_unboxed; DISPATCH(); }