Skip to content

Commit 876791a

Browse files
youknowoneyouknowone
committed
Fix CI: dead store elimination, formatting, snapshot
- Add eliminate_dead_stores pass before peephole_optimize: consecutive STORE_FAST to the same variable are replaced with POP_TOP (apply_static_swaps from CPython flowgraph.c) - Remove 3 expectedFailure decorators in test_peepholer (test_load_fast_unknown_* now pass) - Accept updated async_with snapshot - Fix formatting in ir.rs, mod.rs, Python scripts
1 parent a2bb616 commit 876791a

File tree

7 files changed

+163
-69
lines changed

7 files changed

+163
-69
lines changed

‎Lib/test/test_peepholer.py‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -873,7 +873,6 @@ def f():
873873
self.assertInBytecode(f, 'LOAD_FAST_CHECK')
874874
self.assertNotInBytecode(f, 'LOAD_FAST')
875875

876-
@unittest.expectedFailure # TODO: RUSTPYTHON; RETURN_VALUE
877876
def test_load_fast_unknown_because_del(self):
878877
def f():
879878
x = 1
@@ -928,7 +927,6 @@ def f():
928927
self.assertInBytecode(f, 'LOAD_FAST_BORROW')
929928
self.assertNotInBytecode(f, 'LOAD_FAST_CHECK')
930929

931-
@unittest.expectedFailure # TODO: RUSTPYTHON; L5 to L6 -> L6 [1] lasti
932930
def test_load_fast_unknown_after_error(self):
933931
def f():
934932
try:
@@ -940,7 +938,6 @@ def f():
940938
# Assert that it doesn't occur in the LOAD_FAST_CHECK branch.
941939
self.assertInBytecode(f, 'LOAD_FAST_CHECK')
942940

943-
@unittest.expectedFailure # TODO: RUSTPYTHON; L5 to L6 -> L6 [1] lasti
944941
def test_load_fast_unknown_after_error_2(self):
945942
def f():
946943
try:

‎crates/codegen/src/compile.rs‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2624,7 +2624,19 @@ impl Compiler {
26242624
for elt in value_elts {
26252625
self.compile_expression(elt)?;
26262626
}
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();
26272633
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+
}
26282640
self.compile_store(target)?;
26292641
}
26302642
return Ok(());

‎crates/codegen/src/ir.rs‎

Lines changed: 91 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,9 @@ impl CodeInfo {
209209

210210
// DCE always runs (removes dead code after terminal instructions)
211211
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();
212215
// Peephole optimizer creates superinstructions matching CPython
213216
self.peephole_optimize();
214217

@@ -1328,6 +1331,65 @@ impl CodeInfo {
13281331
}
13291332
}
13301333

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+
13311393
/// Peephole optimization: combine consecutive instructions into super-instructions
13321394
fn peephole_optimize(&mut self) {
13331395
for block in &mut self.blocks {
@@ -1353,41 +1415,44 @@ impl CodeInfo {
13531415
if line1 > 0 && line2 > 0 && line1 != line2 {
13541416
None
13551417
} 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+
}
13691431
}
13701432
}
13711433
// 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).
13721437
(Instruction::StoreFast { .. }, Instruction::StoreFast { .. }) => {
13731438
let line1 = curr.location.line.get() as i32;
13741439
let line2 = next.location.line.get() as i32;
13751440
if line1 > 0 && line2 > 0 && line1 != line2 {
13761441
None
13771442
} 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+
}
13911456
}
13921457
}
13931458
// Note: StoreFast + LoadFast → StoreFastLoadFast is done in a

‎crates/codegen/src/snapshots/rustpython_codegen__compile__tests__nested_double_async_with.snap‎

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

‎crates/vm/src/vm/mod.rs‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -135,9 +135,7 @@ struct ExceptionStack {
135135
impl Default for ExceptionStack {
136136
fn default() -> Self {
137137
// Thread's base `_PyErr_StackItem` – always present.
138-
Self {
139-
stack: vec![None],
140-
}
138+
Self { stack: vec![None] }
141139
}
142140
}
143141

‎scripts/compare_bytecode.py‎

Lines changed: 25 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@
2525
PROJECT_ROOT = os.path.dirname(SCRIPT_DIR)
2626
DIS_DUMP = os.path.join(SCRIPT_DIR, "dis_dump.py")
2727
DEFAULT_REPORT = os.path.join(PROJECT_ROOT, "compare_bytecode.report")
28+
29+
2830
def find_rustpython():
2931
"""Locate the RustPython binary, allowing release builds only."""
3032
if "RUSTPYTHON" in os.environ:
@@ -47,7 +49,9 @@ def collect_targets(lib_dir, pattern=None):
4749
"""Collect Python files to compare, relative to lib_dir."""
4850
targets = []
4951
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+
)
5155
for fname in sorted(files):
5256
if not fname.endswith(".py"):
5357
continue
@@ -222,7 +226,9 @@ def compare_code_summary(cp_code, rp_code):
222226
rp_list = rp_by_name.get(name, [])
223227
for i in range(max(len(cp_list), len(rp_list))):
224228
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+
)
226232
diff_code_objects += child_objects
227233
diff_instructions += child_insts
228234
else:
@@ -308,7 +314,10 @@ def main():
308314
sys.exit(1)
309315
if not os.path.isfile(DIS_DUMP):
310316
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+
)
312321
sys.exit(1)
313322

314323
targets = collect_targets(args.lib_dir, args.filter)
@@ -467,10 +476,16 @@ def pct(n):
467476
else:
468477
list_limit = 0 if args.summary_json else max(args.list_limit, 0)
469478
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}):")
471482
top = sorted(
472483
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+
),
474489
reverse=True,
475490
)[:list_limit]
476491
for item in top:
@@ -504,7 +519,11 @@ def pct(n):
504519
else [item["path"] for item in diff_summaries],
505520
"top_diff_files": sorted(
506521
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+
),
508527
reverse=True,
509528
)[: min(20, len(diff_summaries))],
510529
"rp_error_files": [fp for fp, _ in rp_error_files],

‎scripts/dis_dump.py‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -243,7 +243,8 @@ def _extract_instructions(code):
243243
if target_idx is None or argval_is_raw:
244244
target_idx = None # force recalculation
245245
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
247248
# Try different cache sizes (NOT_TAKEN=1 for JUMP_BACKWARD, 0 for NO_INTERRUPT)
248249
if "NO_INTERRUPT" in inst.opname:
249250
cache_order = (0, 1, 2)
@@ -335,7 +336,9 @@ def main():
335336
default=None,
336337
help="Read newline-separated target paths from this file",
337338
)
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+
)
339342
parser.add_argument(
340343
"--progress",
341344
type=int,

0 commit comments

Comments
 (0)