Skip to content

Commit fba13cc

Browse files
committed
ZJIT: Pass a &blk argument through to a direct C call
A C method takes its block out of the frame's specval exactly like an ISEQ method does, and ZJIT already writes a literal block's handler there for `CCallWithFrame` and `CCallVariadic`. So the reduction the previous commits added for `&blk` -- a Proc is its own block handler, and the block param proxy resolves to this frame's -- works for a C callee too; only `unspecializable_c_call_type` stood in the way. The one difference is the stack: the ISEQ path takes the block argument out of the caller's stack because the callee's parameters are laid out there, while a C frame is simply pushed over the argument slots. So the C path keeps the original frame state and only tells `gen_push_frame` that one more slot is consumed. The inline bodies and the leaf `CCall` fast path still opt out, as they do for a literal block: neither pushes a frame to carry the handler. On lobsters this converts `Hash#fetch(k, &blk)`, `Array#bsearch_index(&blk)` and friends: dynamic_send_count 10,527,955 -> 10,380,293 one_or_more_complex_arg_pass 1,423,400 -> 1,018,033 send_block_arg_not_nil 57,695 -> 319,890 The block-arg counter grows because a C site whose block argument cannot be reduced now reports why it stayed dynamic instead of being lumped in with the complex-argument sites; those calls were already dynamic before.
1 parent 4e402fa commit fba13cc

4 files changed

Lines changed: 99 additions & 38 deletions

File tree

zjit/src/codegen.rs

