From e8144a121752d2db5bafc8bc7297b56bea2cce9a Mon Sep 17 00:00:00 2001 From: Daichi Kamiyama <32436625+dak2@users.noreply.github.com> Date: Tue, 26 May 2026 23:14:49 +0900 Subject: [PATCH 01/11] ZJIT: Fix reference to YJIT in zjit.rb [ci skip] --- zjit.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zjit.rb b/zjit.rb index cd4f9188b208cc..9c7309d67b4c56 100644 --- a/zjit.rb +++ b/zjit.rb @@ -7,7 +7,7 @@ # This module may not exist if ZJIT does not support the particular platform # for which CRuby is built. module RubyVM::ZJIT - # Blocks that are called when YJIT is enabled + # Blocks that are called when ZJIT is enabled @jit_hooks = [] # Avoid calling a Ruby method here to avoid interfering with compilation tests if Primitive.rb_zjit_get_stats_file_path_p From 986a7ed4fab7175a8ec55b799992b8ac24f4f011 Mon Sep 17 00:00:00 2001 From: Earlopain <14981592+Earlopain@users.noreply.github.com> Date: Tue, 26 May 2026 11:54:56 +0200 Subject: [PATCH 02/11] [ruby/prism] Don't replicate unary method bug in parser translator Closes https://github.com/ruby/prism/issues/4112 Also see https://github.com/ruby/prism/issues/2501, for which this was done. The expectation in rubocop is incorrect (produces code with semantic difference), so the test should be updated instead. In any way, it should also have applied to `+`, where the same happens https://github.com/ruby/prism/commit/1f8cae2a75 --- lib/prism/translation/parser/compiler.rb | 21 --------------------- test/prism/fixtures/unary_method_calls.txt | 6 ++++++ 2 files changed, 6 insertions(+), 21 deletions(-) diff --git a/lib/prism/translation/parser/compiler.rb b/lib/prism/translation/parser/compiler.rb index d4e94ae5e03dcc..aaccee64c18c2a 100644 --- a/lib/prism/translation/parser/compiler.rb +++ b/lib/prism/translation/parser/compiler.rb @@ -297,11 +297,6 @@ def visit_call_node(node) if node.call_operator_loc.nil? case name - when :-@ - case (receiver = node.receiver).type - when :integer_node, :float_node, :rational_node, :imaginary_node - return visit(numeric_negate(node.message_loc, receiver)) - end when :! return visit_block(builder.not_op(token(node.message_loc), token(node.opening_loc), visit(node.receiver), token(node.closing_loc)), block) when :=~ @@ -1973,22 +1968,6 @@ def multi_target_elements(node) elements end - # Negate the value of a numeric node. This is a special case where you - # have a negative sign on one line and then a number on the next line. - # In normal Ruby, this will always be a method call. The parser gem, - # however, marks this as a numeric literal. We have to massage the tree - # here to get it into the correct form. - def numeric_negate(message_loc, receiver) - case receiver.type - when :integer_node, :float_node - receiver.copy(value: -receiver.value, location: message_loc.join(receiver.location)) - when :rational_node - receiver.copy(numerator: -receiver.numerator, location: message_loc.join(receiver.location)) - when :imaginary_node - receiver.copy(numeric: numeric_negate(message_loc, receiver.numeric), location: message_loc.join(receiver.location)) - end - end - # Blocks can have a special set of parameters that automatically expand # when given arrays if they have a single required parameter and no # other parameters. diff --git a/test/prism/fixtures/unary_method_calls.txt b/test/prism/fixtures/unary_method_calls.txt index dda85e4bdbf6f9..a8327d23cc34ff 100644 --- a/test/prism/fixtures/unary_method_calls.txt +++ b/test/prism/fixtures/unary_method_calls.txt @@ -1,2 +1,8 @@ 42.~@ 42.!@ + +- +42 + ++ +42 From d6fa8e3e0f876114bf4ab9c8961a8e10e32ac9db Mon Sep 17 00:00:00 2001 From: Alex Kitchens Date: Tue, 26 May 2026 08:41:06 -0500 Subject: [PATCH 03/11] [DOC] Fix indentation in thread_sync.rb documentation examples --- thread_sync.rb | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/thread_sync.rb b/thread_sync.rb index c9d37772d7c19f..18c7cc7adc9d7a 100644 --- a/thread_sync.rb +++ b/thread_sync.rb @@ -11,23 +11,23 @@ class Thread # # Example: # - # queue = Thread::Queue.new + # queue = Thread::Queue.new # - # producer = Thread.new do - # 5.times do |i| - # sleep rand(i) # simulate expense - # queue << i - # puts "#{i} produced" - # end - # end + # producer = Thread.new do + # 5.times do |i| + # sleep rand(i) # simulate expense + # queue << i + # puts "#{i} produced" + # end + # end # - # consumer = Thread.new do - # 5.times do |i| - # value = queue.pop - # sleep rand(i/2) # simulate expense - # puts "consumed #{value}" - # end - # end + # consumer = Thread.new do + # 5.times do |i| + # value = queue.pop + # sleep rand(i/2) # simulate expense + # puts "consumed #{value}" + # end + # end # # consumer.join class Queue @@ -42,13 +42,13 @@ class Queue # # Example: # - # q = Thread::Queue.new + # q = Thread::Queue.new # #=> # # q.empty? # #=> true # - # q = Thread::Queue.new([1, 2, 3]) - # #=> # + # q = Thread::Queue.new([1, 2, 3]) + # #=> # # q.empty? # #=> false # q.pop @@ -113,7 +113,7 @@ def push(object) # # Example: # - # q = Thread::Queue.new + # q = Thread::Queue.new # Thread.new{ # while e = q.deq # wait for nil to break loop # # ... From 01a19896e700148df61b91479a4761c124ff05ff Mon Sep 17 00:00:00 2001 From: Kevin Newton Date: Tue, 26 May 2026 11:31:23 -0400 Subject: [PATCH 04/11] [ruby/prism] Fix parser translation when escaped newline with trailing content https://github.com/ruby/prism/commit/81e07f3cdb --- lib/prism/translation/parser/compiler.rb | 2 +- test/prism/fixtures/escaped_newline_with_trailing_content.txt | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) create mode 100644 test/prism/fixtures/escaped_newline_with_trailing_content.txt diff --git a/lib/prism/translation/parser/compiler.rb b/lib/prism/translation/parser/compiler.rb index aaccee64c18c2a..d11db12ae6a9a4 100644 --- a/lib/prism/translation/parser/compiler.rb +++ b/lib/prism/translation/parser/compiler.rb @@ -2178,7 +2178,7 @@ def string_nodes_from_line_continuations(unescaped, escaped, start_offset, openi else lines.sum do |line| count = line.scan(/(\\*)n/).count { |(backslashes)| backslashes&.length&.odd? } - count -= 1 if !line.end_with?("\n") && count > 0 + count -= 1 if line.match?(/(?:\A|[^\\])(?:\\\\)*\\n\z/) && count > 0 count end end diff --git a/test/prism/fixtures/escaped_newline_with_trailing_content.txt b/test/prism/fixtures/escaped_newline_with_trailing_content.txt new file mode 100644 index 00000000000000..fe947a3f102152 --- /dev/null +++ b/test/prism/fixtures/escaped_newline_with_trailing_content.txt @@ -0,0 +1,2 @@ +"A +B\nCC" From dc0bb7a577fc75aebba56e61f30e56cdeb86f3b2 Mon Sep 17 00:00:00 2001 From: Alan Wu Date: Sat, 23 May 2026 16:09:15 -0400 Subject: [PATCH 05/11] YJIT: Use rb_reg_new_from_values() instead of rb_reg_new_ary() To sync up with ZJIT and insns.def. --- yjit/bindgen/src/main.rs | 2 +- yjit/src/codegen.rs | 37 +++++----------------------------- yjit/src/cruby_bindings.inc.rs | 6 +++++- 3 files changed, 11 insertions(+), 34 deletions(-) diff --git a/yjit/bindgen/src/main.rs b/yjit/bindgen/src/main.rs index a6a24387b3b45d..373d5abb42c835 100644 --- a/yjit/bindgen/src/main.rs +++ b/yjit/bindgen/src/main.rs @@ -180,7 +180,7 @@ fn main() { .allowlist_function("rb_reg_match_post") .allowlist_function("rb_reg_match_last") .allowlist_function("rb_reg_nth_match") - .allowlist_function("rb_reg_new_ary") + .allowlist_function("rb_reg_new_from_values") // `ruby_value_type` is a C enum and this stops it from // prefixing all the members with the name of the type diff --git a/yjit/src/codegen.rs b/yjit/src/codegen.rs index 1146192dc4cc02..743a10e15a3ebe 100644 --- a/yjit/src/codegen.rs +++ b/yjit/src/codegen.rs @@ -10180,45 +10180,18 @@ fn gen_toregexp( let opt = jit.get_arg(0).as_i64(); let cnt = jit.get_arg(1).as_usize(); - // Save the PC and SP because this allocates an object and could - // raise an exception. + // Allocates objects and could raise an exception. jit_prepare_non_leaf_call(jit, asm); let values_ptr = asm.lea(asm.ctx.sp_opnd(-(cnt as i32))); - let ary = asm.ccall( - rb_ary_tmp_new_from_values as *const u8, - vec![ - Opnd::Imm(0), - cnt.into(), - values_ptr, - ] + let regexp = asm.ccall( + rb_reg_new_from_values as _, + vec![cnt.into(), values_ptr, opt.into()], ); asm.stack_pop(cnt); // Let ccall spill them - - // Save the array so we can clear it later - asm.cpush(ary); - asm.cpush(ary); // Alignment - - let val = asm.ccall( - rb_reg_new_ary as *const u8, - vec![ - ary, - Opnd::Imm(opt), - ] - ); - - // The actual regex is in RAX now. Pop the temp array from - // rb_ary_tmp_new_from_values into C arg regs so we can clear it - let ary = asm.cpop(); // Alignment - asm.cpop_into(ary); - - // The value we want to push on the stack is in RAX right now let stack_ret = asm.stack_push(Type::UnknownHeap); - asm.mov(stack_ret, val); - - // Clear the temp array. - asm.ccall(rb_ary_clear as *const u8, vec![ary]); + asm.mov(stack_ret, regexp); Some(KeepCompiling) } diff --git a/yjit/src/cruby_bindings.inc.rs b/yjit/src/cruby_bindings.inc.rs index 4ad88bb7d00888..ef7f6c0a9eff99 100644 --- a/yjit/src/cruby_bindings.inc.rs +++ b/yjit/src/cruby_bindings.inc.rs @@ -1076,7 +1076,11 @@ extern "C" { pub fn rb_obj_info_dump(obj: VALUE); pub fn rb_class_allocate_instance(klass: VALUE) -> VALUE; pub fn rb_obj_equal(obj1: VALUE, obj2: VALUE) -> VALUE; - pub fn rb_reg_new_ary(ary: VALUE, options: ::std::os::raw::c_int) -> VALUE; + pub fn rb_reg_new_from_values( + cnt: ::std::os::raw::c_long, + elements: *const VALUE, + opt: ::std::os::raw::c_int, + ) -> VALUE; pub fn rb_ary_tmp_new_from_values( arg1: VALUE, arg2: ::std::os::raw::c_long, From b1bf8f31ca1ae1f784c3d3319090e1360177b109 Mon Sep 17 00:00:00 2001 From: Alan Wu Date: Sat, 23 May 2026 16:11:22 -0400 Subject: [PATCH 06/11] ZJIT: Delete binding for unused rb_reg_new_ary() --- zjit/bindgen/src/main.rs | 1 - zjit/src/cruby_bindings.inc.rs | 1 - 2 files changed, 2 deletions(-) diff --git a/zjit/bindgen/src/main.rs b/zjit/bindgen/src/main.rs index 2cde74facd66c1..32b75117c852f9 100644 --- a/zjit/bindgen/src/main.rs +++ b/zjit/bindgen/src/main.rs @@ -215,7 +215,6 @@ fn main() { .allowlist_function("rb_reg_match_post") .allowlist_function("rb_reg_match_last") .allowlist_function("rb_reg_nth_match") - .allowlist_function("rb_reg_new_ary") .allowlist_function("rb_reg_new_from_values") .allowlist_var("ARG_ENCODING_FIXED") .allowlist_var("ARG_ENCODING_NONE") diff --git a/zjit/src/cruby_bindings.inc.rs b/zjit/src/cruby_bindings.inc.rs index 5a7c3de606c5f1..21a2f6bbb479dd 100644 --- a/zjit/src/cruby_bindings.inc.rs +++ b/zjit/src/cruby_bindings.inc.rs @@ -2059,7 +2059,6 @@ unsafe extern "C" { pub fn rb_const_get(space: VALUE, name: ID) -> VALUE; pub fn rb_class_allocate_instance(klass: VALUE) -> VALUE; pub fn rb_obj_equal(obj1: VALUE, obj2: VALUE) -> VALUE; - pub fn rb_reg_new_ary(ary: VALUE, options: ::std::os::raw::c_int) -> VALUE; pub fn rb_reg_new_from_values( cnt: ::std::os::raw::c_long, elements: *const VALUE, From e4cba2df17b0dfb18e1b1d16195d54a875c53dbc Mon Sep 17 00:00:00 2001 From: Alan Wu Date: Sat, 23 May 2026 16:21:31 -0400 Subject: [PATCH 07/11] Delete now-unused rb_reg_new_ary() No more usages outside re.c, so let's clean it up. Thanks to @nobu for noticing! --- internal/re.h | 1 - re.c | 8 +------- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/internal/re.h b/internal/re.h index 3ad364a1a69a1f..6c0aee6d06241a 100644 --- a/internal/re.h +++ b/internal/re.h @@ -68,7 +68,6 @@ VALUE rb_reg_equal(VALUE re1, VALUE re2); VALUE rb_backref_set_string(VALUE string, long pos, long len); void rb_match_unbusy(VALUE); int rb_match_count(VALUE match); -VALUE rb_reg_new_ary(VALUE ary, int options); VALUE rb_reg_new_from_values(long cnt, const VALUE *elements, int opt); VALUE rb_reg_last_defined(VALUE match); diff --git a/re.c b/re.c index ec337cd21cf2f6..63e2db81ac2570 100644 --- a/re.c +++ b/re.c @@ -3522,17 +3522,11 @@ rb_reg_init_str_enc(VALUE re, VALUE s, rb_encoding *enc, int options) return re; } -VALUE -rb_reg_new_ary(VALUE ary, int opt) -{ - return rb_reg_new_str(rb_reg_preprocess_dregexp(ary, opt), opt); -} - VALUE rb_reg_new_from_values(long cnt, const VALUE *elements, int opt) { const VALUE ary = rb_ary_tmp_new_from_values(0, cnt, elements); - VALUE val = rb_reg_new_ary(ary, (int)opt); + VALUE val = rb_reg_new_str(rb_reg_preprocess_dregexp(ary, opt), opt); rb_ary_clear(ary); return val; } From 012b72fdd61fd925b6e0362688da1e81d9f9c865 Mon Sep 17 00:00:00 2001 From: Satoshi Tagomori Date: Wed, 20 May 2026 22:22:55 +0900 Subject: [PATCH 08/11] [Box] dump the effective box on the control frame info --- vm_dump.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/vm_dump.c b/vm_dump.c index 4a7b0df5c6c0eb..00b03ff58d117b 100644 --- a/vm_dump.c +++ b/vm_dump.c @@ -90,6 +90,9 @@ control_frame_dump(const rb_execution_context_t *ec, const rb_control_frame_t *c break; case VM_FRAME_MAGIC_CFUNC: magic = "CFUNC"; + if (me) { + box = me->def->box; + } break; case VM_FRAME_MAGIC_IFUNC: magic = "IFUNC"; From 29a596f87deb06779bef6549d88e6f7b26974c3d Mon Sep 17 00:00:00 2001 From: Satoshi Tagomori Date: Sat, 23 May 2026 23:29:57 +0900 Subject: [PATCH 09/11] [Box] Invalidate the method cache of the owner too The owner could be different from the housing class when the housing module is prepended. --- test/ruby/test_box.rb | 16 ++++++++++++++++ vm_method.c | 5 +++++ 2 files changed, 21 insertions(+) diff --git a/test/ruby/test_box.rb b/test/ruby/test_box.rb index a39979109fe909..b07349ebbdd299 100644 --- a/test/ruby/test_box.rb +++ b/test/ruby/test_box.rb @@ -1180,4 +1180,20 @@ def test_user_box_iclass_with_module_modified_in_another_box assert_equal :box2, box2.eval("Class.new { include Math }.new.box2_test") end; end + + def test_method_invalidation_between_boxes + assert_separately([ENV_ENABLE_BOX], __FILE__, __LINE__, "#{<<~"begin;"}\n#{<<~'end;'}", ignore_stderr: true) + begin; + b = Ruby::Box.new + b.eval(<<~'RUBY') + Module.prepend(Module.new) + class C; end + class D < C; end + def C.===(x) = true + RUBY + + assert String === "x" + assert b # to prevent GCing b + end; + end end diff --git a/vm_method.c b/vm_method.c index 5a99f684c02659..1c160e80b004bf 100644 --- a/vm_method.c +++ b/vm_method.c @@ -487,6 +487,11 @@ clear_method_cache_by_id_in_class(VALUE klass, ID mid) // replace the cme that will be invalid in the all classexts invalidate_callable_method_entry_in_every_m_table(klass_housing_cme, mid, cme); + // owner may be a boxable class with per-box classext copies of its m_tbl + // (klass_housing_cme may be a non-boxable origin ICLASS that doesn't cover them) + if (klass_housing_cme != owner) { + invalidate_callable_method_entry_in_every_m_table(owner, mid, cme); + } } vm_cme_invalidate((rb_callable_method_entry_t *)cme); From b2715bf268583ee476000db66797aa0858980b12 Mon Sep 17 00:00:00 2001 From: Satoshi Tagomori Date: Tue, 26 May 2026 23:18:15 +0900 Subject: [PATCH 10/11] [Box] Invalidate callable me on per-box origin IClasses --- test/ruby/test_box.rb | 22 +++++++++++++++++++++- vm_method.c | 41 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 1 deletion(-) diff --git a/test/ruby/test_box.rb b/test/ruby/test_box.rb index b07349ebbdd299..a425c5eb7d6632 100644 --- a/test/ruby/test_box.rb +++ b/test/ruby/test_box.rb @@ -1181,7 +1181,7 @@ def test_user_box_iclass_with_module_modified_in_another_box end; end - def test_method_invalidation_between_boxes + def test_method_invalidation_between_boxes_1 assert_separately([ENV_ENABLE_BOX], __FILE__, __LINE__, "#{<<~"begin;"}\n#{<<~'end;'}", ignore_stderr: true) begin; b = Ruby::Box.new @@ -1196,4 +1196,24 @@ def C.===(x) = true assert b # to prevent GCing b end; end + + def test_method_invalidation_between_boxes_2 + assert_separately([ENV_ENABLE_BOX], __FILE__, __LINE__, "#{<<~"begin;"}\n#{<<~'end;'}", ignore_stderr: true) + begin; + PrepM = Module.new + Module.prepend(PrepM) + Module.new.include?(Module.new) + + b = Ruby::Box.new + b.eval(<<~'RUBY') + Module.class_eval { def _test_method; end } + + class C; end + class D < C; end + def C.include?(x) = true + RUBY + + Module.new.include?(Module.new) + end; + end end diff --git a/vm_method.c b/vm_method.c index 1c160e80b004bf..fb34426bf269a7 100644 --- a/vm_method.c +++ b/vm_method.c @@ -404,6 +404,30 @@ invalidate_callable_method_entry_in_every_m_table_i(rb_classext_t *ext, bool is_ } } +struct collect_per_box_origins_arg { + VALUE owner; + VALUE klass_housing_cme; + VALUE origins; // Array of origins +}; + +static void +collect_per_box_origins_i(rb_classext_t *ext, bool is_prime, VALUE box_value, void *data) +{ + struct collect_per_box_origins_arg *arg = (struct collect_per_box_origins_arg *)data; + VALUE origin = RCLASSEXT_ORIGIN(ext); + + if (origin == arg->owner || origin == arg->klass_housing_cme) { + return; + } + long len = RARRAY_LEN(arg->origins); + for (long i = 0; i < len; i++) { + if (RARRAY_AREF(arg->origins, i) == origin) { + return; + } + } + rb_ary_push(arg->origins, origin); +} + static void invalidate_callable_method_entry_in_every_m_table(VALUE klass, ID mid, const rb_callable_method_entry_t *cme) { @@ -492,6 +516,23 @@ clear_method_cache_by_id_in_class(VALUE klass, ID mid) if (klass_housing_cme != owner) { invalidate_callable_method_entry_in_every_m_table(owner, mid, cme); } + // Also update per-box origin ICLASSes. When ensure_origin is called in + // one box's context, it creates a per-box origin ICLASS whose m_tbl is + // a copy of owner's m_tbl at that time. The current execution box may + // not see these origins via RCLASS_ORIGIN(owner), so we find them by + // iterating all of owner's classexts and checking their origin_ fields. + { + VALUE origins = rb_ary_new(); + struct collect_per_box_origins_arg origins_arg = { + .owner = owner, + .klass_housing_cme = klass_housing_cme, + .origins = origins, + }; + rb_class_classext_foreach(owner, collect_per_box_origins_i, &origins_arg); + for (long i = 0; i < RARRAY_LEN(origins); i++) { + invalidate_callable_method_entry_in_every_m_table(RARRAY_AREF(origins, i), mid, cme); + } + } } vm_cme_invalidate((rb_callable_method_entry_t *)cme); From ce7f3ed832a50e493e3b6cb592791aa1fe784d85 Mon Sep 17 00:00:00 2001 From: alanwu-shopify-inc <286122947+alanwu-shopify-inc@users.noreply.github.com> Date: Tue, 26 May 2026 13:56:56 -0400 Subject: [PATCH 11/11] ZJIT: LIR CCall survivors: Use one stack layout across arches Previously, the last push, in case the total number of pushes was odd, ended up in a different location relative to the stack top on x64 versus A64. On A64, it ends up at `sp[0]`, whereas on x64 it ends up at `sp[1]`. The difference was due to `CPush` not having consistent architectural effects in the two arches. Normalize to `CPushPair` instead. Now A64 will write one more slot, due to using STP. x64 now uses the two-byte `push 0` instead of the one-byte `push rdi`. The odd push now always lives at `sp[1]`, leaving no gap between it and the rest of the survivors. This consistency will benefit future stack map work. Also remove some pointless `Vec` allocations. --- zjit/src/asm/x86_64/mod.rs | 3 +++ zjit/src/backend/arm64/mod.rs | 19 ++++++++++---- zjit/src/backend/lir.rs | 45 +++++++++++----------------------- zjit/src/backend/x86_64/mod.rs | 28 ++++++++++----------- 4 files changed, 45 insertions(+), 50 deletions(-) diff --git a/zjit/src/asm/x86_64/mod.rs b/zjit/src/asm/x86_64/mod.rs index 628a83b99cfe46..c2733e783f2642 100644 --- a/zjit/src/asm/x86_64/mod.rs +++ b/zjit/src/asm/x86_64/mod.rs @@ -1145,6 +1145,9 @@ pub fn push(cb: &mut CodeBlock, opnd: X86Opnd) { X86Opnd::Mem(_mem) => { write_rm(cb, false, false, X86Opnd::None, opnd, Some(6), &[0xff]); }, + X86Opnd::Imm(X86Imm { value: 0, .. }) | X86Opnd::UImm(X86UImm { value: 0, .. }) => { + cb.write_bytes(&[0x6a, 0x00]); + } _ => unreachable!() } } diff --git a/zjit/src/backend/arm64/mod.rs b/zjit/src/backend/arm64/mod.rs index 867b829eccdd87..abe439cb1233df 100644 --- a/zjit/src/backend/arm64/mod.rs +++ b/zjit/src/backend/arm64/mod.rs @@ -1407,8 +1407,10 @@ impl Assembler { emit_push(cb, opnd.into()); }, Insn::CPushPair(opnd0, opnd1) => { + let first_push = if let Opnd::UImm(0) | Opnd::Imm(0) = opnd0 { X31 } else { opnd0.into() }; + let second_push = if let Opnd::UImm(0) | Opnd::Imm(0) = opnd1 { X31 } else { opnd1.into() }; // Second operand ends up at the lower stack address - stp_pre(cb, opnd1.into(), opnd0.into(), A64Opnd::new_mem(64, C_SP_REG, -C_SP_STEP)); + stp_pre(cb, second_push, first_push, A64Opnd::new_mem(64, C_SP_REG, -C_SP_STEP)); }, Insn::CPop { out } => { emit_pop(cb, out.into()); @@ -1417,8 +1419,15 @@ impl Assembler { emit_pop(cb, opnd.into()); }, Insn::CPopPairInto(opnd0, opnd1) => { + let mut first_pop = opnd0.into(); + let second_pop = opnd1.into(); + // Avoid illegal load pair into the same register + // by sinking the first pop to the zero register. + if first_pop == second_pop { + first_pop = X31; + } // First operand is popped from the lower stack address - ldp_post(cb, opnd0.into(), opnd1.into(), A64Opnd::new_mem(64, C_SP_REG, C_SP_STEP)); + ldp_post(cb, first_pop, second_pop, A64Opnd::new_mem(64, C_SP_REG, C_SP_STEP)); }, Insn::CCall { fptr, .. } => { match fptr { @@ -2792,17 +2801,17 @@ mod tests { 0x10: mov x4, #5 0x14: stp x1, x0, [sp, #-0x10]! 0x18: stp x3, x2, [sp, #-0x10]! - 0x1c: str x4, [sp, #-0x10]! + 0x1c: stp xzr, x4, [sp, #-0x10]! 0x20: mov x16, #0 0x24: blr x16 - 0x28: ldr x4, [sp], #0x10 + 0x28: ldp xzr, x4, [sp], #0x10 0x2c: ldp x3, x2, [sp], #0x10 0x30: ldp x1, x0, [sp], #0x10 0x34: adds x0, x0, x1 0x38: adds x0, x2, x3 0x3c: adds x0, x2, x4 "); - assert_snapshot!(cb.hexdump(), @"200080d2410080d2620080d2830080d2a40080d2e103bfa9e30bbfa9e40f1ff8100080d200023fd6e40741f8e30bc1a8e103c1a8000001ab400003ab400004ab"); + assert_snapshot!(cb.hexdump(), @"200080d2410080d2620080d2830080d2a40080d2e103bfa9e30bbfa9ff13bfa9100080d200023fd6ff13c1a8e30bc1a8e103c1a8000001ab400003ab400004ab"); } #[test] diff --git a/zjit/src/backend/lir.rs b/zjit/src/backend/lir.rs index a8d03ad69a8fc1..0ef79b215eddc2 100644 --- a/zjit/src/backend/lir.rs +++ b/zjit/src/backend/lir.rs @@ -2384,27 +2384,16 @@ impl Assembler _ => unreachable!(), }) .collect(); - let survivor_push_groups: Vec> = survivor_regs - .chunks(2) - .map(|group| group.to_vec()) - .collect(); // Push all survivors on the stack, pairing adjacent pushes when possible. - let needs_alignment = cfg!(target_arch = "x86_64") && survivors.len() % 2 == 1; - for group in &survivor_push_groups { - match group.as_slice() { - [left, right] => new_insns.push(Insn::CPushPair(*left, *right)), - [reg] => new_insns.push(Insn::CPush(*reg)), + for group in survivor_regs.chunks(2) { + match group { + &[left, right] => new_insns.push(Insn::CPushPair(left, right)), + &[reg] => new_insns.push(Insn::CPushPair(reg, 0.into())), _ => unreachable!(), } new_ids.push(None); } - // Maintain 16-byte stack alignment for x86_64 - if needs_alignment { - new_insns.push(Insn::CPush(Opnd::Reg(ALLOC_REGS[0]))); - new_ids.push(None); - } - // Extract arguments from CCall, clear opnds assert!(opnds.len() <= regs.len()); @@ -2472,17 +2461,11 @@ impl Assembler new_ids.push(None); } - // Pop alignment padding (if needed) - if needs_alignment { - new_insns.push(Insn::CPopInto(Opnd::Reg(ALLOC_REGS[0]))); - new_ids.push(None); - } - // Restore all survivors in reverse stack order, pairing adjacent pops when possible. - for group in survivor_push_groups.iter().rev() { - match group.as_slice() { - [left, right] => new_insns.push(Insn::CPopPairInto(*right, *left)), - [reg] => new_insns.push(Insn::CPopInto(*reg)), + for group in survivor_regs.chunks(2).rev() { + match group { + &[reg] => new_insns.push(Insn::CPopPairInto(reg, reg)), + &[left, right] => new_insns.push(Insn::CPopPairInto(right, left)), _ => unreachable!(), } new_ids.push(None); @@ -4673,8 +4656,8 @@ mod tests { let insns = &asm.basic_blocks[b1.0].insns; // Find CPush and CPopInto - they should be balanced. - let pushes: Vec<_> = insns.iter().filter(|i| matches!(i, Insn::CPush(_))).collect(); - let pops: Vec<_> = insns.iter().filter(|i| matches!(i, Insn::CPopInto(_))).collect(); + let pushes: Vec<_> = insns.iter().filter(|i| matches!(i, Insn::CPushPair(..))).collect(); + let pops: Vec<_> = insns.iter().filter(|i| matches!(i, Insn::CPopPairInto(..))).collect(); assert_eq!(pushes.len(), pops.len(), "CPush/CPopInto should be balanced"); assert!(!pushes.is_empty(), "Expected at least one saved register across CCall"); @@ -4684,10 +4667,10 @@ mod tests { Allocation::Fixed(reg) => Opnd::Reg(reg), _ => unreachable!(), }; - let pushed_v1 = pushes.iter().any(|insn| matches!(insn, Insn::CPush(opnd) if *opnd == v1_reg)); - let popped_v1 = pops.iter().any(|insn| matches!(insn, Insn::CPopInto(opnd) if *opnd == v1_reg)); - assert!(pushed_v1, "CPush should save v1's register"); - assert!(popped_v1, "CPopInto should restore v1's register"); + let pushed_v1 = pushes.iter().any(|insn| matches!(**insn, Insn::CPushPair(first, second) if first == v1_reg || second == v1_reg)); + let popped_v1 = pops.iter().any(|insn| matches!(**insn, Insn::CPopPairInto(first, second) if first == v1_reg || second == v1_reg)); + assert!(pushed_v1, "CPushPair should save v1's register"); + assert!(popped_v1, "CPopPairInto should restore v1's register"); // The CCall should have empty opnds and out = C_RET_OPND (rewritten to regs[0]) let ccall = insns.iter().find(|i| matches!(i, Insn::CCall { .. })).unwrap(); diff --git a/zjit/src/backend/x86_64/mod.rs b/zjit/src/backend/x86_64/mod.rs index d8d930dfceac54..80abd15a6b6d4c 100644 --- a/zjit/src/backend/x86_64/mod.rs +++ b/zjit/src/backend/x86_64/mod.rs @@ -1985,22 +1985,22 @@ mod tests { 0x1c: push rdx 0x1d: push rcx 0x1e: push r8 - 0x20: push rdi - 0x21: mov eax, 0 - 0x26: call rax - 0x28: pop rdi + 0x20: push 0 + 0x22: mov eax, 0 + 0x27: call rax 0x29: pop r8 - 0x2b: pop rcx - 0x2c: pop rdx - 0x2d: pop rsi - 0x2e: pop rdi - 0x2f: add rdi, rsi - 0x32: mov rdi, rdx - 0x35: add rdi, rcx - 0x38: mov rdi, rdx - 0x3b: add rdi, r8 + 0x2b: pop r8 + 0x2d: pop rcx + 0x2e: pop rdx + 0x2f: pop rsi + 0x30: pop rdi + 0x31: add rdi, rsi + 0x34: mov rdi, rdx + 0x37: add rdi, rcx + 0x3a: mov rdi, rdx + 0x3d: add rdi, r8 "); - assert_snapshot!(cb.hexdump(), @"bf01000000be02000000ba03000000b90400000041b80500000057565251415057b800000000ffd05f4158595a5e5f4801f74889d74801cf4889d74c01c7"); + assert_snapshot!(cb.hexdump(), @"bf01000000be02000000ba03000000b90400000041b8050000005756525141506a00b800000000ffd041584158595a5e5f4801f74889d74801cf4889d74c01c7"); } #[test]