GeezerVison is showing github.com — Fix CI: dead store elimination, formatting, snapshot · RustPython/RustPython@8b264e5
File treeExpand file treeCollapse file tree Open diff view settings Expand file treeCollapse file tree Open diff view settings Collapse file Copy file name to clipboardExpand all lines: Lib/test/test_peepholer.py -3Lines changed: 0 additions & 3 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -873,7 +873,6 @@ def f(): | 873 | 873 | self.assertInBytecode(f, 'LOAD_FAST_CHECK') | 874 | 874 | self.assertNotInBytecode(f, 'LOAD_FAST') | 875 | 875 |
| 876 | | - @unittest.expectedFailure # TODO: RUSTPYTHON; RETURN_VALUE | 877 | 876 | def test_load_fast_unknown_because_del(self): | 878 | 877 | def f(): | 879 | 878 | x = 1 | @@ -928,7 +927,6 @@ def f(): | 928 | 927 | self.assertInBytecode(f, 'LOAD_FAST_BORROW') | 929 | 928 | self.assertNotInBytecode(f, 'LOAD_FAST_CHECK') | 930 | 929 |
| 931 | | - @unittest.expectedFailure # TODO: RUSTPYTHON; L5 to L6 -> L6 [1] lasti | 932 | 930 | def test_load_fast_unknown_after_error(self): | 933 | 931 | def f(): | 934 | 932 | try: | @@ -940,7 +938,6 @@ def f(): | 940 | 938 | # Assert that it doesn't occur in the LOAD_FAST_CHECK branch. | 941 | 939 | self.assertInBytecode(f, 'LOAD_FAST_CHECK') | 942 | 940 |
| 943 | | - @unittest.expectedFailure # TODO: RUSTPYTHON; L5 to L6 -> L6 [1] lasti | 944 | 941 | def test_load_fast_unknown_after_error_2(self): | 945 | 942 | def f(): | 946 | 943 | try: | |
Collapse file Copy file name to clipboardExpand all lines: crates/codegen/src/compile.rs +12Lines changed: 12 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -2624,7 +2624,19 @@ impl Compiler { | 2624 | 2624 | for elt in value_elts { | 2625 | 2625 | self.compile_expression(elt)?; | 2626 | 2626 | } | | 2627 | + // Stores happen in reverse (TOS = last value → first store). | | 2628 | + // When the same name appears multiple times in the target | | 2629 | + // tuple, only the first store (getting the rightmost value) | | 2630 | + // matters; later stores to the same name are dead. | | 2631 | + // Replace them with POP_TOP (apply_static_swaps). | | 2632 | + let mut seen = std::collections::HashSet::new(); | 2627 | 2633 | for target in target_elts.iter().rev() { | | 2634 | + if let ast::Expr::Name(ast::ExprName { id, .. }) = target | | 2635 | + && !seen.insert(id.as_str()) | | 2636 | + { | | 2637 | + emit!(self, Instruction::PopTop); | | 2638 | + continue; | | 2639 | + } | 2628 | 2640 | self.compile_store(target)?; | 2629 | 2641 | } | 2630 | 2642 | return Ok(()); | |
Collapse file Copy file name to clipboardExpand all lines: crates/codegen/src/ir.rs +91-26Lines changed: 91 additions & 26 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -209,6 +209,9 @@ impl CodeInfo { | 209 | 209 |
| 210 | 210 | // DCE always runs (removes dead code after terminal instructions) | 211 | 211 | self.dce(); | | 212 | + // Dead store elimination for duplicate STORE_FAST targets | | 213 | + // (apply_static_swaps in CPython's flowgraph.c) | | 214 | + self.eliminate_dead_stores(); | 212 | 215 | // Peephole optimizer creates superinstructions matching CPython | 213 | 216 | self.peephole_optimize(); | 214 | 217 |
| @@ -1328,6 +1331,65 @@ impl CodeInfo { | 1328 | 1331 | } | 1329 | 1332 | } | 1330 | 1333 |
| | 1334 | + /// Eliminate dead stores in STORE_FAST sequences (apply_static_swaps). | | 1335 | + /// | | 1336 | + /// In sequences of consecutive STORE_FAST instructions (from tuple unpacking), | | 1337 | + /// if the same variable is stored to more than once, only the first store | | 1338 | + /// (which gets TOS — the rightmost value) matters. Later stores to the | | 1339 | + /// same variable are dead and replaced with POP_TOP. | | 1340 | + /// Simplified apply_static_swaps (CPython flowgraph.c): | | 1341 | + /// In STORE_FAST sequences that follow UNPACK_SEQUENCE / UNPACK_EX, | | 1342 | + /// replace duplicate stores to the same variable with POP_TOP. | | 1343 | + /// UNPACK pushes values so stores execute left-to-right; the LAST | | 1344 | + /// store to a variable carries the final value, earlier ones are dead. | | 1345 | + fn eliminate_dead_stores(&mut self) { | | 1346 | + for block in &mut self.blocks { | | 1347 | + let instrs = &mut block.instructions; | | 1348 | + let len = instrs.len(); | | 1349 | + let mut i = 0; | | 1350 | + while i < len { | | 1351 | + // Look for UNPACK_SEQUENCE or UNPACK_EX | | 1352 | + let is_unpack = matches!( | | 1353 | + instrs[i].instr, | | 1354 | + AnyInstruction::Real( | | 1355 | + Instruction::UnpackSequence { .. } | Instruction::UnpackEx { .. } | | 1356 | + ) | | 1357 | + ); | | 1358 | + if !is_unpack { | | 1359 | + i += 1; | | 1360 | + continue; | | 1361 | + } | | 1362 | + // Scan the run of STORE_FAST right after the unpack | | 1363 | + let run_start = i + 1; | | 1364 | + let mut run_end = run_start; | | 1365 | + while run_end < len | | 1366 | + && matches!( | | 1367 | + instrs[run_end].instr, | | 1368 | + AnyInstruction::Real(Instruction::StoreFast { .. }) | | 1369 | + ) | | 1370 | + { | | 1371 | + run_end += 1; | | 1372 | + } | | 1373 | + if run_end - run_start >= 2 { | | 1374 | + // Pass 1: find the LAST occurrence of each variable | | 1375 | + let mut last_occurrence = std::collections::HashMap::new(); | | 1376 | + for (j, instr) in instrs[run_start..run_end].iter().enumerate() { | | 1377 | + last_occurrence.insert(u32::from(instr.arg), j); | | 1378 | + } | | 1379 | + // Pass 2: non-last stores to the same variable are dead | | 1380 | + for (j, instr) in instrs[run_start..run_end].iter_mut().enumerate() { | | 1381 | + let idx = u32::from(instr.arg); | | 1382 | + if last_occurrence[&idx] != j { | | 1383 | + instr.instr = AnyInstruction::Real(Instruction::PopTop); | | 1384 | + instr.arg = OpArg::new(0); | | 1385 | + } | | 1386 | + } | | 1387 | + } | | 1388 | + i = run_end.max(i + 1); | | 1389 | + } | | 1390 | + } | | 1391 | + } | | 1392 | + | 1331 | 1393 | /// Peephole optimization: combine consecutive instructions into super-instructions | 1332 | 1394 | fn peephole_optimize(&mut self) { | 1333 | 1395 | for block in &mut self.blocks { | @@ -1353,41 +1415,44 @@ impl CodeInfo { | 1353 | 1415 | if line1 > 0 && line2 > 0 && line1 != line2 { | 1354 | 1416 | None | 1355 | 1417 | } else { | 1356 | | - let idx1 = u32::from(curr.arg); | 1357 | | - let idx2 = u32::from(next.arg); | 1358 | | - if idx1 < 16 && idx2 < 16 { | 1359 | | - let packed = (idx1 << 4) | idx2; | 1360 | | - Some(( | 1361 | | - Instruction::LoadFastLoadFast { | 1362 | | - var_nums: Arg::marker(), | 1363 | | - }, | 1364 | | - OpArg::new(packed), | 1365 | | - )) | 1366 | | - } else { | 1367 | | - None | 1368 | | - } | | 1418 | + let idx1 = u32::from(curr.arg); | | 1419 | + let idx2 = u32::from(next.arg); | | 1420 | + if idx1 < 16 && idx2 < 16 { | | 1421 | + let packed = (idx1 << 4) | idx2; | | 1422 | + Some(( | | 1423 | + Instruction::LoadFastLoadFast { | | 1424 | + var_nums: Arg::marker(), | | 1425 | + }, | | 1426 | + OpArg::new(packed), | | 1427 | + )) | | 1428 | + } else { | | 1429 | + None | | 1430 | + } | 1369 | 1431 | } | 1370 | 1432 | } | 1371 | 1433 | // StoreFast + StoreFast -> StoreFastStoreFast (if both indices < 16) | | 1434 | + // Dead store elimination: if both store to the same variable, | | 1435 | + // the first store is dead. Replace it with POP_TOP (like | | 1436 | + // apply_static_swaps in CPython's flowgraph.c). | 1372 | 1437 | (Instruction::StoreFast { .. }, Instruction::StoreFast { .. }) => { | 1373 | 1438 | let line1 = curr.location.line.get() as i32; | 1374 | 1439 | let line2 = next.location.line.get() as i32; | 1375 | 1440 | if line1 > 0 && line2 > 0 && line1 != line2 { | 1376 | 1441 | None | 1377 | 1442 | } else { | 1378 | | - let idx1 = u32::from(curr.arg); | 1379 | | - let idx2 = u32::from(next.arg); | 1380 | | - if idx1 < 16 && idx2 < 16 { | 1381 | | - let packed = (idx1 << 4) | idx2; | 1382 | | - Some(( | 1383 | | - Instruction::StoreFastStoreFast { | 1384 | | - var_nums: Arg::marker(), | 1385 | | - }, | 1386 | | - OpArg::new(packed), | 1387 | | - )) | 1388 | | - } else { | 1389 | | - None | 1390 | | - } | | 1443 | + let idx1 = u32::from(curr.arg); | | 1444 | + let idx2 = u32::from(next.arg); | | 1445 | + if idx1 < 16 && idx2 < 16 { | | 1446 | + let packed = (idx1 << 4) | idx2; | | 1447 | + Some(( | | 1448 | + Instruction::StoreFastStoreFast { | | 1449 | + var_nums: Arg::marker(), | | 1450 | + }, | | 1451 | + OpArg::new(packed), | | 1452 | + )) | | 1453 | + } else { | | 1454 | + None | | 1455 | + } | 1391 | 1456 | } | 1392 | 1457 | } | 1393 | 1458 | // Note: StoreFast + LoadFast → StoreFastLoadFast is done in a | |
Collapse file Copy file name to clipboardExpand all lines: crates/codegen/src/snapshots/rustpython_codegen__compile__tests__nested_double_async_with.snap +29-29Lines changed: 29 additions & 29 deletions Collapse file Copy file name to clipboardExpand all lines: crates/vm/src/vm/mod.rs +1-3Lines changed: 1 addition & 3 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -135,9 +135,7 @@ struct ExceptionStack { | 135 | 135 | impl Default for ExceptionStack { | 136 | 136 | fn default() -> Self { | 137 | 137 | // Thread's base `_PyErr_StackItem` – always present. | 138 | | - Self { | 139 | | - stack: vec![None], | 140 | | - } | | 138 | + Self { stack: vec![None] } | 141 | 139 | } | 142 | 140 | } | 143 | 141 |
| |
Collapse file Copy file name to clipboardExpand all lines: scripts/compare_bytecode.py +25-6Lines changed: 25 additions & 6 deletions | Original file line number | Diff line number | Diff line change |
|---|
| 25 | 25 | PROJECT_ROOT = os.path.dirname(SCRIPT_DIR) | 26 | 26 | DIS_DUMP = os.path.join(SCRIPT_DIR, "dis_dump.py") | 27 | 27 | DEFAULT_REPORT = os.path.join(PROJECT_ROOT, "compare_bytecode.report") | | 28 | + | | 29 | + | 28 | 30 | def find_rustpython(): | 29 | 31 | """Locate the RustPython binary, allowing release builds only.""" | 30 | 32 | if "RUSTPYTHON" in os.environ: | @@ -47,7 +49,9 @@ def collect_targets(lib_dir, pattern=None): | 47 | 49 | """Collect Python files to compare, relative to lib_dir.""" | 48 | 50 | targets = [] | 49 | 51 | for root, dirs, files in os.walk(lib_dir): | 50 | | - dirs[:] = sorted(d for d in dirs if d != "__pycache__" and not d.startswith(".")) | | 52 | + dirs[:] = sorted( | | 53 | + d for d in dirs if d != "__pycache__" and not d.startswith(".") | | 54 | + ) | 51 | 55 | for fname in sorted(files): | 52 | 56 | if not fname.endswith(".py"): | 53 | 57 | continue | @@ -222,7 +226,9 @@ def compare_code_summary(cp_code, rp_code): | 222 | 226 | rp_list = rp_by_name.get(name, []) | 223 | 227 | for i in range(max(len(cp_list), len(rp_list))): | 224 | 228 | if i < len(cp_list) and i < len(rp_list): | 225 | | - child_objects, child_insts = compare_code_summary(cp_list[i], rp_list[i]) | | 229 | + child_objects, child_insts = compare_code_summary( | | 230 | + cp_list[i], rp_list[i] | | 231 | + ) | 226 | 232 | diff_code_objects += child_objects | 227 | 233 | diff_instructions += child_insts | 228 | 234 | else: | @@ -308,7 +314,10 @@ def main(): | 308 | 314 | sys.exit(1) | 309 | 315 | if not os.path.isfile(DIS_DUMP): | 310 | 316 | print("Error: disassembler helper not found: %s" % DIS_DUMP, file=sys.stderr) | 311 | | - print(" Expected scripts/dis_dump.py from origin/bytecode-parity", file=sys.stderr) | | 317 | + print( | | 318 | + " Expected scripts/dis_dump.py from origin/bytecode-parity", | | 319 | + file=sys.stderr, | | 320 | + ) | 312 | 321 | sys.exit(1) | 313 | 322 |
| 314 | 323 | targets = collect_targets(args.lib_dir, args.filter) | @@ -467,10 +476,16 @@ def pct(n): | 467 | 476 | else: | 468 | 477 | list_limit = 0 if args.summary_json else max(args.list_limit, 0) | 469 | 478 | if diff_summaries and list_limit: | 470 | | - p("Top differing files (%d shown of %d):" % (min(list_limit, len(diff_summaries)), len(diff_summaries))) | | 479 | + shown = min(list_limit, len(diff_summaries)) | | 480 | + total = len(diff_summaries) | | 481 | + p(f"Top differing files ({shown} shown of {total}):") | 471 | 482 | top = sorted( | 472 | 483 | diff_summaries, | 473 | | - key=lambda item: (item["diff_instructions"], item["diff_code_objects"], item["path"]), | | 484 | + key=lambda item: ( | | 485 | + item["diff_instructions"], | | 486 | + item["diff_code_objects"], | | 487 | + item["path"], | | 488 | + ), | 474 | 489 | reverse=True, | 475 | 490 | )[:list_limit] | 476 | 491 | for item in top: | @@ -504,7 +519,11 @@ def pct(n): | 504 | 519 | else [item["path"] for item in diff_summaries], | 505 | 520 | "top_diff_files": sorted( | 506 | 521 | diff_summaries, | 507 | | - key=lambda item: (item["diff_instructions"], item["diff_code_objects"], item["path"]), | | 522 | + key=lambda item: ( | | 523 | + item["diff_instructions"], | | 524 | + item["diff_code_objects"], | | 525 | + item["path"], | | 526 | + ), | 508 | 527 | reverse=True, | 509 | 528 | )[: min(20, len(diff_summaries))], | 510 | 529 | "rp_error_files": [fp for fp, _ in rp_error_files], | |
Collapse file Copy file name to clipboardExpand all lines: scripts/dis_dump.py +5-2Lines changed: 5 additions & 2 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -243,7 +243,8 @@ def _extract_instructions(code): | 243 | 243 | if target_idx is None or argval_is_raw: | 244 | 244 | target_idx = None # force recalculation | 245 | 245 | if is_backward: | 246 | | - # Target = current_offset + INSTRUCTION_SIZE + cache_size - arg * INSTRUCTION_SIZE | | 246 | + # Target = current_offset + INSTR_SIZE + cache | | 247 | + # - arg * INSTR_SIZE | 247 | 248 | # Try different cache sizes (NOT_TAKEN=1 for JUMP_BACKWARD, 0 for NO_INTERRUPT) | 248 | 249 | if "NO_INTERRUPT" in inst.opname: | 249 | 250 | cache_order = (0, 1, 2) | @@ -335,7 +336,9 @@ def main(): | 335 | 336 | default=None, | 336 | 337 | help="Read newline-separated target paths from this file", | 337 | 338 | ) | 338 | | - parser.add_argument("targets", nargs="*", help="Python files or directories to process") | | 339 | + parser.add_argument( | | 340 | + "targets", nargs="*", help="Python files or directories to process" | | 341 | + ) | 339 | 342 | parser.add_argument( | 340 | 343 | "--progress", | 341 | 344 | type=int, | |
|