Skip to content

Commit 7ed41f1

Browse files
committed
ZJIT: Support variadic C calls
This reduces the `dynamic_send_count` in `liquid-render` by ~21%
1 parent bb33557 commit 7ed41f1

3 files changed

Lines changed: 196 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
@@ -396,6 +396,9 @@ fn gen_insn(cb: &mut CodeBlock, jit: &mut JITState, asm: &mut Assembler, functio
396396
&Insn::GuardBlockParamProxy { level, state } => no_output!(gen_guard_block_param_proxy(jit, asm, level, &function.frame_state(state))),
397397
Insn::PatchPoint { invariant, state } => no_output!(gen_patch_point(jit, asm, invariant, &function.frame_state(*state))),
398398
Insn::CCall { cfun, args, name: _, return_type: _, elidable: _ } => gen_ccall(asm, *cfun, opnds!(args)),
399+
Insn::CCallVariadic { cfun, recv, args, name: _, cme, state } => {
400+
gen_ccall_variadic(jit, asm, *cfun, opnd!(recv), opnds!(args), *cme, &function.frame_state(*state))
401+
}
399402
Insn::GetIvar { self_val, id, state: _ } => gen_getivar(asm, opnd!(self_val), *id),
400403
Insn::SetGlobal { id, val, state } => no_output!(gen_setglobal(jit, asm, *id, opnd!(val), &function.frame_state(*state))),
401404
Insn::GetGlobal { id, state } => gen_getglobal(jit, asm, *id, &function.frame_state(*state)),
@@ -637,6 +640,79 @@ fn gen_ccall(asm: &mut Assembler, cfun: *const u8, args: Vec<Opnd>) -> lir::Opnd
637640
asm.ccall(cfun, args)
638641
}
639642

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

‎zjit/src/hir.rs‎

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

