Skip to content

Commit 271be0a

Browse files
composerinteraliacomposerinteralia
authored
ZJIT: Implement classvar get and set (ruby#14918)
Shopify#649 Class vars are a bit more involved than ivars, since we need to get the class from the cref, so this calls out to `rb_vm_getclassvariable` and `rb_vm_setclassvariable` like YJIT.
1 parent da214cf commit 271be0a

File tree

3 files changed

+124
-2
lines changed

3 files changed

+124
-2
lines changed

‎test/ruby/test_zjit.rb‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1664,6 +1664,28 @@ def test() = @foo = 1
16641664
}
16651665
end
16661666

1667+
def test_getclassvariable
1668+
assert_compiles '42', %q{
1669+
class Foo
1670+
def self.test = @@x
1671+
end
1672+
1673+
Foo.class_variable_set(:@@x, 42)
1674+
Foo.test()
1675+
}
1676+
end
1677+
1678+
def test_setclassvariable
1679+
assert_compiles '42', %q{
1680+
class Foo
1681+
def self.test = @@x = 42
1682+
end
1683+
1684+
Foo.test()
1685+
Foo.class_variable_get(:@@x)
1686+
}
1687+
end
1688+
16671689
def test_attr_reader
16681690
assert_compiles '[4, 4]', %q{
16691691
class C

‎zjit/src/codegen.rs‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -426,6 +426,8 @@ fn gen_insn(cb: &mut CodeBlock, jit: &mut JITState, asm: &mut Assembler, functio
426426
&Insn::GetLocal { ep_offset, level, use_sp, .. } => gen_getlocal(asm, ep_offset, level, use_sp),
427427
&Insn::SetLocal { val, ep_offset, level } => no_output!(gen_setlocal(asm, opnd!(val), function.type_of(val), ep_offset, level)),
428428
Insn::GetConstantPath { ic, state } => gen_get_constant_path(jit, asm, *ic, &function.frame_state(*state)),
429+
Insn::GetClassVar { id, ic, state } => gen_getclassvar(jit, asm, *id, *ic, &function.frame_state(*state)),
430+
Insn::SetClassVar { id, val, ic, state } => no_output!(gen_setclassvar(jit, asm, *id, opnd!(val), *ic, &function.frame_state(*state))),
429431
Insn::SetIvar { self_val, id, val, state } => no_output!(gen_setivar(jit, asm, opnd!(self_val), *id, opnd!(val), &function.frame_state(*state))),
430432
Insn::SideExit { state, reason } => no_output!(gen_side_exit(jit, asm, reason, &function.frame_state(*state))),
431433
Insn::PutSpecialObject { value_type } => gen_putspecialobject(asm, *value_type),
@@ -832,6 +834,20 @@ fn gen_setivar(jit: &mut JITState, asm: &mut Assembler, recv: Opnd, id: ID, val:
832834
asm_ccall!(asm, rb_ivar_set, recv, id.0.into(), val);
833835
}
834836

837+
fn gen_getclassvar(jit: &mut JITState, asm: &mut Assembler, id: ID, ic: *const iseq_inline_cvar_cache_entry, state: &FrameState) -> Opnd {
838+
gen_prepare_non_leaf_call(jit, asm, state);
839+
840+
let iseq = asm.load(Opnd::mem(64, CFP, RUBY_OFFSET_CFP_ISEQ));
841+
asm_ccall!(asm, rb_vm_getclassvariable, iseq, CFP, id.0.into(), Opnd::const_ptr(ic))
842+
}
843+
844+
fn gen_setclassvar(jit: &mut JITState, asm: &mut Assembler, id: ID, val: Opnd, ic: *const iseq_inline_cvar_cache_entry, state: &FrameState) {
845+
gen_prepare_non_leaf_call(jit, asm, state);
846+
847+
let iseq = asm.load(Opnd::mem(64, CFP, RUBY_OFFSET_CFP_ISEQ));
848+
asm_ccall!(asm, rb_vm_setclassvariable, iseq, CFP, id.0.into(), val, Opnd::const_ptr(ic));
849+
}
850+
835851
/// Look up global variables
836852
fn gen_getglobal(jit: &mut JITState, asm: &mut Assembler, id: ID, state: &FrameState) -> Opnd {
837853
// `Warning` module's method `warn` can be called when reading certain global variables

‎zjit/src/hir.rs‎

Lines changed: 86 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -648,6 +648,11 @@ pub enum Insn {
648648
GetSpecialSymbol { symbol_type: SpecialBackrefSymbol, state: InsnId },
649649
GetSpecialNumber { nth: u64, state: InsnId },
650650

651+
/// Get a class variable `id`
652+
GetClassVar { id: ID, ic: *const iseq_inline_cvar_cache_entry, state: InsnId },
653+
/// Set a class variable `id` to `val`
654+
SetClassVar { id: ID, val: InsnId, ic: *const iseq_inline_cvar_cache_entry, state: InsnId },
655+
651656
/// Own a FrameState so that instructions can look up their dominating FrameState when
652657
/// generating deopt side-exits and frame reconstruction metadata. Does not directly generate
653658
/// any code.
@@ -811,7 +816,7 @@ impl Insn {
811816
match self {
812817
Insn::Jump(_)
813818
| Insn::IfTrue { .. } | Insn::IfFalse { .. } | Insn::EntryPoint { .. } | Insn::Return { .. }
814-
| Insn::PatchPoint { .. } | Insn::SetIvar { .. } | Insn::ArrayExtend { .. }
819+
| Insn::PatchPoint { .. } | Insn::SetIvar { .. } | Insn::SetClassVar { .. } | Insn::ArrayExtend { .. }
815820
| Insn::ArrayPush { .. } | Insn::SideExit { .. } | Insn::SetGlobal { .. }
816821
| Insn::SetLocal { .. } | Insn::Throw { .. } | Insn::IncrCounter(_) | Insn::IncrCounterPtr { .. }
817822
| Insn::CheckInterrupts { .. } | Insn::GuardBlockParamProxy { .. } => false,
@@ -1130,6 +1135,8 @@ impl<'a> std::fmt::Display for InsnPrinter<'a> {
11301135
Insn::SetLocal { val, level, ep_offset } => write!(f, "SetLocal l{level}, EP@{ep_offset}, {val}"),
11311136
Insn::GetSpecialSymbol { symbol_type, .. } => write!(f, "GetSpecialSymbol {symbol_type:?}"),
11321137
Insn::GetSpecialNumber { nth, .. } => write!(f, "GetSpecialNumber {nth}"),
1138+
Insn::GetClassVar { id, .. } => write!(f, "GetClassVar :{}", id.contents_lossy()),
1139+
Insn::SetClassVar { id, val, .. } => write!(f, "SetClassVar :{}, {val}", id.contents_lossy()),
11331140
Insn::ToArray { val, .. } => write!(f, "ToArray {val}"),
11341141
Insn::ToNewArray { val, .. } => write!(f, "ToNewArray {val}"),
11351142
Insn::ArrayExtend { left, right, .. } => write!(f, "ArrayExtend {left}, {right}"),
@@ -1716,6 +1723,8 @@ impl Function {
17161723
&LoadIvarEmbedded { self_val, id, index } => LoadIvarEmbedded { self_val: find!(self_val), id, index },
17171724
&LoadIvarExtended { self_val, id, index } => LoadIvarExtended { self_val: find!(self_val), id, index },
17181725
&SetIvar { self_val, id, val, state } => SetIvar { self_val: find!(self_val), id, val: find!(val), state },
1726+
&GetClassVar { id, ic, state } => GetClassVar { id, ic, state },
1727+
&SetClassVar { id, val, ic, state } => SetClassVar { id, val: find!(val), ic, state },
17191728
&SetLocal { val, ep_offset, level } => SetLocal { val: find!(val), ep_offset, level },
17201729
&GetSpecialSymbol { symbol_type, state } => GetSpecialSymbol { symbol_type, state },
17211730
&GetSpecialNumber { nth, state } => GetSpecialNumber { nth, state },
@@ -1765,7 +1774,7 @@ impl Function {
17651774
Insn::Param { .. } => unimplemented!("params should not be present in block.insns"),
17661775
Insn::SetGlobal { .. } | Insn::Jump(_) | Insn::EntryPoint { .. }
17671776
| Insn::IfTrue { .. } | Insn::IfFalse { .. } | Insn::Return { .. } | Insn::Throw { .. }
1768-
| Insn::PatchPoint { .. } | Insn::SetIvar { .. } | Insn::ArrayExtend { .. }
1777+
| Insn::PatchPoint { .. } | Insn::SetIvar { .. } | Insn::SetClassVar { .. } | Insn::ArrayExtend { .. }
17691778
| Insn::ArrayPush { .. } | Insn::SideExit { .. } | Insn::SetLocal { .. } | Insn::IncrCounter(_)
17701779
| Insn::CheckInterrupts { .. } | Insn::GuardBlockParamProxy { .. } | Insn::IncrCounterPtr { .. } =>
17711780
panic!("Cannot infer type of instruction with no output: {}", self.insns[insn.0]),
@@ -1848,6 +1857,7 @@ impl Function {
18481857
Insn::LoadIvarExtended { .. } => types::BasicObject,
18491858
Insn::GetSpecialSymbol { .. } => types::BasicObject,
18501859
Insn::GetSpecialNumber { .. } => types::BasicObject,
1860+
Insn::GetClassVar { .. } => types::BasicObject,
18511861
Insn::ToNewArray { .. } => types::ArrayExact,
18521862
Insn::ToArray { .. } => types::ArrayExact,
18531863
Insn::ObjToString { .. } => types::BasicObject,
@@ -3156,6 +3166,13 @@ impl Function {
31563166
worklist.push_back(val);
31573167
worklist.push_back(state);
31583168
}
3169+
&Insn::GetClassVar { state, .. } => {
3170+
worklist.push_back(state);
3171+
}
3172+
&Insn::SetClassVar { val, state, .. } => {
3173+
worklist.push_back(val);
3174+
worklist.push_back(state);
3175+
}
31593176
&Insn::ArrayPush { array, val, state } => {
31603177
worklist.push_back(array);
31613178
worklist.push_back(val);
@@ -4639,6 +4656,20 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result<Function, ParseError> {
46394656
let val = state.stack_pop()?;
46404657
fun.push_insn(block, Insn::SetIvar { self_val: self_param, id, val, state: exit_id });
46414658
}
4659+
YARVINSN_getclassvariable => {
4660+
let id = ID(get_arg(pc, 0).as_u64());
4661+
let ic = get_arg(pc, 1).as_ptr();
4662+
let exit_id = fun.push_insn(block, Insn::Snapshot { state: exit_state });
4663+
let result = fun.push_insn(block, Insn::GetClassVar { id, ic, state: exit_id });
4664+
state.stack_push(result);
4665+
}
4666+
YARVINSN_setclassvariable => {
4667+
let id = ID(get_arg(pc, 0).as_u64());
4668+
let ic = get_arg(pc, 1).as_ptr();
4669+
let exit_id = fun.push_insn(block, Insn::Snapshot { state: exit_state });
4670+
let val = state.stack_pop()?;
4671+
fun.push_insn(block, Insn::SetClassVar { id, val, ic, state: exit_id });
4672+
}
46424673
YARVINSN_opt_reverse => {
46434674
// Reverse the order of the top N stack items.
46444675
let n = get_arg(pc, 0).as_usize();
@@ -7429,6 +7460,59 @@ mod tests {
74297460
assert_eq!(VALUE::fixnum_from_usize(1), result);
74307461
}
74317462

7463+
#[test]
7464+
fn test_getclassvariable() {
7465+
eval("
7466+
class Foo
7467+
def self.test = @@foo
7468+
end
7469+
");
7470+
let iseq = crate::cruby::with_rubyvm(|| get_method_iseq("Foo", "test"));
7471+
assert!(iseq_contains_opcode(iseq, YARVINSN_getclassvariable), "iseq Foo.test does not contain getclassvariable");
7472+
let function = iseq_to_hir(iseq).unwrap();
7473+
assert_snapshot!(hir_string_function(&function), @r"
7474+
fn test@<compiled>:3:
7475+
bb0():
7476+
EntryPoint interpreter
7477+
v1:BasicObject = LoadSelf
7478+
Jump bb2(v1)
7479+
bb1(v4:BasicObject):
7480+
EntryPoint JIT(0)
7481+
Jump bb2(v4)
7482+
bb2(v6:BasicObject):
7483+
v11:BasicObject = GetClassVar :@@foo
7484+
CheckInterrupts
7485+
Return v11
7486+
");
7487+
}
7488+
7489+
#[test]
7490+
fn test_setclassvariable() {
7491+
eval("
7492+
class Foo
7493+
def self.test = @@foo = 42
7494+
end
7495+
");
7496+
let iseq = crate::cruby::with_rubyvm(|| get_method_iseq("Foo", "test"));
7497+
assert!(iseq_contains_opcode(iseq, YARVINSN_setclassvariable), "iseq Foo.test does not contain setclassvariable");
7498+
let function = iseq_to_hir(iseq).unwrap();
7499+
assert_snapshot!(hir_string_function(&function), @r"
7500+
fn test@<compiled>:3:
7501+
bb0():
7502+
EntryPoint interpreter
7503+
v1:BasicObject = LoadSelf
7504+
Jump bb2(v1)
7505+
bb1(v4:BasicObject):
7506+
EntryPoint JIT(0)
7507+
Jump bb2(v4)
7508+
bb2(v6:BasicObject):
7509+
v10:Fixnum[42] = Const Value(42)
7510+
SetClassVar :@@foo, v10
7511+
CheckInterrupts
7512+
Return v10
7513+
");
7514+
}
7515+
74327516
#[test]
74337517
fn test_setglobal() {
74347518
eval("

0 commit comments

Comments
 (0)