Skip to content

Commit 89e6607

Browse files
cocolatococolato
authored
gh-139109: Replace _CHECK_STACK_SPACE with _CHECK_STACK_SPACE_OPERAND in JIT optiimizer (GH-144394)
1 parent 141fd8b commit 89e6607

File tree

5 files changed

+44
-64
lines changed

5 files changed

+44
-64
lines changed

‎Lib/test/test_capi/test_opt.py‎

Lines changed: 30 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1022,7 +1022,6 @@ def return_hello():
10221022
# Constant narrowing allows constant folding for second comparison
10231023
self.assertLessEqual(count_ops(ex, "_COMPARE_OP_STR"), 1)
10241024

1025-
@unittest.skip("gh-139109 WIP")
10261025
def test_combine_stack_space_checks_sequential(self):
10271026
def dummy12(x):
10281027
return x - 1
@@ -1046,12 +1045,14 @@ def testfunc(n):
10461045
self.assertEqual(uop_names.count("_PUSH_FRAME"), 2)
10471046
self.assertEqual(uop_names.count("_RETURN_VALUE"), 2)
10481047
self.assertEqual(uop_names.count("_CHECK_STACK_SPACE"), 0)
1049-
self.assertEqual(uop_names.count("_CHECK_STACK_SPACE_OPERAND"), 1)
1050-
# sequential calls: max(12, 13) == 13
1051-
largest_stack = _testinternalcapi.get_co_framesize(dummy13.__code__)
1052-
self.assertIn(("_CHECK_STACK_SPACE_OPERAND", largest_stack), uops_and_operands)
1048+
# Each call gets its own _CHECK_STACK_SPACE_OPERAND
1049+
self.assertEqual(uop_names.count("_CHECK_STACK_SPACE_OPERAND"), 2)
1050+
# Each _CHECK_STACK_SPACE_OPERAND has the framesize of its function
1051+
self.assertIn(("_CHECK_STACK_SPACE_OPERAND",
1052+
_testinternalcapi.get_co_framesize(dummy12.__code__)), uops_and_operands)
1053+
self.assertIn(("_CHECK_STACK_SPACE_OPERAND",
1054+
_testinternalcapi.get_co_framesize(dummy13.__code__)), uops_and_operands)
10531055

1054-
@unittest.skip("gh-139109 WIP")
10551056
def test_combine_stack_space_checks_nested(self):
10561057
def dummy12(x):
10571058
return x + 3
@@ -1074,15 +1075,12 @@ def testfunc(n):
10741075
self.assertEqual(uop_names.count("_PUSH_FRAME"), 2)
10751076
self.assertEqual(uop_names.count("_RETURN_VALUE"), 2)
10761077
self.assertEqual(uop_names.count("_CHECK_STACK_SPACE"), 0)
1077-
self.assertEqual(uop_names.count("_CHECK_STACK_SPACE_OPERAND"), 1)
1078-
# nested calls: 15 + 12 == 27
1079-
largest_stack = (
1080-
_testinternalcapi.get_co_framesize(dummy15.__code__) +
1081-
_testinternalcapi.get_co_framesize(dummy12.__code__)
1082-
)
1083-
self.assertIn(("_CHECK_STACK_SPACE_OPERAND", largest_stack), uops_and_operands)
1078+
self.assertEqual(uop_names.count("_CHECK_STACK_SPACE_OPERAND"), 2)
1079+
self.assertIn(("_CHECK_STACK_SPACE_OPERAND",
1080+
_testinternalcapi.get_co_framesize(dummy15.__code__)), uops_and_operands)
1081+
self.assertIn(("_CHECK_STACK_SPACE_OPERAND",
1082+
_testinternalcapi.get_co_framesize(dummy12.__code__)), uops_and_operands)
10841083

1085-
@unittest.skip("gh-139109 WIP")
10861084
def test_combine_stack_space_checks_several_calls(self):
10871085
def dummy12(x):
10881086
return x + 3
@@ -1110,15 +1108,14 @@ def testfunc(n):
11101108
self.assertEqual(uop_names.count("_PUSH_FRAME"), 4)
11111109
self.assertEqual(uop_names.count("_RETURN_VALUE"), 4)
11121110
self.assertEqual(uop_names.count("_CHECK_STACK_SPACE"), 0)
1113-
self.assertEqual(uop_names.count("_CHECK_STACK_SPACE_OPERAND"), 1)
1114-
# max(12, 18 + max(12, 13)) == 31
1115-
largest_stack = (
1116-
_testinternalcapi.get_co_framesize(dummy18.__code__) +
1117-
_testinternalcapi.get_co_framesize(dummy13.__code__)
1118-
)
1119-
self.assertIn(("_CHECK_STACK_SPACE_OPERAND", largest_stack), uops_and_operands)
1111+
self.assertEqual(uop_names.count("_CHECK_STACK_SPACE_OPERAND"), 4)
1112+
self.assertIn(("_CHECK_STACK_SPACE_OPERAND",
1113+
_testinternalcapi.get_co_framesize(dummy12.__code__)), uops_and_operands)
1114+
self.assertIn(("_CHECK_STACK_SPACE_OPERAND",
1115+
_testinternalcapi.get_co_framesize(dummy13.__code__)), uops_and_operands)
1116+
self.assertIn(("_CHECK_STACK_SPACE_OPERAND",
1117+
_testinternalcapi.get_co_framesize(dummy18.__code__)), uops_and_operands)
11201118

