From 03a95944280d39b78308bd2bfc21ccb14c861587 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Wed, 20 May 2026 19:12:18 +0900 Subject: [PATCH 1/3] Align cleanup bytecode layout with CPython --- crates/codegen/src/compile.rs | 617 ++++++++++++++++++++++++++++++++-- crates/codegen/src/ir.rs | 200 ++++++++++- 2 files changed, 782 insertions(+), 35 deletions(-) diff --git a/crates/codegen/src/compile.rs b/crates/codegen/src/compile.rs index 2e9703a9bd..013fd1d967 100644 --- a/crates/codegen/src/compile.rs +++ b/crates/codegen/src/compile.rs @@ -836,6 +836,10 @@ impl Compiler { }) } + fn statements_are_single_with(body: &[ast::Stmt]) -> bool { + matches!(body, [ast::Stmt::With(_)]) + } + fn statements_end_with_try_finally(body: &[ast::Stmt]) -> bool { body.last().is_some_and(|stmt| { matches!( @@ -1089,6 +1093,11 @@ impl Compiler { }) } + fn statements_end_with_conditional(body: &[ast::Stmt]) -> bool { + body.last() + .is_some_and(|stmt| matches!(stmt, ast::Stmt::If(_))) + } + fn statements_end_with_conditional_scope_exit(&self, body: &[ast::Stmt]) -> bool { body.last().is_some_and(|stmt| match stmt { ast::Stmt::Assert(_) => self.opts.optimize == 0, @@ -1168,11 +1177,9 @@ impl Compiler { fn statements_end_with_loop_fallthrough(&mut self, body: &[ast::Stmt]) -> CompileResult { match body.last() { - Some(ast::Stmt::For(ast::StmtFor { body, .. })) => { - Ok(!Self::statements_contain_direct_break(body)) - } Some(ast::Stmt::While(ast::StmtWhile { test, body, .. })) => { - Ok(!matches!(self.constant_expr_truthiness(test)?, Some(true)) + Ok(!matches!(test.as_ref(), ast::Expr::BoolOp(_)) + && !matches!(self.constant_expr_truthiness(test)?, Some(true)) && !Self::statements_contain_direct_break(body)) } _ => Ok(false), @@ -1192,6 +1199,38 @@ impl Compiler { } } + fn statements_end_with_while_true_without_break( + &mut self, + body: &[ast::Stmt], + ) -> CompileResult { + match body.last() { + Some(ast::Stmt::While(ast::StmtWhile { test, body, .. })) => { + Ok(matches!(self.constant_expr_truthiness(test)?, Some(true)) + && !Self::statements_contain_direct_break(body)) + } + _ => Ok(false), + } + } + + fn statements_are_single_with_while_true_without_break( + &mut self, + body: &[ast::Stmt], + ) -> CompileResult { + let [ast::Stmt::With(ast::StmtWith { body, is_async, .. })] = body else { + return Ok(false); + }; + if *is_async { + return Ok(false); + } + match body.last() { + Some(ast::Stmt::While(ast::StmtWhile { test, body, .. })) => { + Ok(matches!(self.constant_expr_truthiness(test)?, Some(true)) + && !Self::statements_contain_direct_break(body)) + } + _ => Ok(false), + } + } + fn statements_end_with_while_true_tail_direct_break( &mut self, body: &[ast::Stmt], @@ -1360,17 +1399,6 @@ impl Compiler { } } - fn has_resuming_bare_except(handlers: &[ast::ExceptHandler]) -> bool { - handlers.iter().any(|handler| { - let ast::ExceptHandler::ExceptHandler(ast::ExceptHandlerExceptHandler { - type_, - body, - .. - }) = handler; - type_.is_none() && !Self::statements_end_with_scope_exit(body) - }) - } - fn statements_end_with_optimized_finally_entry_scope_exit(&self, body: &[ast::Stmt]) -> bool { body.last() .is_some_and(|stmt| self.statement_ends_with_optimized_finally_entry_scope_exit(stmt)) @@ -1394,8 +1422,7 @@ impl Compiler { finalbody, .. }) => { - !finalbody.is_empty() - && !Self::statements_end_with_open_conditional_fallthrough(finalbody) + !finalbody.is_empty() && !Self::statements_end_with_conditional(finalbody) || (!handlers.is_empty() && Self::statements_end_with_scope_exit(body)) } ast::Stmt::If(ast::StmtIf { @@ -4347,6 +4374,7 @@ impl Compiler { let preserve_finally_exit_empty_label = (self.fallthrough_has_statement_successor || preserve_async_try_except_finally_scope_exit) && (!handlers.is_empty() + || Self::statements_are_single_with(finalbody) || Self::statements_contain_for_with_conditional_body(finalbody) || (preserve_async_try_except_finally_scope_exit && Self::statements_contain_conditional_scope_exit(finalbody))); @@ -4380,10 +4408,6 @@ impl Compiler { // End block - continuation point after try-finally // Normal path jumps here to skip exception path blocks let end_block = self.new_block(); - if Self::has_resuming_bare_except(handlers) { - self.disable_load_fast_borrow_for_block(end_block); - } - // Emit NOP at the try: line so LINE events fire for it emit!(self, Instruction::Nop); @@ -4414,6 +4438,8 @@ impl Compiler { if handlers.is_empty() { let preserve_finally_entry_nop = self.preserves_finally_entry_nop(body) || self.statements_end_with_loop_fallthrough(body)?; + let force_remove_finally_entry_nop = + self.statements_are_single_with_while_true_without_break(body)?; let preserve_while_break_end_label_before_finally = self.statements_end_with_while_true_tail_direct_break(body)?; @@ -4426,6 +4452,9 @@ impl Compiler { emit!(self, PseudoInstruction::PopBlock); if preserve_finally_entry_nop { self.preserve_last_redundant_nop(); + } else if force_remove_finally_entry_nop { + self.set_no_location(); + self.force_remove_last_no_location_nop(); } else { self.set_no_location(); self.remove_last_no_location_nop(); @@ -4913,10 +4942,6 @@ impl Compiler { let handlers_stop_before_try_end = handlers_end_with_scope_exit || (handlers_end_with_continue && self.ctx.loop_data.is_some()); let body_exits_scope = Self::statements_end_with_scope_exit(body); - if Self::has_resuming_bare_except(handlers) { - self.disable_load_fast_borrow_for_block(end_block); - } - emit!( self, PseudoInstruction::SetupFinally { @@ -7372,8 +7397,10 @@ impl Compiler { self.compile_with(items, body, is_async)?; } - let nested_multiline_with_cleanup_target_nop = - !is_async && Self::statements_end_with_scope_exit(body) && { + let nested_multiline_with_cleanup_target_nop = !is_async + && !self.fallthrough_has_local_statement_successor + && Self::statements_end_with_scope_exit(body) + && { let parent_with_ranges: Vec<_> = self .current_code_info() .fblock @@ -7397,8 +7424,9 @@ impl Compiler { || Self::statements_end_with_try_except_else_handler_scope_exit(body) || Self::statements_end_with_try_finally(body) || self.statements_end_with_loop_fallthrough(body)?); - let remove_while_true_break_cleanup_target_nop = - !is_async && self.statements_end_with_while_true_direct_break(body)?; + let remove_while_true_break_cleanup_target_nop = !is_async + && (self.statements_end_with_while_true_direct_break(body)? + || self.statements_end_with_while_true_without_break(body)?); let preserve_while_true_tail_break_successor_load_fast = !is_async && self.statements_end_with_while_true_tail_direct_break(body)?; let preserve_try_except_tail_successor_load_fast = @@ -18562,6 +18590,63 @@ def f(self, f, closed, new_key): ); } + #[test] + fn test_nested_finally_closed_conditional_falls_through_without_extra_entry_nop() { + let code = compile_exec( + "\ +def f(was_enabled, faulthandler, sys, orig_stderr): + try: + try: + faulthandler.enable() + faulthandler.disable() + finally: + if was_enabled: + faulthandler.enable() + else: + faulthandler.disable() + finally: + sys.stderr = orig_stderr +", + ); + let f = find_code(&code, "f").expect("missing f code"); + let ops: Vec<_> = f + .instructions + .iter() + .map(|unit| unit.op) + .filter(|op| !matches!(op, Instruction::Cache | Instruction::NotTaken)) + .collect(); + + assert!( + ops.windows(3).any(|window| { + matches!( + window, + [ + Instruction::Nop, + Instruction::LoadFastBorrowLoadFastBorrow { .. } + | Instruction::LoadFastLoadFast { .. }, + Instruction::StoreAttr { .. }, + ] + ) + }), + "CPython keeps the inner finally cleanup anchor before the outer finalbody, got ops={ops:?}" + ); + assert!( + !ops.windows(4).any(|window| { + matches!( + window, + [ + Instruction::Nop, + Instruction::Nop, + Instruction::LoadFastBorrowLoadFastBorrow { .. } + | Instruction::LoadFastLoadFast { .. }, + Instruction::StoreAttr { .. }, + ] + ) + }), + "closed conditional inner finalbody should not add a second outer finalbody-entry NOP, got ops={ops:?}" + ); + } + #[test] fn test_with_try_finally_normal_cleanup_keeps_redundant_jump_nop() { let code = compile_exec( @@ -19131,6 +19216,133 @@ def f(close, dup, first, second): ); } + #[test] + fn test_try_finally_boolop_while_fallthrough_drops_finalbody_entry_nop() { + let code = compile_exec( + "\ +def f(active, socket_map, asyncore): + try: + while active and socket_map: + asyncore.loop(timeout=0.1, count=1) + finally: + asyncore.close_all(ignore_all=True) +", + ); + let f = find_code(&code, "f").expect("missing f code"); + let ops: Vec<_> = f + .instructions + .iter() + .map(|unit| unit.op) + .filter(|op| !matches!(op, Instruction::Cache)) + .collect(); + + assert!( + ops.windows(3).any(|window| { + matches!( + window, + [ + Instruction::JumpBackward { .. }, + Instruction::LoadFastBorrow { .. } | Instruction::LoadFast { .. }, + Instruction::LoadAttr { .. }, + ] + ) + }), + "CPython removes the no-location POP_BLOCK NOP before a boolop-while try/finally finalbody, got ops={ops:?}" + ); + assert!( + !ops.windows(4).any(|window| { + matches!( + window, + [ + Instruction::JumpBackward { .. }, + Instruction::Nop, + Instruction::LoadFastBorrow { .. } | Instruction::LoadFast { .. }, + Instruction::LoadAttr { .. }, + ] + ) + }), + "boolop-while try/finally finalbody should not keep a POP_BLOCK NOP, got ops={ops:?}" + ); + } + + #[test] + fn test_try_finally_with_infinite_loop_body_drops_finalbody_entry_nop() { + let code = compile_exec( + "\ +def f(self, func, args, kwargs): + try: + with self.assertRaises(ZeroDivisionError) as cm: + while True: + self.setAlarm(self.alarm_time) + func(*args, **kwargs) + finally: + self.setAlarm(0) +", + ); + let f = find_code(&code, "f").expect("missing f code"); + let ops: Vec<_> = f + .instructions + .iter() + .map(|unit| unit.op) + .filter(|op| !matches!(op, Instruction::Cache)) + .collect(); + + assert!( + ops.windows(3).any(|window| { + matches!( + window, + [ + Instruction::Reraise { .. }, + Instruction::LoadFast { .. }, + Instruction::LoadAttr { .. }, + ] + ) + }), + "CPython removes the no-location POP_BLOCK NOP before the normal finalbody after a with-wrapped infinite loop, got ops={ops:?}" + ); + assert!( + !ops.windows(4).any(|window| { + matches!( + window, + [ + Instruction::Reraise { .. }, + Instruction::Nop, + Instruction::LoadFast { .. }, + Instruction::LoadAttr { .. }, + ] + ) + }), + "with-wrapped infinite loop try/finally should not keep a finalbody-entry NOP, got ops={ops:?}" + ); + } + + #[test] + fn test_try_finally_with_finalbody_blocks_following_with_borrow() { + let code = compile_exec( + "\ +def f(self, sock, socket, HOST, OSError, TypeError): + try: + sock.bind((HOST, 0)) + socket.close(sock.fileno()) + with self.assertRaises(OSError): + sock.listen(1) + finally: + with self.assertRaises(OSError): + sock.close() + with self.assertRaises(TypeError): + socket.close(42, 42) + with self.assertRaises(OSError): + socket.close(-1) +", + ); + let f = find_code(&code, "f").expect("missing f code"); + let strong_self_loads = count_strong_loads_for_vars(f, &["self"]); + assert!( + strong_self_loads >= 2, + "CPython codegen_try_finally() emits USE_LABEL(exit) after a with finalbody, so optimize_load_fast() leaves following with receivers strong; got {strong_self_loads} strong self loads" + ); + } + #[test] fn test_try_finally_loop_direct_break_drops_finalbody_entry_nop() { let code = compile_exec( @@ -19185,6 +19397,149 @@ def f(lines, close): ); } + #[test] + fn test_try_except_finally_handler_normal_exit_keeps_nointerrupt_jump() { + let code = compile_exec( + "\ +def f(): + try: + 2 + except: + 4 + finally: + 6 +", + ); + let f = find_code(&code, "f").expect("missing f code"); + let ops: Vec<_> = f + .instructions + .iter() + .map(|unit| unit.op) + .filter(|op| !matches!(op, Instruction::Cache)) + .collect(); + + assert!( + ops.windows(2).any(|window| { + matches!( + window, + [ + Instruction::PopExcept, + Instruction::JumpBackwardNoInterrupt { .. } + ] + ) + }), + "CPython codegen_try_except() emits JUMP_NO_INTERRUPT to the inner end label; when wrapped by codegen_try_finally(), push_cold_blocks_to_end() preserves it as a backward no-interrupt jump, got ops={ops:?}", + ); + assert!( + !ops.windows(3).any(|window| { + matches!( + window, + [ + Instruction::PopExcept, + Instruction::LoadConst { .. }, + Instruction::ReturnValue, + ] + ) + }), + "try/except/finally handler normal exit should not inline the function epilogue over CPython's no-interrupt jump, got ops={ops:?}", + ); + } + + #[test] + fn test_nested_while_break_keeps_cpython_unreachable_end_epilogue() { + let code = compile_exec( + "\ +def f(): + TRUE = 1 + while TRUE: + while TRUE: + break + break +", + ); + let f = find_code(&code, "f").expect("missing f code"); + let ops: Vec<_> = f + .instructions + .iter() + .map(|unit| unit.op) + .filter(|op| !matches!(op, Instruction::Cache)) + .collect(); + let returns = ops + .iter() + .filter(|op| matches!(op, Instruction::ReturnValue)) + .count(); + + assert_eq!( + returns, 3, + "CPython codegen_while() emits separate anchor/end labels and codegen_break() jumps to loop->fb_exit; after redundant jump removal, the b_next return epilogue still remains, got ops={ops:?}", + ); + } + + #[test] + fn test_while_else_break_keeps_separate_continue_backedges() { + let code = compile_exec( + "\ +def func(): + TRUE = 1 + x = [1] + while x: + x.pop() + while TRUE: + break + else: + continue +", + ); + let func = find_code(&code, "func").expect("missing func code"); + let ops: Vec<_> = func + .instructions + .iter() + .map(|unit| unit.op) + .filter(|op| !matches!(op, Instruction::Cache)) + .collect(); + let jump_backwards = ops + .iter() + .filter(|op| matches!(op, Instruction::JumpBackward { .. })) + .count(); + + assert_eq!( + jump_backwards, 2, + "CPython codegen_break() emits a line-bearing jump to the inner while end, and codegen_while() keeps the else anchor separate from the end label; the break path and else-continue path should remain distinct backedges, got ops={ops:?}", + ); + } + + #[test] + fn test_break_through_finally_assert_tail_keeps_borrow_loads() { + let code = compile_exec( + "\ +def func(): + a, c, d, i = 1, 1, 1, 99 + try: + for i in range(3): + try: + a = 5 + if i > 0: + break + a = 8 + finally: + c = 10 + except: + d = 12 + assert a == 5 and c == 10 and d == 1 +", + ); + let func = find_code(&code, "func").expect("missing func code"); + for name in ["a", "c", "d"] { + let loads = load_fast_ops_for_var(func, name); + assert!( + loads + .iter() + .any(|op| matches!(op, Instruction::LoadFastBorrow { .. })), + "CPython flowgraph.c::optimize_load_fast() keeps assert-tail {name} loads borrowed after a resuming bare except; got loads={loads:?}", + ); + } + } + #[test] fn test_try_except_finally_suppressing_handler_drops_body_exit_nop() { let code = compile_exec( @@ -24289,6 +24644,50 @@ def f(onerror, err, OSError): ); } + #[test] + fn test_named_except_boolop_condition_shares_cleanup_return() { + let code = compile_exec( + "\ +def f(self, module_name, ModuleNotFoundError): + try: + return importlib.import_module(module_name) + except ModuleNotFoundError as error: + if self._warn_on_extension_import and module_name in builtin_hashes: + logging.getLogger(__name__).warning('msg', error, exc_info=error) + return None +", + ); + let f = find_code(&code, "f").expect("missing function code"); + let ops: Vec<_> = f + .instructions + .iter() + .map(|unit| unit.op) + .filter(|op| !matches!(op, Instruction::Cache)) + .collect(); + + let cleanup_return_count = ops + .windows(6) + .filter(|window| { + matches!( + window, + [ + Instruction::PopExcept, + Instruction::LoadConst { .. }, + Instruction::StoreFast { .. } | Instruction::StoreName { .. }, + Instruction::DeleteFast { .. } | Instruction::DeleteName { .. }, + Instruction::LoadConst { .. }, + Instruction::ReturnValue, + ] + ) + }) + .count(); + + assert_eq!( + cleanup_return_count, 1, + "CPython keeps a shared named-except cleanup return when multiple BoolOp false edges target the same cleanup block, got ops={ops:?}" + ); + } + #[test] fn test_listcomp_cleanup_tail_keeps_split_store_fast_pair() { let code = compile_exec( @@ -29036,6 +29435,56 @@ def f(): ); } + #[test] + fn test_nested_terminal_with_before_successor_drops_after_block_nop() { + let code = compile_exec( + "\ +def f(a, b, c): + with a: + with b: + raise c + c() +", + ); + let f = find_code(&code, "f").expect("missing f code"); + let ops: Vec<_> = f + .instructions + .iter() + .map(|unit| unit.op) + .filter(|op| !matches!(op, Instruction::Cache)) + .collect(); + + assert!( + ops.windows(4).any(|window| { + matches!( + window, + [ + Instruction::Copy { .. }, + Instruction::PopExcept, + Instruction::Reraise { .. }, + Instruction::LoadFast { .. } | Instruction::LoadFastBorrow { .. }, + ] + ) + }), + "CPython falls through from the terminal inner-with cleanup to the following statement without an after-block NOP, got ops={ops:?}" + ); + assert!( + !ops.windows(5).any(|window| { + matches!( + window, + [ + Instruction::Copy { .. }, + Instruction::PopExcept, + Instruction::Reraise { .. }, + Instruction::Nop, + Instruction::LoadFast { .. } | Instruction::LoadFastBorrow { .. }, + ] + ) + }), + "unexpected inner with after-block NOP before following statement, got ops={ops:?}" + ); + } + #[test] fn test_try_loop_elif_places_return_before_orelse_tail() { let code = compile_exec( @@ -33056,6 +33505,60 @@ def f(cm, source): ); } + #[test] + fn test_with_for_fallthrough_drops_cleanup_nop() { + let code = compile_exec( + "\ +def f(cm, xs, g): + with cm: + for x in xs: + g(x) + return None +", + ); + let f = find_code(&code, "f").expect("missing f code"); + let ops: Vec<_> = f + .instructions + .iter() + .filter(|unit| !matches!(unit.op, Instruction::Cache)) + .map(|unit| unit.op) + .collect(); + + assert!( + ops.windows(6).any(|window| { + matches!( + window, + [ + Instruction::EndFor, + Instruction::PopIter, + Instruction::LoadConst { .. }, + Instruction::LoadConst { .. }, + Instruction::LoadConst { .. }, + Instruction::Call { .. }, + ] + ) + }), + "with cleanup after for fallthrough should directly follow END_FOR/POP_ITER like CPython, got ops={ops:?}" + ); + assert!( + !ops.windows(7).any(|window| { + matches!( + window, + [ + Instruction::EndFor, + Instruction::PopIter, + Instruction::Nop, + Instruction::LoadConst { .. }, + Instruction::LoadConst { .. }, + Instruction::LoadConst { .. }, + Instruction::Call { .. }, + ] + ) + }), + "with cleanup after for fallthrough should not preserve a POP_BLOCK NOP, got ops={ops:?}" + ); + } + #[test] fn test_with_while_true_break_drops_cleanup_nop() { let code = compile_exec( @@ -33095,6 +33598,62 @@ def f(cm, source): ); } + #[test] + fn test_multi_with_while_true_try_except_drops_outer_cleanup_nop() { + let code = compile_exec( + "\ +def f(cm1, cm2, g, E): + with cm1, cm2: + while True: + try: + g() + except E: + pass +", + ); + let f = find_code(&code, "f").expect("missing f code"); + let ops: Vec<_> = f + .instructions + .iter() + .filter(|unit| !matches!(unit.op, Instruction::Cache)) + .map(|unit| unit.op) + .collect(); + + assert!( + ops.windows(6).any(|window| { + matches!( + window, + [ + Instruction::Copy { .. }, + Instruction::PopExcept, + Instruction::Reraise { .. }, + Instruction::LoadConst { .. }, + Instruction::LoadConst { .. }, + Instruction::LoadConst { .. }, + ] + ) + }), + "outer with cleanup after an infinite inner with body should follow the inner cleanup directly like CPython, got ops={ops:?}" + ); + assert!( + !ops.windows(7).any(|window| { + matches!( + window, + [ + Instruction::Copy { .. }, + Instruction::PopExcept, + Instruction::Reraise { .. }, + Instruction::Nop, + Instruction::LoadConst { .. }, + Instruction::LoadConst { .. }, + Instruction::LoadConst { .. }, + ] + ) + }), + "outer with cleanup after an infinite inner with body should not keep a POP_BLOCK NOP, got ops={ops:?}" + ); + } + #[test] fn test_with_final_assert_preserves_cleanup_nop() { let code = compile_exec( diff --git a/crates/codegen/src/ir.rs b/crates/codegen/src/ir.rs index d7e229eecd..c04fa0e06a 100644 --- a/crates/codegen/src/ir.rs +++ b/crates/codegen/src/ir.rs @@ -348,6 +348,7 @@ impl CodeInfo { // later jump normalization / block reordering can create adjacencies // that never exist at this stage in flowgraph.c. self.insert_superinstructions(); + self.remove_redundant_const_pop_top_pairs(); inline_single_predecessor_artificial_expr_exit_blocks(&mut self.blocks); push_cold_blocks_to_end(&mut self.blocks); // CPython resolves line numbers again after cold-block extraction. @@ -381,11 +382,11 @@ impl CodeInfo { canonicalize_empty_label_blocks(&mut self.blocks); inline_small_fast_return_blocks(&mut self.blocks); duplicate_end_returns(&mut self.blocks, &self.metadata); + retarget_conditional_jumps_to_empty_while_exit_epilogue(&mut self.blocks); duplicate_fallthrough_jump_back_targets(&mut self.blocks); duplicate_shared_jump_back_targets(&mut self.blocks); self.dce(); // truncate after terminal in blocks that got return duplicated self.eliminate_unreachable_blocks(); // remove now-unreachable last block - self.remove_redundant_const_pop_top_pairs(); remove_redundant_nops_and_jumps(&mut self.blocks); // Some jump-only blocks only appear after late CFG cleanup. Thread them // once more so loop backedges stay direct instead of becoming @@ -653,7 +654,7 @@ impl CodeInfo { let updated_cache = op.cache_entries() as u32; recompile |= updated_cache != old_cache_entries; info.cache_entries = updated_cache; - let new_arg = if matches!(op, Instruction::EndAsyncFor) { + let mut new_arg = if matches!(op, Instruction::EndAsyncFor) { let arg = offset_after .checked_sub(target_offset + END_SEND_OFFSET) .expect("END_ASYNC_FOR target must be before instruction"); @@ -672,6 +673,22 @@ impl CodeInfo { .expect("forward jump target must be after instruction"); OpArg::new(arg) }; + if matches!( + op.into(), + Opcode::JumpBackward | Opcode::JumpBackwardNoInterrupt + ) && u32::from(new_arg) == 0xff + && target_offset > 0xff + { + // CPython assemble.c::resolve_jump_offsets() + // bootstraps jump sizing from the unresolved + // target index stored in i_oparg. When a + // backward jump lands exactly on the 255-code + // unit boundary and the target is already + // beyond one-byte range, that preliminary + // EXTENDED_ARG increases the resolved backward + // delta to 256 and the fixed point keeps it. + new_arg = OpArg::new(0x100); + } recompile |= new_arg.instr_size() != old_arg_size; info.arg = new_arg; } @@ -932,8 +949,15 @@ impl CodeInfo { } } - for (i, block) in self.blocks.iter_mut().enumerate() { - if !reachable[i] { + for i in 0..self.blocks.len() { + if !reachable[i] + && !preserves_cpython_unreachable_fallthrough_return_epilogue( + &self.blocks, + &reachable, + BlockIdx(i as u32), + ) + { + let block = &mut self.blocks[i]; block.instructions.clear(); } } @@ -4327,6 +4351,7 @@ impl CodeInfo { )); self.add_checks_for_loads_of_uninitialized_variables(); self.insert_superinstructions(); + self.remove_redundant_const_pop_top_pairs(); inline_single_predecessor_artificial_expr_exit_blocks(&mut self.blocks); push_cold_blocks_to_end(&mut self.blocks); trace.push(( @@ -4395,6 +4420,7 @@ impl CodeInfo { )); duplicate_end_returns(&mut self.blocks, &self.metadata); + retarget_conditional_jumps_to_empty_while_exit_epilogue(&mut self.blocks); duplicate_fallthrough_jump_back_targets(&mut self.blocks); duplicate_shared_jump_back_targets(&mut self.blocks); trace.push(( @@ -4415,7 +4441,6 @@ impl CodeInfo { self.debug_block_dump(), )); - self.remove_redundant_const_pop_top_pairs(); remove_redundant_nops_and_jumps(&mut self.blocks); trace.push(( "after_remove_redundant_nops_and_jumps".to_owned(), @@ -6861,6 +6886,56 @@ fn redirect_empty_block_targets(blocks: &mut [Block]) { } } +fn preserves_cpython_unreachable_fallthrough_return_epilogue( + blocks: &[Block], + reachable: &[bool], + idx: BlockIdx, +) -> bool { + let block = &blocks[idx.idx()]; + if !matches!( + block.instructions.as_slice(), + [load, ret] + if load.no_location_exit + && ret.no_location_exit + && instruction_lineno(load) < 0 + && instruction_lineno(ret) < 0 + && matches!(load.instr.real(), Some(Instruction::LoadConst { .. })) + && matches!(ret.instr.real(), Some(Instruction::ReturnValue)) + ) { + return false; + } + + let mut seen = vec![false; blocks.len()]; + let mut stack = vec![idx]; + while let Some(target) = stack.pop() { + if target == BlockIdx::NULL { + continue; + } + for (source_idx, source) in blocks.iter().enumerate() { + if source.next != target || seen[source_idx] { + continue; + } + seen[source_idx] = true; + if reachable[source_idx] { + let [.., load, ret] = source.instructions.as_slice() else { + continue; + }; + if load.no_location_exit + && ret.no_location_exit + && matches!(load.instr.real(), Some(Instruction::LoadConst { .. })) + && matches!(ret.instr.real(), Some(Instruction::ReturnValue)) + && instruction_lineno(ret) > 0 + { + return true; + } + } else if source.instructions.is_empty() { + stack.push(BlockIdx(source_idx as u32)); + } + } + } + false +} + fn redirect_load_fast_passthrough_targets(blocks: &mut [Block]) { fn is_assertion_error_load(info: &InstructionInfo) -> bool { matches!( @@ -10534,6 +10609,17 @@ fn duplicate_shared_jump_back_targets(blocks: &mut Vec) { } fn reorder_lineful_jump_back_runs_by_descending_line(blocks: &mut [Block]) { + fn follows_while_body_anchor_order(blocks: &[Block], body: BlockIdx, anchor: BlockIdx) -> bool { + blocks.iter().any(|block| { + next_nonempty_block(blocks, block.next) == body + && block.instructions.iter().any(|info| { + is_conditional_jump(&info.instr) + && info.target != BlockIdx::NULL + && next_nonempty_block(blocks, info.target) == anchor + }) + }) + } + let mut prev = BlockIdx::NULL; let mut current = BlockIdx(0); while current != BlockIdx::NULL { @@ -10572,6 +10658,11 @@ fn reorder_lineful_jump_back_runs_by_descending_line(blocks: &mut [Block]) { current = blocks[current.idx()].next; continue; } + if follows_while_body_anchor_order(blocks, run[0], run[1]) { + prev = *run.last().expect("non-empty jump-back run"); + current = scan; + continue; + } run.sort_by_key(|block_idx| { Reverse(instruction_lineno(&blocks[block_idx.idx()].instructions[0])) @@ -10677,6 +10768,69 @@ fn duplicate_fallthrough_jump_back_targets(blocks: &mut Vec) { } } +fn retarget_conditional_jumps_to_empty_while_exit_epilogue(blocks: &mut [Block]) { + fn is_no_location_return_epilogue(block: &Block) -> bool { + matches!( + block.instructions.as_slice(), + [load, ret] + if load.no_location_exit + && ret.no_location_exit + && instruction_lineno(load) < 0 + && instruction_lineno(ret) < 0 + && matches!(load.instr.real(), Some(Instruction::LoadConst { .. })) + && matches!(ret.instr.real(), Some(Instruction::ReturnValue)) + ) + } + + fn ends_with_line_marker_implicit_return(block: &Block) -> bool { + matches!( + block.instructions.as_slice(), + [.., marker, load, ret] + if matches!(marker.instr.real(), Some(Instruction::Nop)) + && instruction_lineno(marker) > 0 + && load.no_location_exit + && ret.no_location_exit + && instruction_lineno(ret) > 0 + && matches!(load.instr.real(), Some(Instruction::LoadConst { .. })) + && matches!(ret.instr.real(), Some(Instruction::ReturnValue)) + ) + } + + let retargets: Vec> = blocks + .iter() + .map(|block| { + block + .instructions + .iter() + .map(|instr| { + if instr.target == BlockIdx::NULL || !is_conditional_jump(&instr.instr) { + return instr.target; + } + let target = instr.target; + if !ends_with_line_marker_implicit_return(&blocks[target.idx()]) { + return target; + } + let next = next_nonempty_block(blocks, blocks[target.idx()].next); + if next != BlockIdx::NULL && is_no_location_return_epilogue(&blocks[next.idx()]) + { + next + } else { + target + } + }) + .collect() + }) + .collect(); + + for (block, block_retargets) in blocks.iter_mut().zip(retargets) { + for (instr, target) in block.instructions.iter_mut().zip(block_retargets) { + if target != BlockIdx::NULL { + instr.target = target; + } + } + } +} + /// Duplicate `LOAD_CONST None + RETURN_VALUE` for blocks that fall through /// to the final return block. fn duplicate_end_returns(blocks: &mut Vec, metadata: &CodeUnitMetadata) { @@ -10723,6 +10877,23 @@ fn duplicate_end_returns(blocks: &mut Vec, metadata: &CodeUnitMetadata) { // Get the return instructions to clone let return_insts: Vec = last_insts[last_insts.len() - 2..].to_vec(); let predecessors = compute_predecessors(blocks); + let has_nointerrupt_jump_to_last_block = blocks.iter().any(|block| { + block.instructions.iter().any(|instr| { + matches!( + jump_thread_kind(instr.instr), + Some(JumpThreadKind::NoInterrupt) + ) && instr.target != BlockIdx::NULL + && next_nonempty_block(blocks, instr.target) == last_block + }) + }); + let has_lineful_return_fallthrough_to_last_block = blocks.iter().any(|block| { + block.next != BlockIdx::NULL + && next_nonempty_block(blocks, block.next) == last_block + && block.instructions.last().is_some_and(|instr| { + matches!(instr.instr.real(), Some(Instruction::ReturnValue)) + && instruction_lineno(instr) > 0 + }) + }); // Find non-cold blocks that reach the last return block either by // fallthrough or as an unconditional jump target that should get its own @@ -10752,6 +10923,7 @@ fn duplicate_end_returns(blocks: &mut Vec, metadata: &CodeUnitMetadata) { && next == last_block && has_fallthrough && trailing_conditional_jump_index(block).is_none() + && !has_nointerrupt_jump_to_last_block && !already_has_return { fallthrough_blocks_to_fix.push(current); @@ -10765,6 +10937,12 @@ fn duplicate_end_returns(blocks: &mut Vec, metadata: &CodeUnitMetadata) { if let Some(jump_idx) = jump_idx { let jump = &block.instructions[jump_idx]; if jump.target != BlockIdx::NULL + && !matches!( + jump_thread_kind(jump.instr), + Some(JumpThreadKind::NoInterrupt) + ) + && !(has_lineful_return_fallthrough_to_last_block + && is_conditional_jump(&jump.instr)) && next_nonempty_block(blocks, jump.target) == last_block && (is_conditional_jump(&jump.instr) || predecessors[last_block.idx()] > 1) { @@ -10972,6 +11150,7 @@ fn duplicate_named_except_cleanup_returns(blocks: &mut Vec, metadata: &Co continue; } + let mut conditional_sources = Vec::new(); for block_idx in 0..blocks.len() { if block_idx == target.idx() { continue; @@ -10984,7 +11163,10 @@ fn duplicate_named_except_cleanup_returns(blocks: &mut Vec, metadata: &Co { continue; } - clones.push((BlockIdx(block_idx as u32), instr_idx, target)); + conditional_sources.push((BlockIdx(block_idx as u32), instr_idx)); + } + if let [(block_idx, instr_idx)] = conditional_sources.as_slice() { + clones.push((*block_idx, *instr_idx, target)); } } @@ -11031,6 +11213,12 @@ fn inline_pop_except_return_blocks(blocks: &mut [Block]) { if !jump.instr.is_unconditional_jump() || jump.target == BlockIdx::NULL { continue; } + if matches!( + jump_thread_kind(jump.instr), + Some(JumpThreadKind::NoInterrupt) + ) { + continue; + } let Some(last_real_before_jump) = blocks[block_idx].instructions[..jump_idx] .iter() From 8d1b9593a2c664982be5a4dd7ae57f1269357aee Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Wed, 20 May 2026 21:25:21 +0900 Subject: [PATCH 2/3] Align bytecode anchors with CPython --- crates/codegen/src/compile.rs | 145 ++++++++++++++++++++++++++++------ crates/codegen/src/ir.rs | 100 +++++++++++------------ 2 files changed, 173 insertions(+), 72 deletions(-) diff --git a/crates/codegen/src/compile.rs b/crates/codegen/src/compile.rs index 013fd1d967..3edc728a44 100644 --- a/crates/codegen/src/compile.rs +++ b/crates/codegen/src/compile.rs @@ -8725,6 +8725,10 @@ impl Compiler { num_cases > 1 && is_trailing_wildcard_default(&cases.last().unwrap().pattern); let case_count = num_cases - if has_default { 1 } else { 0 }; + let has_match_or_case = cases + .iter() + .take(case_count) + .any(|m| matches!(m.pattern, ast::Pattern::MatchOr(_))); for (i, m) in cases.iter().enumerate().take(case_count) { // Only copy the subject if not on the last case if i != case_count - 1 { @@ -8758,10 +8762,12 @@ impl Compiler { if let Some(first_stmt) = m.body.first() { self.set_source_range(first_stmt.range()); } - if matches!(m.pattern, ast::Pattern::MatchOr(_)) { - emit!(self, Instruction::Nop); - } emit!(self, Instruction::PopTop); + if let Some(last) = self.current_block().instructions.last_mut() { + // CPython emits NEXT_LOCATION here; resolve it after + // redundant NOP removal so a following pass NOP survives. + last.lineno_override = Some(-2); + } if matches!(m.body.first(), Some(ast::Stmt::Try(_))) { let body_block = self.new_block(); self.switch_to_block(body_block); @@ -8769,7 +8775,7 @@ impl Compiler { } self.compile_statements(&m.body)?; - emit!(self, PseudoInstruction::JumpNoInterrupt { delta: end }); + emit!(self, PseudoInstruction::Jump { delta: end }); if let Some(last) = self.current_block().instructions.last_mut() { last.match_success_jump = true; } @@ -8779,10 +8785,16 @@ impl Compiler { if has_default { let m = &cases[num_cases - 1]; + if has_match_or_case { + // CPython optimize_load_fast() does not borrow loads from the + // trailing default block after an OR-pattern subject copy. + let current = self.current_code_info().current_block; + self.disable_load_fast_borrow_for_block(current); + } self.set_source_range(m.pattern.range()); if num_cases == 1 { emit!(self, Instruction::PopTop); - } else if m.guard.is_none() { + } else { emit!(self, Instruction::Nop); } if let Some(ref guard) = m.guard { @@ -8969,9 +8981,10 @@ impl Compiler { self.switch_to_block(cleanup); emit!(self, Instruction::PopTop); if !condition { + self.set_no_location(); emit!( self, - PseudoInstruction::Jump { + PseudoInstruction::JumpNoInterrupt { delta: target_block } ); @@ -14840,6 +14853,104 @@ def f(buffer, pos, last_char): .count() } + #[test] + fn test_match_or_default_block_keeps_load_fast_strong() { + let code = compile_exec( + r#" +def f(format, other): + match format: + case 1 | 2: + return other + case _: + raise NotImplementedError(other) +"#, + ); + let function = find_code(&code, "f").expect("missing function code"); + let loads = load_fast_ops_for_var(function, "other"); + assert!( + matches!( + loads.as_slice(), + [ + Instruction::LoadFastBorrow { .. }, + Instruction::LoadFastBorrow { .. }, + Instruction::LoadFast { .. }, + ] + ), + "CPython optimize_load_fast() keeps trailing OR-pattern default loads strong, got {loads:?}", + ); + } + + #[test] + fn test_match_success_next_location_preserves_pass_nop() { + let code = compile_exec( + r#" +def f(command): + match command: + case "": + pass + case _ as unknown: + sink(unknown) + return False +"#, + ); + let function = find_code(&code, "f").expect("missing function code"); + let ops = non_cache_instructions(function) + .map(|unit| unit.op) + .collect::>(); + assert!( + ops.windows(3).any(|window| matches!( + window, + [ + Instruction::PopTop, + Instruction::Nop, + Instruction::LoadConst { .. }, + ] + )), + "CPython NEXT_LOCATION keeps the pass NOP after match subject POP_TOP, got {ops:?}", + ); + } + + #[test] + fn test_while_try_body_layout_keeps_false_jump_to_anchor() { + let code = compile_exec( + r#" +def f(stack, itstack, node_to_stack_index): + while True: + while stack: + try: + node = itstack[-1]() + break + except StopIteration: + del node_to_stack_index[stack.pop()] + itstack.pop() + else: + break +"#, + ); + let function = find_code(&code, "f").expect("missing function code"); + let ops = non_cache_instructions(function) + .map(|unit| unit.op) + .collect::>(); + let stack_test = ops + .windows(5) + .find(|window| { + matches!( + window, + [ + Instruction::LoadFastBorrow { .. } | Instruction::LoadFast { .. }, + Instruction::ToBool, + Instruction::PopJumpIfFalse { .. }, + Instruction::NotTaken, + Instruction::Nop, + ] + ) + }) + .unwrap_or_else(|| { + panic!("expected CPython-style while/try false jump to anchor, got {ops:?}") + }); + assert!(matches!(stack_test[2], Instruction::PopJumpIfFalse { .. })); + } + fn localsplus_name(code: &CodeObject, idx: usize) -> Option<&str> { if idx < code.varnames.len() { return Some(code.varnames[idx].as_str()); @@ -26665,7 +26776,7 @@ async def f(): } #[test] - fn test_match_async_inlined_comprehension_success_jump_no_interrupt() { + fn test_match_async_inlined_comprehension_success_jump_layout() { let code = compile_exec( "\ async def f(name_3, name_5): @@ -26688,30 +26799,18 @@ async def f(name_3, name_5): .collect(); assert!( - ops.windows(3).any(|window| { - matches!( - window, - [ - Instruction::PopTop, - Instruction::StoreFast { .. }, - Instruction::JumpBackwardNoInterrupt { .. }, - ] - ) - }), - "expected CPython-style no-interrupt match success backedge after async comprehension cleanup, got ops={ops:?}" - ); - assert!( - !ops.windows(3).any(|window| { + ops.windows(4).any(|window| { matches!( window, [ Instruction::PopTop, + Instruction::JumpForward { .. }, + Instruction::Copy { .. }, Instruction::StoreFast { .. }, - Instruction::JumpBackward { .. }, ] ) }), - "match success cleanup backedge should not be a regular interrupting jump, got ops={ops:?}" + "expected CPython-style plain match success jump before async comprehension case, got ops={ops:?}" ); } diff --git a/crates/codegen/src/ir.rs b/crates/codegen/src/ir.rs index c04fa0e06a..d960981f6f 100644 --- a/crates/codegen/src/ir.rs +++ b/crates/codegen/src/ir.rs @@ -573,6 +573,8 @@ impl CodeInfo { } } + resolve_next_location_overrides(&mut blocks); + // Pre-compute cache_entries for real (non-pseudo) instructions for block in &mut blocks { for instr in &mut block.instructions { @@ -6054,52 +6056,6 @@ fn jump_threading_impl(blocks: &mut [Block], include_conditional: bool) { if target == BlockIdx::NULL { continue; } - if include_conditional && is_conditional_jump(&ins.instr) { - let next = next_nonempty_block(blocks, blocks[bi].next); - let next_is_scope_exit = next != BlockIdx::NULL - && blocks[next.idx()] - .instructions - .last() - .is_some_and(|instr| instr.instr.is_scope_exit()); - if next_is_scope_exit { - let target_pos = block_order.get(target.idx()).copied().unwrap_or(u32::MAX); - let target_first_jump = blocks[target.idx()].instructions.first().copied(); - let threads_match_success_jump_to_forward_nointerrupt = - matches!(ins.instr.real(), Some(Instruction::PopJumpIfNone { .. })) - && target_first_jump - .filter(|target_ins| target_ins.instr.is_unconditional_jump()) - .filter(|target_ins| target_ins.target != BlockIdx::NULL) - .is_some_and(|target_ins| { - let final_target_pos = block_order - .get(target_ins.target.idx()) - .copied() - .unwrap_or(u32::MAX); - jump_thread_kind(target_ins.instr) - == Some(JumpThreadKind::NoInterrupt) - && target_ins.match_success_jump - && final_target_pos > target_pos - }); - let next_raises = blocks[next.idx()].instructions.iter().any(|instr| { - matches!(instr.instr.real(), Some(Instruction::RaiseVarargs { .. })) - }); - let target_is_loop_backedge = blocks[target.idx()] - .instructions - .first() - .filter(|target_ins| target_ins.instr.is_unconditional_jump()) - .map(|target_ins| next_nonempty_block(blocks, target_ins.target)) - .is_some_and(|final_target| { - final_target == BlockIdx(bi as u32) - || comes_before(blocks, final_target, BlockIdx(bi as u32)) - }); - if !(threads_match_success_jump_to_forward_nointerrupt - || block_is_protected(&blocks[bi]) - && next_raises - && target_is_loop_backedge) - { - continue; - } - } - } if include_conditional && is_conditional_jump(&ins.instr) && opposite_short_circuit_target(&blocks[target.idx()], ins.instr) @@ -8564,6 +8520,7 @@ fn reorder_conditional_chain_and_jump_back_blocks(blocks: &mut Vec) { let mut saw_nonempty = false; let mut nonempty_blocks = 0usize; let mut real_instr_count = 0usize; + let mut chain_has_block_push = false; let mut cursor = chain_start; let mut chain_valid = true; while cursor != BlockIdx::NULL && cursor != jump_start { @@ -8583,6 +8540,10 @@ fn reorder_conditional_chain_and_jump_back_blocks(blocks: &mut Vec) { .iter() .filter(|info| info.instr.real().is_some()) .count(); + chain_has_block_push |= blocks[cursor.idx()] + .instructions + .iter() + .any(|info| info.instr.is_block_push()); } chain_end = cursor; cursor = blocks[cursor.idx()].next; @@ -8591,6 +8552,10 @@ fn reorder_conditional_chain_and_jump_back_blocks(blocks: &mut Vec) { current = next; continue; } + if is_generic_false_path_reorder && chain_has_block_push { + current = next; + continue; + } let chain_is_conditional_single_delete_body = chain_is_conditional_single_delete_body(blocks, chain_start, jump_start); if is_generic_false_path_reorder @@ -10295,6 +10260,43 @@ fn resolve_line_numbers(blocks: &mut Vec) { propagate_line_numbers(blocks, &predecessors); } +fn resolve_next_location_overrides(blocks: &mut [Block]) { + let mut order = Vec::new(); + let mut current = BlockIdx(0); + while current != BlockIdx::NULL { + for instr_idx in 0..blocks[current.idx()].instructions.len() { + order.push((current, instr_idx)); + } + current = blocks[current.idx()].next; + } + + for pos in (0..order.len()).rev() { + let (block_idx, instr_idx) = order[pos]; + if blocks[block_idx.idx()].instructions[instr_idx].lineno_override != Some(-2) { + continue; + } + if blocks[block_idx.idx()].instructions[instr_idx] + .instr + .is_scope_exit() + || blocks[block_idx.idx()].instructions[instr_idx] + .instr + .is_unconditional_jump() + || is_conditional_jump(&blocks[block_idx.idx()].instructions[instr_idx].instr) + { + blocks[block_idx.idx()].instructions[instr_idx].lineno_override = Some(-1); + continue; + } + if let Some(&(next_block, next_instr)) = order.get(pos + 1) { + let next = blocks[next_block.idx()].instructions[next_instr]; + blocks[block_idx.idx()].instructions[instr_idx].location = next.location; + blocks[block_idx.idx()].instructions[instr_idx].end_location = next.end_location; + blocks[block_idx.idx()].instructions[instr_idx].lineno_override = next.lineno_override; + } else { + blocks[block_idx.idx()].instructions[instr_idx].lineno_override = Some(-1); + } + } +} + fn find_layout_predecessor(blocks: &[Block], target: BlockIdx) -> BlockIdx { if target == BlockIdx::NULL { return BlockIdx::NULL; @@ -10796,7 +10798,7 @@ fn retarget_conditional_jumps_to_empty_while_exit_epilogue(blocks: &mut [Block]) ) } - let retargets: Vec> = blocks + let jump_targets: Vec> = blocks .iter() .map(|block| { block @@ -10822,8 +10824,8 @@ fn retarget_conditional_jumps_to_empty_while_exit_epilogue(blocks: &mut [Block]) }) .collect(); - for (block, block_retargets) in blocks.iter_mut().zip(retargets) { - for (instr, target) in block.instructions.iter_mut().zip(block_retargets) { + for (block, block_jump_targets) in blocks.iter_mut().zip(jump_targets) { + for (instr, target) in block.instructions.iter_mut().zip(block_jump_targets) { if target != BlockIdx::NULL { instr.target = target; } From aa50896cca0051de10c2a7ae0b0612c649b29c11 Mon Sep 17 00:00:00 2001 From: "Jeong, YunWon" Date: Wed, 20 May 2026 22:51:12 +0900 Subject: [PATCH 3/3] Fix bytecode CFG review regressions --- crates/codegen/src/compile.rs | 109 +++++++++++++++++++++++++++++++++- crates/codegen/src/ir.rs | 32 +++++----- 2 files changed, 121 insertions(+), 20 deletions(-) diff --git a/crates/codegen/src/compile.rs b/crates/codegen/src/compile.rs index 3edc728a44..3f2be38f3b 100644 --- a/crates/codegen/src/compile.rs +++ b/crates/codegen/src/compile.rs @@ -1095,7 +1095,34 @@ impl Compiler { fn statements_end_with_conditional(body: &[ast::Stmt]) -> bool { body.last() - .is_some_and(|stmt| matches!(stmt, ast::Stmt::If(_))) + .is_some_and(Self::statement_ends_with_conditional) + } + + fn statement_ends_with_conditional(stmt: &ast::Stmt) -> bool { + match stmt { + ast::Stmt::If(_) | ast::Stmt::Match(_) => true, + ast::Stmt::With(ast::StmtWith { body, .. }) => { + Self::statements_end_with_conditional(body) + } + ast::Stmt::Try(ast::StmtTry { + body, + handlers, + orelse, + finalbody, + .. + }) => { + if !finalbody.is_empty() { + return Self::statements_end_with_conditional(finalbody); + } + let normal_body = if orelse.is_empty() { body } else { orelse }; + Self::statements_end_with_conditional(normal_body) + || handlers.iter().any(|handler| { + let ast::ExceptHandler::ExceptHandler(handler) = handler; + Self::statements_end_with_conditional(&handler.body) + }) + } + _ => false, + } } fn statements_end_with_conditional_scope_exit(&self, body: &[ast::Stmt]) -> bool { @@ -8716,6 +8743,32 @@ impl Compiler { } } + fn contains_match_or(pattern: &ast::Pattern) -> bool { + match pattern { + ast::Pattern::MatchSequence(match_sequence) => { + match_sequence.patterns.iter().any(contains_match_or) + } + ast::Pattern::MatchMapping(match_mapping) => { + match_mapping.patterns.iter().any(contains_match_or) + } + ast::Pattern::MatchClass(match_class) => { + match_class.arguments.patterns.iter().any(contains_match_or) + || match_class + .arguments + .keywords + .iter() + .any(|keyword| contains_match_or(&keyword.pattern)) + } + ast::Pattern::MatchAs(match_as) => { + match_as.pattern.as_deref().is_some_and(contains_match_or) + } + ast::Pattern::MatchOr(_) => true, + ast::Pattern::MatchValue(_) + | ast::Pattern::MatchSingleton(_) + | ast::Pattern::MatchStar(_) => false, + } + } + self.compile_expression(subject)?; let end = self.new_block(); @@ -8728,7 +8781,7 @@ impl Compiler { let has_match_or_case = cases .iter() .take(case_count) - .any(|m| matches!(m.pattern, ast::Pattern::MatchOr(_))); + .any(|m| contains_match_or(&m.pattern)); for (i, m) in cases.iter().enumerate().take(case_count) { // Only copy the subject if not on the last case if i != case_count - 1 { @@ -14880,6 +14933,28 @@ def f(format, other): ); } + #[test] + fn test_match_nested_or_default_block_keeps_load_fast_strong() { + let code = compile_exec( + r#" +def f(format, other): + match format: + case [1 | 2, value]: + return other + case _: + raise NotImplementedError(other) +"#, + ); + let function = find_code(&code, "f").expect("missing function code"); + let loads = load_fast_ops_for_var(function, "other"); + assert!( + loads + .iter() + .any(|op| matches!(op, Instruction::LoadFast { .. })), + "CPython optimize_load_fast() keeps trailing nested OR-pattern default loads strong, got {loads:?}", + ); + } + #[test] fn test_match_success_next_location_preserves_pass_nop() { let code = compile_exec( @@ -14951,6 +15026,36 @@ def f(stack, itstack, node_to_stack_index): assert!(matches!(stack_test[2], Instruction::PopJumpIfFalse { .. })); } + #[test] + fn test_while_if_not_break_keeps_body_call() { + let code = compile_exec( + r#" +def f(waiters): + while waiters: + waiter = waiters.popleft() + if not waiter.done(): + waiter.set_result(None) + break +"#, + ); + let function = find_code(&code, "f").expect("missing function code"); + let ops = non_cache_instructions(function) + .map(|unit| unit.op) + .collect::>(); + assert!( + ops.windows(4).any(|window| matches!( + window, + [ + Instruction::LoadFastBorrow { .. } | Instruction::LoadFast { .. }, + Instruction::LoadAttr { .. }, + Instruction::LoadConst { .. }, + Instruction::Call { .. }, + ] + )), + "CPython keeps waiter.set_result(None) before the break, got {ops:?}", + ); + } + fn localsplus_name(code: &CodeObject, idx: usize) -> Option<&str> { if idx < code.varnames.len() { return Some(code.varnames[idx].as_str()); diff --git a/crates/codegen/src/ir.rs b/crates/codegen/src/ir.rs index d960981f6f..e9c407c537 100644 --- a/crates/codegen/src/ir.rs +++ b/crates/codegen/src/ir.rs @@ -10787,7 +10787,7 @@ fn retarget_conditional_jumps_to_empty_while_exit_epilogue(blocks: &mut [Block]) fn ends_with_line_marker_implicit_return(block: &Block) -> bool { matches!( block.instructions.as_slice(), - [.., marker, load, ret] + [marker, load, ret] if matches!(marker.instr.real(), Some(Instruction::Nop)) && instruction_lineno(marker) > 0 && load.no_location_exit @@ -10879,23 +10879,6 @@ fn duplicate_end_returns(blocks: &mut Vec, metadata: &CodeUnitMetadata) { // Get the return instructions to clone let return_insts: Vec = last_insts[last_insts.len() - 2..].to_vec(); let predecessors = compute_predecessors(blocks); - let has_nointerrupt_jump_to_last_block = blocks.iter().any(|block| { - block.instructions.iter().any(|instr| { - matches!( - jump_thread_kind(instr.instr), - Some(JumpThreadKind::NoInterrupt) - ) && instr.target != BlockIdx::NULL - && next_nonempty_block(blocks, instr.target) == last_block - }) - }); - let has_lineful_return_fallthrough_to_last_block = blocks.iter().any(|block| { - block.next != BlockIdx::NULL - && next_nonempty_block(blocks, block.next) == last_block - && block.instructions.last().is_some_and(|instr| { - matches!(instr.instr.real(), Some(Instruction::ReturnValue)) - && instruction_lineno(instr) > 0 - }) - }); // Find non-cold blocks that reach the last return block either by // fallthrough or as an unconditional jump target that should get its own @@ -10910,6 +10893,13 @@ fn duplicate_end_returns(blocks: &mut Vec, metadata: &CodeUnitMetadata) { let last_ins = block.instructions.last(); let has_fallthrough = last_ins .is_none_or(|ins| !ins.instr.is_scope_exit() && !ins.instr.is_unconditional_jump()); + let has_nointerrupt_jump_to_last_block = block.instructions.iter().any(|instr| { + matches!( + jump_thread_kind(instr.instr), + Some(JumpThreadKind::NoInterrupt) + ) && instr.target != BlockIdx::NULL + && next_nonempty_block(blocks, instr.target) == last_block + }); // Don't duplicate if block already ends with the same return pattern let already_has_return = block.instructions.len() >= 2 && { let n = block.instructions.len(); @@ -10938,6 +10928,12 @@ fn duplicate_end_returns(blocks: &mut Vec, metadata: &CodeUnitMetadata) { }); if let Some(jump_idx) = jump_idx { let jump = &block.instructions[jump_idx]; + let has_lineful_return_fallthrough_to_last_block = block.next != BlockIdx::NULL + && next_nonempty_block(blocks, block.next) == last_block + && block.instructions.last().is_some_and(|instr| { + matches!(instr.instr.real(), Some(Instruction::ReturnValue)) + && instruction_lineno(instr) > 0 + }); if jump.target != BlockIdx::NULL && !matches!( jump_thread_kind(jump.instr),