Skip to content

Commit 2f42f83

Browse files
authored
gh-144005: Eliminate redundant refcounting in the JIT for BINARY_OP_EXTEND (#144006)
1 parent 70e67f5 commit 2f42f83

File tree

11 files changed

+109
-53
lines changed

11 files changed

+109
-53
lines changed

Include/internal/pycore_opcode_metadata.h

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Include/internal/pycore_uop_ids.h

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Include/internal/pycore_uop_metadata.h

Lines changed: 4 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Lib/test/test_capi/test_opt.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2897,6 +2897,23 @@ def testfunc(n):
28972897
self.assertIn("_POP_TOP_NOP", uops)
28982898
self.assertLessEqual(count_ops(ex, "_POP_TOP"), 2)
28992899

2900+
def test_binary_op_extend_float_long_add_refcount_elimination(self):
2901+
def testfunc(n):
2902+
a = 1.5
2903+
b = 2
2904+
res = 0.0
2905+
for _ in range(n):
2906+
res = a + b
2907+
return res
2908+
2909+
res, ex = self._run_with_optimizer(testfunc, TIER2_THRESHOLD)
2910+
self.assertEqual(res, 3.5)
2911+
self.assertIsNotNone(ex)
2912+
uops = get_opnames(ex)
2913+
self.assertIn("_BINARY_OP_EXTEND", uops)
2914+
self.assertIn("_POP_TOP_NOP", uops)
2915+
self.assertLessEqual(count_ops(ex, "_POP_TOP"), 2)
2916+
29002917
def test_remove_guard_for_slice_list(self):
29012918
def f(n):
29022919
for i in range(n):
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Eliminate redundant refcounting from ``BINARY_OP_EXTEND``.

Modules/_testinternalcapi/test_cases.c.h

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -317,6 +317,9 @@
317317
_PyStackRef left;
318318
_PyStackRef right;
319319
_PyStackRef res;
320+
_PyStackRef l;
321+
_PyStackRef r;
322+
_PyStackRef value;
320323
/* Skip 1 cache entry */
321324
// _GUARD_BINARY_OP_EXTEND
322325
{
@@ -348,25 +351,32 @@
348351
STAT_INC(BINARY_OP, hit);
349352
_PyFrame_SetStackPointer(frame, stack_pointer);
350353
PyObject *res_o = d->action(left_o, right_o);
351-
_PyStackRef tmp = right;
352-
right = PyStackRef_NULL;
353-
stack_pointer[-1] = right;
354-
PyStackRef_CLOSE(tmp);
355-
tmp = left;
356-
left = PyStackRef_NULL;
357-
stack_pointer[-2] = left;
358-
PyStackRef_CLOSE(tmp);
359354
stack_pointer = _PyFrame_GetStackPointer(frame);
360-
stack_pointer += -2;
361-
ASSERT_WITHIN_STACK_BOUNDS(__FILE__, __LINE__);
362355
if (res_o == NULL) {
363356
JUMP_TO_LABEL(error);
364357
}
365358
res = PyStackRef_FromPyObjectSteal(res_o);
359+
l = left;
360+
r = right;
361+
}
362+
// _POP_TOP
363+
{
364+
value = r;
365+
stack_pointer[-2] = res;
366+
stack_pointer[-1] = l;
367+
_PyFrame_SetStackPointer(frame, stack_pointer);
368+
PyStackRef_XCLOSE(value);
369+
stack_pointer = _PyFrame_GetStackPointer(frame);
370+
}
371+
// _POP_TOP
372+
{
373+
value = l;
374+
stack_pointer += -1;
375+
ASSERT_WITHIN_STACK_BOUNDS(__FILE__, __LINE__);
376+
_PyFrame_SetStackPointer(frame, stack_pointer);
377+
PyStackRef_XCLOSE(value);
378+
stack_pointer = _PyFrame_GetStackPointer(frame);
366379
}
367-
stack_pointer[0] = res;
368-
stack_pointer += 1;
369-
ASSERT_WITHIN_STACK_BOUNDS(__FILE__, __LINE__);
370380
DISPATCH();
371381
}
372382

Python/bytecodes.c

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -829,7 +829,7 @@ dummy_func(
829829
DEOPT_IF(!res);
830830
}
831831

832-
op(_BINARY_OP_EXTEND, (descr/4, left, right -- res)) {
832+
op(_BINARY_OP_EXTEND, (descr/4, left, right -- res, l, r)) {
833833
PyObject *left_o = PyStackRef_AsPyObjectBorrow(left);
834834
PyObject *right_o = PyStackRef_AsPyObjectBorrow(right);
835835
assert(INLINE_CACHE_ENTRIES_BINARY_OP == 5);
@@ -838,13 +838,18 @@ dummy_func(
838838
STAT_INC(BINARY_OP, hit);
839839

840840
PyObject *res_o = d->action(left_o, right_o);
841-
DECREF_INPUTS();
842-
ERROR_IF(res_o == NULL);
841+
if (res_o == NULL) {
842+
ERROR_NO_POP();
843+
}
843844
res = PyStackRef_FromPyObjectSteal(res_o);
845+
l = left;
846+
r = right;
847+
DEAD(left);
848+
DEAD(right);
844849
}
845850

846851
macro(BINARY_OP_EXTEND) =
847-
unused/1 + _GUARD_BINARY_OP_EXTEND + rewind/-4 + _BINARY_OP_EXTEND;
852+
unused/1 + _GUARD_BINARY_OP_EXTEND + rewind/-4 + _BINARY_OP_EXTEND + POP_TOP + POP_TOP;
848853

849854
macro(BINARY_OP_INPLACE_ADD_UNICODE) =
850855
_GUARD_TOS_UNICODE + _GUARD_NOS_UNICODE + unused/5 + _BINARY_OP_INPLACE_ADD_UNICODE;

Python/executor_cases.c.h

Lines changed: 10 additions & 14 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Python/generated_cases.c.h

Lines changed: 23 additions & 13 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Python/optimizer_bytecodes.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -311,6 +311,12 @@ dummy_func(void) {
311311
r = right;
312312
}
313313

314+
op(_BINARY_OP_EXTEND, (left, right -- res, l, r)) {
315+
res = sym_new_not_null(ctx);
316+
l = left;
317+
r = right;
318+
}
319+
314320
op(_BINARY_OP_INPLACE_ADD_UNICODE, (left, right -- res)) {
315321
if (sym_is_const(ctx, left) && sym_is_const(ctx, right)) {
316322
assert(PyUnicode_CheckExact(sym_get_const(ctx, left)));

0 commit comments

Comments
 (0)