1121-
@unittest.skip("gh-139109 WIP")
11221119
def test_combine_stack_space_checks_several_calls_different_order(self):
11231120
# same as `several_calls` but with top-level calls reversed
11241121
def dummy12(x):
@@ -1147,15 +1144,15 @@ def testfunc(n):
11471144
self.assertEqual(uop_names.count("_PUSH_FRAME"), 4)
11481145
self.assertEqual(uop_names.count("_RETURN_VALUE"), 4)
11491146
self.assertEqual(uop_names.count("_CHECK_STACK_SPACE"), 0)
1150-
self.assertEqual(uop_names.count("_CHECK_STACK_SPACE_OPERAND"), 1)
1151-
# max(18 + max(12, 13), 12) == 31
1152-
largest_stack = (
1153-
_testinternalcapi.get_co_framesize(dummy18.__code__) +
1154-
_testinternalcapi.get_co_framesize(dummy13.__code__)
1155-
)
1156-
self.assertIn(("_CHECK_STACK_SPACE_OPERAND", largest_stack), uops_and_operands)
1157-
1158-
@unittest.skip("gh-139109 WIP")
1147+
self.assertEqual(uop_names.count("_CHECK_STACK_SPACE_OPERAND"), 4)
1148+
self.assertIn(("_CHECK_STACK_SPACE_OPERAND",
1149+
_testinternalcapi.get_co_framesize(dummy12.__code__)), uops_and_operands)
1150+
self.assertIn(("_CHECK_STACK_SPACE_OPERAND",
1151+
_testinternalcapi.get_co_framesize(dummy13.__code__)), uops_and_operands)
1152+
self.assertIn(("_CHECK_STACK_SPACE_OPERAND",
1153+
_testinternalcapi.get_co_framesize(dummy18.__code__)), uops_and_operands)
1154+
1155+
@unittest.skip("reopen when we combine multiple stack space checks into one")
11591156
def test_combine_stack_space_complex(self):
11601157
def dummy0(x):
11611158
return x
@@ -1205,7 +1202,7 @@ def testfunc(n):
12051202
("_CHECK_STACK_SPACE_OPERAND", largest_stack), uops_and_operands
12061203
)
12071204

1208-
@unittest.skip("gh-139109 WIP")
1205+
@unittest.skip("reopen when we combine multiple stack space checks into one")
12091206
def test_combine_stack_space_checks_large_framesize(self):
12101207
# Create a function with a large framesize. This ensures _CHECK_STACK_SPACE is
12111208
# actually doing its job. Note that the resulting trace hits
@@ -1267,7 +1264,7 @@ def testfunc(n):
12671264
("_CHECK_STACK_SPACE_OPERAND", largest_stack), uops_and_operands
12681265
)
12691266

1270-
@unittest.skip("gh-139109 WIP")
1267+
@unittest.skip("reopen when we combine multiple stack space checks into one")
12711268
def test_combine_stack_space_checks_recursion(self):
12721269
def dummy15(x):
12731270
while x > 0:

‎Python/bytecodes.c‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5372,7 +5372,6 @@ dummy_func(
53725372
tier2 op(_CHECK_STACK_SPACE_OPERAND, (framesize/2 --)) {
53735373
assert(framesize <= INT_MAX);
53745374
DEOPT_IF(!_PyThreadState_HasStackSpace(tstate, framesize));
5375-
DEOPT_IF(tstate->py_recursion_remaining <= 1);
53765375
}
53775376

53785377
op(_SAVE_RETURN_OFFSET, (--)) {

‎Python/executor_cases.c.h‎

Lines changed: 0 additions & 26 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: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1068,13 +1068,17 @@ dummy_func(void) {
10681068
}
10691069

10701070
op(_CHECK_STACK_SPACE, (unused, unused, unused[oparg] -- unused, unused, unused[oparg])) {
1071+
assert((this_instr + 4)->opcode == _PUSH_FRAME);
1072+
PyCodeObject *co = get_code_with_logging((this_instr + 4));
1073+
if (co == NULL) {
1074+
ctx->done = true;
1075+
break;
1076+
}
1077+
ADD_OP(_CHECK_STACK_SPACE_OPERAND, 0, co->co_framesize);
10711078
}
10721079

10731080
op (_CHECK_STACK_SPACE_OPERAND, (framesize/2 -- )) {
10741081
(void)framesize;
1075-
/* We should never see _CHECK_STACK_SPACE_OPERANDs.
1076-
* They are only created at the end of this pass. */
1077-
Py_UNREACHABLE();
10781082
}
10791083

10801084
op(_PUSH_FRAME, (new_frame -- )) {

‎Python/optimizer_cases.c.h‎

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

0 commit comments

Comments
 (0)