From 809a9a9f2972127ea4b8bba027102197661c7196 Mon Sep 17 00:00:00 2001 From: Takashi Kokubun Date: Wed, 3 Sep 2025 13:14:28 -0700 Subject: [PATCH 1/6] ZJIT: Count exits coming from jit_exception (#14428) --- vm.c | 21 +++++++++++++++++++-- yjit.c | 2 +- zjit.c | 16 ++++++++++------ zjit.h | 4 ++-- zjit/src/codegen.rs | 20 ++++++++++++++------ zjit/src/stats.rs | 3 +++ 6 files changed, 49 insertions(+), 17 deletions(-) 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/codegen.rs b/zjit/src/codegen.rs index 0d461cb0aa15dd..766203492b3e61 100644 --- a/zjit/src/codegen.rs +++ b/zjit/src/codegen.rs @@ -65,9 +65,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 +79,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 +105,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); diff --git a/zjit/src/stats.rs b/zjit/src/stats.rs index 77f6d99fc7efb8..8e06654bee7a40 100644 --- a/zjit/src/stats.rs +++ b/zjit/src/stats.rs @@ -116,6 +116,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 +181,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 +196,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, From 5283443bdaff8e1cf591499662fd109acda6ad4f Mon Sep 17 00:00:00 2001 From: Aiden Fox Ivey Date: Wed, 3 Sep 2025 15:28:15 -0400 Subject: [PATCH 2/6] ZJIT: Add documentation note in doc/zjit.md --- doc/zjit.md | 4 ++++ 1 file changed, 4 insertions(+) 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: From a8a2f1f06c3435aaa47c82512f5bf0e3a605132e Mon Sep 17 00:00:00 2001 From: Aiden Fox Ivey Date: Wed, 3 Sep 2025 15:28:31 -0400 Subject: [PATCH 3/6] ZJIT: Fix documentation build warnings --- zjit/src/distribution.rs | 2 +- zjit/src/gc.rs | 2 +- zjit/src/invariants.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/zjit/src/distribution.rs b/zjit/src/distribution.rs index 5927ffa5c944ba..c75bba944e0991 100644 --- a/zjit/src/distribution.rs +++ b/zjit/src/distribution.rs @@ -4,7 +4,7 @@ #[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 diff --git a/zjit/src/gc.rs b/zjit/src/gc.rs index efc77fabc00345..6469fc0bf6a646 100644 --- a/zjit/src/gc.rs +++ b/zjit/src/gc.rs @@ -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/invariants.rs b/zjit/src/invariants.rs index 9935336bc0fc76..5b014320ead0fe 100644 --- a/zjit/src/invariants.rs +++ b/zjit/src/invariants.rs @@ -63,7 +63,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()); From 8a350775aab898ea0f0fad5663b259caa840fc1f Mon Sep 17 00:00:00 2001 From: Aiden Fox Ivey Date: Wed, 3 Sep 2025 15:41:45 -0400 Subject: [PATCH 4/6] ZJIT: Add missing module doc comments --- zjit/src/asm/mod.rs | 2 ++ zjit/src/backend/mod.rs | 2 ++ zjit/src/bitset.rs | 2 ++ zjit/src/cast.rs | 2 ++ zjit/src/codegen.rs | 2 ++ zjit/src/distribution.rs | 2 ++ zjit/src/gc.rs | 2 +- zjit/src/hir_type/mod.rs | 2 ++ zjit/src/invariants.rs | 2 ++ zjit/src/options.rs | 2 ++ zjit/src/profile.rs | 2 ++ zjit/src/state.rs | 2 ++ zjit/src/stats.rs | 2 ++ 13 files changed, 25 insertions(+), 1 deletion(-) 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/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/bitset.rs b/zjit/src/bitset.rs index 895bac8e33232a..b108b173f3cbea 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; diff --git a/zjit/src/cast.rs b/zjit/src/cast.rs index bacc7245f378a9..197de876ea6f01 100644 --- a/zjit/src/cast.rs +++ b/zjit/src/cast.rs @@ -1,3 +1,5 @@ +//! Optimized [usize] casting trait. + /// 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 766203492b3e61..222c43d73a7c54 100644 --- a/zjit/src/codegen.rs +++ b/zjit/src/codegen.rs @@ -1,3 +1,5 @@ +//! This module is for native code generation. + use std::cell::{Cell, RefCell}; use std::rc::Rc; use std::ffi::{c_int, c_long, c_void}; diff --git a/zjit/src/distribution.rs b/zjit/src/distribution.rs index c75bba944e0991..3e951371bdc56f 100644 --- a/zjit/src/distribution.rs +++ b/zjit/src/distribution.rs @@ -1,3 +1,5 @@ +//! 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. diff --git a/zjit/src/gc.rs b/zjit/src/gc.rs index 6469fc0bf6a646..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; diff --git a/zjit/src/hir_type/mod.rs b/zjit/src/hir_type/mod.rs index d6d43984251e57..12c0f31fa72c1f 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}; diff --git a/zjit/src/invariants.rs b/zjit/src/invariants.rs index 5b014320ead0fe..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}; diff --git a/zjit/src/options.rs b/zjit/src/options.rs index f0a7f0bc7b60fe..62dcc1e0f5449f 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::*; diff --git a/zjit/src/profile.rs b/zjit/src/profile.rs index f2b12516c72747..06d5d4b2b774c6 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)] 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 8e06654bee7a40..252a75db20e749 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}}; From a6d397e29c61030f130f8d83e6bfe6d99ebbae91 Mon Sep 17 00:00:00 2001 From: Aiden Fox Ivey Date: Wed, 3 Sep 2025 17:45:54 -0400 Subject: [PATCH 5/6] ZJIT: Ensure `clippy` passes and silence unnecessary warnings (#14439) --- .github/workflows/zjit-ubuntu.yml | 19 +++++ zjit/src/asm/arm64/inst/branch.rs | 2 +- zjit/src/asm/arm64/inst/load_literal.rs | 2 + zjit/src/asm/arm64/inst/mov.rs | 4 +- zjit/src/asm/arm64/mod.rs | 6 +- zjit/src/asm/arm64/opnd.rs | 5 +- zjit/src/asm/x86_64/mod.rs | 9 +-- zjit/src/asm/x86_64/tests.rs | 6 +- zjit/src/backend/arm64/mod.rs | 25 +++---- zjit/src/backend/lir.rs | 61 ++++++++-------- zjit/src/backend/x86_64/mod.rs | 12 ++-- zjit/src/bitset.rs | 32 ++++----- zjit/src/cast.rs | 2 + zjit/src/codegen.rs | 18 +++-- zjit/src/cruby.rs | 16 ++--- zjit/src/cruby_methods.rs | 2 +- zjit/src/disasm.rs | 2 +- zjit/src/distribution.rs | 2 +- zjit/src/hir.rs | 94 +++++++++++++------------ zjit/src/hir_type/hir_type.inc.rs | 2 +- zjit/src/hir_type/mod.rs | 4 +- zjit/src/lib.rs | 4 ++ zjit/src/options.rs | 2 +- zjit/src/profile.rs | 7 +- zjit/src/stats.rs | 5 +- 25 files changed, 183 insertions(+), 160 deletions(-) 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/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/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/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 b108b173f3cbea..b5b69abdeeb62d 100644 --- a/zjit/src/bitset.rs +++ b/zjit/src/bitset.rs @@ -67,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] @@ -119,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 197de876ea6f01..c6d11ef4af18dc 100644 --- a/zjit/src/cast.rs +++ b/zjit/src/cast.rs @@ -1,5 +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 222c43d73a7c54..6d4e5cfab5e437 100644 --- a/zjit/src/codegen.rs +++ b/zjit/src/codegen.rs @@ -1,5 +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}; @@ -51,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 @@ -169,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"); @@ -240,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(); @@ -519,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)) } @@ -532,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); } } @@ -1097,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); @@ -1854,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 3e951371bdc56f..7a496ffd8dfc09 100644 --- a/zjit/src/distribution.rs +++ b/zjit/src/distribution.rs @@ -107,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/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 12c0f31fa72c1f..926d6d306fff7b 100644 --- a/zjit/src/hir_type/mod.rs +++ b/zjit/src/hir_type/mod.rs @@ -642,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); } @@ -669,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/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 62dcc1e0f5449f..3efcb933bb52ec 100644 --- a/zjit/src/options.rs +++ b/zjit/src/options.rs @@ -95,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 06d5d4b2b774c6..4d33aadf2bc1f0 100644 --- a/zjit/src/profile.rs +++ b/zjit/src/profile.rs @@ -103,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); } } @@ -148,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/stats.rs b/zjit/src/stats.rs index 252a75db20e749..cc6c9452e6d26f 100644 --- a/zjit/src/stats.rs +++ b/zjit/src/stats.rs @@ -304,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 From bbef1772aab330bffb0c11fb0df2c909bfdce989 Mon Sep 17 00:00:00 2001 From: Takashi Kokubun Date: Wed, 3 Sep 2025 16:11:22 -0700 Subject: [PATCH 6/6] Revert "test_gc.rb: Attempt to stabilize test_interrupt_in_finalizer" This reverts commit c1c0b32445c66e343c136faa28d7a0f0f46d96a2. This test is clearly not reliable: https://github.com/ruby/ruby/actions/runs/17446920961/job/49543543423 I want to skip this unstable test on GitHub Actions to make sure this test doesn't prevent PRs from getting merged. We can continue to monitor the state of this test on RubyCI and ci.rvm.jp. --- test/ruby/test_gc.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 }