617+
/// Call a variadic C function with signature: func(int argc, VALUE *argv, VALUE recv)
618+
/// This handles frame setup, argv creation, and frame teardown all in one
619+
CCallVariadic {
620+
cfun: *const u8,
621+
recv: InsnId,
622+
args: Vec<InsnId>,
623+
cme: *const rb_callable_method_entry_t,
624+
name: ID,
625+
state: InsnId,
626+
},
627+
617628
/// Un-optimized fallback implementation (dynamic dispatch) for send-ish instructions
618629
/// Ignoring keyword arguments etc for now
619630
SendWithoutBlock {
@@ -931,6 +942,13 @@ impl<'a> std::fmt::Display for InsnPrinter<'a> {
931942
}
932943
Ok(())
933944
},
945+
Insn::CCallVariadic { cfun, recv, args, name, .. } => {
946+
write!(f, "CCallVariadic {}@{:p}, {recv}", name.contents_lossy(), self.ptr_map.map_ptr(cfun))?;
947+
for arg in args {
948+
write!(f, ", {arg}")?;
949+
}
950+
Ok(())
951+
},
934952
Insn::Snapshot { state } => write!(f, "Snapshot {}", state.print(self.ptr_map)),
935953
Insn::Defined { op_type, v, .. } => {
936954
// op_type (enum defined_type) printing logic from iseq.c.
@@ -1412,6 +1430,9 @@ impl Function {
14121430
&HashDup { val, state } => HashDup { val: find!(val), state },
14131431
&ObjectAlloc { val, state } => ObjectAlloc { val: find!(val), state },
14141432
&CCall { cfun, ref args, name, return_type, elidable } => CCall { cfun, args: find_vec!(args), name, return_type, elidable },
1433+
&CCallVariadic { cfun, recv, ref args, cme, name, state } => CCallVariadic {
1434+
cfun, recv: find!(recv), args: find_vec!(args), cme, name, state
1435+
},
14151436
&Defined { op_type, obj, pushval, v, state } => Defined { op_type, obj, pushval, v: find!(v), state: find!(state) },
14161437
&DefinedIvar { self_val, pushval, id, state } => DefinedIvar { self_val: find!(self_val), pushval, id, state },
14171438
&NewArray { ref elements, state } => NewArray { elements: find_vec!(elements), state: find!(state) },
@@ -1498,6 +1519,7 @@ impl Function {
14981519
Insn::NewRangeFixnum { .. } => types::RangeExact,
14991520
Insn::ObjectAlloc { .. } => types::HeapObject,
15001521
Insn::CCall { return_type, .. } => *return_type,
1522+
Insn::CCallVariadic { .. } => types::BasicObject,
15011523
Insn::GuardType { val, guard_type, .. } => self.type_of(*val).intersection(*guard_type),
15021524
Insn::GuardTypeNot { .. } => types::BasicObject,
15031525
Insn::GuardBitEquals { val, expected, .. } => self.type_of(*val).intersection(Type::from_value(*expected)),
@@ -2027,9 +2049,42 @@ impl Function {
20272049
return Ok(());
20282050
}
20292051
}
2052+
// Variadic method
20302053
-1 => {
2031-
// (argc, argv, self) parameter form
2032-
// Falling through for now
2054+
// The method gets a pointer to the first argument
2055+
// rb_f_puts(int argc, VALUE *argv, VALUE recv)
2056+
let ci_flags = unsafe { vm_ci_flag(call_info) };
2057+
if ci_flags & VM_CALL_ARGS_SIMPLE != 0 {
2058+
// We need profiled type information for variadic calls
2059+
// to ensure we're calling the right method
2060+
if profiled_type.is_none() {
2061+
return Err(());
2062+
}
2063+
2064+
// Add method redefinition guard
2065+
fun.push_insn(block, Insn::PatchPoint {
2066+
invariant: Invariant::MethodRedefined {
2067+
klass: recv_class,
2068+
method: method_id,
2069+
cme: method
2070+
},
2071+
state
2072+
});
2073+
2074+
let cfun = unsafe { get_mct_func(cfunc) }.cast();
2075+
let ccall = fun.push_insn(block, Insn::CCallVariadic {
2076+
cfun,
2077+
recv,
2078+
args,
2079+
cme: method,
2080+
name: method_id,
2081+
state,
2082+
});
2083+
2084+
fun.make_equal_to(send_insn_id, ccall);
2085+
return Ok(());
2086+
}
2087+
// Fall through for complex cases (splat, kwargs, etc.)
20332088
}
20342089
-2 => {
20352090
// (self, args_ruby_array) parameter form
@@ -2342,6 +2397,11 @@ impl Function {
23422397
worklist.push_back(state)
23432398
}
23442399
Insn::CCall { args, .. } => worklist.extend(args),
2400+
Insn::CCallVariadic { recv, args, state, .. } => {
2401+
worklist.push_back(*recv);
2402+
worklist.extend(args);
2403+
worklist.push_back(*state);
2404+
}
23452405
&Insn::GetIvar { self_val, state, .. } | &Insn::DefinedIvar { self_val, state, .. } => {
23462406
worklist.push_back(self_val);
23472407
worklist.push_back(state);
@@ -6845,6 +6905,26 @@ mod opt_tests {
68456905
");
68466906
}
68476907

6908+
#[test]
6909+
fn test_optimize_variadic_ccall() {
6910+
eval("
6911+
def test
6912+
puts 'Hello'
6913+
end
6914+
test; test
6915+
");
6916+
assert_snapshot!(hir_string("test"), @r"
6917+
fn test@<compiled>:3:
6918+
bb0(v0:BasicObject):
6919+
v4:StringExact[VALUE(0x1000)] = Const Value(VALUE(0x1000))
6920+
v6:StringExact = StringCopy v4
6921+
PatchPoint MethodRedefined(Object@0x1008, puts@0x1010, cme:0x1018)
6922+
v15:BasicObject = CCallVariadic puts@0x1040, v0, v6
6923+
CheckInterrupts
6924+
Return v15
6925+
");
6926+
}
6927+
68486928
#[test]
68496929
fn test_dont_optimize_fixnum_add_if_redefined() {
68506930
eval("

0 commit comments

Comments
 (0)