Skip to content

Commit c928cc2

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 93a9712 commit c928cc2

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
@@ -8846,6 +8846,104 @@ fn test_invokeblock_truncated_block_with_return() {
88468846
assert_snapshot!(assert_compiles_allowing_exits("entry"), @":returned");
88478847
}
88488848

8849+
/// A block that `break`s is dispatched directly, not through `rb_vm_invokeblock()`. The
8850+
/// throw unwinds out of the JIT-pushed block frame, which reports its ISEQ through the
8851+
/// JITFrame rather than `cfp->_iseq`.
8852+
#[test]
8853+
fn test_invokeblock_direct_dispatch_with_break() {
8854+
eval("
8855+
def test
8856+
yield 1
8857+
yield 2
8858+
:not_reached
8859+
end
8860+
def entry
8861+
test { |x| break x * 10 if x == 2 }
8862+
end
8863+
entry; entry
8864+
");
8865+
assert_contains_opcode("test", YARVINSN_invokeblock);
8866+
assert_snapshot!(assert_compiles_allowing_exits("entry"), @"20");
8867+
}
8868+
8869+
/// `break` out of a directly dispatched block nested two `yield`s deep unwinds only out of
8870+
/// the `yield` that owns that block, leaving the outer one to run to completion.
8871+
#[test]
8872+
fn test_invokeblock_direct_dispatch_with_nested_break() {
8873+
eval("
8874+
def test
8875+
yield 1
8876+
:after
8877+
end
8878+
def entry
8879+
test { |a| test { |b| break [:inner, a, b] } }
8880+
end
8881+
entry; entry
8882+
");
8883+
assert_contains_opcode("test", YARVINSN_invokeblock);
8884+
assert_snapshot!(assert_compiles_allowing_exits("entry"), @":after");
8885+
}
8886+
8887+
/// `break` inside a lambda is a `return` from the lambda, not an unwind to the block owner.
8888+
#[test]
8889+
fn test_invokeblock_direct_dispatch_break_in_lambda() {
8890+
eval("
8891+
def test
8892+
yield 1
8893+
end
8894+
def entry
8895+
l = lambda { break :from_lambda }
8896+
[test { |x| x }, l.call]
8897+
end
8898+
entry; entry
8899+
");
8900+
assert_contains_opcode("test", YARVINSN_invokeblock);
8901+
assert_snapshot!(assert_compiles_allowing_exits("entry"), @"[1, :from_lambda]");
8902+
}
8903+
8904+
/// A `break` whose block outlived the method that created it is still an orphan.
8905+
#[test]
8906+
fn test_invokeblock_direct_dispatch_orphan_break() {
8907+
eval("
8908+
def make(&b) = b
8909+
def test
8910+
yield 1
8911+
end
8912+
def entry
8913+
test { |x| x }
8914+
pr = make { break :nope }
8915+
begin
8916+
pr.call
8917+
rescue LocalJumpError
8918+
:orphan
8919+
end
8920+
end
8921+
entry; entry
8922+
");
8923+
assert_contains_opcode("test", YARVINSN_invokeblock);
8924+
assert_snapshot!(assert_compiles_allowing_exits("entry"), @":orphan");
8925+
}
8926+
8927+
/// An `ensure` inside a block that `break`s still runs while the throw unwinds out of the
8928+
/// JIT-pushed block frame.
8929+
#[test]
8930+
fn test_invokeblock_direct_dispatch_break_runs_ensure() {
8931+
eval("
8932+
def test
8933+
yield 1
8934+
:not_reached
8935+
end
8936+
def entry
8937+
ran = false
8938+
out = test { |x| begin; break :broke; ensure; ran = true; end }
8939+
[out, ran]
8940+
end
8941+
entry; entry
8942+
");
8943+
assert_contains_opcode("test", YARVINSN_invokeblock);
8944+
assert_snapshot!(assert_compiles_allowing_exits("entry"), @"[:broke, true]");
8945+
}
8946+
88498947
/// The iterator is inlined into the caller and its `yield` reshapes the arguments for the
88508948
/// block, whose body is then inlined at the yield too. The frame that push lays out has to
88518949
/// 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
@@ -3918,15 +3918,11 @@ fn direct_invoke_block_adapt(iseq: IseqPtr, argc: usize) -> Result<BlockArgAdapt
39183918
std::cmp::Ordering::Less => BlockArgAdapt::NilFill(lead_num - argc),
39193919
std::cmp::Ordering::Greater => BlockArgAdapt::Truncate(lead_num),
39203920
};
3921-
// `break` out of a directly-invoked block frame does not unwind correctly:
3922-
// vm_throw_start() matches the block owner's `cfp->pc` against the CATCH_TYPE_BREAK
3923-
// entry's `cont`, and the PC the owner's frame reports after this dispatch does not
3924-
// match, so the break is reported as an orphan ("break from proc-closure").
3925-
// A plain non-local `return` is looked up by frame type and EP instead of by PC, so
3926-
// blocks that only throw TAG_RETURN -- what this dispatch is for -- are fine.
3927-
if crate::codegen::block_iseq_may_throw(iseq) && !block_iseq_throws_only_return(iseq) {
3928-
return Err(InvokeBlockMayThrow);
3929-
}
3921+
// A `throw` out of the block frame this dispatch pushes unwinds like any other: it
3922+
// longjmps out of every JIT native frame to the enclosing `vm_exec()`, which resumes at
3923+
// the catch entry the throw resolved to. `break` needed `vm_throw_start()` to read the
3924+
// throwing frame's ISEQ through `CFP_ISEQ()` rather than the raw `cfp->_iseq` a
3925+
// JIT-pushed frame never writes; without that it walked off a null ISEQ.
39303926
Ok(adapt)
39313927
}
39323928

0 commit comments

Comments
 (0)