Skip to content

Commit f05c6af

Browse files
committed
Replace _CHECK_STACK_SPACE with _CHECK_STACK_SPACE_OPERAND in JIT optimizer
1 parent 40d07ca commit f05c6af

File tree

3 files changed

+56
-68
lines changed

3 files changed

+56
-68
lines changed

Lib/test/test_capi/test_opt.py

Lines changed: 38 additions & 64 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,14 @@ 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)
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)
11571154

1158-
@unittest.skip("gh-139109 WIP")
11591155
def test_combine_stack_space_complex(self):
11601156
def dummy0(x):
11611157
return x
@@ -1193,19 +1189,8 @@ def testfunc(n):
11931189
self.assertEqual(uop_names.count("_RETURN_VALUE"), 15)
11941190

11951191
self.assertEqual(uop_names.count("_CHECK_STACK_SPACE"), 0)
1196-
self.assertEqual(uop_names.count("_CHECK_STACK_SPACE_OPERAND"), 1)
1197-
largest_stack = (
1198-
_testinternalcapi.get_co_framesize(dummy6.__code__) +
1199-
_testinternalcapi.get_co_framesize(dummy5.__code__) +
1200-
_testinternalcapi.get_co_framesize(dummy2.__code__) +
1201-
_testinternalcapi.get_co_framesize(dummy1.__code__) +
1202-
_testinternalcapi.get_co_framesize(dummy0.__code__)
1203-
)
1204-
self.assertIn(
1205-
("_CHECK_STACK_SPACE_OPERAND", largest_stack), uops_and_operands
1206-
)
1192+
self.assertEqual(uop_names.count("_CHECK_STACK_SPACE_OPERAND"), 15)
12071193

1208-
@unittest.skip("gh-139109 WIP")
12091194
def test_combine_stack_space_checks_large_framesize(self):
12101195
# Create a function with a large framesize. This ensures _CHECK_STACK_SPACE is
12111196
# actually doing its job. Note that the resulting trace hits
@@ -1250,24 +1235,13 @@ def testfunc(n):
12501235

12511236
uops_and_operands = [(opcode, operand) for opcode, _, _, operand in ex]
12521237
uop_names = [uop[0] for uop in uops_and_operands]
1253-
self.assertEqual(uop_names.count("_PUSH_FRAME"), 2)
1254-
self.assertEqual(uop_names.count("_CHECK_STACK_SPACE_OPERAND"), 1)
1255-
1256-
# this hits a different case during trace projection in refcount test runs only,
1257-
# so we need to account for both possibilities
1258-
self.assertIn(uop_names.count("_CHECK_STACK_SPACE"), [0, 1])
1259-
if uop_names.count("_CHECK_STACK_SPACE") == 0:
1260-
largest_stack = (
1261-
_testinternalcapi.get_co_framesize(dummy15.__code__) +
1262-
_testinternalcapi.get_co_framesize(dummy_large.__code__)
1263-
)
1264-
else:
1265-
largest_stack = _testinternalcapi.get_co_framesize(dummy15.__code__)
1266-
self.assertIn(
1267-
("_CHECK_STACK_SPACE_OPERAND", largest_stack), uops_and_operands
1268-
)
1238+
self.assertGreaterEqual(uop_names.count("_PUSH_FRAME"), 1)
1239+
self.assertEqual(uop_names.count("_CHECK_STACK_SPACE"), 0)
1240+
self.assertEqual(uop_names.count("_CHECK_STACK_SPACE_OPERAND"),
1241+
uop_names.count("_PUSH_FRAME"))
1242+
self.assertIn(("_CHECK_STACK_SPACE_OPERAND",
1243+
_testinternalcapi.get_co_framesize(dummy15.__code__)), uops_and_operands)
12691244

1270-
@unittest.skip("gh-139109 WIP")
12711245
def test_combine_stack_space_checks_recursion(self):
12721246
def dummy15(x):
12731247
while x > 0:
@@ -1290,12 +1264,12 @@ def testfunc(n):
12901264

12911265
uops_and_operands = [(opcode, operand) for opcode, _, _, operand in ex]
12921266
uop_names = [uop[0] for uop in uops_and_operands]
1293-
self.assertEqual(uop_names.count("_PUSH_FRAME"), 2)
1267+
self.assertEqual(uop_names.count("_PUSH_FRAME"), 12)
12941268
self.assertEqual(uop_names.count("_RETURN_VALUE"), 0)
1295-
self.assertEqual(uop_names.count("_CHECK_STACK_SPACE"), 1)
1296-
self.assertEqual(uop_names.count("_CHECK_STACK_SPACE_OPERAND"), 1)
1297-
largest_stack = _testinternalcapi.get_co_framesize(dummy15.__code__)
1298-
self.assertIn(("_CHECK_STACK_SPACE_OPERAND", largest_stack), uops_and_operands)
1269+
self.assertEqual(uop_names.count("_CHECK_STACK_SPACE"), 0)
1270+
self.assertEqual(uop_names.count("_CHECK_STACK_SPACE_OPERAND"), 12)
1271+
framesize = _testinternalcapi.get_co_framesize(dummy15.__code__)
1272+
self.assertIn(("_CHECK_STACK_SPACE_OPERAND", framesize), uops_and_operands)
12991273

13001274
def test_many_nested(self):
13011275
# overflow the trace_stack

Python/optimizer_bytecodes.c

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1088,13 +1088,19 @@ dummy_func(void) {
10881088
}
10891089

10901090
op(_CHECK_STACK_SPACE, (unused, unused, unused[oparg] -- unused, unused, unused[oparg])) {
1091+
assert((this_instr + 1)->opcode == _CHECK_RECURSION_REMAINING);
1092+
assert((this_instr + 4)->opcode == _PUSH_FRAME);
1093+
PyCodeObject *co = get_code_with_logging((this_instr + 4));
1094+
if (co == NULL) {
1095+
ctx->done = true;
1096+
break;
1097+
}
1098+
ADD_OP(_CHECK_STACK_SPACE_OPERAND, 0, co->co_framesize);
1099+
REPLACE_OP((this_instr + 1), _NOP, 0, 0);
10911100
}
10921101

10931102
op (_CHECK_STACK_SPACE_OPERAND, (framesize/2 -- )) {
10941103
(void)framesize;
1095-
/* We should never see _CHECK_STACK_SPACE_OPERANDs.
1096-
* They are only created at the end of this pass. */
1097-
Py_UNREACHABLE();
10981104
}
10991105

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

Python/optimizer_cases.c.h

Lines changed: 9 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)