Skip to content

Commit 91bb08f

Browse files
committed
ZJIT: Standardize C call related insn fields
- Add `recv` field to `CCall` and `CCallWithFrame` so now all method dispatch related instructions have `recv` field, separate from `args` field. This ensures consistent pointer arithmetic when generating code for these instructions. - Standardize `recv` field's display position in send related instructions.
1 parent 8dc5822 commit 91bb08f

5 files changed

Lines changed: 102 additions & 91 deletions

File tree

zjit/src/codegen.rs

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -425,12 +425,13 @@ fn gen_insn(cb: &mut CodeBlock, jit: &mut JITState, asm: &mut Assembler, functio
425425
&Insn::GuardLess { left, right, state } => gen_guard_less(jit, asm, opnd!(left), opnd!(right), &function.frame_state(state)),
426426
&Insn::GuardGreaterEq { left, right, state } => gen_guard_greater_eq(jit, asm, opnd!(left), opnd!(right), &function.frame_state(state)),
427427
Insn::PatchPoint { invariant, state } => no_output!(gen_patch_point(jit, asm, invariant, &function.frame_state(*state))),
428-
Insn::CCall { cfunc, args, name, return_type: _, elidable: _ } => gen_ccall(asm, *cfunc, *name, opnds!(args)),
429-
// Give up CCallWithFrame for 7+ args since asm.ccall() doesn't support it.
430-
Insn::CCallWithFrame { cd, state, args, .. } if args.len() > C_ARG_OPNDS.len() =>
428+
Insn::CCall { cfunc, recv, args, name, return_type: _, elidable: _ } => gen_ccall(asm, *cfunc, *name, opnd!(recv), opnds!(args)),
429+
// Give up CCallWithFrame for 7+ args since asm.ccall() supports at most 6 args (recv + args).
430+
// There's no test case for this because no core cfuncs have this many parameters. But C extensions could have such methods.
431+
Insn::CCallWithFrame { cd, state, args, .. } if args.len() + 1 > C_ARG_OPNDS.len() =>
431432
gen_send_without_block(jit, asm, *cd, &function.frame_state(*state), SendFallbackReason::CCallWithFrameTooManyArgs),
432-
Insn::CCallWithFrame { cfunc, name, args, cme, state, blockiseq, .. } =>
433-
gen_ccall_with_frame(jit, asm, *cfunc, *name, opnds!(args), *cme, *blockiseq, &function.frame_state(*state)),
433+
Insn::CCallWithFrame { cfunc, recv, name, args, cme, state, blockiseq, .. } =>
434+
gen_ccall_with_frame(jit, asm, *cfunc, *name, opnd!(recv), opnds!(args), *cme, *blockiseq, &function.frame_state(*state)),
434435
Insn::CCallVariadic { cfunc, recv, args, name, cme, state, return_type: _, elidable: _ } => {
435436
gen_ccall_variadic(jit, asm, *cfunc, *name, opnd!(recv), opnds!(args), *cme, &function.frame_state(*state))
436437
}
@@ -766,6 +767,7 @@ fn gen_ccall_with_frame(
766767
asm: &mut Assembler,
767768
cfunc: *const u8,
768769
name: ID,
770+
recv: Opnd,
769771
args: Vec<Opnd>,
770772
cme: *const rb_callable_method_entry_t,
771773
blockiseq: Option<IseqPtr>,
@@ -774,7 +776,8 @@ fn gen_ccall_with_frame(
774776
gen_incr_counter(asm, Counter::non_variadic_cfunc_optimized_send_count);
775777
gen_stack_overflow_check(jit, asm, state, state.stack_size());
776778

777-
let caller_stack_size = state.stack_size() - args.len();
779+
let args_with_recv_len = args.len() + 1;
780+
let caller_stack_size = state.stack().len() - args_with_recv_len;
778781

779782
// Can't use gen_prepare_non_leaf_call() because we need to adjust the SP
780783
// to account for the receiver and arguments (and block arguments if any)
@@ -794,8 +797,8 @@ fn gen_ccall_with_frame(
794797
VM_BLOCK_HANDLER_NONE.into()
795798
};
796799

797-
gen_push_frame(asm, args.len(), state, ControlFrame {
798-
recv: args[0],
800+
gen_push_frame(asm, args_with_recv_len, state, ControlFrame {
801+
recv,
799802
iseq: None,
800803
cme,
801804
frame_type: VM_FRAME_MAGIC_CFUNC | VM_FRAME_FLAG_CFRAME | VM_ENV_FLAG_LOCAL,
@@ -813,8 +816,10 @@ fn gen_ccall_with_frame(
813816
asm.mov(CFP, new_cfp);
814817
asm.store(Opnd::mem(64, EC, RUBY_OFFSET_EC_CFP), CFP);
815818

819+
let mut cfunc_args = vec![recv];
820+
cfunc_args.extend(args);
816821
asm.count_call_to(&name.contents_lossy());
817-
let result = asm.ccall(cfunc, args);
822+
let result = asm.ccall(cfunc, cfunc_args);
818823

819824
asm_comment!(asm, "pop C frame");
820825
let new_cfp = asm.add(CFP, RUBY_SIZEOF_CONTROL_FRAME.into());
@@ -830,9 +835,11 @@ fn gen_ccall_with_frame(
830835

831836
/// Lowering for [`Insn::CCall`]. This is a low-level raw call that doesn't know
832837
/// anything about the callee, so handling for e.g. GC safety is dealt with elsewhere.
833-
fn gen_ccall(asm: &mut Assembler, cfunc: *const u8, name: ID, args: Vec<Opnd>) -> lir::Opnd {
838+
fn gen_ccall(asm: &mut Assembler, cfunc: *const u8, name: ID, recv: Opnd, args: Vec<Opnd>) -> lir::Opnd {
839+
let mut cfunc_args = vec![recv];
840+
cfunc_args.extend(args);
834841
asm.count_call_to(&name.contents_lossy());
835-
asm.ccall(cfunc, args)
842+
asm.ccall(cfunc, cfunc_args)
836843
}
837844

838845
/// Generate code for a variadic C function call

zjit/src/cruby_methods.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -440,7 +440,8 @@ fn inline_string_eq(fun: &mut hir::Function, block: hir::BlockId, recv: hir::Ins
440440
// TODO(max): Make StringEqual its own opcode so that we can later constant-fold StringEqual(a, a) => true
441441
let result = fun.push_insn(block, hir::Insn::CCall {
442442
cfunc: rb_yarv_str_eql_internal as *const u8,
443-
args: vec![recv, other],
443+
recv,
444+
args: vec![other],
444445
name: ID!(string_eq),
445446
return_type,
446447
elidable,

zjit/src/hir.rs

Lines changed: 29 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -778,12 +778,13 @@ pub enum Insn {
778778

779779
/// Call a C function without pushing a frame
780780
/// `name` is for printing purposes only
781-
CCall { cfunc: *const u8, args: Vec<InsnId>, name: ID, return_type: Type, elidable: bool },
781+
CCall { cfunc: *const u8, recv: InsnId, args: Vec<InsnId>, name: ID, return_type: Type, elidable: bool },
782782

783783
/// Call a C function that pushes a frame
784784
CCallWithFrame {
785785
cd: *const rb_call_data, // cd for falling back to SendWithoutBlock
786786
cfunc: *const u8,
787+
recv: InsnId,
787788
args: Vec<InsnId>,
788789
cme: *const rb_callable_method_entry_t,
789790
name: ID,
@@ -1179,8 +1180,8 @@ impl<'a> std::fmt::Display for InsnPrinter<'a> {
11791180
}
11801181
Ok(())
11811182
}
1182-
Insn::SendForward { cd, args, blockiseq, .. } => {
1183-
write!(f, "SendForward {:p}, :{}", self.ptr_map.map_ptr(blockiseq), ruby_call_method_name(*cd))?;
1183+
Insn::SendForward { recv, cd, args, blockiseq, .. } => {
1184+
write!(f, "SendForward {recv}, {:p}, :{}", self.ptr_map.map_ptr(blockiseq), ruby_call_method_name(*cd))?;
11841185
for arg in args {
11851186
write!(f, ", {arg}")?;
11861187
}
@@ -1239,15 +1240,15 @@ impl<'a> std::fmt::Display for InsnPrinter<'a> {
12391240
Insn::GetConstantPath { ic, .. } => { write!(f, "GetConstantPath {:p}", self.ptr_map.map_ptr(ic)) },
12401241
Insn::IsBlockGiven => { write!(f, "IsBlockGiven") },
12411242
Insn::FixnumBitCheck {val, index} => { write!(f, "FixnumBitCheck {val}, {index}") },
1242-
Insn::CCall { cfunc, args, name, return_type: _, elidable: _ } => {
1243-
write!(f, "CCall {}@{:p}", name.contents_lossy(), self.ptr_map.map_ptr(cfunc))?;
1243+
Insn::CCall { cfunc, recv, args, name, return_type: _, elidable: _ } => {
1244+
write!(f, "CCall {recv}, :{}@{:p}", name.contents_lossy(), self.ptr_map.map_ptr(cfunc))?;
12441245
for arg in args {
12451246
write!(f, ", {arg}")?;
12461247
}
12471248
Ok(())
12481249
},
1249-
Insn::CCallWithFrame { cfunc, args, name, blockiseq, .. } => {
1250-
write!(f, "CCallWithFrame {}@{:p}", name.contents_lossy(), self.ptr_map.map_ptr(cfunc))?;
1250+
Insn::CCallWithFrame { cfunc, recv, args, name, blockiseq, .. } => {
1251+
write!(f, "CCallWithFrame {recv}, :{}@{:p}", name.contents_lossy(), self.ptr_map.map_ptr(cfunc))?;
12511252
for arg in args {
12521253
write!(f, ", {arg}")?;
12531254
}
@@ -1256,8 +1257,8 @@ impl<'a> std::fmt::Display for InsnPrinter<'a> {
12561257
}
12571258
Ok(())
12581259
},
1259-
Insn::CCallVariadic { cfunc, recv, args, name, .. } => {
1260-
write!(f, "CCallVariadic {}@{:p}, {recv}", name.contents_lossy(), self.ptr_map.map_ptr(cfunc))?;
1260+
Insn::CCallVariadic { cfunc, recv, args, name, .. } => {
1261+
write!(f, "CCallVariadic {recv}, :{}@{:p}", name.contents_lossy(), self.ptr_map.map_ptr(cfunc))?;
12611262
for arg in args {
12621263
write!(f, ", {arg}")?;
12631264
}
@@ -1908,10 +1909,11 @@ impl Function {
19081909
&HashAref { hash, key, state } => HashAref { hash: find!(hash), key: find!(key), state },
19091910
&ObjectAlloc { val, state } => ObjectAlloc { val: find!(val), state },
19101911
&ObjectAllocClass { class, state } => ObjectAllocClass { class, state: find!(state) },
1911-
&CCall { cfunc, ref args, name, return_type, elidable } => CCall { cfunc, args: find_vec!(args), name, return_type, elidable },
1912-
&CCallWithFrame { cd, cfunc, ref args, cme, name, state, return_type, elidable, blockiseq } => CCallWithFrame {
1912+
&CCall { cfunc, recv, ref args, name, return_type, elidable } => CCall { cfunc, recv: find!(recv), args: find_vec!(args), name, return_type, elidable },
1913+
&CCallWithFrame { cd, cfunc, recv, ref args, cme, name, state, return_type, elidable, blockiseq } => CCallWithFrame {
19131914
cd,
19141915
cfunc,
1916+
recv: find!(recv),
19151917
args: find_vec!(args),
19161918
cme,
19171919
name,
@@ -2958,7 +2960,7 @@ impl Function {
29582960
send: Insn,
29592961
send_insn_id: InsnId,
29602962
) -> Result<(), ()> {
2961-
let Insn::Send { mut recv, cd, blockiseq, mut args, state, .. } = send else {
2963+
let Insn::Send { mut recv, cd, blockiseq, args, state, .. } = send else {
29622964
return Err(());
29632965
};
29642966

@@ -3027,14 +3029,13 @@ impl Function {
30273029

30283030
// Emit a call
30293031
let cfunc = unsafe { get_mct_func(cfunc) }.cast();
3030-
let mut cfunc_args = vec![recv];
3031-
cfunc_args.append(&mut args);
30323032

30333033
let name = rust_str_to_id(&qualified_method_name(unsafe { (*cme).owner }, unsafe { (*cme).called_id }));
30343034
let ccall = fun.push_insn(block, Insn::CCallWithFrame {
30353035
cd,
30363036
cfunc,
3037-
args: cfunc_args,
3037+
recv,
3038+
args,
30383039
cme,
30393040
name,
30403041
state,
@@ -3068,7 +3069,7 @@ impl Function {
30683069
send: Insn,
30693070
send_insn_id: InsnId,
30703071
) -> Result<(), ()> {
3071-
let Insn::SendWithoutBlock { mut recv, cd, mut args, state, .. } = send else {
3072+
let Insn::SendWithoutBlock { mut recv, cd, args, state, .. } = send else {
30723073
return Err(());
30733074
};
30743075

@@ -3161,14 +3162,12 @@ impl Function {
31613162
// No inlining; emit a call
31623163
let cfunc = unsafe { get_mct_func(cfunc) }.cast();
31633164
let name = rust_str_to_id(&qualified_method_name(unsafe { (*cme).owner }, unsafe { (*cme).called_id }));
3164-
let mut cfunc_args = vec![recv];
3165-
cfunc_args.append(&mut args);
31663165
let return_type = props.return_type;
31673166
let elidable = props.elidable;
31683167
// Filter for a leaf and GC free function
31693168
if props.leaf && props.no_gc {
31703169
fun.push_insn(block, Insn::IncrCounter(Counter::inline_cfunc_optimized_send_count));
3171-
let ccall = fun.push_insn(block, Insn::CCall { cfunc, args: cfunc_args, name, return_type, elidable });
3170+
let ccall = fun.push_insn(block, Insn::CCall { cfunc, recv, args, name, return_type, elidable });
31723171
fun.make_equal_to(send_insn_id, ccall);
31733172
} else {
31743173
if get_option!(stats) {
@@ -3177,7 +3176,8 @@ impl Function {
31773176
let ccall = fun.push_insn(block, Insn::CCallWithFrame {
31783177
cd,
31793178
cfunc,
3180-
args: cfunc_args,
3179+
recv,
3180+
args,
31813181
cme,
31823182
name,
31833183
state,
@@ -3631,19 +3631,22 @@ impl Function {
36313631
| &Insn::SendForward { recv, ref args, state, .. }
36323632
| &Insn::SendWithoutBlock { recv, ref args, state, .. }
36333633
| &Insn::CCallVariadic { recv, ref args, state, .. }
3634+
| &Insn::CCallWithFrame { recv, ref args, state, .. }
36343635
| &Insn::SendWithoutBlockDirect { recv, ref args, state, .. }
36353636
| &Insn::InvokeSuper { recv, ref args, state, .. } => {
36363637
worklist.push_back(recv);
36373638
worklist.extend(args);
36383639
worklist.push_back(state);
36393640
}
3640-
&Insn::CCallWithFrame { ref args, state, .. }
3641-
| &Insn::InvokeBuiltin { ref args, state, .. }
3641+
&Insn::InvokeBuiltin { ref args, state, .. }
36423642
| &Insn::InvokeBlock { ref args, state, .. } => {
36433643
worklist.extend(args);
36443644
worklist.push_back(state)
36453645
}
3646-
Insn::CCall { args, .. } => worklist.extend(args),
3646+
&Insn::CCall { recv, ref args, .. } => {
3647+
worklist.push_back(recv);
3648+
worklist.extend(args);
3649+
}
36473650
&Insn::GetIvar { self_val, state, .. } | &Insn::DefinedIvar { self_val, state, .. } => {
36483651
worklist.push_back(self_val);
36493652
worklist.push_back(state);
@@ -4197,7 +4200,6 @@ impl Function {
41974200
| Insn::IncrCounter { .. }
41984201
| Insn::IncrCounterPtr { .. }
41994202
| Insn::CheckInterrupts { .. }
4200-
| Insn::CCall { .. }
42014203
| Insn::GetClassVar { .. }
42024204
| Insn::GetSpecialNumber { .. }
42034205
| Insn::GetSpecialSymbol { .. }
@@ -4245,6 +4247,8 @@ impl Function {
42454247
| Insn::Send { recv, ref args, .. }
42464248
| Insn::SendForward { recv, ref args, .. }
42474249
| Insn::InvokeSuper { recv, ref args, .. }
4250+
| Insn::CCall { recv, ref args, .. }
4251+
| Insn::CCallWithFrame { recv, ref args, .. }
42484252
| Insn::CCallVariadic { recv, ref args, .. }
42494253
| Insn::ArrayInclude { target: recv, elements: ref args, .. } => {
42504254
self.assert_subtype(insn_id, recv, types::BasicObject)?;
@@ -4254,8 +4258,7 @@ impl Function {
42544258
Ok(())
42554259
}
42564260
// Instructions with a Vec of Ruby objects
4257-
Insn::CCallWithFrame { ref args, .. }
4258-
| Insn::InvokeBuiltin { ref args, .. }
4261+
Insn::InvokeBuiltin { ref args, .. }
42594262
| Insn::InvokeBlock { ref args, .. }
42604263
| Insn::NewArray { elements: ref args, .. }
42614264
| Insn::ArrayHash { elements: ref args, .. }

0 commit comments

Comments
 (0)