Skip to content

Commit 5a3847f

Browse files
committed
ZJIT: Support variadic C calls
This reduces the `dynamic_send_count` in `liquid-render` by ~21%
1 parent 4d003ea commit 5a3847f

3 files changed

Lines changed: 197 additions & 2 deletions

File tree

test/ruby/test_zjit.rb

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2675,6 +2675,44 @@ def test
26752675
}, insns: [:invokeblock]
26762676
end
26772677

2678+
def test_ccall_variadic_with_multiple_args
2679+
assert_compiles "[1, 2, 3]", %q{
2680+
def test
2681+
a = []
2682+
a.push(1, 2, 3)
2683+
a
2684+
end
2685+
2686+
test
2687+
test
2688+
}, insns: [:opt_send_without_block]
2689+
end
2690+
2691+
def test_ccall_variadic_with_no_args
2692+
assert_compiles "[1]", %q{
2693+
def test
2694+
a = [1]
2695+
a.push
2696+
end
2697+
2698+
test
2699+
test
2700+
}, insns: [:opt_send_without_block]
2701+
end
2702+
2703+
def test_ccall_variadic_with_no_args_causing_argument_error
2704+
assert_compiles ":error", %q{
2705+
def test
2706+
format
2707+
rescue ArgumentError
2708+
:error
2709+
end
2710+
2711+
test
2712+
test
2713+
}, insns: [:opt_send_without_block]
2714+
end
2715+
26782716
private
26792717

26802718
# Assert that every method call in `test_script` can be compiled by ZJIT

zjit/src/codegen.rs

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -395,6 +395,9 @@ fn gen_insn(cb: &mut CodeBlock, jit: &mut JITState, asm: &mut Assembler, functio
395395
Insn::GuardBitEquals { val, expected, state } => gen_guard_bit_equals(jit, asm, opnd!(val), *expected, &function.frame_state(*state)),
396396
Insn::PatchPoint { invariant, state } => no_output!(gen_patch_point(jit, asm, invariant, &function.frame_state(*state))),
397397
Insn::CCall { cfun, args, name: _, return_type: _, elidable: _ } => gen_ccall(asm, *cfun, opnds!(args)),
398+
Insn::CCallVariadic { cfun, recv, args, name: _, cme, state } => {
399+
gen_ccall_variadic(jit, asm, *cfun, opnd!(recv), opnds!(args), *cme, &function.frame_state(*state))
400+
}
398401
Insn::GetIvar { self_val, id, state: _ } => gen_getivar(asm, opnd!(self_val), *id),
399402
Insn::SetGlobal { id, val, state } => no_output!(gen_setglobal(jit, asm, *id, opnd!(val), &function.frame_state(*state))),
400403
Insn::GetGlobal { id, state } => gen_getglobal(jit, asm, *id, &function.frame_state(*state)),
@@ -640,6 +643,79 @@ fn gen_ccall(asm: &mut Assembler, cfun: *const u8, args: Vec<Opnd>) -> lir::Opnd
640643
asm.ccall(cfun, args)
641644
}
642645

