Skip to content

Commit 1f40922

Browse files
committed
ZJIT: Dispatch a yield to a block that can break directly
`direct_invoke_block_adapt()` refused any block containing a `throw` other than a plain non-local `return`, so every `yield` to a block with a `break` in it called `rb_vm_invokeblock()` for the life of the process. The refusal was there because `break` came out as an orphan ("break from proc-closure") or a segfault, which the previous commit traces to `vm_throw_start()` reading the raw `cfp->_iseq` of a frame ZJIT pushed. With that read going through `CFP_ISEQ()`, the throw unwinds out of a JIT-pushed block frame the same way it unwinds out of one the interpreter pushed: `rb_zjit_throw()` longjmps past every JIT native frame to the enclosing `vm_exec()`, which resumes at the catch entry the throw resolved to. `Kernel#loop` and `Integer#downto` are the two sites this matters for on lobsters, and both are monomorphic in their block, so they were already one gate away from the direct dispatch. lobsters, 15 iterations: rb_vm_invokeblock 1,350,799 -> 1,223,051 (-9.5%) invokeblock_may_throw 127,811 -> 0 Two master snapshots renumbered: optimize_send_with_block and optimize_send_to_aliased_cfunc now inline Array#map, because inlining it is what unlocks the direct yield inside it.
1 parent b84190e commit 1f40922

2 files changed

Lines changed: 103 additions & 9 deletions

File tree

zjit/src/codegen_tests.rs

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8090,6 +8090,104 @@ fn test_invokeblock_truncated_block_with_return() {
80908090
assert_snapshot!(assert_compiles_allowing_exits("entry"), @":returned");
80918091
}
80928092

8093+
/// A block that `break`s is dispatched directly, not through `rb_vm_invokeblock()`. The
8094+
/// throw unwinds out of the JIT-pushed block frame, which reports its ISEQ through the
8095+
/// JITFrame rather than `cfp->_iseq`.
8096+
#[test]
8097+
fn test_invokeblock_direct_dispatch_with_break() {
8098+
eval("
8099+
def test
8100+
yield 1
8101+
yield 2
8102+
:not_reached
8103+
end
8104+
def entry
8105+
test { |x| break x * 10 if x == 2 }
8106+
end
8107+
entry; entry
8108+
");
8109+
assert_contains_opcode("test", YARVINSN_invokeblock);
8110+
assert_snapshot!(assert_compiles_allowing_exits("entry"), @"20");
8111+
}
8112+
8113+
/// `break` out of a directly dispatched block nested two `yield`s deep unwinds only out of
8114+
/// the `yield` that owns that block, leaving the outer one to run to completion.
8115+
#[test]
8116+
fn test_invokeblock_direct_dispatch_with_nested_break() {
8117+
eval("
8118+
def test
8119+
yield 1
8120+
:after
8121+
end
8122+
def entry
8123+
test { |a| test { |b| break [:inner, a, b] } }
8124+
end
8125+
entry; entry
8126+
");
8127+
assert_contains_opcode("test", YARVINSN_invokeblock);
8128+
assert_snapshot!(assert_compiles_allowing_exits("entry"), @":after");
8129+
}
8130+
8131+
/// `break` inside a lambda is a `return` from the lambda, not an unwind to the block owner.
8132+
#[test]
8133+
fn test_invokeblock_direct_dispatch_break_in_lambda() {
8134+
eval("
8135+
def test
8136+
yield 1
8137+
end
8138+
def entry
8139+
l = lambda { break :from_lambda }
8140+
[test { |x| x }, l.call]
8141+
end
8142+
entry; entry
8143+
");
8144+
assert_contains_opcode("test", YARVINSN_invokeblock);
8145+
assert_snapshot!(assert_compiles_allowing_exits("entry"), @"[1, :from_lambda]");
8146+
}
8147+
8148+
/// A `break` whose block outlived the method that created it is still an orphan.
8149+
#[test]
8150+
fn test_invokeblock_direct_dispatch_orphan_break() {
8151+
eval("
8152+
def make(&b) = b
8153+
def test
8154+
yield 1
8155+
end
8156+
def entry
8157+
test { |x| x }
8158+
pr = make { break :nope }
8159+
begin
8160+
pr.call
8161+
rescue LocalJumpError
8162+
:orphan
8163+
end
8164+
end
8165+
entry; entry
8166+
");
8167+
assert_contains_opcode("test", YARVINSN_invokeblock);
8168+
assert_snapshot!(assert_compiles_allowing_exits("entry"), @":orphan");
8169+
}
8170+
8171+
/// An `ensure` inside a block that `break`s still runs while the throw unwinds out of the
8172+
/// JIT-pushed block frame.
8173+
#[test]
8174+
fn test_invokeblock_direct_dispatch_break_runs_ensure() {
8175+
eval("
8176+
def test
8177+
yield 1
8178+
:not_reached
8179+
end
8180+
def entry
8181+
ran = false
8182+
out = test { |x| begin; break :broke; ensure; ran = true; end }
8183+
[out, ran]
8184+
end
8185+
entry; entry
8186+
");
8187+
assert_contains_opcode("test", YARVINSN_invokeblock);
8188+
assert_snapshot!(assert_compiles_allowing_exits("entry"), @"[:broke, true]");
8189+
}
8190+
80938191
/// The iterator is inlined into the caller and its `yield` reshapes the arguments for the
80948192
/// block, whose body is then inlined at the yield too. The frame that push lays out has to
80958193
/// follow the reshaped arguments, not the interpreter's stack, or the frame the block raises

zjit/src/hir.rs

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3640,15 +3640,11 @@ fn direct_invoke_block_adapt(iseq: IseqPtr, argc: usize) -> Result<BlockArgAdapt
36403640
std::cmp::Ordering::Less => BlockArgAdapt::NilFill(lead_num - argc),
36413641
std::cmp::Ordering::Greater => BlockArgAdapt::Truncate(lead_num),
36423642
};
3643-
// `break` out of a directly-invoked block frame does not unwind correctly:
3644-
// vm_throw_start() matches the block owner's `cfp->pc` against the CATCH_TYPE_BREAK
3645-
// entry's `cont`, and the PC the owner's frame reports after this dispatch does not
3646-
// match, so the break is reported as an orphan ("break from proc-closure").
3647-
// A plain non-local `return` is looked up by frame type and EP instead of by PC, so
3648-
// blocks that only throw TAG_RETURN -- what this dispatch is for -- are fine.
3649-
if crate::codegen::block_iseq_may_throw(iseq) && !block_iseq_throws_only_return(iseq) {
3650-
return Err(InvokeBlockMayThrow);
3651-
}
3643+
// A `throw` out of the block frame this dispatch pushes unwinds like any other: it
3644+
// longjmps out of every JIT native frame to the enclosing `vm_exec()`, which resumes at
3645+
// the catch entry the throw resolved to. `break` needed `vm_throw_start()` to read the
3646+
// throwing frame's ISEQ through `CFP_ISEQ()` rather than the raw `cfp->_iseq` a
3647+
// JIT-pushed frame never writes; without that it walked off a null ISEQ.
36523648
Ok(adapt)
36533649
}
36543650

0 commit comments

Comments
 (0)