-
Notifications
You must be signed in to change notification settings - Fork 19
coroutine, generator, io, exceptions and CPython allocation/layout metadata; five jit fixes #1126
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 24 commits
41e693a
37dfc23
bb63f8f
47204b0
9be3e5f
859aa63
7d29c63
a9a8ce9
c08027d
a97bdeb
ee1785d
c5ee4a9
a68fb4b
2559973
fbb793b
74d6276
39e1ec8
4dbc271
624973a
3a535a4
25707ae
dedc639
ef7dda1
cf40e9e
bfa00bc
8650fc5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1213,6 +1213,17 @@ impl BlackholeInterpreter { | |
| if opcode == self.op_catch_exception { | ||
| return self.route_to_catch(position, exc_value); | ||
| } | ||
| // A guard resume coordinate may name the successor block's entry | ||
| // `-live-`, before that block mirrors the virtualizable and reaches | ||
| // the raising operation's trailing `-live-`. The flattener emits the | ||
| // `catch_exception` immediately after that trailing marker | ||
| // (`flatten.py:206-217`). Walk forward to the first such marker and | ||
| // accept only its immediately-following catch; crossing any other | ||
| // operation after it means this exception belongs to no handler at | ||
| // the resumed call site. | ||
| if let Some(catch_pos) = self.find_catch_after_resume_live(resume_live_pos) { | ||
| return self.route_to_catch(catch_pos, exc_value); | ||
| } | ||
| // Backward case (after-residual-call guard): pyre resumes the post-call | ||
| // `GUARD_NO_EXCEPTION` at the next opcode's `-live-` | ||
| // (`pc_map[fallthrough_pc]`, jitcode_dispatch.rs / capture_resumedata), | ||
|
|
@@ -1365,6 +1376,48 @@ impl BlackholeInterpreter { | |
| None | ||
| } | ||
|
|
||
| fn find_catch_after_resume_live(&self, resume_live_pos: usize) -> Option<usize> { | ||
| let code = &self.jitcode.code; | ||
| let startpoints = self.jitcode.startpoints.as_ref()?; | ||
| let mut points: Vec<usize> = startpoints | ||
| .iter() | ||
| .copied() | ||
| .filter(|&q| q > resume_live_pos) | ||
| .collect(); | ||
| points.sort_unstable(); | ||
| let mut crossed_trailing_live = false; | ||
| for q in points { | ||
| let op = code[q]; | ||
| if op == self.op_catch_exception { | ||
| return crossed_trailing_live.then_some(q); | ||
|
Comment on lines
+1389
to
+1390
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a AGENTS.md reference: AGENTS.md:L14-L19 Useful? React with 👍 / 👎. |
||
| } | ||
| if crossed_trailing_live { | ||
| return None; | ||
| } | ||
| if op == self.op_live { | ||
| crossed_trailing_live = true; | ||
| continue; | ||
| } | ||
| if !matches!( | ||
| op, | ||
| majit_translate::insns::BC_SETFIELD_VABLE_I | ||
| | majit_translate::insns::BC_SETFIELD_VABLE_R | ||
| | majit_translate::insns::BC_SETFIELD_VABLE_F | ||
| | majit_translate::insns::BC_SETARRAYITEM_VABLE_I | ||
| | majit_translate::insns::BC_SETARRAYITEM_VABLE_R | ||
| | majit_translate::insns::BC_SETARRAYITEM_VABLE_F | ||
| ) { | ||
| // Only the codewriter's successor-block virtualizable mirror | ||
| // stores may separate the guard resume coordinate from the | ||
| // raising operation's trailing live marker. Crossing an | ||
| // arbitrary operation would attach its exception to the next | ||
| // operation's handler and silently swallow it. | ||
| return None; | ||
| } | ||
| } | ||
| None | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| } | ||
|
|
||
| /// blackhole.py:424-439 handle_rvmprof_enter. | ||
| pub fn handle_rvmprof_enter(&mut self) { | ||
| let code = &self.jitcode.code; | ||
|
|
@@ -4284,6 +4337,57 @@ mod tests { | |
| assert_eq!(bh.return_type, BhReturnType::Int); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_guard_exception_resume_finds_catch_after_successor_sync() { | ||
| let mut asm = majit_translate::codewriter::assembler::Assembler::new(); | ||
| let mut b = JitCodeBuilder::default(); | ||
| b.load_const_r_value(0, 1); | ||
| b.load_const_i_value(0, 2); | ||
| let resume_pc = b.current_pos(); | ||
| b.live(&mut asm, &[], &[], &[]); | ||
| // The normal-flow successor mirrors virtualizable state before | ||
| // the can-raise block's trailing live/catch pair. | ||
| b.vable_setfield_int_with_base(0, 0, 0); | ||
| b.live(&mut asm, &[], &[], &[]); | ||
| let handler_lbl = b.new_label(); | ||
| b.catch_exception(handler_lbl); | ||
| b.load_const_i_value(2, 99); | ||
| b.int_return(2); | ||
| b.mark_label(handler_lbl); | ||
| let handler_pc = b.current_pos(); | ||
| b.load_const_i_value(2, 42); | ||
| b.int_return(2); | ||
| let jitcode = b.finish(); | ||
|
|
||
| let mut builder = super::build_inline_call_only_bh_builder(); | ||
| let mut bh = builder.acquire_interp(); | ||
| bh.setposition(std::sync::Arc::new(jitcode), resume_pc); | ||
|
|
||
| assert!(bh.handle_exception_in_frame(0xCAFE_F00D)); | ||
| assert_eq!(bh.position, handler_pc); | ||
| } | ||
|
|
||
| #[test] | ||
| fn test_guard_exception_resume_does_not_cross_another_operation() { | ||
| let mut asm = majit_translate::codewriter::assembler::Assembler::new(); | ||
| let mut b = JitCodeBuilder::default(); | ||
| let resume_pc = b.current_pos(); | ||
| b.live(&mut asm, &[], &[], &[]); | ||
| b.load_const_i_value(0, 1); | ||
| b.live(&mut asm, &[], &[], &[]); | ||
| let handler_lbl = b.new_label(); | ||
| b.catch_exception(handler_lbl); | ||
| b.mark_label(handler_lbl); | ||
| b.int_return(0); | ||
| let jitcode = b.finish(); | ||
|
|
||
| let mut builder = super::build_inline_call_only_bh_builder(); | ||
| let mut bh = builder.acquire_interp(); | ||
| bh.setposition(std::sync::Arc::new(jitcode), resume_pc); | ||
|
|
||
| assert!(!bh.handle_exception_in_frame(0xCAFE_F00D)); | ||
| } | ||
|
|
||
| thread_local! { | ||
| /// Address of the interpreter's `exception_last_value` slot, read | ||
| /// back by [`probe_exception_slot_at_record_time`]. | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: youknowone/pyre
Length of output: 193
🏁 Script executed:
Repository: youknowone/pyre
Length of output: 33064
🏁 Script executed:
Repository: youknowone/pyre
Length of output: 25397
🏁 Script executed:
Repository: youknowone/pyre
Length of output: 10045
🏁 Script executed:
Repository: youknowone/pyre
Length of output: 7439
Keep bridge input frame slots from bridge input arg mapping.
The unconditional
enumerate()loop writesJITFRAME_FIXED_SIZE + positionfor every bridge input arg, overwriting the bridge-specific positions set bysetup_input_state. The subsequentcurrent_frame_locloop only updates locations that the regalloc FrameManager actually assigned, so inputs that came from a bridge frame arg can be left mapped by sequential index. Restore or carry forward the original bridge input arg slots before this loop, or apply regalloc frame updates without overwriting inputs that never received acurrent_frame_locbinding.🤖 Prompt for AI Agents