646+
/// Push a C frame for cfunc calls
647+
fn gen_push_cfunc_frame(asm: &mut Assembler, recv: Opnd, cme: *const rb_callable_method_entry_t, sp_offset: i32) {
648+
asm_comment!(asm, "push C frame");
649+
650+
let new_sp = asm.lea(Opnd::mem(64, SP, sp_offset * SIZEOF_VALUE_I32));
651+
// sp[-3]: cme
652+
asm.store(Opnd::mem(64, SP, (sp_offset - 3) * SIZEOF_VALUE_I32), VALUE::from(cme).into());
653+
// sp[-2]: block_handler (specval)
654+
asm.store(Opnd::mem(64, SP, (sp_offset - 2) * SIZEOF_VALUE_I32), VM_BLOCK_HANDLER_NONE.into());
655+
// sp[-1]: frame type (must be a valid FIXNUM)
656+
let frame_type = VM_FRAME_MAGIC_CFUNC | VM_FRAME_FLAG_CFRAME | VM_ENV_FLAG_LOCAL;
657+
asm.store(Opnd::mem(64, SP, (sp_offset - 1) * SIZEOF_VALUE_I32), frame_type.into());
658+
659+
fn cfp_opnd(offset: i32) -> Opnd {
660+
Opnd::mem(64, CFP, offset - (RUBY_SIZEOF_CONTROL_FRAME as i32))
661+
}
662+
663+
asm.mov(cfp_opnd(RUBY_OFFSET_CFP_PC), 0.into()); // C frames don't have a PC
664+
asm.mov(cfp_opnd(RUBY_OFFSET_CFP_SP), new_sp); // SP for new frame
665+
666+
asm.mov(cfp_opnd(RUBY_OFFSET_CFP_ISEQ), 0.into()); // C frames don't have an iseq
667+
asm.mov(cfp_opnd(RUBY_OFFSET_CFP_SELF), recv);
668+
asm.mov(cfp_opnd(RUBY_OFFSET_CFP_BLOCK_CODE), 0.into());
669+
670+
let ep = asm.sub(new_sp, SIZEOF_VALUE.into());
671+
asm.mov(cfp_opnd(RUBY_OFFSET_CFP_EP), ep);
672+
}
673+
674+
/// Pop a C frame after cfunc call
675+
fn gen_pop_cfunc_frame(asm: &mut Assembler) {
676+
asm_comment!(asm, "pop C frame");
677+
let new_cfp = asm.add(CFP, RUBY_SIZEOF_CONTROL_FRAME.into());
678+
asm.mov(CFP, new_cfp);
679+
asm.store(Opnd::mem(64, EC, RUBY_OFFSET_EC_CFP), CFP);
680+
}
681+
682+
/// Generate code for a variadic C function call
683+
/// func(int argc, VALUE *argv, VALUE recv)
684+
fn gen_ccall_variadic(
685+
jit: &mut JITState,
686+
asm: &mut Assembler,
687+
cfun: *const u8,
688+
recv: Opnd,
689+
args: Vec<Opnd>,
690+
cme: *const rb_callable_method_entry_t,
691+
state: &FrameState,
692+
) -> lir::Opnd {
693+
gen_prepare_non_leaf_call(jit, asm, state);
694+
695+
asm_comment!(asm, "pushing C frame for cme=0x{:x}", cme as usize);
696+
// Following YJIT: allocate 3 slots for frame metadata (cme, block_handler, frame_type)
697+
// The new frame's SP will be at current_stack_size + 3
698+
let sp_offset = state.stack().len() as i32 + 3;
699+
gen_push_cfunc_frame(asm, recv, cme, sp_offset);
700+
701+
// Move to the new frame
702+
let new_cfp = asm.lea(Opnd::mem(64, CFP, -(RUBY_SIZEOF_CONTROL_FRAME as i32)));
703+
asm.mov(CFP, new_cfp);
704+
asm.store(Opnd::mem(64, EC, RUBY_OFFSET_EC_CFP), new_cfp);
705+
706+
let argc = args.len();
707+
708+
asm_comment!(asm, "call variadic cfunc: argc={}", argc);
709+
710+
let argv_ptr = gen_push_opnds(jit, asm, &args);
711+
let result = asm.ccall(cfun, vec![argc.into(), argv_ptr, recv]);
712+
gen_pop_opnds(asm, &args);
713+
714+
gen_pop_cfunc_frame(asm);
715+
716+
result
717+
}
718+
643719
/// Emit an uncached instance variable lookup
644720
fn gen_getivar(asm: &mut Assembler, recv: Opnd, id: ID) -> Opnd {
645721
asm_ccall!(asm, rb_ivar_get, recv, id.0.into())

zjit/src/hir.rs

Lines changed: 83 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -581,6 +581,17 @@ pub enum Insn {
581581
/// `name` is for printing purposes only
582582
CCall { cfun: *const u8, args: Vec<InsnId>, name: ID, return_type: Type, elidable: bool },
583583

584+
/// Call a variadic C function with signature: func(int argc, VALUE *argv, VALUE recv)
585+
/// This handles frame setup, argv creation, and frame teardown all in one
586+
CCallVariadic {
587+
cfun: *const u8,
588+
recv: InsnId,
589+
args: Vec<InsnId>,
590+
cme: *const rb_callable_method_entry_t,
591+
name: ID,
592+
state: InsnId,
593+
},
594+
584595
/// Un-optimized fallback implementation (dynamic dispatch) for send-ish instructions
585596
/// Ignoring keyword arguments etc for now
586597
SendWithoutBlock { recv: InsnId, cd: *const rb_call_data, args: Vec<InsnId>, state: InsnId },
@@ -713,6 +724,7 @@ impl Insn {
713724
Insn::LoadIvarEmbedded { .. } => false,
714725
Insn::LoadIvarExtended { .. } => false,
715726
Insn::CCall { elidable, .. } => !elidable,
727+
Insn::CCallVariadic { .. } => true,
716728
// TODO: NewRange is effects free if we can prove the two ends to be Fixnum,
717729
// but we don't have type information here in `impl Insn`. See rb_range_new().
718730
Insn::NewRange { .. } => true,
@@ -888,6 +900,13 @@ impl<'a> std::fmt::Display for InsnPrinter<'a> {
888900
}
889901
Ok(())
890902
},
903+
Insn::CCallVariadic { cfun, recv, args, name, .. } => {
904+
write!(f, "CCallVariadic {}@{:p}, {recv}", name.contents_lossy(), self.ptr_map.map_ptr(cfun))?;
905+
for arg in args {
906+
write!(f, ", {arg}")?;
907+
}
908+
Ok(())
909+
},
891910
Insn::Snapshot { state } => write!(f, "Snapshot {}", state.print(self.ptr_map)),
892911
Insn::Defined { op_type, v, .. } => {
893912
// op_type (enum defined_type) printing logic from iseq.c.
@@ -1368,6 +1387,9 @@ impl Function {
13681387
&HashDup { val, state } => HashDup { val: find!(val), state },
13691388
&ObjectAlloc { val, state } => ObjectAlloc { val: find!(val), state },
13701389
&CCall { cfun, ref args, name, return_type, elidable } => CCall { cfun, args: find_vec!(args), name, return_type, elidable },
1390+
&CCallVariadic { cfun, recv, ref args, cme, name, state } => CCallVariadic {
1391+
cfun, recv: find!(recv), args: find_vec!(args), cme, name, state
1392+
},
13711393
&Defined { op_type, obj, pushval, v, state } => Defined { op_type, obj, pushval, v: find!(v), state: find!(state) },
13721394
&DefinedIvar { self_val, pushval, id, state } => DefinedIvar { self_val: find!(self_val), pushval, id, state },
13731395
&NewArray { ref elements, state } => NewArray { elements: find_vec!(elements), state: find!(state) },
@@ -1455,6 +1477,7 @@ impl Function {
14551477
Insn::NewRangeFixnum { .. } => types::RangeExact,
14561478
Insn::ObjectAlloc { .. } => types::HeapObject,
14571479
Insn::CCall { return_type, .. } => *return_type,
1480+
Insn::CCallVariadic { .. } => types::BasicObject,
14581481
Insn::GuardType { val, guard_type, .. } => self.type_of(*val).intersection(*guard_type),
14591482
Insn::GuardTypeNot { .. } => types::BasicObject,
14601483
Insn::GuardBitEquals { val, expected, .. } => self.type_of(*val).intersection(Type::from_value(*expected)),
@@ -1982,9 +2005,42 @@ impl Function {
19822005
return Ok(());
19832006
}
19842007
}
2008+
// Variadic method
19852009
-1 => {
1986-
// (argc, argv, self) parameter form
1987-
// Falling through for now
2010+
// The method gets a pointer to the first argument
2011+
// rb_f_puts(int argc, VALUE *argv, VALUE recv)
2012+
let ci_flags = unsafe { vm_ci_flag(call_info) };
2013+
if ci_flags & VM_CALL_ARGS_SIMPLE != 0 {
2014+
// We need profiled type information for variadic calls
2015+
// to ensure we're calling the right method
2016+
if profiled_type.is_none() {
2017+
return Err(());
2018+
}
2019+
2020+
// Add method redefinition guard
2021+
fun.push_insn(block, Insn::PatchPoint {
2022+
invariant: Invariant::MethodRedefined {
2023+
klass: recv_class,
2024+
method: method_id,
2025+
cme: method
2026+
},
2027+
state
2028+
});
2029+
2030+
let cfun = unsafe { get_mct_func(cfunc) }.cast();
2031+
let ccall = fun.push_insn(block, Insn::CCallVariadic {
2032+
cfun,
2033+
recv,
2034+
args,
2035+
cme: method,
2036+
name: method_id,
2037+
state,
2038+
});
2039+
2040+
fun.make_equal_to(send_insn_id, ccall);
2041+
return Ok(());
2042+
}
2043+
// Fall through for complex cases (splat, kwargs, etc.)
19882044
}
19892045
-2 => {
19902046
// (self, args_ruby_array) parameter form
@@ -2297,6 +2353,11 @@ impl Function {
22972353
worklist.push_back(state)
22982354
}
22992355
Insn::CCall { args, .. } => worklist.extend(args),
2356+
Insn::CCallVariadic { recv, args, state, .. } => {
2357+
worklist.push_back(*recv);
2358+
worklist.extend(args);
2359+
worklist.push_back(*state);
2360+
}
23002361
&Insn::GetIvar { self_val, state, .. } | &Insn::DefinedIvar { self_val, state, .. } => {
23012362
worklist.push_back(self_val);
23022363
worklist.push_back(state);
@@ -6797,6 +6858,26 @@ mod opt_tests {
67976858
");
67986859
}
67996860

6861+
#[test]
6862+
fn test_optimize_variadic_ccall() {
6863+
eval("
6864+
def test
6865+
puts 'Hello'
6866+
end
6867+
test; test
6868+
");
6869+
assert_snapshot!(hir_string("test"), @r"
6870+
fn test@<compiled>:3:
6871+
bb0(v0:BasicObject):
6872+
v4:StringExact[VALUE(0x1000)] = Const Value(VALUE(0x1000))
6873+
v6:StringExact = StringCopy v4
6874+
PatchPoint MethodRedefined(Object@0x1008, puts@0x1010, cme:0x1018)
6875+
v15:BasicObject = CCallVariadic puts@0x1040, v0, v6
6876+
CheckInterrupts
6877+
Return v15
6878+
");
6879+
}
6880+
68006881
#[test]
68016882
fn test_dont_optimize_fixnum_add_if_redefined() {
68026883
eval("

0 commit comments

Comments
 (0)