From 05a604f2f27afee6e0ffa2c6e1f1f4421b2b945a Mon Sep 17 00:00:00 2001 From: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com> Date: Thu, 22 Feb 2024 19:50:00 +0800 Subject: [PATCH 1/4] Only allow immortal constants in tier 2 --- Python/optimizer_analysis.c | 21 ++++++++++--------- .../tier2_redundancy_eliminator_bytecodes.c | 4 ++-- Python/tier2_redundancy_eliminator_cases.c.h | 4 ++-- 3 files changed, 15 insertions(+), 14 deletions(-) diff --git a/Python/optimizer_analysis.c b/Python/optimizer_analysis.c index 5d2df9aca4e77f..199b8b99c81785 100644 --- a/Python/optimizer_analysis.c +++ b/Python/optimizer_analysis.c @@ -164,10 +164,6 @@ abstractcontext_fini(_Py_UOpsAbstractInterpContext *ctx) return; } ctx->curr_frame_depth = 0; - int tys = ctx->t_arena.ty_curr_number; - for (int i = 0; i < tys; i++) { - Py_CLEAR(ctx->t_arena.arena[i].const_val); - } } static int @@ -218,7 +214,7 @@ ctx_frame_pop( } -// Takes a borrowed reference to const_val, turns that into a strong reference. +// Only accepts immortal constants. static _Py_UOpsSymType* sym_new(_Py_UOpsAbstractInterpContext *ctx, PyObject *const_val) @@ -233,9 +229,8 @@ sym_new(_Py_UOpsAbstractInterpContext *ctx, self->const_val = NULL; self->typ = NULL; self->flags = 0; - - if (const_val != NULL) { - self->const_val = Py_NewRef(const_val); + if (const_val != NULL && _Py_IsImmortal(const_val)) { + self->const_val = const_val; } return self; @@ -317,7 +312,7 @@ sym_new_known_type(_Py_UOpsAbstractInterpContext *ctx, return res; } -// Takes a borrowed reference to const_val. +// Steals a reference to const_val if it is not immortal. static inline _Py_UOpsSymType* sym_new_const(_Py_UOpsAbstractInterpContext *ctx, PyObject *const_val) { @@ -330,7 +325,13 @@ sym_new_const(_Py_UOpsAbstractInterpContext *ctx, PyObject *const_val) return NULL; } sym_set_type(temp, Py_TYPE(const_val)); - sym_set_flag(temp, TRUE_CONST); + // const_val can be NULL if it's not immortal, then it's discarded. + if (temp->const_val != NULL) { + sym_set_flag(temp, TRUE_CONST); + } + if (!_Py_IsImmortal(const_val)) { + Py_DECREF(const_val); + } sym_set_flag(temp, KNOWN); sym_set_flag(temp, NOT_NULL); return temp; diff --git a/Python/tier2_redundancy_eliminator_bytecodes.c b/Python/tier2_redundancy_eliminator_bytecodes.c index e9b556d16c3702..fb84e63dd82f62 100644 --- a/Python/tier2_redundancy_eliminator_bytecodes.c +++ b/Python/tier2_redundancy_eliminator_bytecodes.c @@ -204,7 +204,7 @@ dummy_func(void) { } op(_LOAD_CONST_INLINE, (ptr/4 -- value)) { - OUT_OF_SPACE_IF_NULL(value = sym_new_const(ctx, ptr)); + OUT_OF_SPACE_IF_NULL(value = sym_new_const(ctx, Py_NewRef(ptr))); } op(_LOAD_CONST_INLINE_BORROW, (ptr/4 -- value)) { @@ -212,7 +212,7 @@ dummy_func(void) { } op(_LOAD_CONST_INLINE_WITH_NULL, (ptr/4 -- value, null)) { - OUT_OF_SPACE_IF_NULL(value = sym_new_const(ctx, ptr)); + OUT_OF_SPACE_IF_NULL(value = sym_new_const(ctx, Py_NewRef(ptr))); OUT_OF_SPACE_IF_NULL(null = sym_new_null(ctx)); } diff --git a/Python/tier2_redundancy_eliminator_cases.c.h b/Python/tier2_redundancy_eliminator_cases.c.h index f41fe328195b4d..8c4966239d410b 100644 --- a/Python/tier2_redundancy_eliminator_cases.c.h +++ b/Python/tier2_redundancy_eliminator_cases.c.h @@ -1692,7 +1692,7 @@ case _LOAD_CONST_INLINE: { _Py_UOpsSymType *value; PyObject *ptr = (PyObject *)this_instr->operand; - OUT_OF_SPACE_IF_NULL(value = sym_new_const(ctx, ptr)); + OUT_OF_SPACE_IF_NULL(value = sym_new_const(ctx, Py_NewRef(ptr))); stack_pointer[0] = value; stack_pointer += 1; break; @@ -1711,7 +1711,7 @@ _Py_UOpsSymType *value; _Py_UOpsSymType *null; PyObject *ptr = (PyObject *)this_instr->operand; - OUT_OF_SPACE_IF_NULL(value = sym_new_const(ctx, ptr)); + OUT_OF_SPACE_IF_NULL(value = sym_new_const(ctx, Py_NewRef(ptr))); OUT_OF_SPACE_IF_NULL(null = sym_new_null(ctx)); stack_pointer[0] = value; stack_pointer[1] = null; From 38ee21c2d717eb9a66e050636cd208f4b689899e Mon Sep 17 00:00:00 2001 From: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com> Date: Thu, 22 Feb 2024 19:56:40 +0800 Subject: [PATCH 2/4] Allow borrowed ref for _LOAD_CONST_INLINE --- Python/optimizer_analysis.c | 32 +++++++++++++++---- .../tier2_redundancy_eliminator_bytecodes.c | 4 +-- Python/tier2_redundancy_eliminator_cases.c.h | 4 +-- 3 files changed, 29 insertions(+), 11 deletions(-) diff --git a/Python/optimizer_analysis.c b/Python/optimizer_analysis.c index 199b8b99c81785..9bd9263a845ae8 100644 --- a/Python/optimizer_analysis.c +++ b/Python/optimizer_analysis.c @@ -226,12 +226,9 @@ sym_new(_Py_UOpsAbstractInterpContext *ctx, return NULL; } ctx->t_arena.ty_curr_number++; - self->const_val = NULL; self->typ = NULL; self->flags = 0; - if (const_val != NULL && _Py_IsImmortal(const_val)) { - self->const_val = const_val; - } + self->const_val = const_val; return self; } @@ -325,15 +322,36 @@ sym_new_const(_Py_UOpsAbstractInterpContext *ctx, PyObject *const_val) return NULL; } sym_set_type(temp, Py_TYPE(const_val)); - // const_val can be NULL if it's not immortal, then it's discarded. - if (temp->const_val != NULL) { + sym_set_flag(temp, KNOWN); + sym_set_flag(temp, NOT_NULL); + // If the constant is not immortal, discard it. + if (_Py_IsImmortal(const_val)) { sym_set_flag(temp, TRUE_CONST); } - if (!_Py_IsImmortal(const_val)) { + else { Py_DECREF(const_val); + temp->const_val = NULL; + } + return temp; +} + +// Same as sym_new_const, but borrows const_val, assuming it will be kept alive. +// Only safe if we know const_val has a strong reference elsewhere. +static inline _Py_UOpsSymType* +sym_new_const_borrow(_Py_UOpsAbstractInterpContext *ctx, PyObject *const_val) +{ + assert(const_val != NULL); + _Py_UOpsSymType *temp = sym_new( + ctx, + const_val + ); + if (temp == NULL) { + return NULL; } + sym_set_type(temp, Py_TYPE(const_val)); sym_set_flag(temp, KNOWN); sym_set_flag(temp, NOT_NULL); + sym_set_flag(temp, TRUE_CONST); return temp; } diff --git a/Python/tier2_redundancy_eliminator_bytecodes.c b/Python/tier2_redundancy_eliminator_bytecodes.c index fb84e63dd82f62..1752f8eb72b429 100644 --- a/Python/tier2_redundancy_eliminator_bytecodes.c +++ b/Python/tier2_redundancy_eliminator_bytecodes.c @@ -204,7 +204,7 @@ dummy_func(void) { } op(_LOAD_CONST_INLINE, (ptr/4 -- value)) { - OUT_OF_SPACE_IF_NULL(value = sym_new_const(ctx, Py_NewRef(ptr))); + OUT_OF_SPACE_IF_NULL(value = sym_new_const_borrow(ctx, ptr)); } op(_LOAD_CONST_INLINE_BORROW, (ptr/4 -- value)) { @@ -212,7 +212,7 @@ dummy_func(void) { } op(_LOAD_CONST_INLINE_WITH_NULL, (ptr/4 -- value, null)) { - OUT_OF_SPACE_IF_NULL(value = sym_new_const(ctx, Py_NewRef(ptr))); + OUT_OF_SPACE_IF_NULL(value = sym_new_const_borrow(ctx, ptr)); OUT_OF_SPACE_IF_NULL(null = sym_new_null(ctx)); } diff --git a/Python/tier2_redundancy_eliminator_cases.c.h b/Python/tier2_redundancy_eliminator_cases.c.h index 8c4966239d410b..5c1b3906a72797 100644 --- a/Python/tier2_redundancy_eliminator_cases.c.h +++ b/Python/tier2_redundancy_eliminator_cases.c.h @@ -1692,7 +1692,7 @@ case _LOAD_CONST_INLINE: { _Py_UOpsSymType *value; PyObject *ptr = (PyObject *)this_instr->operand; - OUT_OF_SPACE_IF_NULL(value = sym_new_const(ctx, Py_NewRef(ptr))); + OUT_OF_SPACE_IF_NULL(value = sym_new_const_borrow(ctx, ptr)); stack_pointer[0] = value; stack_pointer += 1; break; @@ -1711,7 +1711,7 @@ _Py_UOpsSymType *value; _Py_UOpsSymType *null; PyObject *ptr = (PyObject *)this_instr->operand; - OUT_OF_SPACE_IF_NULL(value = sym_new_const(ctx, Py_NewRef(ptr))); + OUT_OF_SPACE_IF_NULL(value = sym_new_const_borrow(ctx, ptr)); OUT_OF_SPACE_IF_NULL(null = sym_new_null(ctx)); stack_pointer[0] = value; stack_pointer[1] = null; From c75030d2fe7ac057deb34d4af1f1daf21de475a1 Mon Sep 17 00:00:00 2001 From: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com> Date: Thu, 22 Feb 2024 20:13:02 +0800 Subject: [PATCH 3/4] update comments --- Python/optimizer_analysis.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Python/optimizer_analysis.c b/Python/optimizer_analysis.c index 9bd9263a845ae8..8d9b02b5a77e27 100644 --- a/Python/optimizer_analysis.c +++ b/Python/optimizer_analysis.c @@ -214,7 +214,8 @@ ctx_frame_pop( } -// Only accepts immortal constants. +// Only accepts immortal constants or borrowed constants that will be kept alive +// elsewhere. static _Py_UOpsSymType* sym_new(_Py_UOpsAbstractInterpContext *ctx, PyObject *const_val) @@ -309,7 +310,7 @@ sym_new_known_type(_Py_UOpsAbstractInterpContext *ctx, return res; } -// Steals a reference to const_val if it is not immortal. +// Decrefs const_val if it is not immortal and discards it. static inline _Py_UOpsSymType* sym_new_const(_Py_UOpsAbstractInterpContext *ctx, PyObject *const_val) { From 3ff15c770b20c11705a94ae61e58849c57f3b7ac Mon Sep 17 00:00:00 2001 From: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com> Date: Thu, 22 Feb 2024 21:45:20 +0800 Subject: [PATCH 4/4] make flags simpler --- Python/optimizer_analysis.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/Python/optimizer_analysis.c b/Python/optimizer_analysis.c index 8d9b02b5a77e27..27d61ecf4a5c84 100644 --- a/Python/optimizer_analysis.c +++ b/Python/optimizer_analysis.c @@ -323,8 +323,7 @@ sym_new_const(_Py_UOpsAbstractInterpContext *ctx, PyObject *const_val) return NULL; } sym_set_type(temp, Py_TYPE(const_val)); - sym_set_flag(temp, KNOWN); - sym_set_flag(temp, NOT_NULL); + sym_set_flag(temp, KNOWN | NOT_NULL); // If the constant is not immortal, discard it. if (_Py_IsImmortal(const_val)) { sym_set_flag(temp, TRUE_CONST); @@ -350,9 +349,7 @@ sym_new_const_borrow(_Py_UOpsAbstractInterpContext *ctx, PyObject *const_val) return NULL; } sym_set_type(temp, Py_TYPE(const_val)); - sym_set_flag(temp, KNOWN); - sym_set_flag(temp, NOT_NULL); - sym_set_flag(temp, TRUE_CONST); + sym_set_flag(temp, KNOWN | NOT_NULL | TRUE_CONST); return temp; }