diff --git a/.github/workflows/zjit-ubuntu.yml b/.github/workflows/zjit-ubuntu.yml index 4b0e0dbd5caf21..ea158262ec940a 100644 --- a/.github/workflows/zjit-ubuntu.yml +++ b/.github/workflows/zjit-ubuntu.yml @@ -27,6 +27,25 @@ permissions: contents: read jobs: + lint: + name: cargo clippy + + runs-on: ubuntu-22.04 + + if: >- + ${{!(false + || contains(github.event.head_commit.message, '[DOC]') + || contains(github.event.pull_request.title, '[DOC]') + || contains(github.event.pull_request.labels.*.name, 'Documentation') + || (github.event_name == 'push' && github.event.pull_request.user.login == 'dependabot[bot]') + )}} + + steps: + - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 + + - run: cargo clippy --all-targets --all-features + working-directory: zjit + make: strategy: fail-fast: false diff --git a/doc/zjit.md b/doc/zjit.md index ef15c6dac471ac..04ca0a8bb45de1 100644 --- a/doc/zjit.md +++ b/doc/zjit.md @@ -24,6 +24,10 @@ To run code snippets with ZJIT: You can also try https://www.rubyexplorer.xyz/ to view Ruby YARV disasm output with syntax highlighting in a way that can be easily shared with other team members. +## Documentation + +You can generate and open the source level documentation in your browser using `cargo doc --open --document-private-items`. + ## Testing Make sure you have a `--enable-zjit=dev` build, and install the following tools: diff --git a/test/ruby/test_gc.rb b/test/ruby/test_gc.rb index d88b4f07f6de19..1c4561ed5a45ec 100644 --- a/test/ruby/test_gc.rb +++ b/test/ruby/test_gc.rb @@ -742,12 +742,13 @@ def test_exception_in_finalizer end def test_interrupt_in_finalizer + omit 'randomly hangs on many platforms' if ENV.key?('GITHUB_ACTIONS') bug10595 = '[ruby-core:66825] [Bug #10595]' src = <<-'end;' Signal.trap(:INT, 'DEFAULT') pid = $$ Thread.start do - 1000.times { + 10.times { sleep 0.1 Process.kill("INT", pid) rescue break } diff --git a/vm.c b/vm.c index 60c26cf0188d8a..b947f9aeaebb58 100644 --- a/vm.c +++ b/vm.c @@ -445,7 +445,7 @@ jit_compile(rb_execution_context_t *ec) // At call-threshold, compile the ISEQ with ZJIT. if (body->jit_entry_calls == rb_zjit_call_threshold) { - rb_zjit_compile_iseq(iseq, ec, false); + rb_zjit_compile_iseq(iseq, false); } } #endif @@ -493,8 +493,25 @@ jit_compile_exception(rb_execution_context_t *ec) const rb_iseq_t *iseq = ec->cfp->iseq; struct rb_iseq_constant_body *body = ISEQ_BODY(iseq); - // Increment the ISEQ's call counter and trigger JIT compilation if not compiled +#if USE_ZJIT + if (body->jit_exception == NULL && rb_zjit_enabled_p) { + body->jit_exception_calls++; + + // At profile-threshold, rewrite some of the YARV instructions + // to zjit_* instructions to profile these instructions. + if (body->jit_exception_calls == rb_zjit_profile_threshold) { + rb_zjit_profile_enable(iseq); + } + + // At call-threshold, compile the ISEQ with ZJIT. + if (body->jit_exception_calls == rb_zjit_call_threshold) { + rb_zjit_compile_iseq(iseq, true); + } + } +#endif + #if USE_YJIT + // Increment the ISEQ's call counter and trigger JIT compilation if not compiled if (body->jit_exception == NULL && rb_yjit_enabled_p) { body->jit_exception_calls++; if (body->jit_exception_calls == rb_yjit_call_threshold) { diff --git a/yjit.c b/yjit.c index b38f860ed5e627..ac218a084ca33f 100644 --- a/yjit.c +++ b/yjit.c @@ -655,7 +655,7 @@ rb_yjit_compile_iseq(const rb_iseq_t *iseq, rb_execution_context_t *ec, bool jit else { iseq->body->jit_entry = (rb_jit_func_t)code_ptr; } -} + } } // GC root for interacting with the GC diff --git a/zjit.c b/zjit.c index 7b4952a93eb810..de5d859ba1af43 100644 --- a/zjit.c +++ b/zjit.c @@ -159,18 +159,22 @@ rb_zjit_reserve_addr_space(uint32_t mem_size) void rb_zjit_profile_disable(const rb_iseq_t *iseq); void -rb_zjit_compile_iseq(const rb_iseq_t *iseq, rb_execution_context_t *ec, bool jit_exception) +rb_zjit_compile_iseq(const rb_iseq_t *iseq, bool jit_exception) { RB_VM_LOCKING() { rb_vm_barrier(); // Compile a block version starting at the current instruction - uint8_t *rb_zjit_iseq_gen_entry_point(const rb_iseq_t *iseq, rb_execution_context_t *ec); // defined in Rust - uintptr_t code_ptr = (uintptr_t)rb_zjit_iseq_gen_entry_point(iseq, ec); + uint8_t *rb_zjit_iseq_gen_entry_point(const rb_iseq_t *iseq, bool jit_exception); // defined in Rust + uintptr_t code_ptr = (uintptr_t)rb_zjit_iseq_gen_entry_point(iseq, jit_exception); - // TODO: support jit_exception - iseq->body->jit_entry = (rb_jit_func_t)code_ptr; -} + if (jit_exception) { + iseq->body->jit_exception = (rb_jit_func_t)code_ptr; + } + else { + iseq->body->jit_entry = (rb_jit_func_t)code_ptr; + } + } } extern VALUE *rb_vm_base_ptr(struct rb_control_frame_struct *cfp); diff --git a/zjit.h b/zjit.h index 45e91fa43c77ca..27a7b8a0013da5 100644 --- a/zjit.h +++ b/zjit.h @@ -13,7 +13,7 @@ extern bool rb_zjit_enabled_p; extern uint64_t rb_zjit_call_threshold; extern uint64_t rb_zjit_profile_threshold; -void rb_zjit_compile_iseq(const rb_iseq_t *iseq, rb_execution_context_t *ec, bool jit_exception); +void rb_zjit_compile_iseq(const rb_iseq_t *iseq, bool jit_exception); void rb_zjit_profile_insn(uint32_t insn, rb_execution_context_t *ec); void rb_zjit_profile_enable(const rb_iseq_t *iseq); void rb_zjit_bop_redefined(int redefined_flag, enum ruby_basic_operators bop); @@ -26,7 +26,7 @@ void rb_zjit_before_ractor_spawn(void); void rb_zjit_tracing_invalidate_all(void); #else #define rb_zjit_enabled_p false -static inline void rb_zjit_compile_iseq(const rb_iseq_t *iseq, rb_execution_context_t *ec, bool jit_exception) {} +static inline void rb_zjit_compile_iseq(const rb_iseq_t *iseq, bool jit_exception) {} static inline void rb_zjit_profile_insn(uint32_t insn, rb_execution_context_t *ec) {} static inline void rb_zjit_profile_enable(const rb_iseq_t *iseq) {} static inline void rb_zjit_bop_redefined(int redefined_flag, enum ruby_basic_operators bop) {} diff --git a/zjit/src/asm/arm64/inst/branch.rs b/zjit/src/asm/arm64/inst/branch.rs index 14fcb2e9fd99f5..2db52e5d316410 100644 --- a/zjit/src/asm/arm64/inst/branch.rs +++ b/zjit/src/asm/arm64/inst/branch.rs @@ -89,7 +89,7 @@ mod tests { #[test] fn test_ret() { let result: u32 = Branch::ret(30).into(); - assert_eq!(0xd65f03C0, result); + assert_eq!(0xd65f03c0, result); } #[test] diff --git a/zjit/src/asm/arm64/inst/load_literal.rs b/zjit/src/asm/arm64/inst/load_literal.rs index 817e8935531b2b..a429c0fb53d082 100644 --- a/zjit/src/asm/arm64/inst/load_literal.rs +++ b/zjit/src/asm/arm64/inst/load_literal.rs @@ -1,3 +1,5 @@ +#![allow(clippy::identity_op)] + use super::super::arg::{InstructionOffset, truncate_imm}; /// The size of the operands being operated on. diff --git a/zjit/src/asm/arm64/inst/mov.rs b/zjit/src/asm/arm64/inst/mov.rs index 58877ae94040c7..70814938056f15 100644 --- a/zjit/src/asm/arm64/inst/mov.rs +++ b/zjit/src/asm/arm64/inst/mov.rs @@ -145,14 +145,14 @@ mod tests { fn test_movk_shifted_16() { let inst = Mov::movk(0, 123, 16, 64); let result: u32 = inst.into(); - assert_eq!(0xf2A00f60, result); + assert_eq!(0xf2a00f60, result); } #[test] fn test_movk_shifted_32() { let inst = Mov::movk(0, 123, 32, 64); let result: u32 = inst.into(); - assert_eq!(0xf2C00f60, result); + assert_eq!(0xf2c00f60, result); } #[test] diff --git a/zjit/src/asm/arm64/mod.rs b/zjit/src/asm/arm64/mod.rs index 0576b230907ff5..63ac7823f18d98 100644 --- a/zjit/src/asm/arm64/mod.rs +++ b/zjit/src/asm/arm64/mod.rs @@ -1,4 +1,8 @@ #![allow(dead_code)] // For instructions and operands we're not currently using. +#![allow(clippy::upper_case_acronyms)] +#![allow(clippy::identity_op)] +#![allow(clippy::self_named_constructors)] +#![allow(clippy::unusual_byte_groupings)] use crate::asm::CodeBlock; @@ -1590,7 +1594,7 @@ mod tests { #[test] fn test_nop() { - check_bytes("1f2003d5", |cb| nop(cb)); + check_bytes("1f2003d5", nop); } #[test] diff --git a/zjit/src/asm/arm64/opnd.rs b/zjit/src/asm/arm64/opnd.rs index a77958f7e6eeec..8246bea08e69ee 100644 --- a/zjit/src/asm/arm64/opnd.rs +++ b/zjit/src/asm/arm64/opnd.rs @@ -79,10 +79,7 @@ impl A64Opnd { /// Convenience function to check if this operand is a register. pub fn is_reg(&self) -> bool { - match self { - A64Opnd::Reg(_) => true, - _ => false - } + matches!(self, A64Opnd::Reg(_)) } /// Unwrap a register from an operand. diff --git a/zjit/src/asm/mod.rs b/zjit/src/asm/mod.rs index 08d3571c2d6b6a..8efa5df2701247 100644 --- a/zjit/src/asm/mod.rs +++ b/zjit/src/asm/mod.rs @@ -1,3 +1,5 @@ +//! Model for creating generating textual assembler code. + use std::collections::BTreeMap; use std::fmt; use std::ops::Range; diff --git a/zjit/src/asm/x86_64/mod.rs b/zjit/src/asm/x86_64/mod.rs index f1eaa49f204ac2..854438ad40d231 100644 --- a/zjit/src/asm/x86_64/mod.rs +++ b/zjit/src/asm/x86_64/mod.rs @@ -163,10 +163,7 @@ impl X86Opnd { } pub fn is_some(&self) -> bool { - match self { - X86Opnd::None => false, - _ => true - } + !matches!(self, X86Opnd::None) } } @@ -284,11 +281,11 @@ pub fn mem_opnd(num_bits: u8, base_reg: X86Opnd, disp: i32) -> X86Opnd } else { X86Opnd::Mem( X86Mem { - num_bits: num_bits, + num_bits, base_reg_no: base_reg.reg_no, idx_reg_no: None, scale_exp: 0, - disp: disp, + disp, } ) } diff --git a/zjit/src/asm/x86_64/tests.rs b/zjit/src/asm/x86_64/tests.rs index ec490fd3301333..0268846e10c883 100644 --- a/zjit/src/asm/x86_64/tests.rs +++ b/zjit/src/asm/x86_64/tests.rs @@ -33,7 +33,7 @@ fn test_add() { fn test_add_unsigned() { // ADD r/m8, imm8 check_bytes("4180c001", |cb| add(cb, R8B, uimm_opnd(1))); - check_bytes("4180c07f", |cb| add(cb, R8B, imm_opnd(i8::MAX.try_into().unwrap()))); + check_bytes("4180c07f", |cb| add(cb, R8B, imm_opnd(i8::MAX.into()))); // ADD r/m16, imm16 check_bytes("664183c001", |cb| add(cb, R8W, uimm_opnd(1))); @@ -102,7 +102,7 @@ fn test_cmp() { #[test] fn test_cqo() { - check_bytes("4899", |cb| cqo(cb)); + check_bytes("4899", cqo); } #[test] @@ -341,7 +341,7 @@ fn test_push() { #[test] fn test_ret() { - check_bytes("c3", |cb| ret(cb)); + check_bytes("c3", ret); } #[test] diff --git a/zjit/src/backend/arm64/mod.rs b/zjit/src/backend/arm64/mod.rs index ef5a4aadd6bc22..9a23c2bd132dbf 100644 --- a/zjit/src/backend/arm64/mod.rs +++ b/zjit/src/backend/arm64/mod.rs @@ -183,7 +183,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. /// SCRATCH0 and SCRATCH1 are excluded. -pub const ALLOC_REGS: &'static [Reg] = &[ +pub const ALLOC_REGS: &[Reg] = &[ X0_REG, X1_REG, X2_REG, @@ -386,15 +386,12 @@ impl Assembler let mut opnd_iter = insn.opnd_iter_mut(); while let Some(opnd) = opnd_iter.next() { - match opnd { - Opnd::Value(value) => { - if value.special_const_p() { - *opnd = Opnd::UImm(value.as_u64()); - } else if !is_load { - *opnd = asm.load(*opnd); - } - }, - _ => {} + if let Opnd::Value(value) = opnd { + if value.special_const_p() { + *opnd = Opnd::UImm(value.as_u64()); + } else if !is_load { + *opnd = asm.load(*opnd); + } }; } @@ -481,7 +478,7 @@ impl Assembler // which is both the return value and first argument register if !opnds.is_empty() { let mut args: Vec<(Reg, Opnd)> = vec![]; - for (idx, opnd) in opnds.into_iter().enumerate().rev() { + 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 // a UImm of 0 along as the argument to the move. @@ -1411,7 +1408,7 @@ impl Assembler /// /// If a, b, and c are all registers. fn merge_three_reg_mov( - live_ranges: &Vec, + live_ranges: &[LiveRange], iterator: &mut std::iter::Peekable>, left: &Opnd, right: &Opnd, @@ -1566,7 +1563,7 @@ mod tests { #[test] fn frame_setup_and_teardown() { - const THREE_REGS: &'static [Opnd] = &[Opnd::Reg(X19_REG), Opnd::Reg(X20_REG), Opnd::Reg(X21_REG)]; + const THREE_REGS: &[Opnd] = &[Opnd::Reg(X19_REG), Opnd::Reg(X20_REG), Opnd::Reg(X21_REG)]; // Test 3 preserved regs (odd), odd slot_count { let (mut asm, mut cb) = setup_asm(); @@ -1607,7 +1604,7 @@ mod tests { // Test 4 preserved regs (even), odd slot_count { - static FOUR_REGS: &'static [Opnd] = &[Opnd::Reg(X19_REG), Opnd::Reg(X20_REG), Opnd::Reg(X21_REG), Opnd::Reg(X22_REG)]; + static FOUR_REGS: &[Opnd] = &[Opnd::Reg(X19_REG), Opnd::Reg(X20_REG), Opnd::Reg(X21_REG), Opnd::Reg(X22_REG)]; let (mut asm, mut cb) = setup_asm(); asm.frame_setup(FOUR_REGS, 3); asm.frame_teardown(FOUR_REGS); diff --git a/zjit/src/backend/lir.rs b/zjit/src/backend/lir.rs index 03cd5253bc2729..c67bf8c9ba01e8 100644 --- a/zjit/src/backend/lir.rs +++ b/zjit/src/backend/lir.rs @@ -18,7 +18,7 @@ pub use crate::backend::current::{ }; pub const SCRATCH_OPND: Opnd = Opnd::Reg(Assembler::SCRATCH_REG); -pub static JIT_PRESERVED_REGS: &'static [Opnd] = &[CFP, SP, EC]; +pub static JIT_PRESERVED_REGS: &[Opnd] = &[CFP, SP, EC]; // Memory operand base #[derive(Clone, Copy, PartialEq, Eq, Debug)] @@ -99,8 +99,8 @@ impl Opnd assert!(base_reg.num_bits == 64); Opnd::Mem(Mem { base: MemBase::Reg(base_reg.reg_no), - disp: disp, - num_bits: num_bits, + disp, + num_bits, }) }, @@ -108,8 +108,8 @@ impl Opnd assert!(num_bits <= out_num_bits); Opnd::Mem(Mem { base: MemBase::VReg(idx), - disp: disp, - num_bits: num_bits, + disp, + num_bits, }) }, @@ -1215,8 +1215,8 @@ impl Assembler } // If we find any VReg from previous instructions, extend the live range to insn_idx - let mut opnd_iter = insn.opnd_iter(); - while let Some(opnd) = opnd_iter.next() { + let opnd_iter = insn.opnd_iter(); + for opnd in opnd_iter { match *opnd { Opnd::VReg { idx, .. } | Opnd::Mem(Mem { base: MemBase::VReg(idx), .. }) => { @@ -1243,16 +1243,16 @@ 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: &Vec<(Reg, Opnd)>) -> Vec<(Reg, Opnd)> { + pub fn resolve_parallel_moves(old_moves: &[(Reg, Opnd)]) -> Vec<(Reg, Opnd)> { // Return the index of a move whose destination is not used as a source if any. - fn find_safe_move(moves: &Vec<(Reg, Opnd)>) -> Option { + 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)) }).map(|(index, _)| index) } // Remove moves whose source and destination are the same - let mut old_moves: Vec<(Reg, Opnd)> = old_moves.clone().into_iter() + let mut old_moves: Vec<(Reg, Opnd)> = old_moves.iter().copied() .filter(|&(reg, opnd)| Opnd::Reg(reg) != opnd).collect(); let mut new_moves = vec![]; @@ -1386,19 +1386,19 @@ impl Assembler Some(Opnd::VReg { idx, .. }) => Some(*idx), _ => None, }; - if vreg_idx.is_some() { - if live_ranges[vreg_idx.unwrap()].end() == index { - debug!("Allocating a register for VReg({}) at instruction index {} even though it does not live past this index", vreg_idx.unwrap(), index); + if let Some(vreg_idx) = vreg_idx { + if live_ranges[vreg_idx].end() == index { + debug!("Allocating a register for VReg({}) at instruction index {} even though it does not live past this index", vreg_idx, index); } // This is going to be the output operand that we will set on the // instruction. CCall and LiveReg need to use a specific register. let mut out_reg = match insn { Insn::CCall { .. } => { - Some(pool.take_reg(&C_RET_REG, vreg_idx.unwrap())) + Some(pool.take_reg(&C_RET_REG, vreg_idx)) } Insn::LiveReg { opnd, .. } => { let reg = opnd.unwrap_reg(); - Some(pool.take_reg(®, vreg_idx.unwrap())) + Some(pool.take_reg(®, vreg_idx)) } _ => None }; @@ -1414,7 +1414,7 @@ impl Assembler if let Some(Opnd::VReg{ idx, .. }) = opnd_iter.next() { if live_ranges[*idx].end() == index { if let Some(reg) = reg_mapping[*idx] { - out_reg = Some(pool.take_reg(®, vreg_idx.unwrap())); + out_reg = Some(pool.take_reg(®, vreg_idx)); } } } @@ -1423,21 +1423,19 @@ impl Assembler // Allocate a new register for this instruction if one is not // already allocated. if out_reg.is_none() { - out_reg = match &insn { - _ => match pool.alloc_reg(vreg_idx.unwrap()) { - Some(reg) => Some(reg), - None => { - if get_option!(debug) { - let mut insns = asm.insns; + out_reg = match pool.alloc_reg(vreg_idx) { + Some(reg) => Some(reg), + None => { + if get_option!(debug) { + let mut insns = asm.insns; + insns.push(insn); + for (_, insn) in iterator.by_ref() { insns.push(insn); - while let Some((_, insn)) = iterator.next() { - insns.push(insn); - } - dump_live_regs(insns, live_ranges, regs.len(), index); } - debug!("Register spill not supported"); - return Err(CompileError::RegisterSpillOnAlloc); + dump_live_regs(insns, live_ranges, regs.len(), index); } + debug!("Register spill not supported"); + return Err(CompileError::RegisterSpillOnAlloc); } }; } @@ -1510,7 +1508,7 @@ impl Assembler if is_ccall { // On x86_64, maintain 16-byte stack alignment if cfg!(target_arch = "x86_64") && saved_regs.len() % 2 == 1 { - asm.cpop_into(Opnd::Reg(saved_regs.last().unwrap().0.clone())); + asm.cpop_into(Opnd::Reg(saved_regs.last().unwrap().0)); } // Restore saved registers for &(reg, vreg_idx) in saved_regs.iter().rev() { @@ -1527,7 +1525,6 @@ impl Assembler /// Compile the instructions down to machine code. /// Can fail due to lack of code memory and inopportune code placement, among other reasons. - #[must_use] pub fn compile(self, cb: &mut CodeBlock) -> Result<(CodePtr, Vec), CompileError> { #[cfg(feature = "disasm")] let start_addr = cb.get_write_ptr(); @@ -2038,7 +2035,7 @@ mod tests { assert!(matches!(opnd_iter.next(), Some(Opnd::None))); assert!(matches!(opnd_iter.next(), Some(Opnd::None))); - assert!(matches!(opnd_iter.next(), None)); + assert!(opnd_iter.next().is_none()); } #[test] @@ -2049,7 +2046,7 @@ mod tests { assert!(matches!(opnd_iter.next(), Some(Opnd::None))); assert!(matches!(opnd_iter.next(), Some(Opnd::None))); - assert!(matches!(opnd_iter.next(), None)); + assert!(opnd_iter.next().is_none()); } #[test] diff --git a/zjit/src/backend/mod.rs b/zjit/src/backend/mod.rs index 4922421f18f6dd..ee861f5bd3562a 100644 --- a/zjit/src/backend/mod.rs +++ b/zjit/src/backend/mod.rs @@ -1,3 +1,5 @@ +//! A multi-platform assembler generation backend. + #[cfg(target_arch = "x86_64")] pub mod x86_64; diff --git a/zjit/src/backend/x86_64/mod.rs b/zjit/src/backend/x86_64/mod.rs index 5517384d5f30f5..b18510c29a2972 100644 --- a/zjit/src/backend/x86_64/mod.rs +++ b/zjit/src/backend/x86_64/mod.rs @@ -84,7 +84,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_REG is excluded. -pub const ALLOC_REGS: &'static [Reg] = &[ +pub const ALLOC_REGS: &[Reg] = &[ RDI_REG, RSI_REG, RDX_REG, @@ -155,7 +155,7 @@ impl Assembler let vreg_outlives_insn = |vreg_idx| { live_ranges .get(vreg_idx) - .map_or(false, |live_range: &LiveRange| live_range.end() > index) + .is_some_and(|live_range: &LiveRange| live_range.end() > index) }; // We are replacing instructions here so we know they are already @@ -343,7 +343,7 @@ impl Assembler // Load each operand into the corresponding argument register. if !opnds.is_empty() { let mut args: Vec<(Reg, Opnd)> = vec![]; - for (idx, opnd) in opnds.into_iter().enumerate() { + for (idx, opnd) in opnds.iter_mut().enumerate() { args.push((C_ARG_OPNDS[idx].unwrap_reg(), *opnd)); } asm.parallel_mov(args); @@ -877,18 +877,18 @@ impl Assembler // Error if we couldn't write out everything if cb.has_dropped_bytes() { - return None + None } else { // No bytes dropped, so the pos markers point to valid code for (insn_idx, pos) in pos_markers { if let Insn::PosMarker(callback) = self.insns.get(insn_idx).unwrap() { - callback(pos, &cb); + callback(pos, cb); } else { panic!("non-PosMarker in pos_markers insn_idx={insn_idx} {self:?}"); } } - return Some(gc_offsets) + Some(gc_offsets) } } diff --git a/zjit/src/bitset.rs b/zjit/src/bitset.rs index 895bac8e33232a..b5b69abdeeb62d 100644 --- a/zjit/src/bitset.rs +++ b/zjit/src/bitset.rs @@ -1,3 +1,5 @@ +//! Optimized bitset implementation. + type Entry = u128; const ENTRY_NUM_BITS: usize = Entry::BITS as usize; @@ -65,40 +67,40 @@ mod tests { #[should_panic] fn get_over_capacity_panics() { let set = BitSet::with_capacity(0); - assert_eq!(set.get(0usize), false); + assert!(!set.get(0usize)); } #[test] fn with_capacity_defaults_to_zero() { let set = BitSet::with_capacity(4); - assert_eq!(set.get(0usize), false); - assert_eq!(set.get(1usize), false); - assert_eq!(set.get(2usize), false); - assert_eq!(set.get(3usize), false); + assert!(!set.get(0usize)); + assert!(!set.get(1usize)); + assert!(!set.get(2usize)); + assert!(!set.get(3usize)); } #[test] fn insert_sets_bit() { let mut set = BitSet::with_capacity(4); - assert_eq!(set.insert(1usize), true); - assert_eq!(set.get(1usize), true); + assert!(set.insert(1usize)); + assert!(set.get(1usize)); } #[test] fn insert_with_set_bit_returns_false() { let mut set = BitSet::with_capacity(4); - assert_eq!(set.insert(1usize), true); - assert_eq!(set.insert(1usize), false); + assert!(set.insert(1usize)); + assert!(!set.insert(1usize)); } #[test] fn insert_all_sets_all_bits() { let mut set = BitSet::with_capacity(4); set.insert_all(); - assert_eq!(set.get(0usize), true); - assert_eq!(set.get(1usize), true); - assert_eq!(set.get(2usize), true); - assert_eq!(set.get(3usize), true); + assert!(set.get(0usize)); + assert!(set.get(1usize)); + assert!(set.get(2usize)); + assert!(set.get(3usize)); } #[test] @@ -117,8 +119,8 @@ mod tests { right.insert(1usize); right.insert(2usize); left.intersect_with(&right); - assert_eq!(left.get(0usize), false); - assert_eq!(left.get(1usize), true); - assert_eq!(left.get(2usize), false); + assert!(!left.get(0usize)); + assert!(left.get(1usize)); + assert!(!left.get(2usize)); } } diff --git a/zjit/src/cast.rs b/zjit/src/cast.rs index bacc7245f378a9..c6d11ef4af18dc 100644 --- a/zjit/src/cast.rs +++ b/zjit/src/cast.rs @@ -1,3 +1,7 @@ +//! Optimized [usize] casting trait. + +#![allow(clippy::wrong_self_convention)] + /// Trait for casting to [usize] that allows you to say `.as_usize()`. /// Implementation conditional on the cast preserving the numeric value on /// all inputs and being inexpensive. diff --git a/zjit/src/codegen.rs b/zjit/src/codegen.rs index 0d461cb0aa15dd..6d4e5cfab5e437 100644 --- a/zjit/src/codegen.rs +++ b/zjit/src/codegen.rs @@ -1,3 +1,7 @@ +//! This module is for native code generation. + +#![allow(clippy::let_and_return)] + use std::cell::{Cell, RefCell}; use std::rc::Rc; use std::ffi::{c_int, c_long, c_void}; @@ -49,7 +53,7 @@ impl JITState { /// Retrieve the output of a given instruction that has been compiled fn get_opnd(&self, insn_id: InsnId) -> lir::Opnd { - self.opnds[insn_id.0].expect(&format!("Failed to get_opnd({insn_id})")) + self.opnds[insn_id.0].unwrap_or_else(|| panic!("Failed to get_opnd({insn_id})")) } /// Find or create a label for a given BlockId @@ -65,9 +69,11 @@ impl JITState { } } -/// CRuby API to compile a given ISEQ +/// CRuby API to compile a given ISEQ. +/// If jit_exception is true, compile JIT code for handling exceptions. +/// See jit_compile_exception() for details. #[unsafe(no_mangle)] -pub extern "C" fn rb_zjit_iseq_gen_entry_point(iseq: IseqPtr, _ec: EcPtr) -> *const u8 { +pub extern "C" fn rb_zjit_iseq_gen_entry_point(iseq: IseqPtr, jit_exception: bool) -> *const u8 { // Do not test the JIT code in HIR tests if cfg!(test) { return std::ptr::null(); @@ -77,11 +83,12 @@ pub extern "C" fn rb_zjit_iseq_gen_entry_point(iseq: IseqPtr, _ec: EcPtr) -> *co // with_vm_lock() does nothing if the program doesn't use Ractors. with_vm_lock(src_loc!(), || { let cb = ZJITState::get_code_block(); - let mut code_ptr = with_time_stat(compile_time_ns, || gen_iseq_entry_point(cb, iseq)); + let mut code_ptr = with_time_stat(compile_time_ns, || gen_iseq_entry_point(cb, iseq, jit_exception)); if let Err(err) = &code_ptr { - // Assert that the ISEQ compiles if RubyVM::ZJIT.assert_compiles is enabled - if ZJITState::assert_compiles_enabled() { + // Assert that the ISEQ compiles if RubyVM::ZJIT.assert_compiles is enabled. + // We assert only `jit_exception: false` cases until we support exception handlers. + if ZJITState::assert_compiles_enabled() && !jit_exception { let iseq_location = iseq_get_location(iseq, 0); panic!("Failed to compile: {iseq_location}"); } @@ -102,7 +109,12 @@ pub extern "C" fn rb_zjit_iseq_gen_entry_point(iseq: IseqPtr, _ec: EcPtr) -> *co } /// Compile an entry point for a given ISEQ -fn gen_iseq_entry_point(cb: &mut CodeBlock, iseq: IseqPtr) -> Result { +fn gen_iseq_entry_point(cb: &mut CodeBlock, iseq: IseqPtr, jit_exception: bool) -> Result { + // We don't support exception handlers yet + if jit_exception { + return Err(CompileError::ExceptionHandler); + } + // Compile ISEQ into High-level IR let function = compile_iseq(iseq).inspect_err(|_| { incr_counter!(failed_iseq_count); @@ -159,7 +171,7 @@ fn gen_entry(cb: &mut CodeBlock, iseq: IseqPtr, function: &Function, function_pt gen_entry_params(&mut asm, iseq, function.block(BlockId(0))); // Jump to the first block using a call instruction - asm.ccall(function_ptr.raw_ptr(cb) as *const u8, vec![]); + asm.ccall(function_ptr.raw_ptr(cb), vec![]); // Restore registers for CFP, EC, and SP after use asm_comment!(asm, "return to the interpreter"); @@ -230,7 +242,7 @@ fn gen_iseq_body(cb: &mut CodeBlock, iseq: IseqPtr, function: Option<&Function>, } /// Compile a function -fn gen_function(cb: &mut CodeBlock, iseq: IseqPtr, function: &Function) -> Result<(CodePtr, Vec, Vec>>), CompileError> { +fn gen_function(cb: &mut CodeBlock, iseq: IseqPtr, function: &Function) -> Result<(CodePtr, Vec, Vec), CompileError> { let c_stack_slots = max_num_params(function).saturating_sub(ALLOC_REGS.len()); let mut jit = JITState::new(iseq, function.num_insns(), function.num_blocks(), c_stack_slots); let mut asm = Assembler::new(); @@ -509,7 +521,7 @@ fn gen_defined(jit: &JITState, asm: &mut Assembler, op_type: usize, obj: VALUE, /// can't optimize the level=0 case using the SP register. fn gen_getlocal_with_ep(asm: &mut Assembler, local_ep_offset: u32, level: u32) -> lir::Opnd { let ep = gen_get_ep(asm, level); - let offset = -(SIZEOF_VALUE_I32 * i32::try_from(local_ep_offset).expect(&format!("Could not convert local_ep_offset {local_ep_offset} to i32"))); + let offset = -(SIZEOF_VALUE_I32 * i32::try_from(local_ep_offset).unwrap_or_else(|_| panic!("Could not convert local_ep_offset {local_ep_offset} to i32"))); asm.load(Opnd::mem(64, ep, offset)) } @@ -522,12 +534,12 @@ fn gen_setlocal_with_ep(asm: &mut Assembler, val: Opnd, val_type: Type, local_ep // When we've proved that we're writing an immediate, // we can skip the write barrier. if val_type.is_immediate() { - let offset = -(SIZEOF_VALUE_I32 * i32::try_from(local_ep_offset).expect(&format!("Could not convert local_ep_offset {local_ep_offset} to i32"))); + let offset = -(SIZEOF_VALUE_I32 * i32::try_from(local_ep_offset).unwrap_or_else(|_| panic!("Could not convert local_ep_offset {local_ep_offset} to i32"))); asm.mov(Opnd::mem(64, ep, offset), val); } 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 = c_int::try_from(local_ep_offset).ok().and_then(|idx| idx.checked_mul(-1)).expect(&format!("Could not turn {local_ep_offset} into a negative c_int")); + let local_index = c_int::try_from(local_ep_offset).ok().and_then(|idx| idx.checked_mul(-1)).unwrap_or_else(|| panic!("Could not turn {local_ep_offset} into a negative c_int")); asm_ccall!(asm, rb_vm_env_write, ep, local_index.into(), val); } } @@ -1087,7 +1099,7 @@ fn gen_new_array( fn gen_new_hash( jit: &mut JITState, asm: &mut Assembler, - elements: &Vec<(InsnId, InsnId)>, + elements: &[(InsnId, InsnId)], state: &FrameState, ) -> lir::Opnd { gen_prepare_non_leaf_call(jit, asm, state); @@ -1844,6 +1856,8 @@ pub struct IseqCall { end_addr: Cell>, } +type IseqCallRef = Rc>; + impl IseqCall { /// Allocate a new IseqCall fn new(iseq: IseqPtr) -> Rc> { diff --git a/zjit/src/cruby.rs b/zjit/src/cruby.rs index b82edc6633fcfe..0ef402f073754c 100644 --- a/zjit/src/cruby.rs +++ b/zjit/src/cruby.rs @@ -81,6 +81,7 @@ #![allow(non_camel_case_types)] // A lot of imported CRuby globals aren't all-caps #![allow(non_upper_case_globals)] +#![allow(clippy::upper_case_acronyms)] // Some of this code may not be used yet #![allow(dead_code)] @@ -713,7 +714,7 @@ pub fn rust_str_to_sym(str: &str) -> VALUE { /// Produce an owned Rust String from a C char pointer pub fn cstr_to_rust_string(c_char_ptr: *const c_char) -> Option { - assert!(c_char_ptr != std::ptr::null()); + assert!(!c_char_ptr.is_null()); let c_str: &CStr = unsafe { CStr::from_ptr(c_char_ptr) }; @@ -743,13 +744,13 @@ pub fn iseq_get_location(iseq: IseqPtr, pos: u32) -> String { let iseq_lineno = unsafe { rb_iseq_line_no(iseq, pos as usize) }; let mut s = iseq_name(iseq); - s.push_str("@"); + s.push('@'); if iseq_path == Qnil { s.push_str("None"); } else { s.push_str(&ruby_str_to_rust_string(iseq_path)); } - s.push_str(":"); + s.push(':'); s.push_str(&iseq_lineno.to_string()); s } @@ -762,10 +763,7 @@ fn ruby_str_to_rust_string(v: VALUE) -> String { let str_ptr = unsafe { rb_RSTRING_PTR(v) } as *mut u8; let str_len: usize = unsafe { rb_RSTRING_LEN(v) }.try_into().unwrap(); let str_slice: &[u8] = unsafe { std::slice::from_raw_parts(str_ptr, str_len) }; - match String::from_utf8(str_slice.to_vec()) { - Ok(utf8) => utf8, - Err(_) => String::new(), - } + String::from_utf8(str_slice.to_vec()).unwrap_or_default() } pub fn ruby_sym_to_rust_string(v: VALUE) -> String { @@ -1041,7 +1039,7 @@ pub mod test_utils { /// Make sure the Ruby VM is set up and run a given callback with rb_protect() pub fn with_rubyvm(mut func: impl FnMut() -> T) -> T { - RUBY_VM_INIT.call_once(|| boot_rubyvm()); + RUBY_VM_INIT.call_once(boot_rubyvm); // Set up a callback wrapper to store a return value let mut result: Option = None; @@ -1121,7 +1119,7 @@ pub mod test_utils { if line.len() > spaces { unindented.extend_from_slice(&line.as_bytes()[spaces..]); } else { - unindented.extend_from_slice(&line.as_bytes()); + unindented.extend_from_slice(line.as_bytes()); } } String::from_utf8(unindented).unwrap() diff --git a/zjit/src/cruby_methods.rs b/zjit/src/cruby_methods.rs index 36965c2c00dc1f..8e9e14cc5dda33 100644 --- a/zjit/src/cruby_methods.rs +++ b/zjit/src/cruby_methods.rs @@ -113,7 +113,7 @@ fn annotate_builtin_method(props_map: &mut HashMap<*mut c_void, FnProperties>, c opcode == YARVINSN_opt_invokebuiltin_delegate_leave as i32 { // The first operand is the builtin function pointer let bf_value = *pc.add(1); - let bf_ptr = bf_value.as_ptr() as *const rb_builtin_function; + let bf_ptr: *const rb_builtin_function = bf_value.as_ptr(); if func_ptr.is_null() { func_ptr = (*bf_ptr).func_ptr as *mut c_void; diff --git a/zjit/src/disasm.rs b/zjit/src/disasm.rs index 09864ef64994d9..70a56d780d4edc 100644 --- a/zjit/src/disasm.rs +++ b/zjit/src/disasm.rs @@ -47,5 +47,5 @@ pub fn disasm_addr_range(cb: &CodeBlock, start_addr: usize, end_addr: usize) -> writeln!(&mut out, " {}", format!("{insn}").trim()).unwrap(); } - return out; + out } diff --git a/zjit/src/distribution.rs b/zjit/src/distribution.rs index 5927ffa5c944ba..7a496ffd8dfc09 100644 --- a/zjit/src/distribution.rs +++ b/zjit/src/distribution.rs @@ -1,10 +1,12 @@ +//! Type frequency distribution tracker. + /// This implementation was inspired by the type feedback module from Google's S6, which was /// written in C++ for use with Python. This is a new implementation in Rust created for use with /// Ruby instead of Python. #[derive(Debug, Clone)] pub struct Distribution { /// buckets and counts have the same length - /// buckets[0] is always the most common item + /// `buckets[0]` is always the most common item buckets: [T; N], counts: [usize; N], /// if there is no more room, increment the fallback @@ -105,7 +107,7 @@ impl Distributi DistributionKind::Megamorphic } }; - Self { kind, buckets: dist.buckets.clone() } + Self { kind, buckets: dist.buckets } } pub fn is_monomorphic(&self) -> bool { diff --git a/zjit/src/gc.rs b/zjit/src/gc.rs index efc77fabc00345..49f8cbe2812aca 100644 --- a/zjit/src/gc.rs +++ b/zjit/src/gc.rs @@ -1,4 +1,4 @@ -// This module is responsible for marking/moving objects on GC. +//! This module is responsible for marking/moving objects on GC. use std::cell::RefCell; use std::rc::Rc; @@ -219,7 +219,7 @@ pub fn remove_gc_offsets(payload_ptr: *mut IseqPayload, removed_range: &Range ranges overlap with each other +/// Return true if given `Range` ranges overlap with each other fn ranges_overlap(left: &Range, right: &Range) -> bool where T: PartialOrd { left.start < right.end && right.start < left.end } diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index 1e5153b71101da..91730dc5be0353 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -3,6 +3,8 @@ // We use the YARV bytecode constants which have a CRuby-style name #![allow(non_upper_case_globals)] +#![allow(clippy::if_same_then_else)] +#![allow(clippy::match_like_matches_macro)] use crate::{ cast::IntoUsize, codegen::local_idx_to_ep_offset, cruby::*, gc::{get_or_create_iseq_payload, IseqPayload}, options::{get_option, DumpHIR}, state::ZJITState }; @@ -20,9 +22,9 @@ use crate::stats::Counter; #[derive(Copy, Clone, Ord, PartialOrd, Eq, PartialEq, Hash, Debug)] pub struct InsnId(pub usize); -impl Into for InsnId { - fn into(self) -> usize { - self.0 +impl From for usize { + fn from(val: InsnId) -> Self { + val.0 } } @@ -36,9 +38,9 @@ impl std::fmt::Display for InsnId { #[derive(Copy, Clone, Eq, PartialEq, Hash, Debug)] pub struct BlockId(pub usize); -impl Into for BlockId { - fn into(self) -> usize { - self.0 +impl From for usize { + fn from(val: BlockId) -> Self { + val.0 } } @@ -663,7 +665,7 @@ impl Insn { Insn::NewArray { .. } => false, // NewHash's operands may be hashed and compared for equality, which could have // side-effects. - Insn::NewHash { elements, .. } => elements.len() > 0, + Insn::NewHash { elements, .. } => !elements.is_empty(), Insn::ArrayDup { .. } => false, Insn::HashDup { .. } => false, Insn::Test { .. } => false, @@ -1165,8 +1167,10 @@ impl Function { fn new_block(&mut self, insn_idx: u32) -> BlockId { let id = BlockId(self.blocks.len()); - let mut block = Block::default(); - block.insn_idx = insn_idx; + let block = Block { + insn_idx, + .. Block::default() + }; self.blocks.push(block); id } @@ -1253,11 +1257,11 @@ impl Function { &ToRegexp { opt, ref values, state } => ToRegexp { opt, values: find_vec!(values), state }, &Test { val } => Test { val: find!(val) }, &IsNil { val } => IsNil { val: find!(val) }, - &Jump(ref target) => Jump(find_branch_edge!(target)), + Jump(target) => Jump(find_branch_edge!(target)), &IfTrue { val, ref target } => IfTrue { val: find!(val), target: find_branch_edge!(target) }, &IfFalse { val, ref target } => IfFalse { val: find!(val), target: find_branch_edge!(target) }, - &GuardType { val, guard_type, state } => GuardType { val: find!(val), guard_type: guard_type, state }, - &GuardBitEquals { val, expected, state } => GuardBitEquals { val: find!(val), expected: expected, state }, + &GuardType { val, guard_type, state } => GuardType { val: find!(val), guard_type, state }, + &GuardBitEquals { val, expected, state } => GuardBitEquals { val: find!(val), expected, state }, &GuardShape { val, shape, state } => GuardShape { val: find!(val), shape, state }, &FixnumAdd { left, right, state } => FixnumAdd { left: find!(left), right: find!(right), state }, &FixnumSub { left, right, state } => FixnumSub { left: find!(left), right: find!(right), state }, @@ -1274,7 +1278,7 @@ impl Function { &FixnumOr { left, right } => FixnumOr { left: find!(left), right: find!(right) }, &ObjToString { val, cd, state } => ObjToString { val: find!(val), - cd: cd, + cd, state, }, &AnyToString { val, str, state } => AnyToString { @@ -1284,22 +1288,22 @@ impl Function { }, &SendWithoutBlock { self_val, cd, ref args, state } => SendWithoutBlock { self_val: find!(self_val), - cd: cd, + cd, args: find_vec!(args), state, }, &SendWithoutBlockDirect { self_val, cd, cme, iseq, ref args, state } => SendWithoutBlockDirect { self_val: find!(self_val), - cd: cd, - cme: cme, - iseq: iseq, + cd, + cme, + iseq, args: find_vec!(args), state, }, &Send { self_val, cd, blockiseq, ref args, state } => Send { self_val: find!(self_val), - cd: cd, - blockiseq: blockiseq, + cd, + blockiseq, args: find_vec!(args), state, }, @@ -1518,8 +1522,8 @@ impl Function { /// index, if it is known. This historical type record is not a guarantee and must be checked /// with a GuardType or similar. fn profiled_type_of_at(&self, insn: InsnId, iseq_insn_idx: usize) -> Option { - let Some(ref profiles) = self.profiles else { return None }; - let Some(entries) = profiles.types.get(&iseq_insn_idx) else { return None }; + let profiles = self.profiles.as_ref()?; + let entries = profiles.types.get(&iseq_insn_idx)?; let insn = self.chase_insn(insn); for (entry_insn, entry_type_summary) in entries { if self.union_find.borrow().find_const(*entry_insn) == insn { @@ -1534,19 +1538,19 @@ impl Function { } fn likely_is_fixnum(&self, val: InsnId, profiled_type: ProfiledType) -> bool { - return self.is_a(val, types::Fixnum) || profiled_type.is_fixnum(); + self.is_a(val, types::Fixnum) || profiled_type.is_fixnum() } fn coerce_to_fixnum(&mut self, block: BlockId, val: InsnId, state: InsnId) -> InsnId { if self.is_a(val, types::Fixnum) { return val; } - return self.push_insn(block, Insn::GuardType { val, guard_type: types::Fixnum, state }); + self.push_insn(block, Insn::GuardType { val, guard_type: types::Fixnum, state }) } fn arguments_likely_fixnums(&mut self, left: InsnId, right: InsnId, state: InsnId) -> bool { let frame_state = self.frame_state(state); - let iseq_insn_idx = frame_state.insn_idx as usize; - let left_profiled_type = self.profiled_type_of_at(left, iseq_insn_idx).unwrap_or(ProfiledType::empty()); - let right_profiled_type = self.profiled_type_of_at(right, iseq_insn_idx).unwrap_or(ProfiledType::empty()); + let iseq_insn_idx = frame_state.insn_idx; + let left_profiled_type = self.profiled_type_of_at(left, iseq_insn_idx).unwrap_or_default(); + let right_profiled_type = self.profiled_type_of_at(right, iseq_insn_idx).unwrap_or_default(); self.likely_is_fixnum(left, left_profiled_type) && self.likely_is_fixnum(right, right_profiled_type) } @@ -1667,9 +1671,9 @@ impl Function { self.try_rewrite_fixnum_op(block, insn_id, &|left, right| Insn::FixnumAnd { left, right }, BOP_AND, self_val, args[0], state), Insn::SendWithoutBlock { self_val, args, state, cd, .. } if ruby_call_method_id(cd) == ID!(or) && args.len() == 1 => self.try_rewrite_fixnum_op(block, insn_id, &|left, right| Insn::FixnumOr { left, right }, BOP_OR, self_val, args[0], state), - Insn::SendWithoutBlock { self_val, args, state, cd, .. } if ruby_call_method_id(cd) == ID!(freeze) && args.len() == 0 => + Insn::SendWithoutBlock { self_val, args, state, cd, .. } if ruby_call_method_id(cd) == ID!(freeze) && args.is_empty() => self.try_rewrite_freeze(block, insn_id, self_val, state), - Insn::SendWithoutBlock { self_val, args, state, cd, .. } if ruby_call_method_id(cd) == ID!(minusat) && args.len() == 0 => + Insn::SendWithoutBlock { self_val, args, state, cd, .. } if ruby_call_method_id(cd) == ID!(minusat) && args.is_empty() => self.try_rewrite_uminus(block, insn_id, self_val, state), Insn::SendWithoutBlock { self_val, args, state, cd, .. } if ruby_call_method_id(cd) == ID!(aref) && args.len() == 1 => self.try_rewrite_aref(block, insn_id, self_val, args[0], state), @@ -1807,12 +1811,10 @@ impl Function { // entered the compiler. That means we can just return nil for this // shape + iv name Insn::Const { val: Const::Value(Qnil) } + } else if recv_type.flags().is_embedded() { + Insn::LoadIvarEmbedded { self_val, id, index: ivar_index } } else { - if recv_type.flags().is_embedded() { - Insn::LoadIvarEmbedded { self_val, id, index: ivar_index } - } else { - Insn::LoadIvarExtended { self_val, id, index: ivar_index } - } + Insn::LoadIvarExtended { self_val, id, index: ivar_index } }; let replacement = self.push_insn(block, replacement); self.make_equal_to(insn_id, replacement); @@ -2161,7 +2163,7 @@ impl Function { worklist.push_back(val); worklist.push_back(state); } - &Insn::Snapshot { ref state } => { + Insn::Snapshot { state } => { worklist.extend(&state.stack); worklist.extend(&state.locals); } @@ -2210,7 +2212,7 @@ impl Function { worklist.extend(args); worklist.push_back(state) } - &Insn::CCall { ref args, .. } => worklist.extend(args), + Insn::CCall { args, .. } => worklist.extend(args), &Insn::GetIvar { self_val, state, .. } | &Insn::DefinedIvar { self_val, state, .. } => { worklist.push_back(self_val); worklist.push_back(state); @@ -2273,7 +2275,7 @@ impl Function { } } - fn absorb_dst_block(&mut self, num_in_edges: &Vec, block: BlockId) -> bool { + fn absorb_dst_block(&mut self, num_in_edges: &[u32], block: BlockId) -> bool { let Some(terminator_id) = self.blocks[block.0].insns.last() else { return false }; let Insn::Jump(BranchEdge { target, args }) = self.find(*terminator_id) @@ -2399,8 +2401,8 @@ impl Function { pub fn dump_hir(&self) { // Dump HIR after optimization match get_option!(dump_hir_opt) { - Some(DumpHIR::WithoutSnapshot) => println!("Optimized HIR:\n{}", FunctionPrinter::without_snapshot(&self)), - Some(DumpHIR::All) => println!("Optimized HIR:\n{}", FunctionPrinter::with_snapshot(&self)), + Some(DumpHIR::WithoutSnapshot) => println!("Optimized HIR:\n{}", FunctionPrinter::without_snapshot(self)), + Some(DumpHIR::All) => println!("Optimized HIR:\n{}", FunctionPrinter::with_snapshot(self)), Some(DumpHIR::Debug) => println!("Optimized HIR:\n{:#?}", &self), None => {}, } @@ -2409,7 +2411,7 @@ impl Function { use std::fs::OpenOptions; use std::io::Write; let mut file = OpenOptions::new().append(true).open(filename).unwrap(); - writeln!(file, "{}", FunctionGraphvizPrinter::new(&self)).unwrap(); + writeln!(file, "{}", FunctionGraphvizPrinter::new(self)).unwrap(); } } @@ -2620,7 +2622,7 @@ impl<'a> std::fmt::Display for FunctionGraphvizPrinter<'a> { let iseq_name = iseq_get_location(fun.iseq, 0); write!(f, "digraph G {{ # ")?; write_encoded!(f, "{iseq_name}")?; - write!(f, "\n")?; + writeln!(f)?; writeln!(f, "node [shape=plaintext];")?; writeln!(f, "mode=hier; overlap=false; splines=true;")?; for block_id in fun.rpo() { @@ -2787,7 +2789,7 @@ impl FrameState { // TODO: Modify the register allocator to allow reusing an argument // of another basic block. let mut args = vec![self_param]; - args.extend(self.locals.iter().chain(self.stack.iter()).map(|op| *op)); + args.extend(self.locals.iter().chain(self.stack.iter()).copied()); args } @@ -2939,7 +2941,7 @@ impl ProfileOracle { fn profile_stack(&mut self, state: &FrameState) { let iseq_insn_idx = state.insn_idx; let Some(operand_types) = self.payload.profile.get_operand_types(iseq_insn_idx) else { return }; - let entry = self.types.entry(iseq_insn_idx).or_insert_with(|| vec![]); + let entry = self.types.entry(iseq_insn_idx).or_default(); // operand_types is always going to be <= stack size (otherwise it would have an underflow // at run-time) so use that to drive iteration. for (idx, insn_type_distribution) in operand_types.iter().rev().enumerate() { @@ -2952,7 +2954,7 @@ impl ProfileOracle { fn profile_self(&mut self, state: &FrameState, self_param: InsnId) { let iseq_insn_idx = state.insn_idx; let Some(operand_types) = self.payload.profile.get_operand_types(iseq_insn_idx) else { return }; - let entry = self.types.entry(iseq_insn_idx).or_insert_with(|| vec![]); + let entry = self.types.entry(iseq_insn_idx).or_default(); if operand_types.is_empty() { return; } @@ -3612,7 +3614,7 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result { let call_info = unsafe { rb_get_call_data_ci(cd) }; if let Err(call_type) = unknown_call_type(unsafe { rb_vm_ci_flag(call_info) }) { - assert!(false, "objtostring should not have unknown call type {call_type:?}"); + panic!("objtostring should not have unknown call type {call_type:?}"); } let argc = unsafe { vm_ci_argc((*cd).ci) }; assert_eq!(0, argc, "objtostring should not have args"); @@ -3796,7 +3798,7 @@ mod validation_tests { Err(validation_err) => { assert_eq!(validation_err, expected); } - Ok(_) => assert!(false, "Expected validation error"), + Ok(_) => panic!("Expected validation error"), } } @@ -4114,7 +4116,7 @@ mod tests { #[track_caller] fn hir_string_function(function: &Function) -> String { - format!("{}", FunctionPrinter::without_snapshot(&function)) + format!("{}", FunctionPrinter::without_snapshot(function)) } #[track_caller] diff --git a/zjit/src/hir_type/hir_type.inc.rs b/zjit/src/hir_type/hir_type.inc.rs index 58508740800da3..2e03fdac96b8fc 100644 --- a/zjit/src/hir_type/hir_type.inc.rs +++ b/zjit/src/hir_type/hir_type.inc.rs @@ -66,7 +66,7 @@ mod bits { pub const Symbol: u64 = DynamicSymbol | StaticSymbol; pub const TrueClass: u64 = 1u64 << 40; pub const Undef: u64 = 1u64 << 41; - pub const AllBitPatterns: [(&'static str, u64); 66] = [ + pub const AllBitPatterns: [(&str, u64); 66] = [ ("Any", Any), ("RubyValue", RubyValue), ("Immediate", Immediate), diff --git a/zjit/src/hir_type/mod.rs b/zjit/src/hir_type/mod.rs index d6d43984251e57..926d6d306fff7b 100644 --- a/zjit/src/hir_type/mod.rs +++ b/zjit/src/hir_type/mod.rs @@ -1,3 +1,5 @@ +//! High-level intermediate representation types. + #![allow(non_upper_case_globals)] use crate::cruby::{Qfalse, Qnil, Qtrue, VALUE, RUBY_T_ARRAY, RUBY_T_STRING, RUBY_T_HASH, RUBY_T_CLASS, RUBY_T_MODULE}; use crate::cruby::{rb_cInteger, rb_cFloat, rb_cArray, rb_cHash, rb_cString, rb_cSymbol, rb_cObject, rb_cTrueClass, rb_cFalseClass, rb_cNilClass, rb_cRange, rb_cSet, rb_cRegexp, rb_cClass, rb_cModule, rb_zjit_singleton_class_p}; @@ -640,7 +642,7 @@ mod tests { #[test] fn integer_has_exact_ruby_class() { - assert_eq!(Type::fixnum(3).exact_ruby_class(), Some(unsafe { rb_cInteger }.into())); + assert_eq!(Type::fixnum(3).exact_ruby_class(), Some(unsafe { rb_cInteger })); assert_eq!(types::Fixnum.exact_ruby_class(), None); assert_eq!(types::Integer.exact_ruby_class(), None); } @@ -667,7 +669,7 @@ mod tests { #[test] fn integer_has_ruby_class() { - assert_eq!(Type::fixnum(3).inexact_ruby_class(), Some(unsafe { rb_cInteger }.into())); + assert_eq!(Type::fixnum(3).inexact_ruby_class(), Some(unsafe { rb_cInteger })); assert_eq!(types::Fixnum.inexact_ruby_class(), None); assert_eq!(types::Integer.inexact_ruby_class(), None); } diff --git a/zjit/src/invariants.rs b/zjit/src/invariants.rs index 9935336bc0fc76..345d815bf2054c 100644 --- a/zjit/src/invariants.rs +++ b/zjit/src/invariants.rs @@ -1,3 +1,5 @@ +//! Code invalidation and patching for speculative optimizations. + use std::{collections::{HashMap, HashSet}, mem}; use crate::{backend::lir::{asm_comment, Assembler}, cruby::{rb_callable_method_entry_t, rb_gc_location, ruby_basic_operators, src_loc, with_vm_lock, IseqPtr, RedefinitionFlag, ID, VALUE}, gc::IseqPayload, hir::Invariant, options::debug, state::{zjit_enabled_p, ZJITState}, virtualmem::CodePtr}; @@ -63,7 +65,7 @@ impl Invariants { Self::update_iseq_references(&mut self.no_ep_escape_iseqs); } - /// Update ISEQ references in a given HashSet + /// Update ISEQ references in a given `HashSet` fn update_iseq_references(iseqs: &mut HashSet) { let mut moved: Vec = Vec::with_capacity(iseqs.len()); diff --git a/zjit/src/lib.rs b/zjit/src/lib.rs index b36bf6515ebadd..e6f743fa1e6782 100644 --- a/zjit/src/lib.rs +++ b/zjit/src/lib.rs @@ -1,6 +1,10 @@ #![allow(dead_code)] #![allow(static_mut_refs)] +#![allow(clippy::enum_variant_names)] +#![allow(clippy::too_many_arguments)] +#![allow(clippy::needless_bool)] + // Add std docs to cargo doc. #[doc(inline)] pub use std; diff --git a/zjit/src/options.rs b/zjit/src/options.rs index f0a7f0bc7b60fe..3efcb933bb52ec 100644 --- a/zjit/src/options.rs +++ b/zjit/src/options.rs @@ -1,3 +1,5 @@ +//! Configurable options for ZJIT. + use std::{ffi::{CStr, CString}, ptr::null}; use std::os::raw::{c_char, c_int, c_uint}; use crate::cruby::*; @@ -93,7 +95,7 @@ impl Default for Options { /// `ruby --help` descriptions for user-facing options. Do not add options for ZJIT developers. /// Note that --help allows only 80 chars per line, including indentation, and it also puts the /// description in a separate line if the option name is too long. 80-char limit --> | (any character beyond this `|` column fails the test) -pub const ZJIT_OPTIONS: &'static [(&str, &str)] = &[ +pub const ZJIT_OPTIONS: &[(&str, &str)] = &[ // TODO: Hide --zjit-exec-mem-size from ZJIT_OPTIONS once we add --zjit-mem-size (Shopify/ruby#686) ("--zjit-exec-mem-size=num", "Size of executable memory block in MiB (default: 64)."), diff --git a/zjit/src/profile.rs b/zjit/src/profile.rs index f2b12516c72747..4d33aadf2bc1f0 100644 --- a/zjit/src/profile.rs +++ b/zjit/src/profile.rs @@ -1,3 +1,5 @@ +//! Profiler for runtime information. + // We use the YARV bytecode constants which have a CRuby-style name #![allow(non_upper_case_globals)] @@ -101,13 +103,14 @@ fn profile_operands(profiler: &mut Profiler, profile: &mut IseqProfile, n: usize if types.is_empty() { types.resize(n, TypeDistribution::new()); } - for i in 0..n { + + for (i, profile_type) in types.iter_mut().enumerate() { let obj = profiler.peek_at_stack((n - i - 1) as isize); // TODO(max): Handle GC-hidden classes like Array, Hash, etc and make them look normal or // drop them or something let ty = ProfiledType::new(obj); unsafe { rb_gc_writebarrier(profiler.iseq.into(), ty.class()) }; - types[i].observe(ty); + profile_type.observe(ty); } } @@ -146,7 +149,7 @@ impl Flags { /// opt_send_without_block/opt_plus/... should store: /// * the class of the receiver, so we can do method lookup /// * the shape of the receiver, so we can optimize ivar lookup -/// with those two, pieces of information, we can also determine when an object is an immediate: +/// with those two, pieces of information, we can also determine when an object is an immediate: /// * Integer + IS_IMMEDIATE == Fixnum /// * Float + IS_IMMEDIATE == Flonum /// * Symbol + IS_IMMEDIATE == StaticSymbol diff --git a/zjit/src/state.rs b/zjit/src/state.rs index 6608f1ea23c10f..02bba3b7a3e8a8 100644 --- a/zjit/src/state.rs +++ b/zjit/src/state.rs @@ -1,3 +1,5 @@ +//! Runtime state of ZJIT. + use crate::codegen::{gen_exit_trampoline, gen_exit_trampoline_with_counter, gen_function_stub_hit_trampoline}; use crate::cruby::{self, rb_bug_panic_hook, rb_vm_insn_count, EcPtr, Qnil, VALUE, VM_INSTRUCTION_SIZE}; use crate::cruby_methods; diff --git a/zjit/src/stats.rs b/zjit/src/stats.rs index 77f6d99fc7efb8..cc6c9452e6d26f 100644 --- a/zjit/src/stats.rs +++ b/zjit/src/stats.rs @@ -1,3 +1,5 @@ +//! Counters and associated methods for events when ZJIT is run. + use std::time::Instant; use crate::{cruby::*, hir::ParseError, options::get_option, state::{zjit_enabled_p, ZJITState}}; @@ -116,6 +118,7 @@ make_counters! { // compile_error_: Compile error reasons compile_error_iseq_stack_too_large, + compile_error_exception_handler, compile_error_out_of_memory, compile_error_register_spill_on_ccall, compile_error_register_spill_on_alloc, @@ -180,6 +183,7 @@ pub fn exit_counter_ptr_for_call_type(call_type: crate::hir::CallType) -> *mut u #[derive(Clone, Debug, PartialEq)] pub enum CompileError { IseqStackTooLarge, + ExceptionHandler, OutOfMemory, RegisterSpillOnAlloc, RegisterSpillOnCCall, @@ -194,6 +198,7 @@ pub fn exit_counter_for_compile_error(compile_error: &CompileError) -> Counter { use crate::stats::Counter::*; match compile_error { IseqStackTooLarge => compile_error_iseq_stack_too_large, + ExceptionHandler => compile_error_exception_handler, OutOfMemory => compile_error_out_of_memory, RegisterSpillOnAlloc => compile_error_register_spill_on_alloc, RegisterSpillOnCCall => compile_error_register_spill_on_ccall, @@ -299,11 +304,10 @@ pub extern "C" fn rb_zjit_stats(_ec: EcPtr, _self: VALUE, target_key: VALUE) -> // Set side-exit counters for UnhandledYARVInsn let exit_counters = ZJITState::get_exit_counters(); - for op_idx in 0..VM_INSTRUCTION_SIZE as usize { + for (op_idx, count) in exit_counters.iter().enumerate().take(VM_INSTRUCTION_SIZE as usize) { let op_name = insn_name(op_idx); let key_string = "unhandled_yarv_insn_".to_owned() + &op_name; - let count = exit_counters[op_idx]; - set_stat_usize!(hash, &key_string, count); + set_stat_usize!(hash, &key_string, *count); } // Only ZJIT_STATS builds support rb_vm_insn_count