Skip to content

Commit 8cefb70

Browse files
tekknolagiXrXr
andauthored
ZJIT: Re-apply attr_writer inlining (ruby#14678)
This re-applies ruby#14629 / 40bb476 by reverting ruby#14673 / d439377. Co-authored-by: Alan Wu <XrXr@users.noreply.github.com>
1 parent df2d1d5 commit 8cefb70

2 files changed

Lines changed: 121 additions & 1 deletion

File tree

test/ruby/test_zjit.rb

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1642,7 +1642,7 @@ def test(c) = c.foo
16421642
}, call_threshold: 2, insns: [:opt_send_without_block]
16431643
end
16441644

1645-
def test_attr_accessor
1645+
def test_attr_accessor_getivar
16461646
assert_compiles '[4, 4]', %q{
16471647
class C
16481648
attr_accessor :foo
@@ -1658,6 +1658,47 @@ def test(c) = c.foo
16581658
}, call_threshold: 2, insns: [:opt_send_without_block]
16591659
end
16601660

1661+
def test_attr_accessor_setivar
1662+
assert_compiles '[5, 5]', %q{
1663+
class C
1664+
attr_accessor :foo
1665+
1666+
def initialize
1667+
@foo = 4
1668+
end
1669+
end
1670+
1671+
def test(c)
1672+
c.foo = 5
1673+
c.foo
1674+
end
1675+
1676+
c = C.new
1677+
[test(c), test(c)]
1678+
}, call_threshold: 2, insns: [:opt_send_without_block]
1679+
end
1680+
1681+
def test_attr_writer
1682+
assert_compiles '[5, 5]', %q{
1683+
class C
1684+
attr_writer :foo
1685+
1686+
def initialize
1687+
@foo = 4
1688+
end
1689+
1690+
def get_foo = @foo
1691+
end
1692+
1693+
def test(c)
1694+
c.foo = 5
1695+
c.get_foo
1696+
end
1697+
c = C.new
1698+
[test(c), test(c)]
1699+
}, call_threshold: 2, insns: [:opt_send_without_block]
1700+
end
1701+
16611702
def test_uncached_getconstant_path
16621703
assert_compiles RUBY_COPYRIGHT.dump, %q{
16631704
def test = RUBY_COPYRIGHT

zjit/src/hir.rs

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2043,6 +2043,23 @@ impl Function {
20432043
}
20442044
let getivar = self.push_insn(block, Insn::GetIvar { self_val: recv, id, state });
20452045
self.make_equal_to(insn_id, getivar);
2046+
} else if let (VM_METHOD_TYPE_ATTRSET, &[val]) = (def_type, args.as_slice()) {
2047+
self.push_insn(block, Insn::PatchPoint { invariant: Invariant::MethodRedefined { klass, method: mid, cme }, state });
2048+
if let Some(profiled_type) = profiled_type {
2049+
recv = self.push_insn(block, Insn::GuardType { val: recv, guard_type: Type::from_profiled_type(profiled_type), state });
2050+
}
2051+
let id = unsafe { get_cme_def_body_attr_id(cme) };
2052+
2053+
// Check if we're accessing ivars of a Class or Module object as they require single-ractor mode.
2054+
// We omit gen_prepare_non_leaf_call on gen_setivar, so it's unsafe to raise for multi-ractor mode.
2055+
if unsafe { rb_zjit_singleton_class_p(klass) } {
2056+
let attached = unsafe { rb_class_attached_object(klass) };
2057+
if unsafe { RB_TYPE_P(attached, RUBY_T_CLASS) || RB_TYPE_P(attached, RUBY_T_MODULE) } {
2058+
self.push_insn(block, Insn::PatchPoint { invariant: Invariant::SingleRactorMode, state });
2059+
}
2060+
}
2061+
self.push_insn(block, Insn::SetIvar { self_val: recv, id, val, state });
2062+
self.make_equal_to(insn_id, val);
20462063
} else {
20472064
self.set_dynamic_send_reason(insn_id, SendWithoutBlockNotOptimizedMethodType(MethodType::from(def_type)));
20482065
self.push_insn_id(block, insn_id); continue;
@@ -12190,4 +12207,66 @@ mod opt_tests {
1219012207
Return v25
1219112208
");
1219212209
}
12210+
12211+
#[test]
12212+
fn test_inline_attr_accessor_set() {
12213+
eval("
12214+
class C
12215+
attr_accessor :foo
12216+
end
12217+
12218+
def test(o) = o.foo = 5
12219+
test C.new
12220+
test C.new
12221+
");
12222+
assert_snapshot!(hir_string("test"), @r"
12223+
fn test@<compiled>:6:
12224+
bb0():
12225+
EntryPoint interpreter
12226+
v1:BasicObject = LoadSelf
12227+
v2:BasicObject = GetLocal l0, SP@4
12228+
Jump bb2(v1, v2)
12229+
bb1(v5:BasicObject, v6:BasicObject):
12230+
EntryPoint JIT(0)
12231+
Jump bb2(v5, v6)
12232+
bb2(v8:BasicObject, v9:BasicObject):
12233+
v14:Fixnum[5] = Const Value(5)
12234+
PatchPoint MethodRedefined(C@0x1000, foo=@0x1008, cme:0x1010)
12235+
v23:HeapObject[class_exact:C] = GuardType v9, HeapObject[class_exact:C]
12236+
SetIvar v23, :@foo, v14
12237+
CheckInterrupts
12238+
Return v14
12239+
");
12240+
}
12241+
12242+
#[test]
12243+
fn test_inline_attr_writer_set() {
12244+
eval("
12245+
class C
12246+
attr_writer :foo
12247+
end
12248+
12249+
def test(o) = o.foo = 5
12250+
test C.new
12251+
test C.new
12252+
");
12253+
assert_snapshot!(hir_string("test"), @r"
12254+
fn test@<compiled>:6:
12255+
bb0():
12256+
EntryPoint interpreter
12257+
v1:BasicObject = LoadSelf
12258+
v2:BasicObject = GetLocal l0, SP@4
12259+
Jump bb2(v1, v2)
12260+
bb1(v5:BasicObject, v6:BasicObject):
12261+
EntryPoint JIT(0)
12262+
Jump bb2(v5, v6)
12263+
bb2(v8:BasicObject, v9:BasicObject):
12264+
v14:Fixnum[5] = Const Value(5)
12265+
PatchPoint MethodRedefined(C@0x1000, foo=@0x1008, cme:0x1010)
12266+
v23:HeapObject[class_exact:C] = GuardType v9, HeapObject[class_exact:C]
12267+
SetIvar v23, :@foo, v14
12268+
CheckInterrupts
12269+
Return v14
12270+
");
12271+
}
1219312272
}

0 commit comments

Comments
 (0)