Lines changed: 20 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -785,12 +785,14 @@ fn gen_insn(cb: &mut CodeBlock, jit: &mut JITState, asm: &mut Assembler, functio
785785
Insn::PatchPoint { invariant, state } => no_output!(gen_patch_point(jit, asm, function, invariant, &function.frame_state(*state))),
786786
Insn::CCall { cfunc, recv, args, name, owner, return_type: _, elidable: _ } => gen_ccall(asm, *cfunc, *name, *owner, opnd!(recv), opnds!(args)),
787787
Insn::CCallWithFrame(insn) => {
788-
let CCallWithFrameData { cfunc, recv, name, args, cme, state, block, .. } = &**insn;
789-
gen_ccall_with_frame(jit, asm, function, *cfunc, *name, opnd!(recv), opnds!(args), *cme, *block, &function.frame_state(*state))
788+
let CCallWithFrameData { cfunc, recv, name, args, cme, state, block, block_arg, .. } = &**insn;
789+
let block_arg = block_arg.map(|block_arg| opnd!(block_arg));
790+
gen_ccall_with_frame(jit, asm, function, *cfunc, *name, opnd!(recv), opnds!(args), *cme, *block, block_arg, &function.frame_state(*state))
790791
}
791792
Insn::CCallVariadic(insn) => {
792-
let CCallVariadicData { cfunc, recv, name, args, cme, state, block, .. } = &**insn;
793-
gen_ccall_variadic(jit, asm, function, *cfunc, *name, opnd!(recv), opnds!(args), *cme, *block, &function.frame_state(*state))
793+
let CCallVariadicData { cfunc, recv, name, args, cme, state, block, block_arg, .. } = &**insn;
794+
let block_arg = block_arg.map(|block_arg| opnd!(block_arg));
795+
gen_ccall_variadic(jit, asm, function, *cfunc, *name, opnd!(recv), opnds!(args), *cme, *block, block_arg, &function.frame_state(*state))
794796
}
795797
Insn::GetIvar { self_val, id, ic, state } => gen_getivar(asm, opnd!(self_val), *id, *ic, &function.frame_state(*state)),
796798
Insn::SetGlobal { id, val, state } => no_output!(gen_setglobal(jit, asm, function, *id, opnd!(val), &function.frame_state(*state))),
@@ -1137,13 +1139,16 @@ fn gen_ccall_with_frame(
11371139
args: Vec<Opnd>,
11381140
cme: *const rb_callable_method_entry_t,
11391141
block: Option<BlockHandler>,
1142+
block_arg: Option<lir::Opnd>,
11401143
state: &FrameState,
11411144
) -> lir::Opnd {
11421145
gen_incr_counter(asm, Counter::non_variadic_cfunc_optimized_send_count);
11431146
gen_stack_overflow_check(jit, asm, function, state, state.stack_size());
11441147

1145-
let args_with_recv_len = args.len() + 1;
1146-
let caller_stack_size = state.stack().len() - args_with_recv_len;
1148+
// A `&blk` argument keeps its VM stack slot, above the arguments and below the frame this
1149+
// pushes, so the frame setup consumes one more slot than the C function has arguments.
1150+
let stack_slots_consumed = args.len() + 1 + usize::from(block_arg.is_some());
1151+
let caller_stack_size = state.stack().len() - stack_slots_consumed;
11471152

11481153
// Can't use gen_prepare_non_leaf_call() because we need to adjust the SP
11491154
// to account for the receiver and arguments (and block arguments if any)
@@ -1160,10 +1165,10 @@ fn gen_ccall_with_frame(
11601165
let cfp_self_addr = asm.lea(Opnd::mem(64, CFP, RUBY_OFFSET_CFP_SELF));
11611166
asm.or(cfp_self_addr, Opnd::Imm(1))
11621167
} else {
1163-
VM_BLOCK_HANDLER_NONE.into()
1168+
block_arg.unwrap_or_else(|| VM_BLOCK_HANDLER_NONE.into())
11641169
};
11651170

1166-
gen_push_frame(asm, args_with_recv_len, state, ControlFrame {
1171+
gen_push_frame(asm, stack_slots_consumed, state, ControlFrame {
11671172
recv,
11681173
iseq: None,
11691174
cme,
@@ -1230,16 +1235,16 @@ fn gen_ccall_variadic(
12301235
args: Vec<Opnd>,
12311236
cme: *const rb_callable_method_entry_t,
12321237
block: Option<BlockHandler>,
1238+
block_arg: Option<lir::Opnd>,
12331239
state: &FrameState,
12341240
) -> lir::Opnd {
12351241
gen_incr_counter(asm, Counter::variadic_cfunc_optimized_send_count);
12361242
gen_stack_overflow_check(jit, asm, function, state, state.stack_size());
12371243

1238-
let args_with_recv_len = args.len() + 1;
1239-
1240-
// Compute the caller's stack size after consuming recv and args.
1241-
// state.stack() includes recv + args, so subtract both.
1242-
let caller_stack_size = state.stack_size() - args_with_recv_len;
1244+
// Compute the caller's stack size after consuming recv, args and, when the site passes one,
1245+
// the `&blk` argument's slot. state.stack() includes all of them.
1246+
let stack_slots_consumed = args.len() + 1 + usize::from(block_arg.is_some());
1247+
let caller_stack_size = state.stack_size() - stack_slots_consumed;
12431248

12441249
// Can't use gen_prepare_non_leaf_call() because we need to adjust the SP
12451250
// to account for the receiver and arguments (like gen_ccall_with_frame does)
@@ -1251,10 +1256,10 @@ fn gen_ccall_variadic(
12511256
let block_handler_specval = if let Some(BlockHandler::BlockIseq(blockiseq)) = block {
12521257
gen_block_handler_specval(asm, blockiseq)
12531258
} else {
1254-
VM_BLOCK_HANDLER_NONE.into()
1259+
block_arg.unwrap_or_else(|| VM_BLOCK_HANDLER_NONE.into())
12551260
};
12561261

1257-
gen_push_frame(asm, args_with_recv_len, state, ControlFrame {
1262+
gen_push_frame(asm, stack_slots_consumed, state, ControlFrame {
12581263
recv,
12591264
iseq: None,
12601265
cme,

zjit/src/codegen_tests.rs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2465,6 +2465,28 @@ fn test_send_block_param_proxy_from_block_body() {
24652465
"), @"[[199, 202], [9, nil]]");
24662466
}
24672467

2468+
#[test]
2469+
fn test_send_forwards_block_arg_to_cfunc() {
2470+
// A C method takes its block from the frame's specval too, so `&blk` forwarding reaches
2471+
// `Hash#fetch` (variadic) and `Array#bsearch_index` (fixed arity) as a direct C call. The
2472+
// no-block calls must still see no block.
2473+
assert_snapshot!(inspect("
2474+
def fetch(h, k, &b) = h.fetch(k, &b)
2475+
def search(a, &b) = a.bsearch_index(&b)
2476+
def each_with_proc(a, p) = a.each(&p)
2477+
out = []
2478+
300.times do
2479+
out << fetch({ a: 1 }, :b) { |k| \"no #{k}\" }
2480+
out << fetch({ a: 1 }, :a) { |k| \"no #{k}\" }
2481+
out << search([1, 3, 5, 7]) { |x| x >= 5 }
2482+
out << (fetch({ a: 1 }, :b) rescue :keyerror)
2483+
end
2484+
seen = []
2485+
each_with_proc([1, 2], ->(x) { seen << x })
2486+
[out.last(4), seen]
2487+
"), @r#"[["no b", 1, 2, :keyerror], [1, 2]]"#);
2488+
}
2489+
24682490
#[test]
24692491
fn test_send_symbol_block_arg() {
24702492
assert_snapshot!(inspect("

zjit/src/hir.rs

Lines changed: 55 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1018,6 +1018,10 @@ pub struct CCallWithFrameData {
10181018
pub return_type: Type,
10191019
pub elidable: bool,
10201020
pub block: Option<BlockHandler>,
1021+
/// See [`SendDirectData::block_arg`]. Unlike the ISEQ case this is *not* taken off the VM
1022+
/// stack: the frame the C method runs in is pushed over the argument slots, so the frame
1023+
/// setup counts the block argument's slot even though the C function never sees it.
1024+
pub block_arg: Option<InsnId>,
10211025
}
10221026

10231027
/// Payload of [`Insn::SendDirect`]. Boxed in the enum to keep `Insn` small.
@@ -1051,6 +1055,8 @@ pub struct CCallVariadicData {
10511055
pub return_type: Type,
10521056
pub elidable: bool,
10531057
pub block: Option<BlockHandler>,
1058+
/// See [`CCallWithFrameData::block_arg`].
1059+
pub block_arg: Option<InsnId>,
10541060
}
10551061

10561062
/// An instruction in the SSA IR. The output of an instruction is referred to by the index of
@@ -1700,11 +1706,13 @@ macro_rules! for_each_operand_impl {
17001706
Insn::CCallWithFrame(insn) => {
17011707
$visit_one!(insn.recv);
17021708
$visit_many!(insn.args);
1709+
$visit_many!(insn.block_arg);
17031710
$visit_one!(insn.state);
17041711
}
17051712
Insn::CCallVariadic(insn) => {
17061713
$visit_one!(insn.recv);
17071714
$visit_many!(insn.args);
1715+
$visit_many!(insn.block_arg);
17081716
$visit_one!(insn.state);
17091717
}
17101718
Insn::InvokeBlock { args, state, .. } => {
@@ -2483,7 +2491,7 @@ impl<'a> std::fmt::Display for InsnPrinter<'a> {
24832491
Ok(())
24842492
},
24852493
Insn::CCallWithFrame(insn) => {
2486-
let CCallWithFrameData { cfunc, recv, args, name, cme, block, .. } = &**insn;
2494+
let CCallWithFrameData { cfunc, recv, args, name, cme, block, block_arg, .. } = &**insn;
24872495
write!(f, "CCallWithFrame {recv}, :{}@{:p}", qualified_method_name(unsafe { (**cme).owner }, *name), self.ptr_map.map_ptr(*cfunc))?;
24882496
write_separated!(f, ", ", ", ", args);
24892497
match block {
@@ -2493,12 +2501,18 @@ impl<'a> std::fmt::Display for InsnPrinter<'a> {
24932501
write!(f, ", block=&block")?,
24942502
None => {}
24952503
}
2504+
if let Some(block_arg) = block_arg {
2505+
write!(f, ", block=&{block_arg}")?;
2506+
}
24962507
Ok(())
24972508
},
24982509
Insn::CCallVariadic(insn) => {
2499-
let CCallVariadicData { cfunc, recv, args, name, cme, .. } = &**insn;
2510+
let CCallVariadicData { cfunc, recv, args, name, cme, block_arg, .. } = &**insn;
25002511
write!(f, "CCallVariadic {recv}, :{}@{:p}", qualified_method_name(unsafe { (**cme).owner }, *name), self.ptr_map.map_ptr(*cfunc))?;
25012512
write_separated!(f, ", ", ", ", args);
2513+
if let Some(block_arg) = block_arg {
2514+
write!(f, ", block=&{block_arg}")?;
2515+
}
25022516
Ok(())
25032517
},
25042518
Insn::IncrCounterPtr { .. } => write!(f, "IncrCounterPtr"),
@@ -5883,7 +5897,12 @@ impl Function {
58835897
}
58845898
let mut stripped_block_arg = false;
58855899
let mut send_block_arg = None;
5886-
if send_block == Some(BlockHandler::BlockArg) && def_type == VM_METHOD_TYPE_ISEQ {
5900+
// A C method's frame carries the block handler in its specval just like an
5901+
// ISEQ frame's, so the same reduction applies; the difference is that the
5902+
// block argument keeps its VM stack slot, which the C frame setup accounts
5903+
// for. Nothing else reads `args` positionally for a C call.
5904+
if send_block == Some(BlockHandler::BlockArg)
5905+
&& matches!(def_type, VM_METHOD_TYPE_ISEQ | VM_METHOD_TYPE_CFUNC) {
58875906
// The block arg is the last element in args
58885907
if let Some(&block_arg) = args.last() {
58895908
let statically_nil = self.is_a(block_arg, types::NilClass);
@@ -6245,12 +6264,13 @@ impl Function {
62456264
cme: *const rb_callable_method_entry_struct,
62466265
method_id: ID,
62476266
argc: u32,
6267+
// The call site's flags with `VM_CALL_ARGS_BLOCKARG` cleared when
6268+
// `block_arg` already holds the handler the interpreter would have
6269+
// built from it.
6270+
ci_flags: u32,
6271+
block_arg: Option<InsnId>,
62486272
) -> Result<(), ()> {
6249-
let call_info = unsafe { (*cd).ci };
6250-
6251-
let ci_flags = unsafe { vm_ci_flag(call_info) };
6252-
// When seeing &block argument, fall back to dynamic dispatch for now
6253-
// TODO: Support block forwarding
6273+
// Argument shapes the C frame setup cannot reproduce.
62546274
if unspecializable_c_call_type(ci_flags) {
62556275
// Only count features NOT already counted in type_specialize.
62566276
if !unspecializable_call_type(ci_flags) {
@@ -6265,6 +6285,10 @@ impl Function {
62656285
Some(BlockHandler::BlockIseq(blockiseq)) => Some(blockiseq),
62666286
None => None,
62676287
};
6288+
// A block reaches the callee either way, so neither the inline
6289+
// bodies nor the leaf fast path (which push no frame to carry the
6290+
// handler) can serve this call.
6291+
let passes_block = blockiseq.is_some() || block_arg.is_some();
62686292

62696293
let cfunc = unsafe { get_cme_def_body_cfunc(cme) };
62706294
// Find the `argc` (arity) of the C method, which describes the parameters it expects
@@ -6279,10 +6303,8 @@ impl Function {
62796303
}
62806304
let props = props.unwrap_or_default();
62816305
let return_type = props.return_type;
6282-
let elidable = match blockiseq {
6283-
Some(_) => false, // Don't consider cfuncs with block arguments as elidable for now
6284-
None => props.elidable,
6285-
};
6306+
// Don't consider cfuncs with block arguments as elidable for now
6307+
let elidable = !passes_block && props.elidable;
62866308

62876309
match cfunc_argc {
62886310
0.. => {
@@ -6309,7 +6331,7 @@ impl Function {
63096331
}
63106332

63116333
// Try inlining the cfunc into HIR. Only inline if we don't have a block argument
6312-
if blockiseq.is_none() {
6334+
if !passes_block {
63136335
let tmp_block = fun.new_block(u32::MAX);
63146336
if let Some(replacement) = (props.inline)(fun, tmp_block, recv, &args, state) {
63156337
// Copy contents of tmp_block to block
@@ -6352,6 +6374,7 @@ impl Function {
63526374
return_type,
63536375
elidable,
63546376
block: blockiseq.map(BlockHandler::BlockIseq),
6377+
block_arg,
63556378
})));
63566379
fun.insn_types[ccall] = fun.infer_type(ccall);
63576380
fun.make_equal_to(send_insn_id, ccall);
@@ -6376,7 +6399,7 @@ impl Function {
63766399
}
63776400

63786401
// Try inlining the cfunc into HIR. Only inline if we don't have a block argument
6379-
if blockiseq.is_none() {
6402+
if !passes_block {
63806403
let tmp_block = fun.new_block(u32::MAX);
63816404
if let Some(replacement) = (props.inline)(fun, tmp_block, recv, &args, state) {
63826405
// Copy contents of tmp_block to block
@@ -6419,6 +6442,7 @@ impl Function {
64196442
return_type,
64206443
elidable,
64216444
block: blockiseq.map(BlockHandler::BlockIseq),
6445+
block_arg,
64226446
})));
64236447
fun.insn_types[ccall] = fun.infer_type(ccall);
64246448
fun.make_equal_to(send_insn_id, ccall);
@@ -6434,7 +6458,7 @@ impl Function {
64346458
}
64356459

64366460
let ccall_argc = if send_mid_override.is_some() { args.len() as u32 } else { unsafe { vm_ci_argc(ci) } };
6437-
if reduce_send_to_ccall(self, block, insn_id, recv, cd, send_block, args, state, klass, profiled_type, cme, mid, ccall_argc).is_ok() {
6461+
if reduce_send_to_ccall(self, block, insn_id, recv, cd, send_block, args, state, klass, profiled_type, cme, mid, ccall_argc, flags_for_check, send_block_arg).is_ok() {
64386462
continue;
64396463
}
64406464

@@ -6694,6 +6718,7 @@ impl Function {
66946718
return_type,
66956719
elidable,
66966720
block: None,
6721+
block_arg: None,
66976722
})))
66986723
};
66996724
self.make_equal_to(insn_id, ccall);
@@ -6743,6 +6768,7 @@ impl Function {
67436768
return_type,
67446769
elidable,
67456770
block: None,
6771+
block_arg: None,
67466772
})))
67476773
};
67486774
self.make_equal_to(insn_id, ccall);
@@ -9135,13 +9161,19 @@ impl Function {
91359161
for &arg in &insn.args {
91369162
self.assert_subtype(insn_id, arg, types::BasicObject)?;
91379163
}
9164+
if let Some(block_arg) = insn.block_arg {
9165+
self.assert_subtype(insn_id, block_arg, types::BasicObject)?;
9166+
}
91389167
Ok(())
91399168
}
91409169
Insn::CCallVariadic(ref insn) => {
91419170
self.assert_subtype(insn_id, insn.recv, types::BasicObject)?;
91429171
for &arg in &insn.args {
91439172
self.assert_subtype(insn_id, arg, types::BasicObject)?;
91449173
}
9174+
if let Some(block_arg) = insn.block_arg {
9175+
self.assert_subtype(insn_id, block_arg, types::BasicObject)?;
9176+
}
91459177
Ok(())
91469178
}
91479179
Insn::ArrayPackBuffer { ref elements, fmt, buffer, .. } => {
@@ -10132,9 +10164,10 @@ struct AddIseqResult {
1013210164
profiles: ProfileOracle,
1013310165
}
1013410166

10135-
/// Whether any receiver class this site profiled resolves the call to an ISEQ method, which is
10136-
/// the only method type whose frame setup `type_specialize` can hand a `&blk` block handler to.
10137-
fn profiled_recv_has_iseq_callee(
10167+
/// Whether any receiver class this site profiled resolves the call to a method whose frame setup
10168+
/// `type_specialize` can hand a `&blk` block handler to. Only ISEQ and C methods get such a
10169+
/// frame; the rest keep the dynamic send whatever the block argument is.
10170+
fn profiled_recv_takes_block_handler(
1013810171
fun: &Function,
1013910172
profiles: &ProfileOracle,
1014010173
recv: InsnId,
@@ -10151,7 +10184,7 @@ fn profiled_recv_has_iseq_callee(
1015110184
cme = unsafe { rb_aliased_callable_method_entry(cme) };
1015210185
def_type = unsafe { get_cme_def_type(cme) };
1015310186
}
10154-
def_type == VM_METHOD_TYPE_ISEQ
10187+
matches!(def_type, VM_METHOD_TYPE_ISEQ | VM_METHOD_TYPE_CFUNC)
1015510188
})
1015610189
}
1015710190

@@ -11699,10 +11732,9 @@ fn add_iseq_to_hir(
1169911732
&& args.last().is_some_and(|arg| block_param_proxy_values.contains(arg))
1170011733
&& block_arg_summary.as_ref().is_some_and(|summary| summary.buckets().iter().any(|profiled_type|
1170111734
!profiled_type.is_empty() && profiled_type.class() == proxy_class))
11702-
// Only an ISEQ callee's frame setup takes the handler; a C method reads
11703-
// its block from a frame ZJIT does not build for a `&blk` argument, so
11704-
// its call stays dynamic and the branch would be dead weight.
11705-
&& profiled_recv_has_iseq_callee(fun, &profiles, recv, exit_id, cd);
11735+
// Only an ISEQ or C callee's frame setup takes the handler; for anything
11736+
// else the call stays dynamic and the branch would be dead weight.
11737+
&& profiled_recv_takes_block_handler(fun, &profiles, recv, exit_id, cd);
1170611738
let proxy_join = if proxy_split {
1170711739
let block_arg_insn = *args.last().unwrap();
1170811740
let join_block = fun.new_block(insn_idx);

0 commit comments

Comments
 (0)