From 882e16776866813e0df40e835b9f6cf1b5999d1c Mon Sep 17 00:00:00 2001 From: Andrew Konchin Date: Tue, 28 Oct 2025 17:25:50 +0200 Subject: [PATCH 01/12] Update to ruby/spec@3bc45ba --- spec/ruby/core/proc/element_reference_spec.rb | 2 +- spec/ruby/core/symbol/inspect_spec.rb | 4 ++++ spec/ruby/language/assignments_spec.rb | 4 ++-- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/spec/ruby/core/proc/element_reference_spec.rb b/spec/ruby/core/proc/element_reference_spec.rb index 9077e44c346254..81ceb91af5c40c 100644 --- a/spec/ruby/core/proc/element_reference_spec.rb +++ b/spec/ruby/core/proc/element_reference_spec.rb @@ -17,7 +17,7 @@ it_behaves_like :proc_call_on_proc_or_lambda, :call end -describe "Proc#[] with frozen_string_literals" do +describe "Proc#[] with frozen_string_literal: true/false" do it "doesn't duplicate frozen strings" do ProcArefSpecs.aref.frozen?.should be_false ProcArefSpecs.aref_freeze.frozen?.should be_true diff --git a/spec/ruby/core/symbol/inspect_spec.rb b/spec/ruby/core/symbol/inspect_spec.rb index 6dbb36c2adbf91..26229da9442807 100644 --- a/spec/ruby/core/symbol/inspect_spec.rb +++ b/spec/ruby/core/symbol/inspect_spec.rb @@ -96,6 +96,10 @@ :"foo " => ":\"foo \"", :" foo" => ":\" foo\"", :" " => ":\" \"", + + :"ê" => ":ê", + :"测" => ":测", + :"🦊" => ":🦊", } symbols.each do |input, expected| diff --git a/spec/ruby/language/assignments_spec.rb b/spec/ruby/language/assignments_spec.rb index d469459c43d727..c4adf73c1cbf67 100644 --- a/spec/ruby/language/assignments_spec.rb +++ b/spec/ruby/language/assignments_spec.rb @@ -93,7 +93,7 @@ def []=(*args, **kw) end ruby_version_is "3.4" do - it "raies SyntaxError when given keyword arguments in index assignments" do + it "raises SyntaxError when given keyword arguments in index assignments" do a = @klass.new -> { eval "a[1, 2, 3, b: 4] = 5" }.should raise_error(SyntaxError, /keywords are not allowed in index assignment expressions|keyword arg given in index assignment/) # prism|parse.y @@ -236,7 +236,7 @@ def []=(*args, **kw) end ruby_version_is "3.4" do - it "raies SyntaxError when given keyword arguments in index assignments" do + it "raises SyntaxError when given keyword arguments in index assignments" do a = @klass.new -> { eval "a[1, 2, 3, b: 4] += 5" }.should raise_error(SyntaxError, /keywords are not allowed in index assignment expressions|keyword arg given in index assignment/) # prism|parse.y From 599de290a030927734eac93db66de18c653b6ed2 Mon Sep 17 00:00:00 2001 From: Takashi Kokubun Date: Tue, 28 Oct 2025 13:14:47 -0700 Subject: [PATCH 02/12] YJIT, ZJIT: Fix unnecessary `use` of macros https://github.com/ruby/ruby/actions/runs/18887695798/job/53907237061?pr=14975 --- yjit/src/stats.rs | 1 - zjit/src/codegen.rs | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/yjit/src/stats.rs b/yjit/src/stats.rs index a53d23435b49a7..817c842cf4144d 100644 --- a/yjit/src/stats.rs +++ b/yjit/src/stats.rs @@ -318,7 +318,6 @@ macro_rules! ptr_to_counter { } }; } -pub(crate) use ptr_to_counter; // Declare all the counters we track make_counters! { diff --git a/zjit/src/codegen.rs b/zjit/src/codegen.rs index 2b71be0e15ba71..63b5b6cb52cab3 100644 --- a/zjit/src/codegen.rs +++ b/zjit/src/codegen.rs @@ -2008,6 +2008,7 @@ macro_rules! c_callable { extern "C" fn $f $args $(-> $ret)? $body }; } +#[cfg(test)] pub(crate) use c_callable; c_callable! { From d0c7234bc79e9b0e415c29ae3250bde12d791b4c Mon Sep 17 00:00:00 2001 From: Takashi Kokubun Date: Tue, 28 Oct 2025 13:18:05 -0700 Subject: [PATCH 03/12] ZJIT: Support ParallelMov into memory (#14975) --- zjit/src/backend/arm64/mod.rs | 35 ++++---- zjit/src/backend/lir.rs | 155 ++++++++++++++++++++++++++++----- zjit/src/backend/x86_64/mod.rs | 26 +++--- zjit/src/codegen.rs | 19 +--- 4 files changed, 169 insertions(+), 66 deletions(-) diff --git a/zjit/src/backend/arm64/mod.rs b/zjit/src/backend/arm64/mod.rs index 867ad493ec2821..e090d8ce446aaa 100644 --- a/zjit/src/backend/arm64/mod.rs +++ b/zjit/src/backend/arm64/mod.rs @@ -212,13 +212,7 @@ impl Assembler { /// Return true if opnd contains a scratch reg pub fn has_scratch_reg(opnd: Opnd) -> bool { - match opnd { - Opnd::Reg(_) => opnd == SCRATCH_OPND, - Opnd::Mem(Mem { base: MemBase::Reg(reg_no), .. }) => { - reg_no == SCRATCH_OPND.unwrap_reg().reg_no - } - _ => false, - } + Self::has_reg(opnd, SCRATCH_OPND.unwrap_reg()) } /// Get the list of registers from which we will allocate on this platform @@ -492,7 +486,7 @@ impl Assembler { // Note: the iteration order is reversed to avoid corrupting x0, // which is both the return value and first argument register if !opnds.is_empty() { - let mut args: Vec<(Reg, Opnd)> = vec![]; + let mut args: Vec<(Opnd, Opnd)> = vec![]; for (idx, opnd) in opnds.iter_mut().enumerate().rev() { // If the value that we're sending is 0, then we can use // the zero register, so in this case we'll just send @@ -502,7 +496,7 @@ impl Assembler { Opnd::Mem(_) => split_memory_address(asm, *opnd), _ => *opnd }; - args.push((C_ARG_OPNDS[idx].unwrap_reg(), value)); + args.push((C_ARG_OPNDS[idx], value)); } asm.parallel_mov(args); } @@ -725,10 +719,21 @@ impl Assembler { asm.lea_into(SCRATCH_OPND, Opnd::mem(64, SCRATCH_OPND, 0)); asm.incr_counter(SCRATCH_OPND, value); } + &mut Insn::Mov { dest, src } => { + match dest { + Opnd::Reg(_) => asm.load_into(dest, src), + Opnd::Mem(_) => asm.store(dest, src), + _ => asm.push_insn(insn), + } + } // Resolve ParallelMov that couldn't be handled without a scratch register. Insn::ParallelMov { moves } => { - for (reg, opnd) in Self::resolve_parallel_moves(moves, Some(SCRATCH_OPND)).unwrap() { - asm.load_into(Opnd::Reg(reg), opnd); + for (dst, src) in Self::resolve_parallel_moves(moves, Some(SCRATCH_OPND)).unwrap() { + match dst { + Opnd::Reg(_) => asm.load_into(dst, src), + Opnd::Mem(_) => asm.store(dst, src), + _ => asm.mov(dst, src), + } } } _ => { @@ -2385,7 +2390,7 @@ mod tests { } #[test] - fn test_reorder_c_args_no_cycle() { + fn test_ccall_resolve_parallel_moves_no_cycle() { crate::options::rb_zjit_prepare_options(); let (mut asm, mut cb) = setup_asm(); @@ -2403,7 +2408,7 @@ mod tests { } #[test] - fn test_reorder_c_args_single_cycle() { + fn test_ccall_resolve_parallel_moves_single_cycle() { crate::options::rb_zjit_prepare_options(); let (mut asm, mut cb) = setup_asm(); @@ -2426,7 +2431,7 @@ mod tests { } #[test] - fn test_reorder_c_args_two_cycles() { + fn test_ccall_resolve_parallel_moves_two_cycles() { crate::options::rb_zjit_prepare_options(); let (mut asm, mut cb) = setup_asm(); @@ -2453,7 +2458,7 @@ mod tests { } #[test] - fn test_reorder_c_args_large_cycle() { + fn test_ccall_resolve_parallel_moves_large_cycle() { crate::options::rb_zjit_prepare_options(); let (mut asm, mut cb) = setup_asm(); diff --git a/zjit/src/backend/lir.rs b/zjit/src/backend/lir.rs index eb49b419d62046..72ebeaf11be61a 100644 --- a/zjit/src/backend/lir.rs +++ b/zjit/src/backend/lir.rs @@ -454,9 +454,9 @@ pub enum Insn { /// Shift a value left by a certain amount. LShift { opnd: Opnd, shift: Opnd, out: Opnd }, - /// A set of parallel moves into registers. + /// A set of parallel moves into registers or memory. /// The backend breaks cycles if there are any cycles between moves. - ParallelMov { moves: Vec<(Reg, Opnd)> }, + ParallelMov { moves: Vec<(Opnd, Opnd)> }, // A low-level mov instruction. It accepts two operands. Mov { dest: Opnd, src: Opnd }, @@ -1227,6 +1227,15 @@ impl Assembler asm } + /// Return true if `opnd` is or depends on `reg` + pub fn has_reg(opnd: Opnd, reg: Reg) -> bool { + match opnd { + Opnd::Reg(opnd_reg) => opnd_reg == reg, + Opnd::Mem(Mem { base: MemBase::Reg(reg_no), .. }) => reg_no == reg.reg_no, + _ => false, + } + } + pub fn expect_leaf_ccall(&mut self, stack_size: usize) { self.leaf_ccall_stack_size = Some(stack_size); } @@ -1307,17 +1316,23 @@ impl Assembler // Shuffle register moves, sometimes adding extra moves using scratch_reg, // so that they will not rewrite each other before they are used. - pub fn resolve_parallel_moves(old_moves: &[(Reg, Opnd)], scratch_reg: Option) -> Option> { + pub fn resolve_parallel_moves(old_moves: &[(Opnd, Opnd)], scratch_opnd: Option) -> Option> { // Return the index of a move whose destination is not used as a source if any. - fn find_safe_move(moves: &[(Reg, Opnd)]) -> Option { - moves.iter().enumerate().find(|&(_, &(dest_reg, _))| { - moves.iter().all(|&(_, src_opnd)| src_opnd != Opnd::Reg(dest_reg)) + fn find_safe_move(moves: &[(Opnd, Opnd)]) -> Option { + moves.iter().enumerate().find(|&(_, &(dst, src))| { + // Check if `dst` is used in other moves. If `dst` is not used elsewhere, it's safe to write into `dst` now. + moves.iter().filter(|&&other_move| other_move != (dst, src)).all(|&(other_dst, other_src)| + match dst { + Opnd::Reg(reg) => !Assembler::has_reg(other_dst, reg) && !Assembler::has_reg(other_src, reg), + _ => other_dst != dst && other_src != dst, + } + ) }).map(|(index, _)| index) } // Remove moves whose source and destination are the same - let mut old_moves: Vec<(Reg, Opnd)> = old_moves.iter().copied() - .filter(|&(reg, opnd)| Opnd::Reg(reg) != opnd).collect(); + let mut old_moves: Vec<(Opnd, Opnd)> = old_moves.iter().copied() + .filter(|&(dst, src)| dst != src).collect(); let mut new_moves = vec![]; while !old_moves.is_empty() { @@ -1326,18 +1341,19 @@ impl Assembler new_moves.push(old_moves.remove(index)); } - // No safe move. Load the source of one move into scratch_reg, and - // then load scratch_reg into the destination when it's safe. + // No safe move. Load the source of one move into scratch_opnd, and + // then load scratch_opnd into the destination when it's safe. if !old_moves.is_empty() { - // If scratch_reg is None, return None and leave it to *_split_with_scratch_regs to resolve it. - let scratch_reg = scratch_reg?.unwrap_reg(); + // If scratch_opnd is None, return None and leave it to *_split_with_scratch_regs to resolve it. + let scratch_opnd = scratch_opnd?; + let scratch_reg = scratch_opnd.unwrap_reg(); // Make sure it's safe to use scratch_reg - assert!(old_moves.iter().all(|&(_, opnd)| opnd != Opnd::Reg(scratch_reg))); + assert!(old_moves.iter().all(|&(dst, src)| !Self::has_reg(dst, scratch_reg) && !Self::has_reg(src, scratch_reg))); - // Move scratch_reg <- opnd, and delay reg <- scratch_reg - let (reg, opnd) = old_moves.remove(0); - new_moves.push((scratch_reg, opnd)); - old_moves.push((reg, Opnd::Reg(scratch_reg))); + // Move scratch_opnd <- src, and delay dst <- scratch_opnd + let (dst, src) = old_moves.remove(0); + new_moves.push((scratch_opnd, src)); + old_moves.push((dst, scratch_opnd)); } } Some(new_moves) @@ -1551,8 +1567,8 @@ impl Assembler Insn::ParallelMov { moves } => { // For trampolines that use scratch registers, attempt to lower ParallelMov without scratch_reg. if let Some(moves) = Self::resolve_parallel_moves(&moves, None) { - for (reg, opnd) in moves { - asm.load_into(Opnd::Reg(reg), opnd); + for (dst, src) in moves { + asm.mov(dst, src); } } else { // If it needs a scratch_reg, leave it to *_split_with_scratch_regs to handle it. @@ -1980,7 +1996,7 @@ impl Assembler { out } - pub fn parallel_mov(&mut self, moves: Vec<(Reg, Opnd)>) { + pub fn parallel_mov(&mut self, moves: Vec<(Opnd, Opnd)>) { self.push_insn(Insn::ParallelMov { moves }); } @@ -2096,6 +2112,10 @@ pub(crate) use asm_ccall; mod tests { use super::*; + fn scratch_reg() -> Opnd { + Assembler::new_with_scratch_reg().1 + } + #[test] fn test_opnd_iter() { let insn = Insn::Add { left: Opnd::None, right: Opnd::None, out: Opnd::None }; @@ -2125,4 +2145,99 @@ mod tests { let mem = Opnd::mem(64, SP, 0); asm.load_into(mem, mem); } + + #[test] + fn test_resolve_parallel_moves_reorder_registers() { + let result = Assembler::resolve_parallel_moves(&[ + (C_ARG_OPNDS[0], SP), + (C_ARG_OPNDS[1], C_ARG_OPNDS[0]), + ], None); + assert_eq!(result, Some(vec![ + (C_ARG_OPNDS[1], C_ARG_OPNDS[0]), + (C_ARG_OPNDS[0], SP), + ])); + } + + #[test] + fn test_resolve_parallel_moves_give_up_register_cycle() { + // If scratch_opnd is not given, it cannot break cycles. + let result = Assembler::resolve_parallel_moves(&[ + (C_ARG_OPNDS[0], C_ARG_OPNDS[1]), + (C_ARG_OPNDS[1], C_ARG_OPNDS[0]), + ], None); + assert_eq!(result, None); + } + + #[test] + fn test_resolve_parallel_moves_break_register_cycle() { + let scratch_reg = scratch_reg(); + let result = Assembler::resolve_parallel_moves(&[ + (C_ARG_OPNDS[0], C_ARG_OPNDS[1]), + (C_ARG_OPNDS[1], C_ARG_OPNDS[0]), + ], Some(scratch_reg)); + assert_eq!(result, Some(vec![ + (scratch_reg, C_ARG_OPNDS[1]), + (C_ARG_OPNDS[1], C_ARG_OPNDS[0]), + (C_ARG_OPNDS[0], scratch_reg), + ])); + } + + #[test] + fn test_resolve_parallel_moves_break_memory_memory_cycle() { + let scratch_reg = scratch_reg(); + let result = Assembler::resolve_parallel_moves(&[ + (Opnd::mem(64, C_ARG_OPNDS[0], 0), C_ARG_OPNDS[1]), + (C_ARG_OPNDS[1], Opnd::mem(64, C_ARG_OPNDS[0], 0)), + ], Some(scratch_reg)); + assert_eq!(result, Some(vec![ + (scratch_reg, C_ARG_OPNDS[1]), + (C_ARG_OPNDS[1], Opnd::mem(64, C_ARG_OPNDS[0], 0)), + (Opnd::mem(64, C_ARG_OPNDS[0], 0), scratch_reg), + ])); + } + + #[test] + fn test_resolve_parallel_moves_break_register_memory_cycle() { + let scratch_reg = scratch_reg(); + let result = Assembler::resolve_parallel_moves(&[ + (C_ARG_OPNDS[0], C_ARG_OPNDS[1]), + (C_ARG_OPNDS[1], Opnd::mem(64, C_ARG_OPNDS[0], 0)), + ], Some(scratch_reg)); + assert_eq!(result, Some(vec![ + (scratch_reg, C_ARG_OPNDS[1]), + (C_ARG_OPNDS[1], Opnd::mem(64, C_ARG_OPNDS[0], 0)), + (C_ARG_OPNDS[0], scratch_reg), + ])); + } + + #[test] + fn test_resolve_parallel_moves_reorder_memory_destination() { + let scratch_reg = scratch_reg(); + let result = Assembler::resolve_parallel_moves(&[ + (C_ARG_OPNDS[0], SP), + (Opnd::mem(64, C_ARG_OPNDS[0], 0), CFP), + ], Some(scratch_reg)); + assert_eq!(result, Some(vec![ + (Opnd::mem(64, C_ARG_OPNDS[0], 0), CFP), + (C_ARG_OPNDS[0], SP), + ])); + } + + #[test] + #[should_panic] + fn test_resolve_parallel_moves_into_same_register() { + Assembler::resolve_parallel_moves(&[ + (C_ARG_OPNDS[0], SP), + (C_ARG_OPNDS[0], CFP), + ], Some(scratch_reg())); + } + + #[test] + #[should_panic] + fn test_resolve_parallel_moves_into_same_memory() { + Assembler::resolve_parallel_moves(&[ + (Opnd::mem(64, C_ARG_OPNDS[0], 0), SP), + (Opnd::mem(64, C_ARG_OPNDS[0], 0), CFP), + ], Some(scratch_reg())); + } } diff --git a/zjit/src/backend/x86_64/mod.rs b/zjit/src/backend/x86_64/mod.rs index bd2421823c9f3c..f76be64ec0025c 100644 --- a/zjit/src/backend/x86_64/mod.rs +++ b/zjit/src/backend/x86_64/mod.rs @@ -107,13 +107,7 @@ impl Assembler { /// Return true if opnd contains a scratch reg pub fn has_scratch_reg(opnd: Opnd) -> bool { - match opnd { - Opnd::Reg(_) => opnd == SCRATCH_OPND, - Opnd::Mem(Mem { base: MemBase::Reg(reg_no), .. }) => { - reg_no == SCRATCH_OPND.unwrap_reg().reg_no - } - _ => false, - } + Self::has_reg(opnd, SCRATCH_OPND.unwrap_reg()) } /// Get the list of registers from which we can allocate on this platform @@ -354,9 +348,9 @@ impl Assembler { // Load each operand into the corresponding argument register. if !opnds.is_empty() { - let mut args: Vec<(Reg, Opnd)> = vec![]; + let mut args: Vec<(Opnd, Opnd)> = vec![]; for (idx, opnd) in opnds.iter_mut().enumerate() { - args.push((C_ARG_OPNDS[idx].unwrap_reg(), *opnd)); + args.push((C_ARG_OPNDS[idx], *opnd)); } asm.parallel_mov(args); } @@ -489,8 +483,8 @@ impl Assembler { } // Resolve ParallelMov that couldn't be handled without a scratch register. Insn::ParallelMov { moves } => { - for (reg, opnd) in Self::resolve_parallel_moves(&moves, Some(SCRATCH_OPND)).unwrap() { - asm.load_into(Opnd::Reg(reg), opnd); + for (dst, src) in Self::resolve_parallel_moves(&moves, Some(SCRATCH_OPND)).unwrap() { + asm.mov(dst, src) } } // Handle various operand combinations for spills on compile_side_exits. @@ -1368,7 +1362,7 @@ mod tests { } #[test] - fn test_reorder_c_args_no_cycle() { + fn test_ccall_resolve_parallel_moves_no_cycle() { crate::options::rb_zjit_prepare_options(); let (mut asm, mut cb) = setup_asm(); @@ -1386,7 +1380,7 @@ mod tests { } #[test] - fn test_reorder_c_args_single_cycle() { + fn test_ccall_resolve_parallel_moves_single_cycle() { crate::options::rb_zjit_prepare_options(); let (mut asm, mut cb) = setup_asm(); @@ -1409,7 +1403,7 @@ mod tests { } #[test] - fn test_reorder_c_args_two_cycles() { + fn test_ccall_resolve_parallel_moves_two_cycles() { crate::options::rb_zjit_prepare_options(); let (mut asm, mut cb) = setup_asm(); @@ -1436,7 +1430,7 @@ mod tests { } #[test] - fn test_reorder_c_args_large_cycle() { + fn test_ccall_resolve_parallel_moves_large_cycle() { crate::options::rb_zjit_prepare_options(); let (mut asm, mut cb) = setup_asm(); @@ -1461,7 +1455,7 @@ mod tests { #[test] #[ignore] - fn test_reorder_c_args_with_insn_out() { + fn test_ccall_resolve_parallel_moves_with_insn_out() { let (mut asm, mut cb) = setup_asm(); let rax = asm.load(Opnd::UImm(1)); diff --git a/zjit/src/codegen.rs b/zjit/src/codegen.rs index 63b5b6cb52cab3..5ead4870dff517 100644 --- a/zjit/src/codegen.rs +++ b/zjit/src/codegen.rs @@ -8,7 +8,7 @@ use std::ffi::{c_int, c_long, c_void}; use std::slice; use crate::asm::Label; -use crate::backend::current::{Reg, ALLOC_REGS}; +use crate::backend::current::ALLOC_REGS; use crate::invariants::{ track_bop_assumption, track_cme_assumption, track_no_ep_escape_assumption, track_no_trace_point_assumption, track_single_ractor_assumption, track_stable_constant_names_assumption, track_no_singleton_class_assumption @@ -1024,20 +1024,9 @@ fn gen_branch_params(jit: &mut JITState, asm: &mut Assembler, branch: &BranchEdg } asm_comment!(asm, "set branch params: {}", branch.args.len()); - let mut moves: Vec<(Reg, Opnd)> = vec![]; - for (idx, &arg) in branch.args.iter().enumerate() { - match param_opnd(idx) { - Opnd::Reg(reg) => { - // If a parameter is a register, we need to parallel-move it - moves.push((reg, jit.get_opnd(arg))); - }, - param => { - // If a parameter is memory, we set it beforehand - asm.mov(param, jit.get_opnd(arg)); - } - } - } - asm.parallel_mov(moves); + asm.parallel_mov(branch.args.iter().enumerate().map(|(idx, &arg)| + (param_opnd(idx), jit.get_opnd(arg)) + ).collect()); } /// Compile a constant From afb0d43181429d393b3db614cd8246c33c331626 Mon Sep 17 00:00:00 2001 From: Takashi Kokubun Date: Tue, 28 Oct 2025 13:47:15 -0700 Subject: [PATCH 04/12] ZJIT: Drop --seed from test-all on CI (#14976) --- .github/workflows/zjit-macos.yml | 3 --- .github/workflows/zjit-ubuntu.yml | 3 --- 2 files changed, 6 deletions(-) diff --git a/.github/workflows/zjit-macos.yml b/.github/workflows/zjit-macos.yml index 0d2b564c41c802..f687672ac7a9e3 100644 --- a/.github/workflows/zjit-macos.yml +++ b/.github/workflows/zjit-macos.yml @@ -36,13 +36,11 @@ jobs: run_opts: '--zjit-call-threshold=1' specopts: '-T --zjit-call-threshold=1' configure: '--enable-zjit=dev' - test_all_opts: '--seed=46450' - test_task: 'check' run_opts: '--zjit-disable-hir-opt --zjit-call-threshold=1' specopts: '-T --zjit-disable-hir-opt -T --zjit-call-threshold=1' configure: '--enable-zjit=dev' - test_all_opts: '--seed=46450' - test_task: 'zjit-check' # zjit-test + quick feedback of test_zjit.rb configure: '--enable-yjit=dev --enable-zjit' @@ -127,7 +125,6 @@ jobs: TEST_BUNDLED_GEMS_ALLOW_FAILURES: '' SYNTAX_SUGGEST_TIMEOUT: '5' PRECHECK_BUNDLED_GEMS: 'no' - TESTS: ${{ matrix.test_all_opts }} continue-on-error: ${{ matrix.continue-on-test_task || false }} - name: Dump crash logs diff --git a/.github/workflows/zjit-ubuntu.yml b/.github/workflows/zjit-ubuntu.yml index ec92cd5342588c..5163076be5d98a 100644 --- a/.github/workflows/zjit-ubuntu.yml +++ b/.github/workflows/zjit-ubuntu.yml @@ -55,13 +55,11 @@ jobs: run_opts: '--zjit-call-threshold=1' specopts: '-T --zjit-call-threshold=1' configure: '--enable-zjit=dev' - test_all_opts: '--seed=39471' - test_task: 'check' run_opts: '--zjit-disable-hir-opt --zjit-call-threshold=1' specopts: '-T --zjit-disable-hir-opt -T --zjit-call-threshold=1' configure: '--enable-zjit=dev' - test_all_opts: '--seed=39471' - test_task: 'zjit-check' # zjit-test + quick feedback of test_zjit.rb configure: '--enable-yjit --enable-zjit=dev' @@ -177,7 +175,6 @@ jobs: SYNTAX_SUGGEST_TIMEOUT: '5' ZJIT_BINDGEN_DIFF_OPTS: '--exit-code' CLANG_PATH: ${{ matrix.clang_path }} - TESTS: ${{ matrix.test_all_opts }} continue-on-error: ${{ matrix.continue-on-test_task || false }} - name: Dump crash logs From 5a14999d0d54b3af4b91860922168d0e3de19bc7 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 28 Oct 2025 12:21:26 +0000 Subject: [PATCH 05/12] Bump actions/upload-artifact from 4 to 5 Bumps [actions/upload-artifact](https://github.com/actions/upload-artifact) from 4 to 5. - [Release notes](https://github.com/actions/upload-artifact/releases) - [Commits](https://github.com/actions/upload-artifact/compare/v4...v5) --- updated-dependencies: - dependency-name: actions/upload-artifact dependency-version: '5' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] --- .github/workflows/check_misc.yml | 2 +- .github/workflows/scorecards.yml | 2 +- .github/workflows/wasm.yml | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/check_misc.yml b/.github/workflows/check_misc.yml index 38c23b41107a28..bbbaae07b20765 100644 --- a/.github/workflows/check_misc.yml +++ b/.github/workflows/check_misc.yml @@ -103,7 +103,7 @@ jobs: }} - name: Upload docs - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@v5 with: path: html name: ${{ steps.docs.outputs.htmlout }} diff --git a/.github/workflows/scorecards.yml b/.github/workflows/scorecards.yml index 34a301ddb020d5..ec3dc873b1a877 100644 --- a/.github/workflows/scorecards.yml +++ b/.github/workflows/scorecards.yml @@ -64,7 +64,7 @@ jobs: # Upload the results as artifacts (optional). Commenting out will disable uploads of run results in SARIF # format to the repository Actions tab. - name: "Upload artifact" - uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 + uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 with: name: SARIF file path: results.sarif diff --git a/.github/workflows/wasm.yml b/.github/workflows/wasm.yml index fcb062a4c1d5c0..10c6afbe5e4228 100644 --- a/.github/workflows/wasm.yml +++ b/.github/workflows/wasm.yml @@ -140,7 +140,7 @@ jobs: - run: tar cfz ../install.tar.gz -C ../install . - name: Upload artifacts - uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 + uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 with: name: ruby-wasm-install path: ${{ github.workspace }}/install.tar.gz @@ -168,7 +168,7 @@ jobs: - name: Save Pull Request number if: ${{ github.event_name == 'pull_request' }} run: echo "${{ github.event.pull_request.number }}" >> ${{ github.workspace }}/github-pr-info.txt - - uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2 + - uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0 if: ${{ github.event_name == 'pull_request' }} with: name: github-pr-info From 8dc276f3e1a2d16814eace023840e9eabe7e4101 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 28 Oct 2025 12:21:57 +0000 Subject: [PATCH 06/12] Bump github.com/microsoft/vcpkg from master to 2025.10.17 Bumps [github.com/microsoft/vcpkg](https://github.com/microsoft/vcpkg) from master to 2025.10.17. This release includes the previously tagged commit. - [Release notes](https://github.com/microsoft/vcpkg/releases) - [Commits](https://github.com/microsoft/vcpkg/compare/4334d8b4c8916018600212ab4dd4bbdc343065d1...74e6536215718009aae747d86d84b78376bf9e09) --- updated-dependencies: - dependency-name: github.com/microsoft/vcpkg dependency-version: 2025.10.17 dependency-type: direct:production ... Signed-off-by: dependabot[bot] --- vcpkg.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vcpkg.json b/vcpkg.json index b8b0faf93edb1a..1e4b65d89c9632 100644 --- a/vcpkg.json +++ b/vcpkg.json @@ -7,5 +7,5 @@ "openssl", "zlib" ], - "builtin-baseline": "4334d8b4c8916018600212ab4dd4bbdc343065d1" + "builtin-baseline": "74e6536215718009aae747d86d84b78376bf9e09" } \ No newline at end of file From d6d095e2fc4e6ec4d965811de98333bd71076555 Mon Sep 17 00:00:00 2001 From: Takashi Kokubun Date: Tue, 28 Oct 2025 16:06:38 -0700 Subject: [PATCH 07/12] ZJIT: Rename SCRATCH_OPND to SCRATCH0_OPND for x86_64 --- zjit/src/backend/x86_64/mod.rs | 48 +++++++++++++++++----------------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/zjit/src/backend/x86_64/mod.rs b/zjit/src/backend/x86_64/mod.rs index f76be64ec0025c..2590ceaf7d0e2b 100644 --- a/zjit/src/backend/x86_64/mod.rs +++ b/zjit/src/backend/x86_64/mod.rs @@ -83,7 +83,7 @@ impl From<&Opnd> for X86Opnd { /// List of registers that can be used for register allocation. /// This has the same number of registers for x86_64 and arm64. -/// SCRATCH_OPND is excluded. +/// SCRATCH0_OPND is excluded. pub const ALLOC_REGS: &[Reg] = &[ RDI_REG, RSI_REG, @@ -97,17 +97,17 @@ pub const ALLOC_REGS: &[Reg] = &[ /// Special scratch register for intermediate processing. It should be used only by /// [`Assembler::x86_split_with_scratch_reg`] or [`Assembler::new_with_scratch_reg`]. -const SCRATCH_OPND: Opnd = Opnd::Reg(R11_REG); +const SCRATCH0_OPND: Opnd = Opnd::Reg(R11_REG); impl Assembler { /// Return an Assembler with scratch registers disabled in the backend, and a scratch register. pub fn new_with_scratch_reg() -> (Self, Opnd) { - (Self::new_with_accept_scratch_reg(true), SCRATCH_OPND) + (Self::new_with_accept_scratch_reg(true), SCRATCH0_OPND) } /// Return true if opnd contains a scratch reg pub fn has_scratch_reg(opnd: Opnd) -> bool { - Self::has_reg(opnd, SCRATCH_OPND.unwrap_reg()) + Self::has_reg(opnd, SCRATCH0_OPND.unwrap_reg()) } /// Get the list of registers from which we can allocate on this platform @@ -387,15 +387,15 @@ impl Assembler { pub fn x86_split_with_scratch_reg(self) -> Assembler { /// For some instructions, we want to be able to lower a 64-bit operand /// without requiring more registers to be available in the register - /// allocator. So we just use the SCRATCH_OPND register temporarily to hold + /// allocator. So we just use the SCRATCH0_OPND register temporarily to hold /// the value before we immediately use it. fn split_64bit_immediate(asm: &mut Assembler, opnd: Opnd) -> Opnd { match opnd { Opnd::Imm(value) => { // 32-bit values will be sign-extended if imm_num_bits(value) > 32 { - asm.mov(SCRATCH_OPND, opnd); - SCRATCH_OPND + asm.mov(SCRATCH0_OPND, opnd); + SCRATCH0_OPND } else { opnd } @@ -403,8 +403,8 @@ impl Assembler { Opnd::UImm(value) => { // 32-bit values will be sign-extended if imm_num_bits(value as i64) > 32 { - asm.mov(SCRATCH_OPND, opnd); - SCRATCH_OPND + asm.mov(SCRATCH0_OPND, opnd); + SCRATCH0_OPND } else { Opnd::Imm(value as i64) } @@ -460,8 +460,8 @@ impl Assembler { match (opnd, out) { // Split here for compile_side_exits (Opnd::Mem(_), Opnd::Mem(_)) => { - asm.lea_into(SCRATCH_OPND, opnd); - asm.store(out, SCRATCH_OPND); + asm.lea_into(SCRATCH0_OPND, opnd); + asm.store(out, SCRATCH0_OPND); } _ => { asm.push_insn(insn); @@ -470,20 +470,20 @@ impl Assembler { } Insn::LeaJumpTarget { target, out } => { if let Target::Label(_) = target { - asm.push_insn(Insn::LeaJumpTarget { out: SCRATCH_OPND, target: target.clone() }); - asm.mov(*out, SCRATCH_OPND); + asm.push_insn(Insn::LeaJumpTarget { out: SCRATCH0_OPND, target: target.clone() }); + asm.mov(*out, SCRATCH0_OPND); } } // Convert Opnd::const_ptr into Opnd::Mem. This split is done here to give // a register for compile_side_exits. &mut Insn::IncrCounter { mem, value } => { assert!(matches!(mem, Opnd::UImm(_))); - asm.load_into(SCRATCH_OPND, mem); - asm.incr_counter(Opnd::mem(64, SCRATCH_OPND, 0), value); + asm.load_into(SCRATCH0_OPND, mem); + asm.incr_counter(Opnd::mem(64, SCRATCH0_OPND, 0), value); } // Resolve ParallelMov that couldn't be handled without a scratch register. Insn::ParallelMov { moves } => { - for (dst, src) in Self::resolve_parallel_moves(&moves, Some(SCRATCH_OPND)).unwrap() { + for (dst, src) in Self::resolve_parallel_moves(&moves, Some(SCRATCH0_OPND)).unwrap() { asm.mov(dst, src) } } @@ -496,14 +496,14 @@ impl Assembler { let src = match src { Opnd::Reg(_) => src, Opnd::Mem(_) => { - asm.mov(SCRATCH_OPND, src); - SCRATCH_OPND + asm.mov(SCRATCH0_OPND, src); + SCRATCH0_OPND } Opnd::Imm(imm) => { // For 64 bit destinations, 32-bit values will be sign-extended if num_bits == 64 && imm_num_bits(imm) > 32 { - asm.mov(SCRATCH_OPND, src); - SCRATCH_OPND + asm.mov(SCRATCH0_OPND, src); + SCRATCH0_OPND } else if uimm_num_bits(imm as u64) <= num_bits { // If the bit string is short enough for the destination, use the unsigned representation. // Note that 64-bit and negative values are ruled out. @@ -515,15 +515,15 @@ impl Assembler { Opnd::UImm(imm) => { // For 64 bit destinations, 32-bit values will be sign-extended if num_bits == 64 && imm_num_bits(imm as i64) > 32 { - asm.mov(SCRATCH_OPND, src); - SCRATCH_OPND + asm.mov(SCRATCH0_OPND, src); + SCRATCH0_OPND } else { src.into() } } Opnd::Value(_) => { - asm.load_into(SCRATCH_OPND, src); - SCRATCH_OPND + asm.load_into(SCRATCH0_OPND, src); + SCRATCH0_OPND } src @ (Opnd::None | Opnd::VReg { .. }) => panic!("Unexpected source operand during x86_split_with_scratch_reg: {src:?}"), }; From 7428dc7348c37703dbc36fa959f8b4254af17a60 Mon Sep 17 00:00:00 2001 From: Takashi Kokubun Date: Tue, 28 Oct 2025 15:07:19 -0700 Subject: [PATCH 08/12] ZJIT: Migrate an arm64 register from emit to split --- zjit/src/backend/arm64/mod.rs | 205 +++++++++++++++++++--------------- 1 file changed, 113 insertions(+), 92 deletions(-) diff --git a/zjit/src/backend/arm64/mod.rs b/zjit/src/backend/arm64/mod.rs index e090d8ce446aaa..234f5ac0590ba5 100644 --- a/zjit/src/backend/arm64/mod.rs +++ b/zjit/src/backend/arm64/mod.rs @@ -113,8 +113,8 @@ fn emit_jmp_ptr(cb: &mut CodeBlock, dst_ptr: CodePtr, padding: bool) { b(cb, InstructionOffset::from_bytes((dst_addr - src_addr) as i32)); 1 } else { - let num_insns = emit_load_value(cb, Assembler::EMIT0_OPND, dst_addr as u64); - br(cb, Assembler::EMIT0_OPND); + let num_insns = emit_load_value(cb, Assembler::EMIT_OPND, dst_addr as u64); + br(cb, Assembler::EMIT_OPND); num_insns + 1 }; @@ -181,7 +181,7 @@ fn emit_load_value(cb: &mut CodeBlock, rd: A64Opnd, value: u64) -> usize { /// List of registers that can be used for register allocation. /// This has the same number of registers for x86_64 and arm64. -/// SCRATCH_OPND, EMIT0_OPND, and EMIT1_OPND are excluded. +/// SCRATCH_OPND, SCRATCH1_OPND, and EMIT_OPND are excluded. pub const ALLOC_REGS: &[Reg] = &[ X0_REG, X1_REG, @@ -193,26 +193,24 @@ pub const ALLOC_REGS: &[Reg] = &[ X12_REG, ]; -/// Special scratch register for intermediate processing. It should be used only by +/// Special scratch registers for intermediate processing. They should be used only by /// [`Assembler::arm64_split_with_scratch_reg`] or [`Assembler::new_with_scratch_reg`]. -const SCRATCH_OPND: Opnd = Opnd::Reg(X15_REG); +const SCRATCH0_OPND: Opnd = Opnd::Reg(X15_REG); +const SCRATCH1_OPND: Opnd = Opnd::Reg(X17_REG); impl Assembler { - /// Special registers for intermediate processing in arm64_emit. It should be used only by arm64_emit. - /// TODO: Remove the use of these registers by splitting instructions in arm64_split_with_scratch_reg. - const EMIT0_REG: Reg = X16_REG; - const EMIT1_REG: Reg = X17_REG; - const EMIT0_OPND: A64Opnd = A64Opnd::Reg(Self::EMIT0_REG); - const EMIT1_OPND: A64Opnd = A64Opnd::Reg(Self::EMIT1_REG); + /// Special register for intermediate processing in arm64_emit. It should be used only by arm64_emit. + const EMIT_REG: Reg = X16_REG; + const EMIT_OPND: A64Opnd = A64Opnd::Reg(Self::EMIT_REG); /// Return an Assembler with scratch registers disabled in the backend, and a scratch register. pub fn new_with_scratch_reg() -> (Self, Opnd) { - (Self::new_with_accept_scratch_reg(true), SCRATCH_OPND) + (Self::new_with_accept_scratch_reg(true), SCRATCH0_OPND) } /// Return true if opnd contains a scratch reg pub fn has_scratch_reg(opnd: Opnd) -> bool { - Self::has_reg(opnd, SCRATCH_OPND.unwrap_reg()) + Self::has_reg(opnd, SCRATCH0_OPND.unwrap_reg()) } /// Get the list of registers from which we will allocate on this platform @@ -688,10 +686,20 @@ impl Assembler { fn arm64_split_with_scratch_reg(self) -> Assembler { let mut asm = Assembler::new_with_asm(&self); asm.accept_scratch_reg = true; - let iterator = self.insns.into_iter().enumerate().peekable(); + let mut iterator = self.insns.into_iter().enumerate().peekable(); - for (_, mut insn) in iterator { + while let Some((_, mut insn)) = iterator.next() { match &mut insn { + &mut Insn::Mul { out, .. } => { + asm.push_insn(insn); + + // If the next instruction is JoMul + if matches!(iterator.peek(), Some((_, Insn::JoMul(_)))) { + // Produce a register that is all zeros or all ones + // Based on the sign bit of the 64-bit mul result + asm.push_insn(Insn::RShift { out: SCRATCH0_OPND, opnd: out, shift: Opnd::UImm(63) }); + } + } // For compile_side_exits, support splitting simple C arguments here Insn::CCall { opnds, .. } if !opnds.is_empty() => { for (i, opnd) in opnds.iter().enumerate() { @@ -704,20 +712,43 @@ impl Assembler { match (opnd, out) { // Split here for compile_side_exits (Opnd::Mem(_), Opnd::Mem(_)) => { - asm.lea_into(SCRATCH_OPND, opnd); - asm.store(out, SCRATCH_OPND); + asm.lea_into(SCRATCH0_OPND, opnd); + asm.store(out, SCRATCH0_OPND); } _ => { asm.push_insn(insn); } } } - // Convert Opnd::const_ptr into Opnd::Mem. It's split here compile_side_exits. &mut Insn::IncrCounter { mem, value } => { + // Convert Opnd::const_ptr into Opnd::Mem. + // It's split here to support IncrCounter in compile_side_exits. assert!(matches!(mem, Opnd::UImm(_))); - asm.load_into(SCRATCH_OPND, mem); - asm.lea_into(SCRATCH_OPND, Opnd::mem(64, SCRATCH_OPND, 0)); - asm.incr_counter(SCRATCH_OPND, value); + asm.load_into(SCRATCH0_OPND, mem); + asm.lea_into(SCRATCH0_OPND, Opnd::mem(64, SCRATCH0_OPND, 0)); + + // Create a local loop to atomically increment a counter using SCRATCH1_OPND to check if it succeeded. + // Note that arm64_emit will peek at the next Cmp to set a status into SCRATCH1_OPND on IncrCounter. + let label = asm.new_label("incr_counter_loop"); + asm.write_label(label.clone()); + asm.incr_counter(SCRATCH0_OPND, value); + asm.cmp(SCRATCH1_OPND, 0.into()); + asm.jne(label); + } + &mut Insn::Store { dest, src } => { + let Opnd::Mem(Mem { num_bits: dest_num_bits, disp: dest_disp, .. }) = dest else { + panic!("Insn::Store destination must be Opnd::Mem: {dest:?}, {src:?}"); + }; + + // Split dest using a scratch register if necessary. + let dest = if mem_disp_fits_bits(dest_disp) { + dest + } else { + asm.lea_into(SCRATCH0_OPND, dest); + Opnd::mem(dest_num_bits, SCRATCH0_OPND, 0) + }; + + asm.store(dest, src); } &mut Insn::Mov { dest, src } => { match dest { @@ -728,7 +759,7 @@ impl Assembler { } // Resolve ParallelMov that couldn't be handled without a scratch register. Insn::ParallelMov { moves } => { - for (dst, src) in Self::resolve_parallel_moves(moves, Some(SCRATCH_OPND)).unwrap() { + for (dst, src) in Self::resolve_parallel_moves(moves, Some(SCRATCH0_OPND)).unwrap() { match dst { Opnd::Reg(_) => asm.load_into(dst, src), Opnd::Mem(_) => asm.store(dst, src), @@ -805,8 +836,8 @@ impl Assembler { // that if it doesn't match it will skip over the // instructions used for branching. bcond(cb, Condition::inverse(CONDITION), (load_insns + 2).into()); - emit_load_value(cb, Assembler::EMIT0_OPND, dst_addr as u64); - br(cb, Assembler::EMIT0_OPND); + emit_load_value(cb, Assembler::EMIT_OPND, dst_addr as u64); + br(cb, Assembler::EMIT_OPND); // Here we'll return the number of instructions that it // took to write out the destination address + 1 for the @@ -870,8 +901,8 @@ impl Assembler { } else { cbz(cb, reg, InstructionOffset::from_insns(load_insns + 2)); } - emit_load_value(cb, Assembler::EMIT0_OPND, dst_addr); - br(cb, Assembler::EMIT0_OPND); + emit_load_value(cb, Assembler::EMIT_OPND, dst_addr); + br(cb, Assembler::EMIT_OPND); } } else { unreachable!("We should only generate Joz/Jonz with side-exit targets"); @@ -1039,25 +1070,25 @@ impl Assembler { } }, Insn::Mul { left, right, out } => { - // If the next instruction is jo (jump on overflow) + // If the next instruction is JoMul with RShift created by arm64_split_with_scratch_reg match (self.insns.get(insn_idx + 1), self.insns.get(insn_idx + 2)) { - (Some(Insn::JoMul(_)), _) | - (Some(Insn::PosMarker(_)), Some(Insn::JoMul(_))) => { + (Some(Insn::RShift { out: out_sign, opnd: out_opnd, shift: out_shift }), Some(Insn::JoMul(_))) => { // Compute the high 64 bits - smulh(cb, Self::EMIT0_OPND, left.into(), right.into()); + smulh(cb, Self::EMIT_OPND, left.into(), right.into()); // Compute the low 64 bits // This may clobber one of the input registers, // so we do it after smulh mul(cb, out.into(), left.into(), right.into()); - // Produce a register that is all zeros or all ones - // Based on the sign bit of the 64-bit mul result - asr(cb, Self::EMIT1_OPND, out.into(), A64Opnd::UImm(63)); + // Insert the shift instruction created by arm64_split_with_scratch_reg + // to prepare the register that has the sign bit of the high 64 bits after mul. + asr(cb, out_sign.into(), out_opnd.into(), out_shift.into()); + insn_idx += 1; // skip the next Insn::RShift // If the high 64-bits are not all zeros or all ones, // matching the sign bit, then we have an overflow - cmp(cb, Self::EMIT0_OPND, Self::EMIT1_OPND); + cmp(cb, Self::EMIT_OPND, out_sign.into()); // Insn::JoMul will emit_conditional_jump::<{Condition::NE}> } _ => { @@ -1087,11 +1118,6 @@ impl Assembler { lsl(cb, out.into(), opnd.into(), shift.into()); }, Insn::Store { dest, src } => { - // With minor exceptions, as long as `dest` is a Mem, all forms of `src` are accepted. - let &Opnd::Mem(Mem { num_bits: dest_num_bits, base: MemBase::Reg(base_reg_no), disp }) = dest else { - panic!("Unexpected Insn::Store destination in arm64_emit: {dest:?}"); - }; - // Split src into EMIT0_OPND if necessary let src_reg: A64Reg = match src { Opnd::Reg(reg) => *reg, @@ -1099,16 +1125,16 @@ impl Assembler { Opnd::UImm(0) | Opnd::Imm(0) => XZR_REG, // Immediates &Opnd::Imm(imm) => { - emit_load_value(cb, Self::EMIT0_OPND, imm as u64); - Self::EMIT0_REG + emit_load_value(cb, Self::EMIT_OPND, imm as u64); + Self::EMIT_REG } &Opnd::UImm(imm) => { - emit_load_value(cb, Self::EMIT0_OPND, imm); - Self::EMIT0_REG + emit_load_value(cb, Self::EMIT_OPND, imm); + Self::EMIT_REG } &Opnd::Value(value) => { - emit_load_gc_value(cb, &mut gc_offsets, Self::EMIT0_OPND, value); - Self::EMIT0_REG + emit_load_gc_value(cb, &mut gc_offsets, Self::EMIT_OPND, value); + Self::EMIT_REG } src_mem @ &Opnd::Mem(Mem { num_bits: src_num_bits, base: MemBase::Reg(src_base_reg_no), disp: src_disp }) => { // For mem-to-mem store, load the source into EMIT0_OPND @@ -1116,36 +1142,28 @@ impl Assembler { src_mem.into() } else { // Split the load address into EMIT0_OPND first if necessary - load_effective_address(cb, Self::EMIT0_OPND, src_base_reg_no, src_disp); - A64Opnd::new_mem(dest_num_bits, Self::EMIT0_OPND, 0) + load_effective_address(cb, Self::EMIT_OPND, src_base_reg_no, src_disp); + A64Opnd::new_mem(dest.rm_num_bits(), Self::EMIT_OPND, 0) }; match src_num_bits { - 64 | 32 => ldur(cb, Self::EMIT0_OPND, src_mem), - 16 => ldurh(cb, Self::EMIT0_OPND, src_mem), - 8 => ldurb(cb, Self::EMIT0_OPND, src_mem), + 64 | 32 => ldur(cb, Self::EMIT_OPND, src_mem), + 16 => ldurh(cb, Self::EMIT_OPND, src_mem), + 8 => ldurb(cb, Self::EMIT_OPND, src_mem), num_bits => panic!("unexpected num_bits: {num_bits}") }; - Self::EMIT0_REG + Self::EMIT_REG } src @ (Opnd::Mem(_) | Opnd::None | Opnd::VReg { .. }) => panic!("Unexpected source operand during arm64_emit: {src:?}") }; let src = A64Opnd::Reg(src_reg); - // Split dest into EMIT1_OPND if necessary. - let dest = if mem_disp_fits_bits(disp) { - dest.into() - } else { - load_effective_address(cb, Self::EMIT1_OPND, base_reg_no, disp); - A64Opnd::new_mem(dest_num_bits, Self::EMIT1_OPND, 0) - }; - // This order may be surprising but it is correct. The way // the Arm64 assembler works, the register that is going to // be stored is first and the address is second. However in // our IR we have the address first and the register second. - match dest_num_bits { - 64 | 32 => stur(cb, src, dest), - 16 => sturh(cb, src, dest), + match dest.rm_num_bits() { + 64 | 32 => stur(cb, src, dest.into()), + 16 => sturh(cb, src, dest.into()), num_bits => panic!("unexpected dest num_bits: {} (src: {:#?}, dest: {:#?})", num_bits, src, dest), } }, @@ -1219,7 +1237,7 @@ impl Assembler { } else { // Use a scratch reg for `out += displacement` let disp_reg = if out_reg_no == base_reg_no { - Self::EMIT0_OPND + Self::EMIT_OPND } else { out }; @@ -1234,10 +1252,10 @@ impl Assembler { if let Target::Label(label_idx) = target { // Set output to the raw address of the label cb.label_ref(*label_idx, 4, |cb, end_addr, dst_addr| { - adr(cb, Self::EMIT0_OPND, A64Opnd::new_imm(dst_addr - (end_addr - 4))); + adr(cb, Self::EMIT_OPND, A64Opnd::new_imm(dst_addr - (end_addr - 4))); }); - mov(cb, out.into(), Self::EMIT0_OPND); + mov(cb, out.into(), Self::EMIT_OPND); } else { // Set output to the jump target's raw address let target_code = target.unwrap_code_ptr(); @@ -1262,15 +1280,15 @@ impl Assembler { } // Push the flags/state register - mrs(cb, Self::EMIT0_OPND, SystemRegister::NZCV); - emit_push(cb, Self::EMIT0_OPND); + mrs(cb, Self::EMIT_OPND, SystemRegister::NZCV); + emit_push(cb, Self::EMIT_OPND); }, Insn::CPopAll => { let regs = Assembler::get_caller_save_regs(); // Pop the state/flags register - msr(cb, SystemRegister::NZCV, Self::EMIT0_OPND); - emit_pop(cb, Self::EMIT0_OPND); + msr(cb, SystemRegister::NZCV, Self::EMIT_OPND); + emit_pop(cb, Self::EMIT_OPND); for reg in regs.into_iter().rev() { emit_pop(cb, A64Opnd::Reg(reg)); @@ -1286,8 +1304,8 @@ impl Assembler { if b_offset_fits_bits((dst_addr - src_addr) / 4) { bl(cb, InstructionOffset::from_bytes((dst_addr - src_addr) as i32)); } else { - emit_load_value(cb, Self::EMIT0_OPND, dst_addr as u64); - blr(cb, Self::EMIT0_OPND); + emit_load_value(cb, Self::EMIT_OPND, dst_addr as u64); + blr(cb, Self::EMIT_OPND); } }, Insn::CRet { .. } => { @@ -1364,21 +1382,24 @@ impl Assembler { last_patch_pos = Some(cb.get_write_pos()); }, Insn::IncrCounter { mem, value } => { - let label = cb.new_label("incr_counter_loop".to_string()); - cb.write_label(label); + // Get the status register allocated by arm64_split_with_scratch_reg + let Some(Insn::Cmp { + left: status_reg @ Opnd::Reg(_), + right: Opnd::UImm(_) | Opnd::Imm(_), + }) = self.insns.get(insn_idx + 1) else { + panic!("arm64_split_with_scratch_reg should add Cmp after IncrCounter: {:?}", self.insns.get(insn_idx + 1)); + }; - ldaxr(cb, Self::EMIT0_OPND, mem.into()); - add(cb, Self::EMIT0_OPND, Self::EMIT0_OPND, value.into()); + // Attempt to increment a counter + ldaxr(cb, Self::EMIT_OPND, mem.into()); + add(cb, Self::EMIT_OPND, Self::EMIT_OPND, value.into()); // The status register that gets used to track whether or // not the store was successful must be 32 bytes. Since we // store the EMIT registers as their 64-bit versions, we // need to rewrap it here. - let status = A64Opnd::Reg(Self::EMIT1_REG.with_num_bits(32)); - stlxr(cb, status, Self::EMIT0_OPND, mem.into()); - - cmp(cb, Self::EMIT1_OPND, A64Opnd::new_uimm(0)); - emit_conditional_jump::<{Condition::NE}>(cb, Target::Label(label)); + let status = A64Opnd::Reg(status_reg.unwrap_reg().with_num_bits(32)); + stlxr(cb, status, Self::EMIT_OPND, mem.into()); }, Insn::Breakpoint => { brk(cb, A64Opnd::None); @@ -1865,19 +1886,19 @@ mod tests { asm.store(large_mem, large_mem); asm.compile_with_num_regs(&mut cb, 0); - assert_disasm_snapshot!(cb.disasm(), @" - 0x0: sub x16, sp, #0x305 - 0x4: ldur x16, [x16] - 0x8: stur x16, [x0] - 0xc: ldur x16, [x0] - 0x10: sub x17, sp, #0x305 - 0x14: stur x16, [x17] - 0x18: sub x16, sp, #0x305 - 0x1c: ldur x16, [x16] - 0x20: sub x17, sp, #0x305 - 0x24: stur x16, [x17] + assert_disasm_snapshot!(cb.disasm(), @r" + 0x0: sub x16, sp, #0x305 + 0x4: ldur x16, [x16] + 0x8: stur x16, [x0] + 0xc: sub x15, sp, #0x305 + 0x10: ldur x16, [x0] + 0x14: stur x16, [x15] + 0x18: sub x15, sp, #0x305 + 0x1c: sub x16, sp, #0x305 + 0x20: ldur x16, [x16] + 0x24: stur x16, [x15] "); - assert_snapshot!(cb.hexdump(), @"f0170cd1100240f8100000f8100040f8f1170cd1300200f8f0170cd1100240f8f1170cd1300200f8"); + assert_snapshot!(cb.hexdump(), @"f0170cd1100240f8100000f8ef170cd1100040f8f00100f8ef170cd1f0170cd1100240f8f00100f8"); } #[test] From 4925bec65d17b1330d6b39f0c5a3ffe7ed9abde2 Mon Sep 17 00:00:00 2001 From: Peter Zhu Date: Sun, 26 Oct 2025 15:23:05 -0400 Subject: [PATCH 09/12] Fix TestString#test_encode_fallback_raise_memory_leak The method and aref cases need to accept a parameter. --- test/ruby/test_string.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/ruby/test_string.rb b/test/ruby/test_string.rb index 6445832798ec1a..e4c14fd488456d 100644 --- a/test/ruby/test_string.rb +++ b/test/ruby/test_string.rb @@ -3756,12 +3756,12 @@ def test_encode_fallback_raise_memory_leak fallback = proc { raise } RUBY "method" => <<~RUBY, - def my_method = raise + def my_method(_str) = raise fallback = method(:my_method) RUBY "aref" => <<~RUBY, fallback = Object.new - def fallback.[] = raise + def fallback.[](_str) = raise RUBY }.each do |type, code| assert_no_memory_leak([], '', <<~RUBY, "fallback type is #{type}", rss: true) From 9f4a76ff51530e3ff7a9adf0c689f75f1a1cc6d2 Mon Sep 17 00:00:00 2001 From: Peter Zhu Date: Sun, 26 Oct 2025 15:24:44 -0400 Subject: [PATCH 10/12] Add a custom error class to TestString#test_encode_fallback_raise_memory_leak This prevents a generic RuntimeError from being raised so we can ensure that the correct error is being rescued. --- test/ruby/test_string.rb | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/test/ruby/test_string.rb b/test/ruby/test_string.rb index e4c14fd488456d..f6adef6efc16da 100644 --- a/test/ruby/test_string.rb +++ b/test/ruby/test_string.rb @@ -3750,26 +3750,28 @@ def test_chilled_string_substring def test_encode_fallback_raise_memory_leak { "hash" => <<~RUBY, - fallback = Hash.new { raise } + fallback = Hash.new { raise MyError } RUBY "proc" => <<~RUBY, - fallback = proc { raise } + fallback = proc { raise MyError } RUBY "method" => <<~RUBY, - def my_method(_str) = raise + def my_method(_str) = raise MyError fallback = method(:my_method) RUBY "aref" => <<~RUBY, fallback = Object.new - def fallback.[](_str) = raise + def fallback.[](_str) = raise MyError RUBY }.each do |type, code| assert_no_memory_leak([], '', <<~RUBY, "fallback type is #{type}", rss: true) + class MyError < StandardError; end + #{code} 100_000.times do |i| "\\ufffd".encode(Encoding::US_ASCII, fallback:) - rescue + rescue MyError end RUBY end From 80be97e4a2c878d7c5a129b245f1e2430b99b19b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Luiz=20Tiago=20Soares?= Date: Tue, 28 Oct 2025 20:48:38 -0300 Subject: [PATCH 11/12] ZJIT: Fill `cfp->pc` with trap value for C methods in debug builds --- zjit/src/codegen.rs | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/zjit/src/codegen.rs b/zjit/src/codegen.rs index 5ead4870dff517..2ac7ca348ad492 100644 --- a/zjit/src/codegen.rs +++ b/zjit/src/codegen.rs @@ -25,6 +25,13 @@ use crate::hir_type::{types, Type}; use crate::options::get_option; use crate::cast::IntoUsize; +/// Sentinel program counter stored in C frames when runtime checks are enabled. +const PC_POISON: Option<*const VALUE> = if cfg!(feature = "runtime_checks") { + Some(usize::MAX as *const VALUE) +} else { + None +}; + /// Ephemeral code generation state struct JITState { /// Instruction sequence for the method being compiled @@ -741,6 +748,7 @@ fn gen_ccall_with_frame( iseq: None, cme, frame_type: VM_FRAME_MAGIC_CFUNC | VM_FRAME_FLAG_CFRAME | VM_ENV_FLAG_LOCAL, + pc: PC_POISON, specval: block_handler_specval, }); @@ -798,6 +806,7 @@ fn gen_ccall_variadic( cme, frame_type: VM_FRAME_MAGIC_CFUNC | VM_FRAME_FLAG_CFRAME | VM_ENV_FLAG_LOCAL, specval: VM_BLOCK_HANDLER_NONE.into(), + pc: PC_POISON, }); asm_comment!(asm, "switch to new SP register"); @@ -1205,6 +1214,7 @@ fn gen_send_without_block_direct( iseq: Some(iseq), cme, frame_type, + pc: None, specval, }); @@ -1828,6 +1838,7 @@ struct ControlFrame { /// The [`VM_ENV_DATA_INDEX_SPECVAL`] slot of the frame. /// For the type of frames we push, block handler or the parent EP. specval: lir::Opnd, + pc: Option<*const VALUE>, } /// Compile an interpreter frame @@ -1862,8 +1873,11 @@ fn gen_push_frame(asm: &mut Assembler, argc: usize, state: &FrameState, frame: C // cfp_opnd(RUBY_OFFSET_CFP_SP): written by the callee frame on side-exits or non-leaf calls asm.mov(cfp_opnd(RUBY_OFFSET_CFP_ISEQ), VALUE::from(iseq).into()); } else { - // C frames don't have a PC and ISEQ - asm.mov(cfp_opnd(RUBY_OFFSET_CFP_PC), 0.into()); + // C frames don't have a PC and ISEQ in normal operation. + // When runtime checks are enabled we poison the PC so accidental reads stand out. + if let Some(pc) = frame.pc { + asm.mov(cfp_opnd(RUBY_OFFSET_CFP_PC), Opnd::const_ptr(pc)); + } let new_sp = asm.lea(Opnd::mem(64, SP, (ep_offset + 1) * SIZEOF_VALUE_I32)); asm.mov(cfp_opnd(RUBY_OFFSET_CFP_SP), new_sp); asm.mov(cfp_opnd(RUBY_OFFSET_CFP_ISEQ), 0.into()); From f8a333ae193017999b38f6a4838582cc2c333063 Mon Sep 17 00:00:00 2001 From: Max Bernstein Date: Tue, 28 Oct 2025 20:11:27 -0400 Subject: [PATCH 12/12] ZJIT: Add type checker to HIR (#14978) Allow instructions to constrain their operands' input types to avoid accidentally creating invalid HIR. --- zjit/src/hir.rs | 162 ++++++++++++++++++++++++++++++++++++++++++++++ zjit/src/stats.rs | 4 ++ 2 files changed, 166 insertions(+) diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index 3f68764722b448..b284ae6c118d86 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -1356,6 +1356,9 @@ pub enum ValidationError { OperandNotDefined(BlockId, InsnId, InsnId), /// The offending block and instruction DuplicateInstruction(BlockId, InsnId), + /// The offending instruction, its operand, expected type string, actual type string + MismatchedOperandType(InsnId, InsnId, String, String), + MiscValidationError(InsnId, String), } fn can_direct_send(iseq: *const rb_iseq_t) -> bool { @@ -3565,11 +3568,170 @@ impl Function { Ok(()) } + fn assert_subtype(&self, user: InsnId, operand: InsnId, expected: Type) -> Result<(), ValidationError> { + let actual = self.type_of(operand); + if !actual.is_subtype(expected) { + return Err(ValidationError::MismatchedOperandType(user, operand, format!("{}", expected), format!("{}", actual))); + } + Ok(()) + } + + fn validate_insn_type(&self, insn_id: InsnId) -> Result<(), ValidationError> { + let insn_id = self.union_find.borrow().find_const(insn_id); + let insn = self.find(insn_id); + match insn { + Insn::StringCopy { val, .. } => self.assert_subtype(insn_id, val, types::StringExact), + Insn::StringIntern { val, .. } => self.assert_subtype(insn_id, val, types::StringExact), + Insn::ArrayDup { val, .. } => self.assert_subtype(insn_id, val, types::ArrayExact), + Insn::StringAppend { recv, other, .. } => { + self.assert_subtype(insn_id, recv, types::StringExact)?; + self.assert_subtype(insn_id, other, types::String) + } + Insn::NewHash { ref elements, .. } => { + if elements.len() % 2 != 0 { + return Err(ValidationError::MiscValidationError(insn_id, "NewHash elements length is not even".to_string())); + } + Ok(()) + } + Insn::NewRangeFixnum { low, high, .. } => { + self.assert_subtype(insn_id, low, types::Fixnum)?; + self.assert_subtype(insn_id, high, types::Fixnum) + } + Insn::ArrayExtend { left, right, .. } => { + // TODO(max): Do left and right need to be ArrayExact? + self.assert_subtype(insn_id, left, types::Array)?; + self.assert_subtype(insn_id, right, types::Array) + } + Insn::ArrayPush { array, .. } => self.assert_subtype(insn_id, array, types::Array), + Insn::ArrayPop { array, .. } => self.assert_subtype(insn_id, array, types::Array), + Insn::ArrayLength { array, .. } => self.assert_subtype(insn_id, array, types::Array), + Insn::HashAref { hash, .. } => self.assert_subtype(insn_id, hash, types::Hash), + Insn::HashDup { val, .. } => self.assert_subtype(insn_id, val, types::HashExact), + Insn::ObjectAllocClass { class, .. } => { + let has_leaf_allocator = unsafe { rb_zjit_class_has_default_allocator(class) } || class_has_leaf_allocator(class); + if !has_leaf_allocator { + return Err(ValidationError::MiscValidationError(insn_id, "ObjectAllocClass must have leaf allocator".to_string())); + } + Ok(()) + } + Insn::Test { val } => self.assert_subtype(insn_id, val, types::BasicObject), + Insn::IsNil { val } => self.assert_subtype(insn_id, val, types::BasicObject), + Insn::IsMethodCfunc { val, .. } => self.assert_subtype(insn_id, val, types::BasicObject), + Insn::IsBitEqual { left, right } + | Insn::IsBitNotEqual { left, right } => { + if self.is_a(left, types::CInt) && self.is_a(right, types::CInt) { + // TODO(max): Check that int sizes match + Ok(()) + } else if self.is_a(left, types::CPtr) && self.is_a(right, types::CPtr) { + Ok(()) + } else if self.is_a(left, types::RubyValue) && self.is_a(right, types::RubyValue) { + Ok(()) + } else { + return Err(ValidationError::MiscValidationError(insn_id, "IsBitEqual can only compare CInt/CInt or RubyValue/RubyValue".to_string())); + } + } + Insn::BoxBool { val } => self.assert_subtype(insn_id, val, types::CBool), + Insn::SetGlobal { val, .. } => self.assert_subtype(insn_id, val, types::BasicObject), + Insn::GetIvar { self_val, .. } => self.assert_subtype(insn_id, self_val, types::BasicObject), + Insn::SetIvar { self_val, val, .. } => { + self.assert_subtype(insn_id, self_val, types::BasicObject)?; + self.assert_subtype(insn_id, val, types::BasicObject) + } + Insn::DefinedIvar { self_val, .. } => self.assert_subtype(insn_id, self_val, types::BasicObject), + Insn::LoadIvarEmbedded { self_val, .. } => self.assert_subtype(insn_id, self_val, types::HeapBasicObject), + Insn::LoadIvarExtended { self_val, .. } => self.assert_subtype(insn_id, self_val, types::HeapBasicObject), + Insn::SetLocal { val, .. } => self.assert_subtype(insn_id, val, types::BasicObject), + Insn::SetClassVar { val, .. } => self.assert_subtype(insn_id, val, types::BasicObject), + Insn::IfTrue { val, .. } | Insn::IfFalse { val, .. } => self.assert_subtype(insn_id, val, types::CBool), + Insn::SendWithoutBlock { recv, ref args, .. } + | Insn::SendWithoutBlockDirect { recv, ref args, .. } + | Insn::Send { recv, ref args, .. } + | Insn::SendForward { recv, ref args, .. } + | Insn::InvokeSuper { recv, ref args, .. } + | Insn::CCallVariadic { recv, ref args, .. } => { + self.assert_subtype(insn_id, recv, types::BasicObject)?; + for &arg in args { + self.assert_subtype(insn_id, arg, types::BasicObject)?; + } + Ok(()) + } + Insn::CCallWithFrame { ref args, .. } + | Insn::InvokeBuiltin { ref args, .. } + | Insn::InvokeBlock { ref args, .. } => { + for &arg in args { + self.assert_subtype(insn_id, arg, types::BasicObject)?; + } + Ok(()) + } + Insn::Return { val } => self.assert_subtype(insn_id, val, types::BasicObject), + Insn::Throw { val, .. } => self.assert_subtype(insn_id, val, types::BasicObject), + Insn::FixnumAdd { left, right, .. } + | Insn::FixnumSub { left, right, .. } + | Insn::FixnumMult { left, right, .. } + | Insn::FixnumDiv { left, right, .. } + | Insn::FixnumMod { left, right, .. } + | Insn::FixnumEq { left, right } + | Insn::FixnumNeq { left, right } + | Insn::FixnumLt { left, right } + | Insn::FixnumLe { left, right } + | Insn::FixnumGt { left, right } + | Insn::FixnumGe { left, right } + | Insn::FixnumAnd { left, right } + | Insn::FixnumOr { left, right } + | Insn::FixnumXor { left, right } + => { + self.assert_subtype(insn_id, left, types::Fixnum)?; + self.assert_subtype(insn_id, right, types::Fixnum) + } + Insn::ObjToString { val, .. } => self.assert_subtype(insn_id, val, types::BasicObject), + Insn::AnyToString { val, str, .. } => { + self.assert_subtype(insn_id, val, types::BasicObject)?; + self.assert_subtype(insn_id, str, types::BasicObject) + } + Insn::GuardType { val, .. } => self.assert_subtype(insn_id, val, types::BasicObject), + Insn::GuardTypeNot { val, .. } => self.assert_subtype(insn_id, val, types::BasicObject), + Insn::GuardBitEquals { val, expected, .. } => { + match expected { + Const::Value(_) => self.assert_subtype(insn_id, val, types::RubyValue), + Const::CInt8(_) => self.assert_subtype(insn_id, val, types::CInt8), + Const::CInt16(_) => self.assert_subtype(insn_id, val, types::CInt16), + Const::CInt32(_) => self.assert_subtype(insn_id, val, types::CInt32), + Const::CInt64(_) => self.assert_subtype(insn_id, val, types::CInt64), + Const::CUInt8(_) => self.assert_subtype(insn_id, val, types::CUInt8), + Const::CUInt16(_) => self.assert_subtype(insn_id, val, types::CUInt16), + Const::CUInt32(_) => self.assert_subtype(insn_id, val, types::CUInt32), + Const::CUInt64(_) => self.assert_subtype(insn_id, val, types::CUInt64), + Const::CBool(_) => self.assert_subtype(insn_id, val, types::CBool), + Const::CDouble(_) => self.assert_subtype(insn_id, val, types::CDouble), + Const::CPtr(_) => self.assert_subtype(insn_id, val, types::CPtr), + } + } + Insn::GuardShape { val, .. } => self.assert_subtype(insn_id, val, types::BasicObject), + Insn::GuardNotFrozen { val, .. } => self.assert_subtype(insn_id, val, types::BasicObject), + Insn::StringGetbyteFixnum { string, index } => { + self.assert_subtype(insn_id, string, types::String)?; + self.assert_subtype(insn_id, index, types::Fixnum) + } + _ => Ok(()), + } + } + + /// Check that insn types match the expected types for each instruction. + fn validate_types(&self) -> Result<(), ValidationError> { + for block_id in self.rpo() { + for &insn_id in &self.blocks[block_id.0].insns { + self.validate_insn_type(insn_id)?; + } + } + Ok(()) + } + /// Run all validation passes we have. pub fn validate(&self) -> Result<(), ValidationError> { self.validate_block_terminators_and_jumps()?; self.validate_definite_assignment()?; self.validate_insn_uniqueness()?; + self.validate_types()?; Ok(()) } } diff --git a/zjit/src/stats.rs b/zjit/src/stats.rs index 4874d0fe64146b..9965526b7607cb 100644 --- a/zjit/src/stats.rs +++ b/zjit/src/stats.rs @@ -202,6 +202,8 @@ make_counters! { compile_error_validation_jump_target_not_in_rpo, compile_error_validation_operand_not_defined, compile_error_validation_duplicate_instruction, + compile_error_validation_type_check_failure, + compile_error_validation_misc_validation_error, // The number of times YARV instructions are executed on JIT code zjit_insn_count, @@ -320,6 +322,8 @@ pub fn exit_counter_for_compile_error(compile_error: &CompileError) -> Counter { JumpTargetNotInRPO(_) => compile_error_validation_jump_target_not_in_rpo, OperandNotDefined(_, _, _) => compile_error_validation_operand_not_defined, DuplicateInstruction(_, _) => compile_error_validation_duplicate_instruction, + MismatchedOperandType(..) => compile_error_validation_type_check_failure, + MiscValidationError(..) => compile_error_validation_misc_validation_error, }, } }