Skip to content
Draft
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Include/internal/pycore_opcode_metadata.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

19 changes: 10 additions & 9 deletions Include/internal/pycore_uop_ids.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions Include/internal/pycore_uop_metadata.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 13 additions & 0 deletions Lib/test/test_capi/test_opt.py
Original file line number Diff line number Diff line change
Expand Up @@ -2093,6 +2093,19 @@ def testfunc(n):
self.assertNotIn("_POP_CALL_ONE_LOAD_CONST_INLINE_BORROW", uops)
self.assertNotIn("_POP_CALL_TWO_LOAD_CONST_INLINE_BORROW", uops)

def test_call_isinstance_guards_pop_top(self):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't leave a comment on that line, but inside test_call_isinstance_guards_removed we should include the new opcode _SWAP_CALL_TWO_LOAD_CONST_INLINE_BORROW to ensure remove_unneeded_uops is still working as expected (I think it should be since you also updated op_without_push, but just to be sure :))

def testfunc(n):
x = 0
for _ in range(n):
x += isinstance(42, int)
return x

res, ex = self._run_with_optimizer(testfunc, TIER2_THRESHOLD)
self.assertEqual(res, TIER2_THRESHOLD)
self.assertIsNotNone(ex)
uops = get_opnames(ex)
self.assertIn("_POP_TOP_NOP", uops)

def test_call_list_append(self):
def testfunc(n):
a = []
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Eliminate redundant refcounting from ``_CALL_ISINSTANCE``. Patch by Noam
Cohen
29 changes: 22 additions & 7 deletions Python/bytecodes.c
Original file line number Diff line number Diff line change
Expand Up @@ -4349,7 +4349,7 @@ dummy_func(
DEOPT_IF(callable_o != interp->callable_cache.isinstance);
}

op(_CALL_ISINSTANCE, (callable, null, instance, cls -- res)) {
op(_CALL_ISINSTANCE, (callable, null, instance, cls -- res, c1, i, c2)) {
/* isinstance(o, o2) */
STAT_INC(CALL, hit);
PyObject *inst_o = PyStackRef_AsPyObjectBorrow(instance);
Expand All @@ -4358,11 +4358,10 @@ dummy_func(
if (retval < 0) {
ERROR_NO_POP();
}
(void)null; // Silence compiler warnings about unused variables
PyStackRef_CLOSE(cls);
PyStackRef_CLOSE(instance);
DEAD(null);
PyStackRef_CLOSE(callable);
INPUTS_DEAD();
c1 = callable;
i = instance;
c2 = cls;
res = retval ? PyStackRef_True : PyStackRef_False;
assert((!PyStackRef_IsNull(res)) ^ (_PyErr_Occurred(tstate) != NULL));
}
Expand All @@ -4372,7 +4371,10 @@ dummy_func(
unused/2 +
_GUARD_THIRD_NULL +
_GUARD_CALLABLE_ISINSTANCE +
_CALL_ISINSTANCE;
_CALL_ISINSTANCE +
POP_TOP +
POP_TOP +
POP_TOP;

macro(CALL_LIST_APPEND) =
unused/1 +
Expand Down Expand Up @@ -5360,6 +5362,19 @@ dummy_func(
value = PyStackRef_FromPyObjectBorrow(ptr);
}

tier2 op(_SWAP_CALL_TWO_LOAD_CONST_INLINE_BORROW, (ptr/4, callable, null, instance, cls -- value, c1, i, c2)) {
PyStackRef_CLOSE(cls);
PyStackRef_CLOSE(instance);
(void)null; // Silence compiler warnings about unused variables
DEAD(null);
PyStackRef_CLOSE(callable);
assert(_Py_IsImmortal(PyStackRef_AsPyObjectBorrow(callable)));
value = PyStackRef_FromPyObjectBorrow(ptr);
c1 = callable;
i = instance;
c2 = cls;
}

tier2 op(_LOAD_CONST_UNDER_INLINE, (ptr/4, old -- value, new)) {
new = old;
DEAD(old);
Expand Down
75 changes: 54 additions & 21 deletions Python/executor_cases.c.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

38 changes: 28 additions & 10 deletions Python/generated_cases.c.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Python/optimizer_analysis.c
Original file line number Diff line number Diff line change
Expand Up @@ -567,6 +567,7 @@ const uint16_t op_without_push[MAX_UOP_ID + 1] = {
[_POP_TOP_LOAD_CONST_INLINE_BORROW] = _POP_TOP,
[_POP_TWO_LOAD_CONST_INLINE_BORROW] = _POP_TWO,
[_POP_CALL_TWO_LOAD_CONST_INLINE_BORROW] = _POP_CALL_TWO,
[_SWAP_CALL_TWO_LOAD_CONST_INLINE_BORROW] = _POP_CALL_TWO_LOAD_CONST_INLINE_BORROW,
};

const bool op_skip[MAX_UOP_ID + 1] = {
Expand Down
11 changes: 9 additions & 2 deletions Python/optimizer_bytecodes.c
Original file line number Diff line number Diff line change
Expand Up @@ -567,6 +567,13 @@ dummy_func(void) {
value = PyJitRef_Borrow(sym_new_const(ctx, ptr));
}

op(_SWAP_CALL_TWO_LOAD_CONST_INLINE_BORROW, (ptr/4, callable, null, instance, cls -- value, c1, i, c2)) {
value = PyJitRef_Borrow(sym_new_const(ctx, ptr));
c1 = callable;
i = instance;
c2 = cls;
}

op(_POP_TOP, (value -- )) {
PyTypeObject *typ = sym_get_type(value);
if (PyJitRef_IsBorrowed(value) ||
Expand Down Expand Up @@ -997,7 +1004,7 @@ dummy_func(void) {
}
}

op(_CALL_ISINSTANCE, (unused, unused, instance, cls -- res)) {
op(_CALL_ISINSTANCE, (unused, unused, instance, cls -- res, unused, unused, unused)) {
// the result is always a bool, but sometimes we can
// narrow it down to True or False
res = sym_new_type(ctx, &PyBool_Type);
Expand All @@ -1013,7 +1020,7 @@ dummy_func(void) {
out = Py_True;
}
sym_set_const(res, out);
REPLACE_OP(this_instr, _POP_CALL_TWO_LOAD_CONST_INLINE_BORROW, 0, (uintptr_t)out);
REPLACE_OP(this_instr, _SWAP_CALL_TWO_LOAD_CONST_INLINE_BORROW, 0, (uintptr_t)out);
}
}

Expand Down
27 changes: 24 additions & 3 deletions Python/optimizer_cases.c.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading