[pull] master from ruby:master

doc/zjit.md

@@ -155,10 +155,10 @@ make -j
 
 ### Tracing side exits
 
-Through [Stackprof](https://github.com/tmm1/stackprof), detailed information about the methods that the JIT side-exits from can be displayed after some execution of a program. Note that the use of `--zjit-trace-exits` must be used alongside `--zjit-stats`.
+Through [Stackprof](https://github.com/tmm1/stackprof), detailed information about the methods that the JIT side-exits from can be displayed after some execution of a program.
 
 ```bash
-./miniruby --zjit-stats --zjit-trace-exits script.rb
+./miniruby --zjit-trace-exits script.rb
 ```
 
 A file called `zjit_exit_locations.dump` will be created in the same directory as `script.rb`. Viewing the side exited methods can be done with Stackprof:

prism/prism.c

@@ -2622,10 +2622,11 @@ pm_break_node_create(pm_parser_t *parser, const pm_token_t *keyword, pm_argument
 // There are certain flags that we want to use internally but don't want to
 // expose because they are not relevant beyond parsing. Therefore we'll define
 // them here and not define them in config.yml/a header file.
-static const pm_node_flags_t PM_WRITE_NODE_FLAGS_IMPLICIT_ARRAY = 0x4;
-static const pm_node_flags_t PM_CALL_NODE_FLAGS_IMPLICIT_ARRAY = 0x40;
-static const pm_node_flags_t PM_CALL_NODE_FLAGS_COMPARISON = 0x80;
-static const pm_node_flags_t PM_CALL_NODE_FLAGS_INDEX = 0x100;
+static const pm_node_flags_t PM_WRITE_NODE_FLAGS_IMPLICIT_ARRAY = (1 << 2);
+
+static const pm_node_flags_t PM_CALL_NODE_FLAGS_IMPLICIT_ARRAY = ((PM_CALL_NODE_FLAGS_LAST - 1) << 1);
+static const pm_node_flags_t PM_CALL_NODE_FLAGS_COMPARISON = ((PM_CALL_NODE_FLAGS_LAST - 1) << 2);
+static const pm_node_flags_t PM_CALL_NODE_FLAGS_INDEX = ((PM_CALL_NODE_FLAGS_LAST - 1) << 3);
 
 /**
  * Allocate and initialize a new CallNode node. This sets everything to NULL or

prism/templates/include/prism/ast.h.erb

@@ -212,6 +212,8 @@ typedef enum pm_<%= flag.human %> {
     /** <%= value.comment %> */
     PM_<%= flag.human.upcase %>_<%= value.name %> = <%= 1 << (index + Prism::Template::COMMON_FLAGS_COUNT) %>,
     <%- end -%>
+
+    PM_<%= flag.human.upcase %>_LAST,
 } pm_<%= flag.human %>_t;
 <%- end -%>
 

test/ruby/test_zjit.rb

@@ -1642,7 +1642,7 @@ def test(c) = c.foo
     }, call_threshold: 2, insns: [:opt_send_without_block]
   end
 
-  def test_attr_accessor
+  def test_attr_accessor_getivar
     assert_compiles '[4, 4]', %q{
       class C
         attr_accessor :foo
@@ -1658,6 +1658,47 @@ def test(c) = c.foo
     }, call_threshold: 2, insns: [:opt_send_without_block]
   end
 
+  def test_attr_accessor_setivar
+    assert_compiles '[5, 5]', %q{
+      class C
+        attr_accessor :foo
+
+        def initialize
+          @foo = 4
+        end
+      end
+
+      def test(c)
+        c.foo = 5
+        c.foo
+      end
+
+      c = C.new
+      [test(c), test(c)]
+    }, call_threshold: 2, insns: [:opt_send_without_block]
+  end
+
+  def test_attr_writer
+    assert_compiles '[5, 5]', %q{
+      class C
+        attr_writer :foo
+
+        def initialize
+          @foo = 4
+        end
+
+        def get_foo = @foo
+      end
+
+      def test(c)
+        c.foo = 5
+        c.get_foo
+      end
+      c = C.new
+      [test(c), test(c)]
+    }, call_threshold: 2, insns: [:opt_send_without_block]
+  end
+
   def test_uncached_getconstant_path
     assert_compiles RUBY_COPYRIGHT.dump, %q{
       def test = RUBY_COPYRIGHT

zjit.c

@@ -59,6 +59,10 @@ rb_zjit_add_frame(VALUE hash, VALUE frame)
 
         rb_hash_aset(frame_info, ID2SYM(rb_intern("name")), name);
         rb_hash_aset(frame_info, ID2SYM(rb_intern("file")), file);
+        rb_hash_aset(frame_info, ID2SYM(rb_intern("samples")), INT2NUM(0));
+        rb_hash_aset(frame_info, ID2SYM(rb_intern("total_samples")), INT2NUM(0));
+        rb_hash_aset(frame_info, ID2SYM(rb_intern("edges")), rb_hash_new());
+        rb_hash_aset(frame_info, ID2SYM(rb_intern("lines")), rb_hash_new());
 
         if (line != INT2FIX(0)) {
             rb_hash_aset(frame_info, ID2SYM(rb_intern("line")), line);
@@ -89,8 +93,9 @@ rb_zjit_exit_locations_dict(VALUE *zjit_raw_samples, int *zjit_line_samples, int
         int line_num = (int)zjit_line_samples[idx];
         idx++;
 
-        rb_ary_push(raw_samples, SIZET2NUM(num));
-        rb_ary_push(line_samples, INT2NUM(line_num));
+        // + 1 as we append an additional sample for the insn
+        rb_ary_push(raw_samples, SIZET2NUM(num + 1));
+        rb_ary_push(line_samples, INT2NUM(line_num + 1));
 
         // Loop through the length of samples_len and add data to the
         // frames hash. Also push the current value onto the raw_samples

zjit.rb

@@ -9,10 +9,10 @@
 module RubyVM::ZJIT
   # Avoid calling a Ruby method here to avoid interfering with compilation tests
   if Primitive.rb_zjit_print_stats_p
-    at_exit {
-      print_stats
-      dump_locations
-    }
+    at_exit { print_stats }
+  end
+  if Primitive.rb_zjit_trace_exit_locations_enabled_p
+    at_exit { dump_locations }
   end
 end
 
@@ -40,56 +40,63 @@ def exit_locations
     frames = results[:frames].dup
     samples_count = 0
 
-    frames.each do |frame_id, frame|
-      frame[:samples] = 0
-      frame[:edges] = {}
-    end
-
     # Loop through the instructions and set the frame hash with the data.
     # We use nonexistent.def for the file name, otherwise insns.def will be displayed
     # and that information isn't useful in this context.
     RubyVM::INSTRUCTION_NAMES.each_with_index do |name, frame_id|
-      frame_hash = { samples: 0, total_samples: 0, edges: {}, name: name, file: "nonexistent.def", line: nil }
+      frame_hash = { samples: 0, total_samples: 0, edges: {}, name: name, file: "nonexistent.def", line: nil, lines: {} }
       results[:frames][frame_id] = frame_hash
       frames[frame_id] = frame_hash
     end
 
     # Loop through the raw_samples and build the hashes for StackProf.
     # The loop is based off an example in the StackProf documentation and therefore
     # this functionality can only work with that library.
-    while raw_samples.length > 0
-      stack_trace = raw_samples.shift(raw_samples.shift + 1)
-      lines = line_samples.shift(line_samples.shift + 1)
+    #
+    # Raw Samples:
+    # [ length, frame1, frame2, frameN, ..., instruction, count
+    #
+    # Line Samples
+    # [ length, line_1, line_2, line_n, ..., dummy value, count
+    i = 0
+    while i < raw_samples.length
+      stack_length = raw_samples[i]
+      i += 1 # consume the stack length
+
+      sample_count = raw_samples[i + stack_length]
+
       prev_frame_id = nil
+      stack_length.times do |idx|
+        idx += i
+        frame_id = raw_samples[idx]
 
-      stack_trace.each_with_index do |frame_id, idx|
         if prev_frame_id
           prev_frame = frames[prev_frame_id]
           prev_frame[:edges][frame_id] ||= 0
-          prev_frame[:edges][frame_id] += 1
+          prev_frame[:edges][frame_id] += sample_count
         end
 
         frame_info = frames[frame_id]
-        frame_info[:total_samples] ||= 0
-        frame_info[:total_samples] += 1
+        frame_info[:total_samples] += sample_count
 
-        frame_info[:lines] ||= {}
-        frame_info[:lines][lines[idx]] ||= [0, 0]
-        frame_info[:lines][lines[idx]][0] += 1
+        frame_info[:lines][line_samples[idx]] ||= [0, 0]
+        frame_info[:lines][line_samples[idx]][0] += sample_count
 
         prev_frame_id = frame_id
       end
 
-      top_frame_id = stack_trace.last
+      i += stack_length # consume the stack
+
+      top_frame_id = prev_frame_id
       top_frame_line = 1
 
-      frames[top_frame_id][:samples] += 1
+      frames[top_frame_id][:samples] += sample_count
       frames[top_frame_id][:lines] ||= {}
       frames[top_frame_id][:lines][top_frame_line] ||= [0, 0]
-      frames[top_frame_id][:lines][top_frame_line][1] += 1
+      frames[top_frame_id][:lines][top_frame_line][1] += sample_count
 
-      samples_count += raw_samples.shift
-      line_samples.shift
+      samples_count += sample_count
+      i += 1
     end
 
     results[:samples] = samples_count
@@ -119,7 +126,7 @@ def dump_exit_locations(filename)
       raise ArgumentError, "--zjit-trace-exits must be enabled to use dump_exit_locations."
     end
 
-    File.write(filename, Marshal.dump(RubyVM::ZJIT.exit_locations))
+    File.binwrite(filename, Marshal.dump(RubyVM::ZJIT.exit_locations))
   end
 
   # Check if `--zjit-stats` is used

zjit/src/codegen.rs

@@ -573,7 +573,7 @@ fn gen_setlocal(asm: &mut Assembler, val: Opnd, val_type: Type, local_ep_offset:
     } else {
         // We're potentially writing a reference to an IMEMO/env object,
         // so take care of the write barrier with a function.
-        let local_index = local_ep_offset * -1;
+        let local_index = -local_ep_offset;
         asm_ccall!(asm, rb_vm_env_write, ep, local_index.into(), val);
     }
 }

zjit/src/hir.rs

@@ -2043,6 +2043,23 @@ impl Function {
                             }
                             let getivar = self.push_insn(block, Insn::GetIvar { self_val: recv, id, state });
                             self.make_equal_to(insn_id, getivar);
+                        } else if let (VM_METHOD_TYPE_ATTRSET, &[val]) = (def_type, args.as_slice()) {
+                            self.push_insn(block, Insn::PatchPoint { invariant: Invariant::MethodRedefined { klass, method: mid, cme }, state });
+                            if let Some(profiled_type) = profiled_type {
+                                recv = self.push_insn(block, Insn::GuardType { val: recv, guard_type: Type::from_profiled_type(profiled_type), state });
+                            }
+                            let id = unsafe { get_cme_def_body_attr_id(cme) };
+
+                            // Check if we're accessing ivars of a Class or Module object as they require single-ractor mode.
+                            // We omit gen_prepare_non_leaf_call on gen_setivar, so it's unsafe to raise for multi-ractor mode.
+                            if unsafe { rb_zjit_singleton_class_p(klass) } {
+                                let attached = unsafe { rb_class_attached_object(klass) };
+                                if unsafe { RB_TYPE_P(attached, RUBY_T_CLASS) || RB_TYPE_P(attached, RUBY_T_MODULE) } {
+                                    self.push_insn(block, Insn::PatchPoint { invariant: Invariant::SingleRactorMode, state });
+                                }
+                            }
+                            self.push_insn(block, Insn::SetIvar { self_val: recv, id, val, state });
+                            self.make_equal_to(insn_id, val);
                         } else {
                             self.set_dynamic_send_reason(insn_id, SendWithoutBlockNotOptimizedMethodType(MethodType::from(def_type)));
                             self.push_insn_id(block, insn_id); continue;
@@ -3263,7 +3280,7 @@ struct BytecodeInfo {
     has_blockiseq: bool,
 }
 
-fn compute_bytecode_info(iseq: *const rb_iseq_t, opt_table: &Vec<u32>) -> BytecodeInfo {
+fn compute_bytecode_info(iseq: *const rb_iseq_t, opt_table: &[u32]) -> BytecodeInfo {
     let iseq_size = unsafe { get_iseq_encoded_size(iseq) };
     let mut insn_idx = 0;
     let mut jump_targets: HashSet<u32> = opt_table.iter().copied().collect();
@@ -4251,7 +4268,7 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result<Function, ParseError> {
 }
 
 /// Compile an entry_block for the interpreter
-fn compile_entry_block(fun: &mut Function, jit_entry_insns: &Vec<u32>) {
+fn compile_entry_block(fun: &mut Function, jit_entry_insns: &[u32]) {
     let entry_block = fun.entry_block;
     fun.push_insn(entry_block, Insn::EntryPoint { jit_entry_idx: None });
 
@@ -12190,4 +12207,66 @@ mod opt_tests {
           Return v25
         ");
     }
+
+    #[test]
+    fn test_inline_attr_accessor_set() {
+        eval("
+            class C
+              attr_accessor :foo
+            end
+
+            def test(o) = o.foo = 5
+            test C.new
+            test C.new
+        ");
+        assert_snapshot!(hir_string("test"), @r"
+        fn test@<compiled>:6:
+        bb0():
+          EntryPoint interpreter
+          v1:BasicObject = LoadSelf
+          v2:BasicObject = GetLocal l0, SP@4
+          Jump bb2(v1, v2)
+        bb1(v5:BasicObject, v6:BasicObject):
+          EntryPoint JIT(0)
+          Jump bb2(v5, v6)
+        bb2(v8:BasicObject, v9:BasicObject):
+          v14:Fixnum[5] = Const Value(5)
+          PatchPoint MethodRedefined(C@0x1000, foo=@0x1008, cme:0x1010)
+          v23:HeapObject[class_exact:C] = GuardType v9, HeapObject[class_exact:C]
+          SetIvar v23, :@foo, v14
+          CheckInterrupts
+          Return v14
+        ");
+    }
+
+    #[test]
+    fn test_inline_attr_writer_set() {
+        eval("
+            class C
+              attr_writer :foo
+            end
+
+            def test(o) = o.foo = 5
+            test C.new
+            test C.new
+        ");
+        assert_snapshot!(hir_string("test"), @r"
+        fn test@<compiled>:6:
+        bb0():
+          EntryPoint interpreter
+          v1:BasicObject = LoadSelf
+          v2:BasicObject = GetLocal l0, SP@4
+          Jump bb2(v1, v2)
+        bb1(v5:BasicObject, v6:BasicObject):
+          EntryPoint JIT(0)
+          Jump bb2(v5, v6)
+        bb2(v8:BasicObject, v9:BasicObject):
+          v14:Fixnum[5] = Const Value(5)
+          PatchPoint MethodRedefined(C@0x1000, foo=@0x1008, cme:0x1010)
+          v23:HeapObject[class_exact:C] = GuardType v9, HeapObject[class_exact:C]
+          SetIvar v23, :@foo, v14
+          CheckInterrupts
+          Return v14
+        ");
+    }
 }

zjit/src/hir_type/mod.rs

@@ -355,8 +355,7 @@ impl Type {
     fn is_builtin(class: VALUE) -> bool {
         types::ExactBitsAndClass
             .iter()
-            .find(|&(_, class_object)| unsafe { **class_object } == class)
-            .is_some()
+            .any(|&(_, class_object)| unsafe { *class_object } == class)
     }
 
     /// Union both types together, preserving specialization if possible.

zjit/src/options.rs

@@ -314,7 +314,7 @@ fn update_profile_threshold() {
 /// Update --zjit-call-threshold for testing
 #[cfg(test)]
 pub fn set_call_threshold(call_threshold: u64) {
-    unsafe { rb_zjit_call_threshold = call_threshold as u64; }
+    unsafe { rb_zjit_call_threshold = call_threshold; }
     rb_zjit_prepare_options();
     update_profile_threshold();
 }

zjit/src/state.rs

@@ -392,7 +392,7 @@ pub extern "C" fn rb_zjit_record_exit_stack(exit_pc: *const VALUE) {
 /// Mark `raw_samples` so they can be used by rb_zjit_add_frame.
 pub fn gc_mark_raw_samples() {
     // Return if ZJIT is not enabled
-    if !zjit_enabled_p() || !get_option!(stats) || !get_option!(trace_side_exits) {
+    if !zjit_enabled_p() || !get_option!(trace_side_exits) {
         return;
     }