Skip to content

Commit f8cb9f3

Browse files
tekknolagitekknolagi
committed
ZJIT: Put optional interpreter cache on both GetIvar and SetIvar
1 parent 0b6daad commit f8cb9f3

File tree

6 files changed

+40
-38
lines changed

6 files changed

+40
-38
lines changed

‎vm_insnhelper.c‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1723,6 +1723,12 @@ rb_vm_setinstancevariable(const rb_iseq_t *iseq, VALUE obj, ID id, VALUE val, IV
17231723
vm_setinstancevariable(iseq, obj, id, val, ic);
17241724
}
17251725

1726+
VALUE
1727+
rb_vm_getinstancevariable(const rb_iseq_t *iseq, VALUE obj, ID id, IVC ic)
1728+
{
1729+
return vm_getinstancevariable(iseq, obj, id, ic);
1730+
}
1731+
17261732
static VALUE
17271733
vm_throw_continue(const rb_execution_context_t *ec, VALUE err)
17281734
{

‎zjit/src/codegen.rs‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -428,16 +428,15 @@ fn gen_insn(cb: &mut CodeBlock, jit: &mut JITState, asm: &mut Assembler, functio
428428
Insn::CCallVariadic { cfunc, recv, args, name, cme, state, return_type: _, elidable: _ } => {
429429
gen_ccall_variadic(jit, asm, *cfunc, *name, opnd!(recv), opnds!(args), *cme, &function.frame_state(*state))
430430
}
431-
Insn::GetIvar { self_val, id, state: _ } => gen_getivar(asm, opnd!(self_val), *id),
431+
Insn::GetIvar { self_val, id, ic, state: _ } => gen_getivar(jit, asm, opnd!(self_val), *id, *ic),
432432
Insn::SetGlobal { id, val, state } => no_output!(gen_setglobal(jit, asm, *id, opnd!(val), &function.frame_state(*state))),
433433
Insn::GetGlobal { id, state } => gen_getglobal(jit, asm, *id, &function.frame_state(*state)),
434434
&Insn::GetLocal { ep_offset, level, use_sp, .. } => gen_getlocal(asm, ep_offset, level, use_sp),
435435
&Insn::SetLocal { val, ep_offset, level } => no_output!(gen_setlocal(asm, opnd!(val), function.type_of(val), ep_offset, level)),
436436
Insn::GetConstantPath { ic, state } => gen_get_constant_path(jit, asm, *ic, &function.frame_state(*state)),
437437
Insn::GetClassVar { id, ic, state } => gen_getclassvar(jit, asm, *id, *ic, &function.frame_state(*state)),
438438
Insn::SetClassVar { id, val, ic, state } => no_output!(gen_setclassvar(jit, asm, *id, opnd!(val), *ic, &function.frame_state(*state))),
439-
Insn::SetIvar { self_val, id, val, state } => no_output!(gen_setivar(jit, asm, opnd!(self_val), *id, opnd!(val), &function.frame_state(*state))),
440-
Insn::SetInstanceVariable { self_val, id, ic, val, state } => no_output!(gen_set_instance_variable(jit, asm, opnd!(self_val), *id, *ic, opnd!(val), &function.frame_state(*state))),
439+
Insn::SetIvar { self_val, id, ic, val, state } => no_output!(gen_setivar(jit, asm, opnd!(self_val), *id, *ic, opnd!(val), &function.frame_state(*state))),
441440
Insn::FixnumBitCheck { val, index } => gen_fixnum_bit_check(asm, opnd!(val), *index),
442441
Insn::SideExit { state, reason } => no_output!(gen_side_exit(jit, asm, reason, &function.frame_state(*state))),
443442
Insn::PutSpecialObject { value_type } => gen_putspecialobject(asm, *value_type),
@@ -881,26 +880,27 @@ fn gen_ccall_variadic(
881880
}
882881

883882
/// Emit an uncached instance variable lookup
884-
fn gen_getivar(asm: &mut Assembler, recv: Opnd, id: ID) -> Opnd {
883+
fn gen_getivar(jit: &mut JITState, asm: &mut Assembler, recv: Opnd, id: ID, ic: *const iseq_inline_iv_cache_entry) -> Opnd {
885884
gen_incr_counter(asm, Counter::dynamic_getivar_count);
886-
asm_ccall!(asm, rb_ivar_get, recv, id.0.into())
885+
if ic.is_null() {
886+
asm_ccall!(asm, rb_ivar_get, recv, id.0.into())
887+
} else {
888+
let iseq = Opnd::Value(jit.iseq.into());
889+
asm_ccall!(asm, rb_vm_getinstancevariable, iseq, recv, id.0.into(), Opnd::const_ptr(ic))
890+
}
887891
}
888892

889893
/// Emit an uncached instance variable store
890-
fn gen_setivar(jit: &mut JITState, asm: &mut Assembler, recv: Opnd, id: ID, val: Opnd, state: &FrameState) {
894+
fn gen_setivar(jit: &mut JITState, asm: &mut Assembler, recv: Opnd, id: ID, ic: *const iseq_inline_iv_cache_entry, val: Opnd, state: &FrameState) {
891895
gen_incr_counter(asm, Counter::dynamic_setivar_count);
892896
// Setting an ivar can raise FrozenError, so we need proper frame state for exception handling.
893897
gen_prepare_non_leaf_call(jit, asm, state);
894-
asm_ccall!(asm, rb_ivar_set, recv, id.0.into(), val);
895-
}
896-
897-
/// Emit an uncached instance variable store using the interpreter inline cache
898-
fn gen_set_instance_variable(jit: &mut JITState, asm: &mut Assembler, recv: Opnd, id: ID, ic: *const iseq_inline_iv_cache_entry, val: Opnd, state: &FrameState) {
899-
gen_incr_counter(asm, Counter::dynamic_setivar_count);
900-
// Setting an ivar can raise FrozenError, so we need proper frame state for exception handling.
901-
gen_prepare_non_leaf_call(jit, asm, state);
902-
let iseq = Opnd::Value(jit.iseq.into());
903-
asm_ccall!(asm, rb_vm_setinstancevariable, iseq, recv, id.0.into(), val, Opnd::const_ptr(ic));
898+
if ic.is_null() {
899+
asm_ccall!(asm, rb_ivar_set, recv, id.0.into(), val);
900+
} else {
901+
let iseq = Opnd::Value(jit.iseq.into());
902+
asm_ccall!(asm, rb_vm_setinstancevariable, iseq, recv, id.0.into(), val, Opnd::const_ptr(ic));
903+
}
904904
}
905905

906906
fn gen_getclassvar(jit: &mut JITState, asm: &mut Assembler, id: ID, ic: *const iseq_inline_cvar_cache_entry, state: &FrameState) -> Opnd {

‎zjit/src/cruby.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,7 @@ unsafe extern "C" {
147147
) -> bool;
148148
pub fn rb_vm_set_ivar_id(obj: VALUE, idx: u32, val: VALUE) -> VALUE;
149149
pub fn rb_vm_setinstancevariable(iseq: IseqPtr, obj: VALUE, id: ID, val: VALUE, ic: IVC);
150+
pub fn rb_vm_getinstancevariable(iseq: IseqPtr, obj: VALUE, id: ID, ic: IVC) -> VALUE;
150151
pub fn rb_aliased_callable_method_entry(
151152
me: *const rb_callable_method_entry_t,
152153
) -> *const rb_callable_method_entry_t;

‎zjit/src/hir.rs‎

Lines changed: 15 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -707,12 +707,10 @@ pub enum Insn {
707707
SetGlobal { id: ID, val: InsnId, state: InsnId },
708708

709709
//NewObject?
710-
/// Get an instance variable `id` from `self_val`
711-
GetIvar { self_val: InsnId, id: ID, state: InsnId },
712-
/// Set `self_val`'s instance variable `id` to `val`
713-
SetIvar { self_val: InsnId, id: ID, val: InsnId, state: InsnId },
714-
/// Set `self_val`'s instance variable `id` to `val` using the interpreter inline cache
715-
SetInstanceVariable { self_val: InsnId, id: ID, ic: *const iseq_inline_iv_cache_entry, val: InsnId, state: InsnId },
710+
/// Get an instance variable `id` from `self_val`, using the inline cache `ic` if present
711+
GetIvar { self_val: InsnId, id: ID, ic: *const iseq_inline_iv_cache_entry, state: InsnId },
712+
/// Set `self_val`'s instance variable `id` to `val`, using the inline cache `ic` if present
713+
SetIvar { self_val: InsnId, id: ID, val: InsnId, ic: *const iseq_inline_iv_cache_entry, state: InsnId },
716714
/// Check whether an instance variable exists on `self_val`
717715
DefinedIvar { self_val: InsnId, id: ID, pushval: VALUE, state: InsnId },
718716

@@ -912,7 +910,7 @@ impl Insn {
912910
| Insn::PatchPoint { .. } | Insn::SetIvar { .. } | Insn::SetClassVar { .. } | Insn::ArrayExtend { .. }
913911
| Insn::ArrayPush { .. } | Insn::SideExit { .. } | Insn::SetGlobal { .. }
914912
| Insn::SetLocal { .. } | Insn::Throw { .. } | Insn::IncrCounter(_) | Insn::IncrCounterPtr { .. }
915-
| Insn::CheckInterrupts { .. } | Insn::GuardBlockParamProxy { .. } | Insn::SetInstanceVariable { .. } | Insn::StoreField { .. } | Insn::WriteBarrier { .. } => false,
913+
| Insn::CheckInterrupts { .. } | Insn::GuardBlockParamProxy { .. } | Insn::StoreField { .. } | Insn::WriteBarrier { .. } => false,
916914
_ => true,
917915
}
918916
}
@@ -1248,7 +1246,6 @@ impl<'a> std::fmt::Display for InsnPrinter<'a> {
12481246
&Insn::StoreField { recv, id, offset, val } => write!(f, "StoreField {recv}, :{}@{:p}, {val}", id.contents_lossy(), self.ptr_map.map_offset(offset)),
12491247
&Insn::WriteBarrier { recv, val } => write!(f, "WriteBarrier {recv}, {val}"),
12501248
Insn::SetIvar { self_val, id, val, .. } => write!(f, "SetIvar {self_val}, :{}, {val}", id.contents_lossy()),
1251-
Insn::SetInstanceVariable { self_val, id, val, .. } => write!(f, "SetInstanceVariable {self_val}, :{}, {val}", id.contents_lossy()),
12521249
Insn::GetGlobal { id, .. } => write!(f, "GetGlobal :{}", id.contents_lossy()),
12531250
Insn::SetGlobal { id, val, .. } => write!(f, "SetGlobal :{}, {val}", id.contents_lossy()),
12541251
&Insn::GetLocal { level, ep_offset, use_sp: true, rest_param } => write!(f, "GetLocal l{level}, SP@{}{}", ep_offset + 1, if rest_param { ", *" } else { "" }),
@@ -1891,12 +1888,11 @@ impl Function {
18911888
&ArrayInclude { ref elements, target, state } => ArrayInclude { elements: find_vec!(elements), target: find!(target), state: find!(state) },
18921889
&DupArrayInclude { ary, target, state } => DupArrayInclude { ary, target: find!(target), state: find!(state) },
18931890
&SetGlobal { id, val, state } => SetGlobal { id, val: find!(val), state },
1894-
&GetIvar { self_val, id, state } => GetIvar { self_val: find!(self_val), id, state },
1891+
&GetIvar { self_val, id, ic, state } => GetIvar { self_val: find!(self_val), id, ic, state },
18951892
&LoadField { recv, id, offset, return_type } => LoadField { recv: find!(recv), id, offset, return_type },
18961893
&StoreField { recv, id, offset, val } => StoreField { recv: find!(recv), id, offset, val: find!(val) },
18971894
&WriteBarrier { recv, val } => WriteBarrier { recv: find!(recv), val: find!(val) },
1898-
&SetIvar { self_val, id, val, state } => SetIvar { self_val: find!(self_val), id, val: find!(val), state },
1899-
&SetInstanceVariable { self_val, id, ic, val, state } => SetInstanceVariable { self_val: find!(self_val), id, ic, val: find!(val), state },
1895+
&SetIvar { self_val, id, ic, val, state } => SetIvar { self_val: find!(self_val), id, ic, val: find!(val), state },
19001896
&GetClassVar { id, ic, state } => GetClassVar { id, ic, state },
19011897
&SetClassVar { id, val, ic, state } => SetClassVar { id, val: find!(val), ic, state },
19021898
&SetLocal { val, ep_offset, level } => SetLocal { val: find!(val), ep_offset, level },
@@ -1951,7 +1947,7 @@ impl Function {
19511947
| Insn::PatchPoint { .. } | Insn::SetIvar { .. } | Insn::SetClassVar { .. } | Insn::ArrayExtend { .. }
19521948
| Insn::ArrayPush { .. } | Insn::SideExit { .. } | Insn::SetLocal { .. } | Insn::IncrCounter(_)
19531949
| Insn::CheckInterrupts { .. } | Insn::GuardBlockParamProxy { .. } | Insn::IncrCounterPtr { .. }
1954-
| Insn::SetInstanceVariable { .. } | Insn::StoreField { .. } | Insn::WriteBarrier { .. } =>
1950+
| Insn::StoreField { .. } | Insn::WriteBarrier { .. } =>
19551951
panic!("Cannot infer type of instruction with no output: {}. See Insn::has_output().", self.insns[insn.0]),
19561952
Insn::Const { val: Const::Value(val) } => Type::from_value(*val),
19571953
Insn::Const { val: Const::CBool(val) } => Type::from_cbool(*val),
@@ -2450,7 +2446,7 @@ impl Function {
24502446
self.push_insn(block, Insn::PatchPoint { invariant: Invariant::SingleRactorMode, state });
24512447
}
24522448
}
2453-
let getivar = self.push_insn(block, Insn::GetIvar { self_val: recv, id, state });
2449+
let getivar = self.push_insn(block, Insn::GetIvar { self_val: recv, id, ic: std::ptr::null(), state });
24542450
self.make_equal_to(insn_id, getivar);
24552451
} else if let (VM_METHOD_TYPE_ATTRSET, &[val]) = (def_type, args.as_slice()) {
24562452
self.push_insn(block, Insn::PatchPoint { invariant: Invariant::MethodRedefined { klass, method: mid, cme }, state });
@@ -2467,7 +2463,7 @@ impl Function {
24672463
self.push_insn(block, Insn::PatchPoint { invariant: Invariant::SingleRactorMode, state });
24682464
}
24692465
}
2470-
self.push_insn(block, Insn::SetIvar { self_val: recv, id, val, state });
2466+
self.push_insn(block, Insn::SetIvar { self_val: recv, id, ic: std::ptr::null(), val, state });
24712467
self.make_equal_to(insn_id, val);
24722468
} else if def_type == VM_METHOD_TYPE_OPTIMIZED {
24732469
let opt_type: OptimizedMethodType = unsafe { get_cme_def_body_optimized_type(cme) }.into();
@@ -2750,7 +2746,7 @@ impl Function {
27502746
assert!(self.blocks[block.0].insns.is_empty());
27512747
for insn_id in old_insns {
27522748
match self.find(insn_id) {
2753-
Insn::GetIvar { self_val, id, state } => {
2749+
Insn::GetIvar { self_val, id, ic: _, state } => {
27542750
let frame_state = self.frame_state(state);
27552751
let Some(recv_type) = self.profiled_type_of_at(self_val, frame_state.insn_idx) else {
27562752
// No (monomorphic/skewed polymorphic) profile info
@@ -3503,8 +3499,7 @@ impl Function {
35033499
worklist.push_back(self_val);
35043500
worklist.push_back(state);
35053501
}
3506-
&Insn::SetIvar { self_val, val, state, .. }
3507-
| &Insn::SetInstanceVariable { self_val, val, state, .. } => {
3502+
&Insn::SetIvar { self_val, val, state, .. } => {
35083503
worklist.push_back(self_val);
35093504
worklist.push_back(val);
35103505
worklist.push_back(state);
@@ -4082,7 +4077,6 @@ impl Function {
40824077
}
40834078
// Instructions with 2 Ruby object operands
40844079
Insn::SetIvar { self_val: left, val: right, .. }
4085-
| Insn::SetInstanceVariable { self_val: left, val: right, .. }
40864080
| Insn::NewRange { low: left, high: right, .. }
40874081
| Insn::AnyToString { val: left, str: right, .. }
40884082
| Insn::WriteBarrier { recv: left, val: right } => {
@@ -5450,11 +5444,12 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result<Function, ParseError> {
54505444
}
54515445
YARVINSN_getinstancevariable => {
54525446
let id = ID(get_arg(pc, 0).as_u64());
5447+
let ic = get_arg(pc, 1).as_ptr();
54535448
// ic is in arg 1
54545449
// Assume single-Ractor mode to omit gen_prepare_non_leaf_call on gen_getivar
54555450
// TODO: We only really need this if self_val is a class/module
54565451
fun.push_insn(block, Insn::PatchPoint { invariant: Invariant::SingleRactorMode, state: exit_id });
5457-
let result = fun.push_insn(block, Insn::GetIvar { self_val: self_param, id, state: exit_id });
5452+
let result = fun.push_insn(block, Insn::GetIvar { self_val: self_param, id, ic, state: exit_id });
54585453
state.stack_push(result);
54595454
}
54605455
YARVINSN_setinstancevariable => {
@@ -5464,7 +5459,7 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result<Function, ParseError> {
54645459
// TODO: We only really need this if self_val is a class/module
54655460
fun.push_insn(block, Insn::PatchPoint { invariant: Invariant::SingleRactorMode, state: exit_id });
54665461
let val = state.stack_pop()?;
5467-
fun.push_insn(block, Insn::SetInstanceVariable { self_val: self_param, id, ic, val, state: exit_id });
5462+
fun.push_insn(block, Insn::SetIvar { self_val: self_param, id, ic, val, state: exit_id });
54685463
}
54695464
YARVINSN_getclassvariable => {
54705465
let id = ID(get_arg(pc, 0).as_u64());

‎zjit/src/hir/opt_tests.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3367,7 +3367,7 @@ mod hir_opt_tests {
33673367
bb2(v6:BasicObject):
33683368
v10:Fixnum[1] = Const Value(1)
33693369
PatchPoint SingleRactorMode
3370-
SetInstanceVariable v6, :@foo, v10
3370+
SetIvar v6, :@foo, v10
33713371
CheckInterrupts
33723372
Return v10
33733373
");

‎zjit/src/hir/tests.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2241,7 +2241,7 @@ pub mod hir_build_tests {
22412241
bb2(v6:BasicObject):
22422242
v10:Fixnum[1] = Const Value(1)
22432243
PatchPoint SingleRactorMode
2244-
SetInstanceVariable v6, :@foo, v10
2244+
SetIvar v6, :@foo, v10
22452245
CheckInterrupts
22462246
Return v10
22472247
");

0 commit comments

Comments
 (0)