diff --git a/configure.ac b/configure.ac index b09d549f76a7b8..24fe2b690790f9 100644 --- a/configure.ac +++ b/configure.ac @@ -2822,6 +2822,12 @@ AS_CASE([$coroutine_type], [yes|''], [ [powerpc64le-freebsd*], [ coroutine_type=ppc64le ], + [riscv64-freebsd*], [ + coroutine_type=riscv64 + ], + [arm*-freebsd*], [ + coroutine_type=arm32 + ], [x86_64-netbsd*], [ coroutine_type=amd64 ], diff --git a/cont.c b/cont.c index 0d0f00f60e0f18..42cebf539c93ad 100644 --- a/cont.c +++ b/cont.c @@ -3738,6 +3738,13 @@ ruby_Init_Continuation_body(void) rb_undef_method(CLASS_OF(rb_cContinuation), "new"); rb_define_method(rb_cContinuation, "call", rb_cont_call, -1); rb_define_method(rb_cContinuation, "[]", rb_cont_call, -1); +#ifdef COROUTINE_SHADOW_STACK + if (coroutine_shadow_stack_enabled()) { + /* Continuations cannot restore previously unwound shadow stack frames. */ + rb_define_global_function("callcc", rb_f_notimplement, 0); + return; + } +#endif rb_define_global_function("callcc", rb_callcc, 0); } diff --git a/coroutine/amd64/Context.S b/coroutine/amd64/Context.S index 4b94d31f30839f..d5ff07f8836c3c 100644 --- a/coroutine/amd64/Context.S +++ b/coroutine/amd64/Context.S @@ -11,14 +11,18 @@ #define TOKEN_PASTE(x,y) x##y +#if defined(__linux__) && defined(__CET__) && (__CET__ & 0x02) != 0 +#define COROUTINE_SHADOW_STACK 1 +#endif + .text .globl PREFIXED_SYMBOL(coroutine_transfer) PREFIXED_SYMBOL(coroutine_transfer): #if defined(__CET__) && (__CET__ & 0x01) != 0 - /* IBT landing pad */ - endbr64 + /* IBT landing pad */ + endbr64 #endif # Make space on the stack for 6 registers: @@ -31,6 +35,20 @@ PREFIXED_SYMBOL(coroutine_transfer): movq %r13, 16(%rsp) movq %r14, 8(%rsp) movq %r15, (%rsp) +#if defined(COROUTINE_SHADOW_STACK) + # A zero target SSP means shadow stacks are not enabled at runtime. + movq (%rsi), %rax + cmpq $0, (%rax) + je 1f + + # Save the source shadow stack pointer: + rdsspq %rbp + pushq %rbp + jmp 2f +1: + pushq $0 +2: +#endif # Save caller stack pointer: movq %rsp, (%rdi) @@ -38,6 +56,18 @@ PREFIXED_SYMBOL(coroutine_transfer): # Restore callee stack pointer: movq (%rsi), %rsp +#if defined(COROUTINE_SHADOW_STACK) + # Restore the destination shadow stack pointer: + popq %rbp + testq %rbp, %rbp + jz 3f + rstorssp -8(%rbp) + + # Save the source shadow stack pointer token: + saveprevssp +3: +#endif + # Restore callee state movq 40(%rsp), %rbp movq 32(%rsp), %rbx @@ -55,6 +85,44 @@ PREFIXED_SYMBOL(coroutine_transfer): # We pop the return address and jump to it ret +#if defined(COROUTINE_SHADOW_STACK) +.globl PREFIXED_SYMBOL(coroutine_initialize_shadow_stack) +PREFIXED_SYMBOL(coroutine_initialize_shadow_stack): +#if defined(__CET__) && (__CET__ & 0x01) != 0 + /* IBT landing pad */ + endbr64 +#endif + # Save the current SSP and switch to the new shadow stack. The restore + # token created by map_shadow_stack is immediately below the supplied SSP. + rdsspq %r8 + rstorssp -8(%rdi) + saveprevssp + + # CALL is the only way to place an arbitrary return address on a shadow + # stack without enabling WRSS. Its return address is the trampoline below. + call 4f + +.globl PREFIXED_SYMBOL(coroutine_start_trampoline) +PREFIXED_SYMBOL(coroutine_start_trampoline): +#if defined(__CET__) && (__CET__ & 0x01) != 0 + /* IBT landing pad */ + endbr64 +#endif + jmp *%r12 + +4: + # Remove CALL's entry from the normal stack while retaining its shadow + # stack entry, then remember the resulting SSP for the new coroutine. + popq %rax + rdsspq %rax + + # Restore the original shadow stack. This leaves a restore token for the + # newly initialized stack immediately below the SSP returned in %rax. + rstorssp -8(%r8) + saveprevssp + ret +#endif + #if (defined(__linux__) || defined(__FreeBSD__)) && defined(__ELF__) .section .note.GNU-stack,"",%progbits #endif @@ -67,10 +135,11 @@ PREFIXED_SYMBOL(coroutine_transfer): # define IBT_FLAG 0x00 #endif -/* We do _NOT_ support CET shadow-stack. Do _not_ add the property for - * this to the Context.o object. If you require CET shadow-stack support, - * for now, consider building with --with-coroutine=ucontext */ -#define SHSTK_FLAG 0x00 +#if defined(COROUTINE_SHADOW_STACK) +# define SHSTK_FLAG 0x02 +#else +# define SHSTK_FLAG 0x00 +#endif .pushsection .note.gnu.property, "a" .p2align 3 diff --git a/coroutine/amd64/Context.h b/coroutine/amd64/Context.h index 0deb9b6dd4fb3e..288da653a83c30 100644 --- a/coroutine/amd64/Context.h +++ b/coroutine/amd64/Context.h @@ -48,6 +48,65 @@ enum {COROUTINE_REGISTERS = 6}; #include #endif +#if defined(__linux__) && defined(__CET__) && (__CET__ & 0x02) != 0 +#define COROUTINE_SHADOW_STACK + +#include +#include +#include +#include + +#ifndef ARCH_SHSTK_STATUS +#define ARCH_SHSTK_STATUS 0x5005 +#endif + +#ifndef ARCH_SHSTK_SHSTK +#define ARCH_SHSTK_SHSTK (1UL << 0) +#endif + +#ifndef SYS_map_shadow_stack +#ifdef __NR_map_shadow_stack +#define SYS_map_shadow_stack __NR_map_shadow_stack +#else +#define SYS_map_shadow_stack 453 +#endif +#endif + +#ifndef SHADOW_STACK_SET_TOKEN +#define SHADOW_STACK_SET_TOKEN (1UL << 0) +#endif + +void *coroutine_initialize_shadow_stack(void *shadow_stack_pointer); +COROUTINE coroutine_start_trampoline(void); + +static inline int coroutine_shadow_stack_enabled(void) +{ + unsigned long features = 0; + + if (syscall(SYS_arch_prctl, ARCH_SHSTK_STATUS, &features) != 0) { + return 0; + } + + return (features & ARCH_SHSTK_SHSTK) != 0; +} + +static inline void *coroutine_allocate_shadow_stack(size_t size) +{ + void *base = (void *)syscall( + SYS_map_shadow_stack, + 0, + size, + SHADOW_STACK_SET_TOKEN + ); + + if (base == MAP_FAILED) { + abort(); + } + + return base; +} +#endif + struct coroutine_context { void **stack_pointer; @@ -66,6 +125,11 @@ struct coroutine_context * implicit fiber, owned by TSan; must not be destroyed). */ int tsan_fiber_owned; #endif + +#if defined(COROUTINE_SHADOW_STACK) + void *shadow_stack; + size_t shadow_stack_size; +#endif }; typedef COROUTINE(* coroutine_start)(struct coroutine_context *from, struct coroutine_context *self); @@ -78,6 +142,11 @@ static inline void coroutine_initialize_main(struct coroutine_context * context) context->tsan_fiber = __tsan_get_current_fiber(); context->tsan_fiber_owned = 0; #endif + +#if defined(COROUTINE_SHADOW_STACK) + context->shadow_stack = NULL; + context->shadow_stack_size = 0; +#endif } static inline void coroutine_initialize( @@ -88,6 +157,11 @@ static inline void coroutine_initialize( ) { assert(start && stack && size >= 1024); +#if defined(COROUTINE_SHADOW_STACK) + void *shadow_stack_pointer = NULL; + void *entry = (void *)(uintptr_t)start; +#endif + #if defined(COROUTINE_SANITIZE_ADDRESS) context->fake_stack = NULL; context->stack_base = stack; @@ -99,15 +173,44 @@ static inline void coroutine_initialize( context->tsan_fiber_owned = 1; #endif +#if defined(COROUTINE_SHADOW_STACK) + if (coroutine_shadow_stack_enabled()) { + size_t shadow_stack_size = (size + 7) & ~(size_t)7; + + context->shadow_stack = coroutine_allocate_shadow_stack(shadow_stack_size); + context->shadow_stack_size = shadow_stack_size; + shadow_stack_pointer = coroutine_initialize_shadow_stack( + (char *)context->shadow_stack + shadow_stack_size + ); + entry = (void *)(uintptr_t)coroutine_start_trampoline; + } else { + context->shadow_stack = NULL; + context->shadow_stack_size = 0; + } +#endif + // Stack grows down. Force 16-byte alignment. char * top = (char*)stack + size; context->stack_pointer = (void**)((uintptr_t)top & ~0xF); *--context->stack_pointer = NULL; +#if defined(COROUTINE_SHADOW_STACK) + *--context->stack_pointer = entry; +#else *--context->stack_pointer = (void*)(uintptr_t)start; +#endif context->stack_pointer -= COROUTINE_REGISTERS; memset(context->stack_pointer, 0, sizeof(void*) * COROUTINE_REGISTERS); + +#if defined(COROUTINE_SHADOW_STACK) + if (shadow_stack_pointer) { + /* coroutine_start_trampoline jumps to the start function in r12. */ + context->stack_pointer[3] = (void *)(uintptr_t)start; + } + + *--context->stack_pointer = shadow_stack_pointer; +#endif } struct coroutine_context * coroutine_transfer(struct coroutine_context * current, struct coroutine_context * target); @@ -126,6 +229,13 @@ static inline void coroutine_destroy(struct coroutine_context * context) context->tsan_fiber_owned = 0; } #endif + +#if defined(COROUTINE_SHADOW_STACK) + if (context->shadow_stack) { + munmap(context->shadow_stack, context->shadow_stack_size); + context->shadow_stack = NULL; + } +#endif } #endif /* COROUTINE_AMD64_CONTEXT_H */ diff --git a/coroutine/arm64/Context.S b/coroutine/arm64/Context.S index ce219c0c4d375c..9884ce91ba8a0d 100644 --- a/coroutine/arm64/Context.S +++ b/coroutine/arm64/Context.S @@ -141,20 +141,20 @@ PREFIXED_SYMBOL(coroutine_transfer): # define PAC_FLAG 0 # endif - # The note section format is described by Note Section in Chapter 5 - # of "System V Application Binary Interface, Edition 4.1". - .pushsection .note.gnu.property, "a" - .p2align 3 - .long 0x4 /* Name size ("GNU\0") */ - .long 0x10 /* Descriptor size */ - .long 0x5 /* Type: NT_GNU_PROPERTY_TYPE_0 */ - .asciz "GNU" /* Name */ - # Begin descriptor - .long 0xc0000000 /* Property type: GNU_PROPERTY_AARCH64_FEATURE_1_AND */ - .long 0x4 /* Property size */ - .long (BTI_FLAG|PAC_FLAG) - .long 0x0 /* 8-byte alignment padding */ - # End descriptor - .popsection + # The note section format is described by Note Section in Chapter 5 + # of "System V Application Binary Interface, Edition 4.1". + .pushsection .note.gnu.property, "a" + .p2align 3 + .long 0x4 /* Name size ("GNU\0") */ + .long 0x10 /* Descriptor size */ + .long 0x5 /* Type: NT_GNU_PROPERTY_TYPE_0 */ + .asciz "GNU" /* Name */ + # Begin descriptor + .long 0xc0000000 /* Property type: GNU_PROPERTY_AARCH64_FEATURE_1_AND */ + .long 0x4 /* Property size */ + .long (BTI_FLAG|PAC_FLAG) + .long 0x0 /* 8-byte alignment padding */ + # End descriptor + .popsection #endif #endif diff --git a/coroutine/ppc/Context.S b/coroutine/ppc/Context.S index f44b2419b4dfee..ba87caef86d87f 100644 --- a/coroutine/ppc/Context.S +++ b/coroutine/ppc/Context.S @@ -46,9 +46,12 @@ PREFIXED_SYMBOL(coroutine_transfer): stw r13,72(r1) ; Save return address - ; Possibly should rather be saved into linkage area, see libphobos and IBM docs stw r0,76(r1) + ; Save nonvolatile CR fields in the caller linkage area + mfcr r0 + stw r0,84(r1) + ; Save stack pointer to first argument stw r1,0(r3) @@ -82,6 +85,10 @@ PREFIXED_SYMBOL(coroutine_transfer): ; Set LR mtlr r0 + ; Restore nonvolatile CR2-CR4 + lwz r0,84(r1) + mtcrf 56,r0 + ; Pop stack frame addi r1,r1,80 diff --git a/coroutine/ppc/Context.h b/coroutine/ppc/Context.h index 8035d08556d77d..8e35a799aa9e95 100644 --- a/coroutine/ppc/Context.h +++ b/coroutine/ppc/Context.h @@ -14,7 +14,7 @@ enum { COROUTINE_REGISTERS = 20 /* 19 general purpose registers (r13-r31) and 1 return address */ - + 4 /* space for fiber_entry() to store the link register */ + + 4 /* caller linkage area, including the condition register save slot */ }; struct coroutine_context diff --git a/coroutine/ppc64/Context.S b/coroutine/ppc64/Context.S index 20a47c61c6914e..c21435d48b7fc9 100644 --- a/coroutine/ppc64/Context.S +++ b/coroutine/ppc64/Context.S @@ -45,9 +45,12 @@ PREFIXED_SYMBOL(coroutine_transfer): std r13,144(r1) ; Save return address - ; Possibly should rather be saved into linkage area, see libphobos and IBM docs std r0,152(r1) + ; Save nonvolatile CR fields in the caller linkage area + mfcr r0 + std r0,168(r1) + ; Save stack pointer to first argument std r1,0(r3) @@ -81,6 +84,10 @@ PREFIXED_SYMBOL(coroutine_transfer): ; Set LR mtlr r0 + ; Restore nonvolatile CR2-CR4 + ld r0,168(r1) + mtcrf 56,r0 + ; Pop stack frame addi r1,r1,160 diff --git a/coroutine/ppc64/Context.h b/coroutine/ppc64/Context.h index 085b475ed58563..2cbfee67f048fe 100644 --- a/coroutine/ppc64/Context.h +++ b/coroutine/ppc64/Context.h @@ -13,7 +13,7 @@ enum { COROUTINE_REGISTERS = 20 /* 19 general purpose registers (r13-r31) and 1 return address */ - + 4 /* space for fiber_entry() to store the link register */ + + 4 /* caller linkage area, including the condition register save slot */ }; struct coroutine_context diff --git a/coroutine/ppc64le/Context.S b/coroutine/ppc64le/Context.S index 819264c245ce9b..0ea8bd3b73ae20 100644 --- a/coroutine/ppc64le/Context.S +++ b/coroutine/ppc64le/Context.S @@ -14,7 +14,7 @@ PREFIXED_SYMBOL(coroutine_transfer): .localentry PREFIXED_SYMBOL(coroutine_transfer), .-PREFIXED_SYMBOL(coroutine_transfer) # Make space on the stack for caller registers - addi 1,1,-160 + addi 1,1,-496 # Save caller registers std 14,0(1) @@ -40,9 +40,57 @@ PREFIXED_SYMBOL(coroutine_transfer): mflr 0 std 0,144(1) - # Save caller special register - mfcr 0 - std 0, 152(1) + # Save caller special register + mfcr 0 + std 0, 152(1) + + # Save non-volatile FP registers f14-f31 (ELFv2 callee-saved). GCC keeps + # GPR values in them under pressure (mtvsrdd/mffprd spills). + stfd 14,160(1) + stfd 15,168(1) + stfd 16,176(1) + stfd 17,184(1) + stfd 18,192(1) + stfd 19,200(1) + stfd 20,208(1) + stfd 21,216(1) + stfd 22,224(1) + stfd 23,232(1) + stfd 24,240(1) + stfd 25,248(1) + stfd 26,256(1) + stfd 27,264(1) + stfd 28,272(1) + stfd 29,280(1) + stfd 30,288(1) + stfd 31,296(1) + + # Save non-volatile vector registers v20-v31 (vs52-vs63). Offsets are + # multiples of 16 from a 16-byte aligned r1, as stvx/lvx require. + li 11,304 + stvx 20,1,11 + li 11,320 + stvx 21,1,11 + li 11,336 + stvx 22,1,11 + li 11,352 + stvx 23,1,11 + li 11,368 + stvx 24,1,11 + li 11,384 + stvx 25,1,11 + li 11,400 + stvx 26,1,11 + li 11,416 + stvx 27,1,11 + li 11,432 + stvx 28,1,11 + li 11,448 + stvx 29,1,11 + li 11,464 + stvx 30,1,11 + li 11,480 + stvx 31,1,11 # Save stack pointer to first argument std 1,0(3) @@ -50,6 +98,50 @@ PREFIXED_SYMBOL(coroutine_transfer): # Load stack pointer from second argument ld 1,0(4) + # Restore non-volatile FP and vector registers + lfd 14,160(1) + lfd 15,168(1) + lfd 16,176(1) + lfd 17,184(1) + lfd 18,192(1) + lfd 19,200(1) + lfd 20,208(1) + lfd 21,216(1) + lfd 22,224(1) + lfd 23,232(1) + lfd 24,240(1) + lfd 25,248(1) + lfd 26,256(1) + lfd 27,264(1) + lfd 28,272(1) + lfd 29,280(1) + lfd 30,288(1) + lfd 31,296(1) + li 11,304 + lvx 20,1,11 + li 11,320 + lvx 21,1,11 + li 11,336 + lvx 22,1,11 + li 11,352 + lvx 23,1,11 + li 11,368 + lvx 24,1,11 + li 11,384 + lvx 25,1,11 + li 11,400 + lvx 26,1,11 + li 11,416 + lvx 27,1,11 + li 11,432 + lvx 28,1,11 + li 11,448 + lvx 29,1,11 + li 11,464 + lvx 30,1,11 + li 11,480 + lvx 31,1,11 + # Restore caller registers ld 14,0(1) ld 15,8(1) @@ -74,14 +166,14 @@ PREFIXED_SYMBOL(coroutine_transfer): ld 0,144(1) mtlr 0 - # Load special registers - ld 0,152(1) - # Restore cr register cr2, cr3 and cr4 (field index 3,4,5) - # (field index is 1-based, field 1 = cr0) using a mask (32|16|8 = 56) - mtcrf 56,0 + # Load special registers + ld 0,152(1) + # Restore cr register cr2, cr3 and cr4 (field index 3,4,5) + # (field index is 1-based, field 1 = cr0) using a mask (32|16|8 = 56) + mtcrf 56,0 # Pop stack frame - addi 1,1,160 + addi 1,1,496 # Jump to return address blr diff --git a/coroutine/ppc64le/Context.h b/coroutine/ppc64le/Context.h index 63ea9f19ff96e7..faad98c20badc9 100644 --- a/coroutine/ppc64le/Context.h +++ b/coroutine/ppc64le/Context.h @@ -13,6 +13,8 @@ enum { COROUTINE_REGISTERS = 20 /* 18 general purpose registers (r14-r31), 1 special register (cr) and 1 return address */ + + 18 /* non-volatile FP registers f14-f31 */ + + 24 /* non-volatile vector registers v20-v31, 16 bytes each */ + 4 /* space for fiber_entry() to store the link register */ }; diff --git a/file.c b/file.c index 42ffc6147319bc..112cf393e88395 100644 --- a/file.c +++ b/file.c @@ -3791,20 +3791,21 @@ rb_file_s_symlink(VALUE klass, VALUE from, VALUE to) * :markup: markdown * * call-seq: - * File.readlink(link_path) -> path + * File.readlink(link_path) -> string * - * Returns the string path to the entry referenced by the given `link_path`: + * Returns the string path to the entry referenced + * by the [symbolic link](rdoc-ref:file/symbolic_links.md) at `link_path`: * * ```ruby - * # Create paths. - * file_path = 'doc/extension.rdoc' # => "doc/extension.rdoc" - * target_path = File.join('..', file_path) # => "../doc/extension.rdoc" - * link_path = 'lib/u.tmp' # => "lib/u.tmp" - * File.symlink(target_path, link_path) - * File.readlink(link_path) # => "../doc/extension.rdoc" - * File.delete(link_path) # Clean up. + * filepath = 'README.md' + * linkpath = 'foo' + * File.symlink(filepath, linkpath) + * File.readlink(linkpath) # => "README.md" + * File.unlink(linkpath) # Clean up. * ``` * + * Raises Errno::EINVAL if the entry referenced by `link_path` + * is not a symbolic link. */ static VALUE diff --git a/gc/mmtk/src/api.rs b/gc/mmtk/src/api.rs index fd63c2b94f3856..c1c87db2c725c8 100644 --- a/gc/mmtk/src/api.rs +++ b/gc/mmtk/src/api.rs @@ -43,7 +43,7 @@ pub extern "C" fn mmtk_is_live_object(object: ObjectReference) -> bool { #[no_mangle] pub extern "C" fn mmtk_is_reachable(object: ObjectReference) -> bool { - object.is_reachable() + binding::object_survives_current_gc(object) } // =============== Bootup =============== @@ -178,6 +178,7 @@ fn mmtk_builder_default_parse_plan() -> PlanSelector { "NoGC" => Some(PlanSelector::NoGC), "MarkSweep" => Some(PlanSelector::MarkSweep), "Immix" => Some(PlanSelector::Immix), + "StickyImmix" => Some(PlanSelector::StickyImmix), _ => None, }) .unwrap_or(PlanSelector::Immix) @@ -265,7 +266,7 @@ pub extern "C" fn mmtk_bind_mutator(tls: VMMutatorThread) -> *mut RubyMutator { #[no_mangle] pub unsafe extern "C" fn mmtk_get_bump_pointer_allocator(m: *mut RubyMutator) -> *mut BumpPointer { match *crate::BINDING.get().unwrap().mmtk.get_options().plan { - PlanSelector::Immix => { + PlanSelector::Immix | PlanSelector::StickyImmix => { let mutator: &mut Mutator = unsafe { &mut *m }; let allocator = unsafe { mutator.allocator_mut(mmtk::util::alloc::AllocatorSelector::Immix(0)) }; @@ -398,7 +399,7 @@ pub extern "C" fn mmtk_declare_weak_references(object: ObjectReference) { #[no_mangle] pub extern "C" fn mmtk_weak_references_alive_p(object: ObjectReference) -> bool { - object.is_reachable() + binding::object_survives_current_gc(object) } #[no_mangle] @@ -515,11 +516,13 @@ pub extern "C" fn mmtk_plan() -> *const u8 { static NO_GC: &[u8] = b"NoGC\0"; static MARK_SWEEP: &[u8] = b"MarkSweep\0"; static IMMIX: &[u8] = b"Immix\0"; + static STICKY_IMMIX: &[u8] = b"StickyImmix\0"; match *crate::BINDING.get().unwrap().mmtk.get_options().plan { PlanSelector::NoGC => NO_GC.as_ptr(), PlanSelector::MarkSweep => MARK_SWEEP.as_ptr(), PlanSelector::Immix => IMMIX.as_ptr(), + PlanSelector::StickyImmix => STICKY_IMMIX.as_ptr(), _ => panic!("Unknown plan"), } } diff --git a/gc/mmtk/src/binding.rs b/gc/mmtk/src/binding.rs index cf5bc881441e4f..793e8cbe2786a9 100644 --- a/gc/mmtk/src/binding.rs +++ b/gc/mmtk/src/binding.rs @@ -4,6 +4,7 @@ use std::sync::Mutex; use std::thread::JoinHandle; use mmtk::util::ObjectReference; +use mmtk::vm::ObjectModel; use mmtk::MMTK; use crate::abi; @@ -106,3 +107,37 @@ impl RubyBinding { objects.contains(&object) } } + +pub(crate) fn object_survives_current_gc(object: ObjectReference) -> bool { + let plan = crate::mmtk().get_plan(); + + let is_nursery_gc = plan + .generational() + .is_some_and(|gen| gen.is_current_gc_nursery()); + + if !is_nursery_gc { + return object.is_reachable(); + } + + if !object.is_reachable() { + return false; + } + + if !is_los_object(object) { + return true; + } + + let byte = crate::object_model::VMObjectModel::LOCAL_LOS_MARK_NURSERY_SPEC + .load_atomic::(object, None, std::sync::atomic::Ordering::SeqCst); + const NURSERY_BIT: u8 = 0b10; + byte & NURSERY_BIT == 0 +} + +fn is_los_object(object: ObjectReference) -> bool { + let access = abi::RubyObjectAccess::from_objref(object); + access.payload_size() + abi::OBJREF_OFFSET + > crate::mmtk() + .get_plan() + .constraints() + .max_non_los_default_alloc_bytes +} diff --git a/gc/mmtk/src/heap/ruby_heap_trigger.rs b/gc/mmtk/src/heap/ruby_heap_trigger.rs index 6188c7f0ced62a..e3d7b05c93d1d9 100644 --- a/gc/mmtk/src/heap/ruby_heap_trigger.rs +++ b/gc/mmtk/src/heap/ruby_heap_trigger.rs @@ -43,29 +43,25 @@ impl GCTriggerPolicy for RubyHeapTrigger { fn on_pause_end(&self, mmtk: &'static MMTK) { if let Some(plan) = mmtk.get_plan().generational() { if plan.is_current_gc_nursery() { - // Nursery GC - } else { - // Full GC + return; } + } - panic!("TODO: support for generational GC not implemented") - } else { - let used_pages = mmtk.get_plan().get_used_pages(); - - let target_min = - (used_pages as f64 * (1.0 + Self::get_config().heap_pages_min_ratio)) as usize; - let target_max = - (used_pages as f64 * (1.0 + Self::get_config().heap_pages_max_ratio)) as usize; - let new_target = - (((used_pages as f64) * (1.0 + Self::get_config().heap_pages_goal_ratio)) as usize) - .clamp( - Self::get_config().min_heap_pages, - Self::get_config().max_heap_pages, - ); - - if used_pages < target_min || used_pages > target_max { - self.target_heap_pages.store(new_target, Ordering::Relaxed); - } + let used_pages = mmtk.get_plan().get_used_pages(); + + let target_min = + (used_pages as f64 * (1.0 + Self::get_config().heap_pages_min_ratio)) as usize; + let target_max = + (used_pages as f64 * (1.0 + Self::get_config().heap_pages_max_ratio)) as usize; + let new_target = (((used_pages as f64) * (1.0 + Self::get_config().heap_pages_goal_ratio)) + as usize) + .clamp( + Self::get_config().min_heap_pages, + Self::get_config().max_heap_pages, + ); + + if used_pages < target_min || used_pages > target_max { + self.target_heap_pages.store(new_target, Ordering::Relaxed); } } diff --git a/gc/mmtk/src/pinning_registry.rs b/gc/mmtk/src/pinning_registry.rs index b498b508f1f97b..335d91b368015d 100644 --- a/gc/mmtk/src/pinning_registry.rs +++ b/gc/mmtk/src/pinning_registry.rs @@ -158,7 +158,7 @@ impl GCWork for RemoveDeadPinnings { .expect("PinningRegistry should not have races during GC."); pinning_objs.retain_mut(|obj| { - if obj.is_live() { + if crate::binding::object_survives_current_gc(*obj) { let new_obj = obj.get_forwarded_object().unwrap_or(*obj); *obj = new_obj; true diff --git a/gc/mmtk/src/weak_proc.rs b/gc/mmtk/src/weak_proc.rs index 20b8f5e4e57fcd..b48ce7197e6426 100644 --- a/gc/mmtk/src/weak_proc.rs +++ b/gc/mmtk/src/weak_proc.rs @@ -7,6 +7,7 @@ use mmtk::util::ObjectReference; use mmtk::vm::ObjectTracerContext; use crate::abi::GCThreadTLS; +use crate::binding::object_survives_current_gc; use crate::upcalls; use crate::Ruby; @@ -137,7 +138,7 @@ fn process_obj_free_candidates(obj_free_candidates: &mut Vec) { let mut new_candidates = Vec::new(); for object in obj_free_candidates.iter().copied() { - if object.is_reachable() { + if object_survives_current_gc(object) { // Forward and add back to the candidate list. let new_object = object.forward(); trace!("Forwarding obj_free candidate: {object} -> {new_object}"); @@ -223,7 +224,7 @@ impl ProcessWeakReferences { *object_ptr = object; } - if object.is_reachable() { + if object_survives_current_gc(object) { (upcalls().handle_weak_references)(object, moving_gc); true @@ -302,7 +303,7 @@ impl GCWork for UpdateWbUnprotectedObjectsList { debug!("Updating {} WB-unprotected objects", old_objects.len()); for object in old_objects { - if object.is_reachable() { + if object_survives_current_gc(object) { // Forward and add back to the candidate list. let new_object = object.forward(); trace!("Forwarding WB-unprotected object: {object} -> {new_object}"); diff --git a/pathname_builtin.rb b/pathname_builtin.rb index d7641cb4a7ae48..32a8234f79093b 100644 --- a/pathname_builtin.rb +++ b/pathname_builtin.rb @@ -1768,18 +1768,19 @@ def open(...) # :yield: file # call-seq: # readlink -> new_pathname # - # Returns a new pathname containing the path to the entry represented by `self`: + # Returns a new pathname containing the path stored + # in the [symbolic link](rdoc-ref:file/symbolic_links.md) entry + # at the path stored in `self`: # # ```ruby - # # Create Pathnames. - # file_pn = Pathname('doc/extension.rdoc') # => # - # target_pn = Pathname('..').join(file_pn) # => # - # link_pn = Pathname('lib/u.tmp') # => # - # link_pn.make_symlink(target_pn) - # link_pn.readlink # => # - # link_pn.delete + # file_pn = Pathname('README.md') + # link_pn = Pathname('foo') + # link_pn.make_symlink(file_pn) + # link_pn.readlink # => # + # link_pn.unlink # Clean up. # ``` # + # Raises Errno::EINVAL if the path in `self` is not the path to a symbolic link. def readlink() self.class.new(File.readlink(@path)) end # :markup: markdown @@ -1828,15 +1829,24 @@ def rename(to) File.rename(@path, to) end # :markup: markdown # # call-seq: - # stat -> File::Stat + # stat -> stat # - # Returns a File::Stat object for the entry at the path in `self`: + # Returns a new File::Stat object for the entry at the path in `self`. + # Follows [symbolic links](file/symbolic_links.md); + # therefore if the entry is a symbolic link, + # the returned object contains information for the target entry, not the symbolic link: # # ```ruby - # Pathname('README.md').stat.inspect - # => "#" - # Pathname('doc').stat.inspect - # => "#" + # file_pn = Pathname('README.md') + # link_pn = Pathname('foo') + # link_pn.make_symlink(file_pn) + # # Method stat follows the symlink, so the birthtimes are the same. + # file_pn.stat.birthtime # => 2026-09-07 13:36:38.939798737 -0500 + # link_pn.stat.birthtime # => 2026-09-07 13:36:38.939798737 -0500 + # # Method lstat does not follow the symlink, so the birthtimes are different. + # file_pn.lstat.birthtime # => 2026-09-07 13:36:38.939798737 -0500 + # link_pn.lstat.birthtime # => 2026-09-08 10:53:41.027337999 -0500 + # link_pn.unlink # Clean up. # ``` # def stat() File.stat(@path) end @@ -1845,25 +1855,24 @@ def stat() File.stat(@path) end # :markup: markdown # # call-seq: - # lstat -> new_stat + # lstat -> stat # - # Returns a File::Stat object for the path in `self`; - # does not follow symbolic links, - # and therefore returns the stat object for that path, + # Returns a File::Stat object for the entry at the path in `self`. + # Does not follow [symbolic links](file/symbolic_links.md); + # therefore the returned object contains information for that entry, # regardless of whether it is a symbolic link: # # ```ruby - # File.write('t.tmp', '') - # sleep(1) - # File.symlink('t.tmp', 'link') - # pn = Pathname('link') - # # => # - # # Method stat: follows link to 't.tmp'. - # pn.stat.ctime # => 2026-06-13 15:02:46.562620885 -0500 - # # Method lstat; does not follow link. - # pn.lstat.ctime # => 2026-06-13 15:02:47.563619647 -0500 - # File.delete('t.tmp') - # File.delete('link') + # file_pn = Pathname('README.md') + # link_pn = Pathname('foo') + # link_pn.make_symlink(file_pn) + # # Method stat follows the symlink, so the birthtimes are the same. + # file_pn.stat.birthtime # => 2026-09-07 13:36:38.939798737 -0500 + # link_pn.stat.birthtime # => 2026-09-07 13:36:38.939798737 -0500 + # # Method lstat does not follow the symlink, so the birthtimes are different. + # file_pn.lstat.birthtime # => 2026-09-07 13:36:38.939798737 -0500 + # link_pn.lstat.birthtime # => 2026-09-08 10:53:41.027337999 -0500 + # link_pn.unlink # Clean up. # ``` # def lstat() File.lstat(@path) end diff --git a/string.c b/string.c index 77cae1b4ccdd51..d1828ca5ddea15 100644 --- a/string.c +++ b/string.c @@ -9542,8 +9542,8 @@ tr_trans_pairs(VALUE str, VALUE pairs_val) tr_buffer_append(&buffer, checkpoint, search.s - checkpoint); } tr_buffer_append_str(&buffer, repl); - search.s += clen; - checkpoint = search.s; + checkpoint = search.s + clen; + search.s++; if (cr == ENC_CODERANGE_7BIT && rb_enc_str_coderange(repl) != ENC_CODERANGE_7BIT) { cr = ENC_CODERANGE_VALID; diff --git a/test/ruby/test_string.rb b/test/ruby/test_string.rb index 34a05eba1bce2a..3280bf83df7579 100644 --- a/test/ruby/test_string.rb +++ b/test/ruby/test_string.rb @@ -2689,6 +2689,7 @@ def test_tr_hash assert_equal S(("."*15 + "<" * 17)), S(("."*15 + "x"*17)).tr({"x"=>"<"}) assert_equal(S("01@3456789abcdefgHij"), S("0123456789abcdefghij").tr("h" => "H", "2" => "@")) + assert_equal(S("UL" * 16 ), S("\u2028<" * 16).tr("\u2028" => "U", "<" => "L")) end def test_tr! diff --git a/test/ruby/test_thread.rb b/test/ruby/test_thread.rb index d0182f9a93fef8..e316c410cce828 100644 --- a/test/ruby/test_thread.rb +++ b/test/ruby/test_thread.rb @@ -1687,6 +1687,26 @@ def test_mn_threads_sub_millisecond_sleep end; end + def test_mn_threads_killed_io_waiter_does_not_spin + assert_separately([{'RUBY_MN_THREADS' => '1'}], "#{<<~"begin;"}\n#{<<~'end;'}", timeout: 30) + begin; + r, w = IO.pipe + th = Thread.new { r.read(1) } + sleep 0.1 # let th park in the M:N poller + th.kill + th.join + w.close # r hangs up while the poller still holds a registration for it + + t0 = Process.clock_gettime(Process::CLOCK_PROCESS_CPUTIME_ID) + sleep 0.3 + cpu = Process.clock_gettime(Process::CLOCK_PROCESS_CPUTIME_ID) - t0 + # A spinning timer thread never reaches its timeout branch, which under + # RUBY_MN_THREADS=2 is the only thing that can serve the exiting Ractor. + assert_operator cpu, :<, 0.15, "timer thread spins on an fd nobody waits on" + r.close + end; + end + # [Bug #21926] def test_thread_join_during_finalizers assert_separately([], "#{<<~"begin;"}\n#{<<~'end;'}", timeout: 60) diff --git a/thread_sched_mn.c b/thread_sched_mn.c index 8476e04e5243c8..636bed468966a0 100644 --- a/thread_sched_mn.c +++ b/thread_sched_mn.c @@ -1281,23 +1281,26 @@ fd_waiters_arm(int fd, struct rb_fd_waiters *e, uint32_t want, bool consumed) } #elif HAVE_SYS_EPOLL_H if (want == 0) { - // A delivered oneshot event has already disarmed the fd; otherwise - // disarm by MOD to no events. Either way the registration stays, so - // the next wait is one MOD instead of DEL + ADD. + // A delivered oneshot event has already disarmed the fd; otherwise DEL + // it. MOD to no events would not do: EPOLLHUP and EPOLLERR are + // reported whatever the mask asks for, and without EPOLLONESHOT they + // are reported level-triggered, so a registration left behind by a + // waiter that gave up (kill, interrupt, timeout) spins the timer + // thread once the fd hangs up -- and a spinning timer thread never + // reaches its timeout branch, the only place that mints an snt. if (!consumed && e->registered) { - struct epoll_event off = { .events = 0, .data = { .u64 = 0 } }; - if (epoll_ctl(timer_th.event_fd, EPOLL_CTL_MOD, fd, &off) == -1) { + if (epoll_ctl(timer_th.event_fd, EPOLL_CTL_DEL, fd, NULL) == -1) { switch (errno) { case EBADF: case ENOENT: // the fd is already closed or gone from the set - e->registered = false; break; default: perror("epoll_ctl"); rb_bug("fd_waiters_arm/epoll_ctl disarm failed (fd:%d errno:%d)", fd, errno); } } + e->registered = false; } // Anything epoll_wait already queued for the old arming is stale now. e->generation++; diff --git a/zjit.c b/zjit.c index 741c99c12a0b64..f22c9299c21a29 100644 --- a/zjit.c +++ b/zjit.c @@ -148,6 +148,13 @@ rb_zjit_compile_iseq(const rb_iseq_t *iseq, rb_execution_context_t *ec, bool jit } } +// This is used by a function stub to install compiled code as the ISEQ's entry point. +void +rb_zjit_iseq_set_jit_entry(const rb_iseq_t *iseq, void *code_ptr) +{ + ISEQ_BODY(iseq)->jit_entry = (rb_jit_func_t)code_ptr; +} + extern VALUE *rb_vm_base_ptr(struct rb_control_frame_struct *cfp); // Convert a given ISEQ's instructions to zjit_* instructions @@ -168,6 +175,26 @@ rb_zjit_profile_enable(const rb_iseq_t *iseq) } } +// Return false if a function stub has not collected enough profiles yet, enabling +// profiling instructions as needed. Return true once enough profiles are collected. +bool +rb_zjit_iseq_has_profiled_enough(const rb_iseq_t *iseq) +{ + struct rb_iseq_constant_body *body = ISEQ_BODY(iseq); + + if (body->jit_entry_calls < rb_zjit_profile_threshold) { + // Skip the unprofiled warmup. The compiled caller already establishes + // that the callee is hot, so go straight to the profiling window. + body->jit_entry_calls = rb_zjit_profile_threshold; + rb_zjit_profile_enable(iseq); + } + else { + body->jit_entry_calls++; + } + + return body->jit_entry_calls >= rb_zjit_call_threshold; +} + // Convert a given ISEQ's ZJIT instructions to bare instructions void rb_zjit_profile_disable(const rb_iseq_t *iseq) diff --git a/zjit/bindgen/src/main.rs b/zjit/bindgen/src/main.rs index f1495d92a6f289..99cb4432566d73 100644 --- a/zjit/bindgen/src/main.rs +++ b/zjit/bindgen/src/main.rs @@ -111,6 +111,8 @@ fn main() { .allowlist_function("ruby_executable_node") .allowlist_function("rb_funcallv") .allowlist_function("rb_protect") + .allowlist_function("rb_zjit_iseq_has_profiled_enough") + .allowlist_function("rb_zjit_iseq_set_jit_entry") .allowlist_function("rb_zjit_profile_disable") .allowlist_function("rb_zjit_profile_enable") .allowlist_function("rb_zjit_insn_to_bare_insn") diff --git a/zjit/src/backend/lir.rs b/zjit/src/backend/lir.rs index 82fdeec96176ea..a95d66fe911d7e 100644 --- a/zjit/src/backend/lir.rs +++ b/zjit/src/backend/lir.rs @@ -6,11 +6,11 @@ use std::mem::take; use std::rc::Rc; use crate::bitset::BitSet; use crate::codegen::{perf_symbol_range_start, perf_symbol_range_end, register_with_perf}; -use crate::cruby::{IseqPtr, RUBY_OFFSET_CFP_ISEQ, RUBY_OFFSET_CFP_JIT_RETURN, RUBY_OFFSET_CFP_PC, RUBY_OFFSET_CFP_SP, SIZEOF_VALUE_I32, VALUE, ZJIT_STACK_MAP_BASE_PTR_INDEX_MASK, ZJIT_STACK_MAP_BASE_PTR_SIZE_SHIFT, ZJIT_STACK_MAP_BASE_PTR_TAG, ZJIT_STACK_MAP_SHIFT, ZJIT_STACK_MAP_SKIP_TAG, ZJIT_STACK_MAP_VREG_TAG, vm_stack_canary, YarvInsnIdx, zjit_jit_frame, local_size_and_idx_to_ep_offset}; +use crate::cruby::{IseqPtr, RUBY_OFFSET_CFP_ISEQ, RUBY_OFFSET_CFP_JIT_RETURN, RUBY_OFFSET_CFP_PC, RUBY_OFFSET_CFP_SP, SIZEOF_VALUE_I32, VALUE, ZJIT_STACK_MAP_BASE_PTR_INDEX_MASK, ZJIT_STACK_MAP_BASE_PTR_SIZE_SHIFT, ZJIT_STACK_MAP_BASE_PTR_TAG, ZJIT_STACK_MAP_SHIFT, ZJIT_STACK_MAP_SKIP_TAG, ZJIT_STACK_MAP_VREG_TAG, vm_stack_canary, zjit_jit_frame, local_size_and_idx_to_ep_offset}; use crate::hir::{Invariant, SideExitReason}; use crate::hir; use crate::options::{TraceExits, PerfMap, get_option}; -use crate::payload::{IseqVersionRef, get_or_create_iseq_payload}; +use crate::payload::IseqVersionRef; use crate::stats::{exit_counter_ptr, exit_counter_ptr_for_opcode, side_exit_counter, CompileError}; use crate::virtualmem::CodePtr; use crate::asm::{CodeBlock, Label}; @@ -628,8 +628,7 @@ pub struct SideExit { /// side exit. The current frame's stack and locals are still handled by /// `stack` and `locals` above. pub stack_map: Option, - /// If set, the side exit will profile the current instruction and invalidate - /// the compiled ISEQ for recompilation. + /// If set, the side exit will invalidate the compiled ISEQ for recompilation. pub recompile: Option, } @@ -639,11 +638,6 @@ pub struct SideExitRecompile { /// The compiled unit whose version must be invalidated to force a recompile. For inlined /// methods, this will be the outer function it was inlined into. pub compiled_iseq: Opnd, - /// The exiting frame's ISEQ, which owns the profile entry for `insn_idx`. For - /// an exit out of inlined code this is the inlined callee, not the compiled unit. - pub frame_iseq: Opnd, - /// The exiting frame's instruction index within `frame_iseq`. - pub insn_idx: u32, } /// Payload of `Target::SideExit`, boxed to keep `Target` (and every `Insn` @@ -3165,15 +3159,9 @@ impl Assembler fn compile_exit_recompile(asm: &mut Assembler, exit: &SideExit) { if let Some(recompile) = &exit.recompile { - let payload = get_or_create_iseq_payload(exit.iseq); - payload.reset_profiles_remaining(recompile.insn_idx as YarvInsnIdx); use crate::codegen::exit_recompile; - asm_comment!(asm, "profile and maybe recompile"); - asm_ccall!(asm, exit_recompile, - recompile.compiled_iseq, - recompile.frame_iseq, - recompile.insn_idx.into() - ); + asm_comment!(asm, "invalidate for recompilation"); + asm_ccall!(asm, exit_recompile, recompile.compiled_iseq); } } diff --git a/zjit/src/codegen.rs b/zjit/src/codegen.rs index 043a477c3fea06..b41eaa4c839f24 100644 --- a/zjit/src/codegen.rs +++ b/zjit/src/codegen.rs @@ -680,6 +680,7 @@ fn gen_insn(cb: &mut CodeBlock, jit: &mut JITState, asm: &mut Assembler, functio Insn::StringCopy { val, chilled, state } => gen_string_copy(jit, asm, function, *val, opnd!(val), *chilled, &function.frame_state(*state)), Insn::StringConcat { strings, state } => gen_string_concat(jit, asm, function, opnds!(strings), &function.frame_state(*state)), &Insn::StringGetbyte { string, index } => gen_string_getbyte(asm, opnd!(string), opnd!(index)), + Insn::StringByteslice { string, beg, len, state } => gen_string_byteslice(asm, opnd!(string), opnd!(beg), opnd!(len), &function.frame_state(*state)), Insn::StringSetbyteFixnum { string, index, value } => gen_string_setbyte_fixnum(asm, opnd!(string), opnd!(index), opnd!(value)), Insn::StringAppend { recv, other, state } => gen_string_append(jit, asm, function, opnd!(recv), opnd!(other), &function.frame_state(*state)), Insn::StringAppendCodepoint { recv, other, state } => gen_string_append_codepoint(jit, asm, function, opnd!(recv), opnd!(other), &function.frame_state(*state)), @@ -694,10 +695,10 @@ fn gen_insn(cb: &mut CodeBlock, jit: &mut JITState, asm: &mut Assembler, functio &Insn::Send { cd, block: Some(BlockHandler::BlockArg), state, reason, .. } => gen_send(jit, asm, function, cd, std::ptr::null(), &function.frame_state(state), reason), &Insn::SendForward { cd, blockiseq, state, reason, .. } => gen_send_forward(jit, asm, function, cd, blockiseq, &function.frame_state(state), reason), Insn::SendDirect(insn) => { - let SendDirectData { cme, iseq, recv, args, kw_bits, jit_entry_idx, block, state, .. } = &**insn; + let SendDirectData { cd, cme, iseq, recv, args, kw_bits, jit_entry_idx, block, state, .. } = &**insn; gen_send_iseq_direct( cb, jit, asm, - function, *cme, *iseq, opnd!(recv), opnds!(args), + function, *cd, *cme, *iseq, opnd!(recv), opnds!(args), *kw_bits, *jit_entry_idx, &function.frame_state(*state), *block, ) } @@ -1106,6 +1107,7 @@ fn gen_ccall_with_frame( frame_type: VM_FRAME_MAGIC_CFUNC | VM_FRAME_FLAG_CFRAME | VM_ENV_FLAG_LOCAL, specval: block_handler_specval, write_block_code: false, + forwarded_argc: None, // cfunc doesn't support forwarded arguments }); asm_comment!(asm, "switch to new SP register"); @@ -1196,6 +1198,7 @@ fn gen_ccall_variadic( frame_type: VM_FRAME_MAGIC_CFUNC | VM_FRAME_FLAG_CFRAME | VM_ENV_FLAG_LOCAL, specval: block_handler_specval, write_block_code: false, + forwarded_argc: None, // cfunc doesn't support forwarded arguments }); asm_comment!(asm, "switch to new SP register"); @@ -1698,6 +1701,7 @@ fn gen_push_inline_frame( frame_type, specval, write_block_code: iseq_may_write_block_code(iseq), + forwarded_argc: None, // `can_inline` rejects forwardable callees }); // Publish the inlined callee's entry JITFrame before the inlined body runs. @@ -1783,6 +1787,7 @@ fn gen_send_iseq_direct( jit: &mut JITState, asm: &mut Assembler, function: &Function, + cd: *const rb_call_data, cme: *const rb_callable_method_entry_t, iseq: IseqPtr, recv: Opnd, @@ -1794,7 +1799,14 @@ fn gen_send_iseq_direct( ) -> lir::Opnd { gen_incr_counter(asm, Counter::iseq_optimized_send_count); - let local_size = unsafe { get_iseq_body_local_table_size(iseq) }.to_usize(); + // The ISEQ of `def foo(...)` takes only 1 parameter for the forwarded callinfo, but the callee + // frame's local_size is increased by the callinfo's argc (see vm_call_iseq_forwardable()) to + // keep the caller's arguments as part of the callee's extra locals. + let forwarding = unsafe { rb_get_iseq_flags_forwardable(iseq) }; + let forwarded_argc = if forwarding { args.len() } else { 0 }; + // Bake the callinfo as a GC offset since a non-packed (`vm_ci_packed_p`) callinfo is a movable imemo_callinfo. + let forwarded_ci = Opnd::Value(unsafe { (*cd).ci }.into()); + let local_size = unsafe { get_iseq_body_local_table_size(iseq) }.to_usize() + forwarded_argc; let stack_growth = state.stack_size() + local_size + unsafe { get_iseq_body_stack_max(iseq) }.to_usize(); gen_stack_overflow_check(jit, asm, function, state, stack_growth); @@ -1840,6 +1852,7 @@ fn gen_send_iseq_direct( frame_type, specval, write_block_code: iseq_may_write_block_code(iseq), + forwarded_argc: Some(forwarded_argc), }); // Write "keyword_bits" to the callee's frame if the callee accepts keywords. @@ -1857,6 +1870,19 @@ fn gen_send_iseq_direct( asm.store(Opnd::mem(64, SP, bits_offset as i32), unspecified_bits.into()); } + // A forwardable callee reads its arguments out of the VM stack using memcpy on rb_vm_sendforward + // (see vm_adjust_stack_forwarding()), so we write these stack slots before the call. The callinfo + // goes into the `...` local, which sits directly above them. + if forwarding { + asm_comment!(asm, "copy forwarded arguments to callee frame"); + let locals_base = state.stack().len() - args.len(); + for (idx, &arg) in args.iter().enumerate() { + asm.store(Opnd::mem(64, SP, ((locals_base + idx) * SIZEOF_VALUE) as i32), arg); + } + // The `...` local on top of the above argument is a method parameter of the callee, so + // the callee will spill the callinfo passed as part of `c_args` into the `...` local. + } + asm_comment!(asm, "switch to new SP register"); let sp_offset = (state.stack().len() + local_size - args.len() + VM_ENV_DATA_SIZE.to_usize()) * SIZEOF_VALUE; let new_sp = asm.add(SP, sp_offset.into()); @@ -1880,7 +1906,13 @@ fn gen_send_iseq_direct( 1 /* recv */ + args.len() + if needs_block { 1 } else { 0 } }); c_args.push(recv); - c_args.extend(&args); + if forwarding { + // The JIT entry of a forwardable ISEQ takes exactly one parameter, the `...` local. + // The forwarded arguments were written to the VM stack slots above. + c_args.push(forwarded_ci); + } else { + c_args.extend(&args); + } if needs_block { if callee_is_bmethod { // For bmethods, specval is the captured EP, not the block handler. @@ -1893,7 +1925,8 @@ fn gen_send_iseq_direct( } // Make a method call. The target address will be rewritten once compiled. - let iseq_call = IseqCall::new(iseq, jit_entry_idx, args.len().try_into().expect("checked in HIR")); + let call_argc = if forwarding { 1 } else { args.len() }; + let iseq_call = IseqCall::new(iseq, jit_entry_idx, call_argc.try_into().expect("checked in HIR")); let dummy_ptr = cb.get_write_ptr().raw_ptr(cb); jit.iseq_calls.push(iseq_call.clone()); let ret = asm.ccall_with_iseq_call(dummy_ptr, c_args, &iseq_call); @@ -2045,6 +2078,7 @@ fn gen_invoke_block_iseq_direct( frame_type: VM_FRAME_MAGIC_BLOCK, specval, write_block_code: iseq_may_write_block_code(block_iseq), + forwarded_argc: None, // `...` is not allowed in block arguments }); asm_comment!(asm, "switch to new SP register"); @@ -3558,6 +3592,9 @@ struct ControlFrame { /// Whether to write block_code = 0 at frame push time. /// True when the callee ISEQ may write to block_code (has send/invokesuper/invokeblock). write_block_code: bool, + /// Number of caller arguments a forwardable callee (`def foo(...)`) keeps below + /// the `...` local. `None` for non-forwardable callees. + forwarded_argc: Option, } /// Compile an interpreter frame @@ -3568,7 +3605,7 @@ fn gen_push_frame(asm: &mut Assembler, argc: usize, state: &FrameState, frame: C asm_comment!(asm, "push cme, specval, frame type"); // ep[-2]: cref of cme let local_size = if let Some(iseq) = frame.iseq { - (unsafe { get_iseq_body_local_table_size(iseq) }) as i32 + (unsafe { get_iseq_body_local_table_size(iseq) }) as i32 + frame.forwarded_argc.unwrap_or(0) as i32 } else { 0 }; @@ -3677,8 +3714,6 @@ fn side_exit_with_recompile(jit: &JITState, function: &Function, state: &FrameSt let mut exit = build_side_exit(jit, function, state); exit.recompile = recompile.map(|_| SideExitRecompile { compiled_iseq: Opnd::Value(VALUE::from(jit.iseq())), - frame_iseq: Opnd::Value(VALUE::from(state.iseq)), - insn_idx: state.insn_idx() as u32, }); Target::SideExit(Box::new(SideExitTarget { exit, reason })) } @@ -3737,24 +3772,22 @@ macro_rules! c_callable { pub(crate) use c_callable; c_callable! { - /// Called from JIT side-exit code to profile operands and trigger recompilation. - /// Once enough profiles are gathered, invalidates the compiled unit for recompilation. + /// Called from JIT side-exit code to invalidate the compiled unit for recompilation. /// /// `compiled_iseq_raw` is the ISEQ that was actually compiled. For an exit out /// of inlined code, the inliner folds the callee's body into the outer ISEQ, so /// the outer ISEQ's version holds the failing guard and must be invalidated to /// force a recompile. For non-inlined code, it is the same as the frame ISEQ. /// - /// `frame_iseq_raw` and `insn_idx` identify the instruction this exit came from, - /// whose re-profiling gates the recompile. Both are baked in at compile time, - /// where the exit already knows them, rather than read back out of the control - /// frame: the control frame describes the exiting frame only because the exit - /// wrote its ISEQ and PC there moments earlier, and an exit path that does not - /// write them would silently gate the recompile on an unrelated instruction. - pub(crate) fn exit_recompile(compiled_iseq_raw: VALUE, frame_iseq_raw: VALUE, insn_idx: u32) { + /// The first exit invalidates the version right away. Invalidation resets the + /// ISEQ's call counter and re-stubs incoming JIT-to-JIT calls, so every entry + /// runs the profiling window in the interpreter before the next compile. + /// + /// TODO: Allow waiting for a configured number of exits before invalidating the ISEQ. + pub(crate) fn exit_recompile(compiled_iseq_raw: VALUE) { // Fast check before taking the VM lock: skip if the compiled unit is already // invalidated or at the version limit. This avoids expensive lock acquisition - // on every shape guard exit after the recompile has already been triggered. + // on every shape guard exit taken by frames still running the invalidated code. // The check is on the compiled unit because that is the version we invalidate. { let compiled_iseq: IseqPtr = compiled_iseq_raw.as_iseq(); @@ -3769,24 +3802,11 @@ c_callable! { with_vm_lock(src_loc!(), || { let compiled_iseq: IseqPtr = compiled_iseq_raw.as_iseq(); - - let should_recompile = with_time_stat(Counter::profile_time_ns, || { - get_or_create_iseq_payload(frame_iseq_raw.as_iseq()) - .profile.done_profiling_at(insn_idx as YarvInsnIdx) - }); - - // Once we have enough profiles, invalidate the compiled unit so it - // recompiles and reads the freshly recorded profile. We invalidate - // `compiled_iseq` rather than `frame_iseq` because an inlined callee has no - // compiled code of its own; the outer function it was folded into is what - // actually got compiled. - if should_recompile { - let payload = get_or_create_iseq_payload(compiled_iseq); - if let Some(version) = payload.versions.last_mut() { - let cb = ZJITState::get_code_block(); - invalidate_iseq_version(cb, compiled_iseq, version); - cb.mark_all_executable(); - } + let payload = get_or_create_iseq_payload(compiled_iseq); + if let Some(version) = payload.versions.last_mut() { + let cb = ZJITState::get_code_block(); + invalidate_iseq_version(cb, compiled_iseq, version); + cb.mark_all_executable(); } }); } @@ -3829,7 +3849,7 @@ c_callable! { // JIT-to-JIT calls don't eagerly fill nils to non-parameter locals. // If we side-exit from function_stub_hit (before JIT code runs), we need to set them here. - fn prepare_for_exit(iseq: IseqPtr, cfp: CfpPtr, sp: *mut VALUE, argc: u16, num_opts_filled: u16, compile_error: &CompileError) { + fn prepare_for_exit(iseq: IseqPtr, cfp: CfpPtr, sp: *mut VALUE, argc: u16, num_opts_filled: u16, compile_error: Option<&CompileError>) { unsafe { // Caller frames are materialized by the materialize_exit trampoline before unwinding native frames. // The current frame's pc and iseq are already set by function_stub_hit before this point. @@ -3892,8 +3912,10 @@ c_callable! { } // Increment a compile error counter for --zjit-stats - if get_option!(stats) { - incr_counter_by(exit_counter_for_compile_error(compile_error), 1); + if let Some(compile_error) = compile_error { + if get_option!(stats) { + incr_counter_by(exit_counter_for_compile_error(compile_error), 1); + } } } @@ -3923,10 +3945,18 @@ c_callable! { // We'll use this Rc again, so increment the ref count decremented by from_raw. unsafe { Rc::increment_strong_count(iseq_call_ptr as *const IseqCall); } - prepare_for_exit(iseq, cfp, sp, argc, num_opts_filled, compile_error); + prepare_for_exit(iseq, cfp, sp, argc, num_opts_filled, Some(compile_error)); return ZJITState::get_materialize_exit_trampoline_with_counter().raw_ptr(cb); } + // Exit to the interpreter until the callee ISEQ collects enough profiles. + if !unsafe { rb_zjit_iseq_has_profiled_enough(iseq) } { + // Preserve the reference owned by the stub when iseq_call is dropped. + unsafe { Rc::increment_strong_count(iseq_call_ptr as *const IseqCall); } + prepare_for_exit(iseq, cfp, sp, argc, num_opts_filled, None); + return ZJITState::get_materialize_exit_trampoline().raw_ptr(cb); + } + // Otherwise, attempt to compile the ISEQ. We have to mark_all_executable() beyond this point. let code_ptr = with_time_stat(compile_time_ns, || function_stub_hit_body(cb, &iseq_call)); if code_ptr.is_ok() { @@ -3938,7 +3968,7 @@ c_callable! { // We'll use this Rc again, so increment the ref count decremented by from_raw. unsafe { Rc::increment_strong_count(iseq_call_ptr as *const IseqCall); } - prepare_for_exit(iseq, cfp, sp, argc, num_opts_filled, &compile_error); + prepare_for_exit(iseq, cfp, sp, argc, num_opts_filled, Some(&compile_error)); ZJITState::get_materialize_exit_trampoline_with_counter() }); cb.mark_all_executable(); @@ -3950,10 +3980,13 @@ c_callable! { /// Compile an ISEQ for a function stub fn function_stub_hit_body(cb: &mut CodeBlock, iseq_call: &IseqCallRef) -> Result { // Compile the stubbed ISEQ - let IseqCodePtrs { jit_entry_ptrs, .. } = gen_iseq(cb, iseq_call.iseq.get(), None).inspect_err(|err| { + let IseqCodePtrs { start_ptr, jit_entry_ptrs } = gen_iseq(cb, iseq_call.iseq.get(), None).inspect_err(|err| { debug!("{err:?}: gen_iseq failed: {}", iseq_get_location(iseq_call.iseq.get(), 0)); })?; + // The compile above generated the interpreter entry along with the JIT-to-JIT entries, so install it now. + unsafe { rb_zjit_iseq_set_jit_entry(iseq_call.iseq.get(), start_ptr.raw_ptr(cb) as *mut c_void); } + // Update the stub to call the code pointer let jit_entry_ptr = jit_entry_ptrs[iseq_call.jit_entry_idx.to_usize()]; let code_addr = jit_entry_ptr.raw_ptr(cb); @@ -4220,6 +4253,11 @@ fn gen_string_getbyte(asm: &mut Assembler, string: Opnd, index: Opnd) -> Opnd { asm.or(byte, Opnd::UImm(1)) } +fn gen_string_byteslice(asm: &mut Assembler, string: Opnd, beg: Opnd, len: Opnd, state: &FrameState) -> Opnd { + gen_prepare_leaf_call_with_gc(asm, state); + asm_ccall!(asm, rb_str_byte_substr, string, beg, len) +} + fn gen_string_setbyte_fixnum(asm: &mut Assembler, string: Opnd, index: Opnd, value: Opnd) -> Opnd { // rb_str_setbyte is not leaf, but we guard types and index ranges in HIR asm_ccall!(asm, rb_str_setbyte, string, index, value) diff --git a/zjit/src/codegen_tests.rs b/zjit/src/codegen_tests.rs index 6f92c749bc0b89..56eb208c31e3a0 100644 --- a/zjit/src/codegen_tests.rs +++ b/zjit/src/codegen_tests.rs @@ -6,7 +6,7 @@ use crate::backend::lir::Assembler; use crate::codegen::max_iseq_versions; use crate::cruby::*; use crate::hir::{Insn, iseq_to_hir}; -use crate::options::{get_option, rb_zjit_prepare_options, set_call_threshold, set_inline_threshold, set_max_versions, set_mem_bytes}; +use crate::options::{CallThreshold, get_option, rb_zjit_prepare_options, set_call_threshold, set_inline_threshold, set_max_versions, set_mem_bytes}; use crate::payload::IseqVersion; use crate::hir::tests::hir_build_tests::assert_contains_opcode; use crate::payload::*; @@ -113,6 +113,76 @@ fn test_nil() { "), @"nil"); } +#[test] +fn test_function_stub_profiles_before_compiling() { + rb_zjit_prepare_options(); + set_inline_threshold(0); + let num_profiles = get_option!(num_profiles); + let call_threshold = CallThreshold::from(num_profiles) + 2; + set_call_threshold(call_threshold); + + eval(&format!(" + class Integer + def zjit_profile_stub_target = self + 1 + end + + def zjit_profile_stub_entry(run) + 1.zjit_profile_stub_target if run + end + + i = 0 + while i < {call_threshold} + zjit_profile_stub_entry(false) + i += 1 + end + ")); + + let entry_iseq = get_method_iseq("self", "zjit_profile_stub_entry"); + let entry_payload = get_or_create_iseq_payload(entry_iseq); + let entry_version = unsafe { entry_payload.versions.last().unwrap().as_ref() }; + assert_eq!(1, entry_version.outgoing.len(), "expected a JIT-to-JIT function stub"); + + let target_iseq = get_method_iseq("1", "zjit_profile_stub_target"); + assert!(get_or_create_iseq_payload(target_iseq).versions.is_empty()); + + // The first stub hit should interpret the callee without compiling it. + assert_eq!(VALUE::fixnum_from_usize(2), eval("zjit_profile_stub_entry(true)")); + assert!(get_or_create_iseq_payload(target_iseq).versions.is_empty()); + + // That hit also enabled profiling instructions, so find `+` by looking for the profiling variant. + let mut insn_idx = 0; + let iseq_size = unsafe { get_iseq_encoded_size(target_iseq) }; + let plus_idx = loop { + assert!(insn_idx < iseq_size, "target ISEQ is not profiling opt_plus"); + let opcode = iseq_opcode_at_idx(target_iseq, insn_idx); + if opcode == YARVINSN_zjit_opt_plus { + break insn_idx as usize; + } + insn_idx += insn_len(opcode as usize); + }; + + // Every remaining stub hit in the profiling window should interpret the callee + // without compiling it. + for _ in 1..num_profiles { + assert_eq!(VALUE::fixnum_from_usize(2), eval("zjit_profile_stub_entry(true)")); + assert!(get_or_create_iseq_payload(target_iseq).versions.is_empty()); + } + + // Verify that the interpreted executions populated the profile for `+`. + assert_eq!( + 2, + get_or_create_iseq_payload(target_iseq) + .profile + .get_operand_types(plus_idx) + .unwrap() + .len(), + ); + + // The following hit observes a completed profiling window and compiles. + assert_eq!(VALUE::fixnum_from_usize(2), eval("zjit_profile_stub_entry(true)")); + assert_eq!(1, get_or_create_iseq_payload(target_iseq).versions.len()); +} + #[test] fn test_putobject() { assert_snapshot!(inspect(" @@ -123,27 +193,71 @@ fn test_putobject() { } #[test] -fn test_recompile_exit_waits_for_interpreter_profiles() { +fn test_recompile_exit_invalidates_on_first_exit() { set_call_threshold(2); eval(" - def recompile_profile_window(a, b) = a + b - recompile_profile_window(1, 2) - recompile_profile_window(1, 2) + def recompile_on_first_exit(a, b) = a + b + recompile_on_first_exit(1, 2) + recompile_on_first_exit(1, 2) "); - let iseq = get_method_iseq("self", "recompile_profile_window"); - let num_profiles = get_option!(num_profiles); - for _ in 0..num_profiles { - eval("recompile_profile_window(1.5, 2.5)"); - } + let iseq = get_method_iseq("self", "recompile_on_first_exit"); let payload = get_or_create_iseq_payload(iseq); + assert_eq!(1, payload.versions.len()); assert!(!unsafe { payload.versions.last().unwrap().as_ref() }.is_invalidated()); - eval("recompile_profile_window(1.5, 2.5)"); + // The first recompile exit invalidates the version right away, so subsequent + // calls re-profile every instruction in the interpreter before recompiling. + eval("recompile_on_first_exit(1.5, 2.5)"); let payload = get_or_create_iseq_payload(iseq); + assert_eq!(1, payload.versions.len()); assert!(unsafe { payload.versions.last().unwrap().as_ref() }.is_invalidated()); } +#[test] +fn test_function_stub_reprofiles_after_invalidation() { + rb_zjit_prepare_options(); + set_inline_threshold(0); + let num_profiles = get_option!(num_profiles); + let call_threshold = CallThreshold::from(num_profiles) + 2; + set_call_threshold(call_threshold); + + eval(&format!(" + def stub_reprofile_target(n) = n + 1 + def stub_reprofile_entry(n) = stub_reprofile_target(n) + + i = 0 + while i < {call_threshold} + stub_reprofile_entry(1) + i += 1 + end + ")); + + let target_iseq = get_method_iseq("self", "stub_reprofile_target"); + let target_payload = get_or_create_iseq_payload(target_iseq); + assert_eq!(1, target_payload.versions.len()); + assert!(!unsafe { target_payload.versions.last().unwrap().as_ref() }.is_invalidated()); + + // The first Float argument misses the Fixnum guard in the callee, and the + // recompile exit invalidates the callee right away, re-stubbing the + // JIT-to-JIT call into it. + assert_eq!(Qtrue, eval("stub_reprofile_entry(1.5) == 2.5")); + let target_payload = get_or_create_iseq_payload(target_iseq); + assert_eq!(1, target_payload.versions.len()); + assert!(unsafe { target_payload.versions.last().unwrap().as_ref() }.is_invalidated()); + + // Every stub hit in the profiling window should interpret the invalidated + // callee without recompiling it. + for _ in 0..num_profiles { + assert_eq!(Qtrue, eval("stub_reprofile_entry(1.5) == 2.5")); + assert_eq!(1, get_or_create_iseq_payload(target_iseq).versions.len()); + } + + // The following hit observes a completed profiling window and recompiles. + assert_eq!(Qtrue, eval("stub_reprofile_entry(1.5) == 2.5")); + assert_eq!(2, get_or_create_iseq_payload(target_iseq).versions.len()); +} + #[test] fn test_dupstring() { eval(r##" @@ -446,6 +560,231 @@ fn test_kwargs_with_max_direct_send_arg_count() { "), @"[[1, 2, 3, 4, 5, 6, 7, 8]]"); } +#[test] +fn test_forwardable_callee_positional_args() { + assert_snapshot!(inspect(" + def target(a, b) = a + b + def fwd(...) = target(...) + 5.times.map { fwd(1, 2) }.uniq + "), @"[3]"); +} + +#[test] +fn test_forwardable_callee_no_args() { + assert_snapshot!(inspect(" + def target = :ok + def fwd(...) = target(...) + 5.times.map { fwd }.uniq + "), @"[:ok]"); +} + +#[test] +fn test_forwardable_callee_kwargs() { + assert_snapshot!(inspect(" + def target(a, b:, c: 3) = [a, b, c] + def fwd(...) = target(...) + 5.times.flat_map { [fwd(1, b: 2), fwd(1, c: 9, b: 2)] }.uniq + "), @"[[1, 2, 3], [1, 2, 9]]"); +} + +#[test] +fn test_forwardable_callee_wrong_number_of_arguments() { + assert_snapshot!(inspect(r#" + def target(a, b) = a + b + def fwd(...) = target(...) + 5.times.map { (fwd(1) rescue $!.message) }.uniq + "#), @r#"["wrong number of arguments (given 1, expected 2)"]"#); +} + +#[test] +fn test_forwardable_callee_unknown_keyword() { + assert_snapshot!(inspect(r#" + def target(a, b:) = [a, b] + def fwd(...) = target(...) + 5.times.map { (fwd(1, z: 2) rescue $!.message) }.uniq + "#), @r#"["missing keyword: :b"]"#); +} + +#[test] +fn test_forwardable_callee_literal_block() { + assert_snapshot!(inspect(" + def target(x) = yield(x) + def fwd(...) = target(...) + 5.times.map { fwd(4) { |v| v * 2 } }.uniq + "), @"[8]"); +} + +// Enabling a TracePoint invalidates the callee's PatchPoint NoTracePoint, so the interpreter takes +// over the frame that the direct send pushed and runs the forwarding call itself. +#[test] +fn test_forwardable_callee_side_exit() { + assert_snapshot!(inspect(" + def target(a, b:) = [a, b] + def fwd(...) = target(...) + def call_fwd = fwd(1, b: 2) + 5.times { call_fwd } + tp = TracePoint.new(:line) { |_| } + tp.enable { 5.times.map { call_fwd }.uniq } + "), @"[[1, 2]]"); +} + +#[test] +fn test_forwardable_callee_block_arg_proc() { + assert_snapshot!(inspect(" + def target(x) = yield(x) + def fwd(...) = target(...) + block = proc { |v| v * 2 } + 5.times.map { fwd(4, &block) }.uniq + "), @"[8]"); +} + +#[test] +fn test_forwardable_callee_block_arg_lambda() { + assert_snapshot!(inspect(" + def target(x) = yield(x) + def fwd(...) = target(...) + block = ->(v) { v * 3 } + 5.times.map { fwd(4, &block) }.uniq + "), @"[12]"); +} + +#[test] +fn test_forwardable_callee_block_arg_symbol() { + assert_snapshot!(inspect(r#" + def target(x) = yield(x) + def fwd(...) = target(...) + 5.times.map { fwd("hello", &:upcase) }.uniq + "#), @r#"["HELLO"]"#); +} + +#[test] +fn test_forwardable_callee_block_arg_method() { + assert_snapshot!(inspect(" + def target(x) = yield(x) + def fwd(...) = target(...) + def double(v) = v * 2 + 5.times.map { fwd(4, &method(:double)) }.uniq + "), @"[8]"); +} + +#[test] +fn test_forwardable_callee_block_arg_to_proc() { + assert_snapshot!(inspect(" + class Doubler + def to_proc = proc { |v| v * 2 } + end + def target(x) = yield(x) + def fwd(...) = target(...) + doubler = Doubler.new + 5.times.map { fwd(4, &doubler) }.uniq + "), @"[8]"); +} + +#[test] +fn test_forwardable_callee_block_arg_nil() { + assert_snapshot!(inspect(" + def target(x) = block_given? ? yield(x) : [:no_block, x] + def fwd(...) = target(...) + 5.times.map { fwd(4, &nil) }.uniq + "), @"[[:no_block, 4]]"); +} + +#[test] +fn test_forwardable_callee_block_arg_not_callable() { + assert_snapshot!(inspect(r#" + def target(x) = yield(x) + def fwd(...) = target(...) + 5.times.map { (fwd(4, &42) rescue $!.class) }.uniq + "#), @"[TypeError]"); +} + +#[test] +fn test_forwardable_callee_splat_call_site_stays_dynamic() { + assert_snapshot!(inspect(" + def target(*a, **k) = [a, k] + def fwd(...) = target(...) + args = [1, 2] + opts = { x: 1 } + 5.times.flat_map { [fwd(*args), fwd(**opts), fwd(&nil)] }.uniq + "), @"[[[1, 2], {}], [[], {x: 1}], [[], {}]]"); +} + +#[test] +fn test_forwardable_callee_ruby2_keywords_flag_survives() { + assert_snapshot!(inspect(" + def target(*a, **k) = [a, k] + def fwd(...) = target(...) + ruby2_keywords def r2k(*a) = fwd(*a) + 5.times.map { r2k(1, k: 2) }.uniq + "), @"[[[1], {k: 2}]]"); +} + +#[test] +fn test_forwardable_callee_chained_forwarding() { + assert_snapshot!(inspect(" + def target(a, b:) = [a, b] + def inner(...) = target(...) + def outer(...) = inner(...) + 5.times.map { outer(1, b: 2) }.uniq + "), @"[[1, 2]]"); +} + +#[test] +fn test_forwardable_callee_with_extra_locals() { + assert_snapshot!(inspect(" + def target(a) = a * 2 + def fwd(...) + extra = 10 + extra + target(...) + end + 5.times.map { fwd(3) }.uniq + "), @"[16]"); +} + +#[test] +fn test_forwardable_callee_super() { + assert_snapshot!(inspect(r#" + class Base + def run(*a, **k) = ["base", a, k] + end + class Child < Base + def run(...) = super + end + c = Child.new + 5.times.map { c.run(1, k: 2) }.uniq + "#), @r#"[["base", [1], {k: 2}]]"#); +} + +#[test] +fn test_explicit_super_to_forwardable_callee() { + assert_snapshot!(inspect(r#" + class Base + def run(...) = fin(...) + def fin(a, b) = ["base", a, b] + end + class Child < Base + def run(a, b) = super(a, b) + end + c = Child.new + 5.times.map { c.run(1, 2) }.uniq + "#), @r#"[["base", 1, 2]]"#); +} + +#[test] +fn test_zsuper_to_forwardable_callee() { + assert_snapshot!(inspect(r#" + class Base + def run(...) = fin(...) + def fin(a, b) = ["base", a, b] + end + class Child < Base + def run(a, b) = super + end + c = Child.new + 5.times.map { c.run(3, 4) }.uniq + "#), @r#"[["base", 3, 4]]"#); +} + #[test] fn test_setlocal_on_eval() { assert_snapshot!(inspect(" @@ -4858,6 +5197,57 @@ fn test_array_pop_arg() { "), @"[33, 42]"); } +#[test] +fn test_string_byteslice_basic() { + assert_snapshot!(inspect(r#" + def test(s, beg, len) = s.byteslice(beg, len) + test("hello", 1, 3) + test("hello", 1, 3) + "#), @r#""ell""#); +} + +#[test] +fn test_string_byteslice_out_of_range_returns_nil() { + assert_snapshot!(inspect(r#" + def test(s, beg, len) = s.byteslice(beg, len) + test("hello", 1, 3) + test("hello", 1, 3) + test("hello", 6, 1) + "#), @"nil"); +} + +#[test] +fn test_string_byteslice_one_arg() { + assert_snapshot!(inspect(r#" + def test(s, beg) = s.byteslice(beg) + test("hello", 1) + test("hello", 1) + "#), @r#""e""#); +} + +#[test] +fn test_string_byteslice_three_args_raises() { + assert_snapshot!(inspect(r#" + def test(s, beg, len, extra) + s.byteslice(beg, len, extra) + rescue ArgumentError + "ArgumentError" + end + test("hello", 1, 3, 5) + test("hello", 1, 3, 5) + "#), @r#""ArgumentError""#); +} + +#[test] +fn test_string_byteslice_bignum_arg_falls_back() { + assert_snapshot!(inspect(r#" + def test(s, beg, len) = s.byteslice(beg, len) + fixnum_result = test("hello", 0, 3) + bignum_result = test("hello", 0, 2**62) + [fixnum_result, bignum_result] + "#), @r#"["hel", "hello"]"#); +} + #[test] fn test_new_range_inclusive() { assert_snapshot!(inspect(" diff --git a/zjit/src/cruby.rs b/zjit/src/cruby.rs index 9212fbc89f9b3f..c7bb63fbfe9820 100644 --- a/zjit/src/cruby.rs +++ b/zjit/src/cruby.rs @@ -887,6 +887,12 @@ impl From<*const rb_callable_method_entry_t> for VALUE { } } +impl From<*const rb_callinfo> for VALUE { + fn from(ci: *const rb_callinfo) -> Self { + VALUE(ci as usize) + } +} + impl From<&str> for VALUE { fn from(value: &str) -> Self { rust_str_to_ruby(value) diff --git a/zjit/src/cruby_bindings.inc.rs b/zjit/src/cruby_bindings.inc.rs index e0161a42373949..d235b7071e490e 100644 --- a/zjit/src/cruby_bindings.inc.rs +++ b/zjit/src/cruby_bindings.inc.rs @@ -2454,7 +2454,12 @@ unsafe extern "C" { pub fn rb_zjit_reserve_low_addr_space(size: usize) -> *mut ::std::os::raw::c_void; pub fn rb_zjit_profile_disable(iseq: *const rb_iseq_t); pub fn rb_zjit_insn_to_bare_insn(insn: ::std::os::raw::c_int) -> ::std::os::raw::c_int; + pub fn rb_zjit_iseq_set_jit_entry( + iseq: *const rb_iseq_t, + code_ptr: *mut ::std::os::raw::c_void, + ); pub fn rb_vm_base_ptr(cfp: *mut rb_control_frame_struct) -> *mut VALUE; + pub fn rb_zjit_iseq_has_profiled_enough(iseq: *const rb_iseq_t) -> bool; pub fn rb_zjit_iseq_insn_set( iseq: *const rb_iseq_t, insn_idx: ::std::os::raw::c_uint, diff --git a/zjit/src/cruby_methods.rs b/zjit/src/cruby_methods.rs index 7becc9eb5536c4..4dcd7ca71854c3 100644 --- a/zjit/src/cruby_methods.rs +++ b/zjit/src/cruby_methods.rs @@ -221,6 +221,7 @@ pub fn init() -> Annotations { annotate!(rb_cString, "size", types::Fixnum, no_gc, leaf, elidable); annotate!(rb_cString, "length", types::Fixnum, no_gc, leaf, elidable); annotate!(rb_cString, "getbyte", inline_string_getbyte); + annotate!(rb_cString, "byteslice", inline_string_byteslice); annotate!(rb_cString, "setbyte", inline_string_setbyte); annotate!(rb_cString, "empty?", inline_string_empty_p, types::BoolExact, no_gc, leaf, elidable); annotate!(rb_cString, "<<", inline_string_append); @@ -509,6 +510,17 @@ fn inline_string_getbyte(fun: &mut hir::Function, block: hir::BlockId, recv: hir None } +fn inline_string_byteslice(fun: &mut hir::Function, block: hir::BlockId, recv: hir::InsnId, args: &[hir::InsnId], state: hir::InsnId) -> Option { + let &[beg, len] = args else { return None; }; + if fun.likely_a(beg, types::Fixnum, state) && fun.likely_a(len, types::Fixnum, state) { + let beg = fun.coerce_to(block, beg, types::Fixnum, state); + let len = fun.coerce_to(block, len, types::Fixnum, state); + Some(fun.push_insn(block, hir::Insn::StringByteslice { string: recv, beg, len, state })) + } else { + None + } +} + fn inline_string_setbyte(fun: &mut hir::Function, block: hir::BlockId, recv: hir::InsnId, args: &[hir::InsnId], state: hir::InsnId) -> Option { let &[index, value] = args else { return None; }; if fun.likely_a(index, types::Fixnum, state) && fun.likely_a(value, types::Fixnum, state) { diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index 23dddf35e6133f..714df07b6a4080 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -1010,6 +1010,8 @@ pub enum Insn { StringConcat { strings: Vec, state: InsnId }, /// Call rb_str_getbyte with known-Fixnum index StringGetbyte { string: InsnId, index: InsnId }, + /// Call rb_str_byte_substr with known-Fixnum beg/len + StringByteslice { string: InsnId, beg: InsnId, len: InsnId, state: InsnId }, StringSetbyteFixnum { string: InsnId, index: InsnId, value: InsnId }, StringAppend { recv: InsnId, other: InsnId, state: InsnId }, StringAppendCodepoint { recv: InsnId, other: InsnId, state: InsnId }, @@ -1430,6 +1432,12 @@ macro_rules! for_each_operand_impl { $visit_one!(*string); $visit_one!(*index); } + Insn::StringByteslice { string, beg, len, state } => { + $visit_one!(*string); + $visit_one!(*beg); + $visit_one!(*len); + $visit_one!(*state); + } Insn::StringSetbyteFixnum { string, index, value } => { $visit_one!(*string); $visit_one!(*index); @@ -1756,6 +1764,7 @@ impl Insn { Insn::StringIntern { .. } => effects::Any, Insn::StringConcat { .. } => effects::Any, Insn::StringGetbyte { .. } => Effect::read_write(abstract_heaps::Other, abstract_heaps::Empty), + Insn::StringByteslice { .. } => allocates.union(Effect::read(abstract_heaps::Other)), Insn::StringSetbyteFixnum { .. } => effects::Any, Insn::StringAppend { .. } => effects::Any, Insn::StringAppendCodepoint { .. } => effects::Any, @@ -2151,6 +2160,9 @@ impl<'a> std::fmt::Display for InsnPrinter<'a> { Insn::StringGetbyte { string, index, .. } => { write!(f, "StringGetbyte {string}, {index}") } + Insn::StringByteslice { string, beg, len, .. } => { + write!(f, "StringByteslice {string}, {beg}, {len}") + } Insn::StringSetbyteFixnum { string, index, value, .. } => { write!(f, "StringSetbyteFixnum {string}, {index}, {value}") } @@ -2651,6 +2663,19 @@ pub enum ValidationError { MiscValidationError(InsnId, String), } +/// Set of flags incompatible with direct sends to forwardable callees. +const FORWARDABLE_CALLEE_BLOCKERS: u32 = + // `gen_send_iseq_direct` currently handles only the interpreter's `vm_call_iseq_forwardable` + // fastpath case on forwardable ISEQs: pass non-`...` arguments to a `...` callee, which sets + // the callinfo of non-`...` arguments into the callee's local variable `...`. + // + // On the other hand, that fastpath and `gen_send_iseq_direct` don't handle the VM_CALL_FORWARDING + // case: pass `...` to a `...` callee, which sets the caller's callinfo into the callee's `...` + // local variable. It needs to be specialized differently. + VM_CALL_FORWARDING + // We only support `def foo(...)` cases for now. + | VM_CALL_ARGS_SPLAT | VM_CALL_KW_SPLAT | VM_CALL_ARGS_BLOCKARG; + /// Check if we can emit SendDirect to the given ISEQ with the given arguments. fn can_direct_send(iseq: *const rb_iseq_t, caller_args: &CallerArguments, has_block: bool, caller_splat: Option) -> Result<(), SendDirectFailure> { let mut complex_arg_counters = vec![]; @@ -2661,7 +2686,9 @@ fn can_direct_send(iseq: *const rb_iseq_t, caller_args: &CallerArguments, has_bl let caller_passes_block_arg = has_block && (caller_args.flags & VM_CALL_ARGS_BLOCKARG) != 0; use Counter::*; - if 0 != params.flags.forwardable() { count_failure(complex_arg_pass_param_forwardable) } + let forwardable = 0 != params.flags.forwardable(); + if forwardable && caller_args.flags & FORWARDABLE_CALLEE_BLOCKERS != 0 + { count_failure(complex_arg_pass_param_forwardable) } if callee_has_block_param && caller_passes_block_arg { count_failure(complex_arg_pass_param_block) } if 0 != params.flags.has_kwrest() { count_failure(complex_arg_pass_param_kwrest) } @@ -2683,6 +2710,16 @@ fn can_direct_send(iseq: *const rb_iseq_t, caller_args: &CallerArguments, has_bl )); } + // A forwardable callee has a single `...` parameter that takes the caller's arguments, and its frame is + // grown by exactly the call site's argument count, so none of the parameter matching below applies to it. + if forwardable { + // `IseqCall` stores argc as u16, and the callee frame has to fit the copied arguments. + if u16::try_from(caller_args.original.len()).is_err() { + return Err(SendDirectFailure::new(OperandTooLarge)); + } + return Ok(()); + } + let lead_num = params.lead_num; let opt_num = params.opt_num; let post_num = params.post_num; @@ -3629,6 +3666,7 @@ impl Function { Insn::StringIntern { .. } => types::Symbol, Insn::StringConcat { .. } => types::StringExact, Insn::StringGetbyte { .. } => types::Fixnum, + Insn::StringByteslice { .. } => types::StringExact.union(types::NilClass), Insn::StringSetbyteFixnum { .. } => types::Fixnum, Insn::StringAppend { .. } => types::StringExact, Insn::StringAppendCodepoint { .. } => types::StringExact, @@ -3664,9 +3702,20 @@ impl Function { Insn::FixnumAdd { .. } => types::Fixnum, Insn::FixnumSub { .. } => types::Fixnum, Insn::FixnumMult { .. } => types::Fixnum, - // FIXNUM_MIN / -1 overflows to a Bignum, so the result is Integer, not Fixnum. - // Downstream Fixnum ops insert their own GuardType(Fixnum) - Insn::FixnumDiv { .. } => types::Integer, + Insn::FixnumDiv { left, right, .. } => { + let left = self.type_of(*left).fixnum_value(); + let right = self.type_of(*right).fixnum_value(); + + // FIXNUM_MIN / -1 overflows to a Bignum, but no other combination does. If we know + // that either operand does not match that case, we can safely assume Fixnum. + if left.is_some_and(|left| left != RUBY_FIXNUM_MIN as i64) + || right.is_some_and(|right| right != -1) + { + types::Fixnum + } else { + types::Integer + } + } Insn::FixnumMod { .. } => types::Fixnum, Insn::FloatAdd { .. } => types::Float, Insn::FloatSub { .. } => types::Float, @@ -3911,6 +3960,14 @@ impl Function { /// Validate and normalize SendDirect arguments without emitting HIR. fn build_send_direct_args(&self, caller_args: &CallerArguments, caller_splat: Option, iseq: IseqPtr, has_block: bool) -> Result { can_direct_send(iseq, caller_args, has_block, caller_splat)?; + // A forwardable callee takes the caller's arguments as is. + if 0 != unsafe { iseq.params() }.flags.forwardable() { + return Ok(SendDirectCall { + args: caller_args.original.iter().copied().map(SendDirectArg::Existing).collect(), + kw_bits: 0, + jit_entry_idx: 0, + }); + } let args = Self::expand_caller_splat_args(caller_args, caller_splat); let (args, kw_bits) = Self::plan_send_direct_keyword_arguments(args, caller_args, iseq) .map_err(SendDirectFailure::new)?; @@ -8012,6 +8069,11 @@ impl Function { self.assert_subtype(insn_id, string, types::String)?; self.assert_subtype(insn_id, index, types::CInt64) }, + Insn::StringByteslice { string, beg, len, .. } => { + self.assert_subtype(insn_id, string, types::String)?; + self.assert_subtype(insn_id, beg, types::Fixnum)?; + self.assert_subtype(insn_id, len, types::Fixnum) + }, Insn::StringSetbyteFixnum { string, index, value } => { self.assert_subtype(insn_id, string, types::String)?; self.assert_subtype(insn_id, index, types::Fixnum)?; diff --git a/zjit/src/hir/opt_tests.rs b/zjit/src/hir/opt_tests.rs index 3658f484b3828d..6b0a32d2992dad 100644 --- a/zjit/src/hir/opt_tests.rs +++ b/zjit/src/hir/opt_tests.rs @@ -450,7 +450,7 @@ mod hir_opt_tests { v10:Fixnum[7] = Const Value(7) v12:Fixnum[0] = Const Value(0) PatchPoint MethodRedefined(Integer@0x1000, /@0x1008, cme:0x1010) - v23:Integer = FixnumDiv v10, v12 + v23:Fixnum = FixnumDiv v10, v12 CheckInterrupts Return v23 "); @@ -691,7 +691,7 @@ mod hir_opt_tests { v15:Fixnum[6] = Const Value(6) PatchPoint MethodRedefined(Integer@0x1008, /@0x1010, cme:0x1018) v26:Fixnum = GuardType v10, Fixnum recompile - v27:Integer = FixnumDiv v26, v15 + v27:Fixnum = FixnumDiv v26, v15 CheckInterrupts Return v27 "); @@ -722,12 +722,105 @@ mod hir_opt_tests { v15:Fixnum[-8] = Const Value(-8) PatchPoint MethodRedefined(Integer@0x1008, /@0x1010, cme:0x1018) v26:Fixnum = GuardType v10, Fixnum recompile + v27:Fixnum = FixnumDiv v26, v15 + CheckInterrupts + Return v27 + "); + } + + #[test] + fn test_fixnum_div_unknown_left_by_negative_one_returns_integer() { + eval(" + def test(n) + n / -1 + end + test 1; test 2 + "); + assert_snapshot!(hir_string("test"), @" + fn test@:3: + bb1(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + v2:CPtr = LoadSP + v3:BasicObject = LoadField v2, :n@0x1000 + Jump bb3(v1, v3) + bb2(): + EntryPoint JIT(0) + v6:BasicObject = LoadArg :self@0 + v7:BasicObject = LoadArg :n@1 + Jump bb3(v6, v7) + bb3(v9:BasicObject, v10:BasicObject): + v15:Fixnum[-1] = Const Value(-1) + PatchPoint MethodRedefined(Integer@0x1008, /@0x1010, cme:0x1018) + v26:Fixnum = GuardType v10, Fixnum recompile v27:Integer = FixnumDiv v26, v15 CheckInterrupts Return v27 "); } + #[test] + fn test_fixnum_div_fixnum_min_left_unknown_right_returns_integer() { + eval(&format!(" + def test(n) + {RUBY_FIXNUM_MIN} / n + end + test 1; test 2 + ")); + assert_snapshot!(hir_string("test"), @" + fn test@:3: + bb1(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + v2:CPtr = LoadSP + v3:BasicObject = LoadField v2, :n@0x1000 + Jump bb3(v1, v3) + bb2(): + EntryPoint JIT(0) + v6:BasicObject = LoadArg :self@0 + v7:BasicObject = LoadArg :n@1 + Jump bb3(v6, v7) + bb3(v9:BasicObject, v10:BasicObject): + v14:Fixnum[-4611686018427387904] = Const Value(-4611686018427387904) + PatchPoint MethodRedefined(Integer@0x1008, /@0x1010, cme:0x1018) + v27:Fixnum = GuardType v10, Fixnum + v28:Integer = FixnumDiv v14, v27 + CheckInterrupts + Return v28 + "); + } + + #[test] + fn test_fixnum_div_non_min_left_unknown_right_returns_fixnum() { + eval(" + def test(n) + 7 / n + end + test 1; test 2 + "); + assert_snapshot!(hir_string("test"), @" + fn test@:3: + bb1(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + v2:CPtr = LoadSP + v3:BasicObject = LoadField v2, :n@0x1000 + Jump bb3(v1, v3) + bb2(): + EntryPoint JIT(0) + v6:BasicObject = LoadArg :self@0 + v7:BasicObject = LoadArg :n@1 + Jump bb3(v6, v7) + bb3(v9:BasicObject, v10:BasicObject): + v14:Fixnum[7] = Const Value(7) + PatchPoint MethodRedefined(Integer@0x1008, /@0x1010, cme:0x1018) + v27:Fixnum = GuardType v10, Fixnum + v28:Fixnum = FixnumDiv v14, v27 + CheckInterrupts + Return v28 + "); + } + #[test] fn test_fold_fixnum_mod_zero_by_zero() { eval(" @@ -10406,9 +10499,15 @@ mod hir_opt_tests { StoreField v6, :@foo@0x1002, v10 Jump bb4() bb6(): - v22:CShape[0x1003] = GuardBitEquals v15, CShape(0x1003) recompile + v22:CShape[0x1003] = Const CShape(0x1003) + v23:CBool = IsBitEqual v15, v22 + CondBranch v23, bb7(), bb8() + bb7(): StoreField v6, :@foo@0x1004, v10 Jump bb4() + bb8(): + SetIvar v6, :@foo, v10 + Jump bb4() bb4(): CheckInterrupts Return v10 @@ -13052,6 +13151,183 @@ mod hir_opt_tests { "); } + #[test] + fn test_optimize_string_byteslice_fixnum() { + eval(r#" + def test(s, beg, len) = s.byteslice(beg, len) + test("foo", 0, 1) + "#); + assert_snapshot!(hir_string("test"), @" + fn test@:2: + bb1(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + v2:CPtr = LoadSP + v3:BasicObject = LoadField v2, :s@0x1000 + v4:BasicObject = LoadField v2, :beg@0x1001 + v5:BasicObject = LoadField v2, :len@0x1002 + Jump bb3(v1, v3, v4, v5) + bb2(): + EntryPoint JIT(0) + v8:BasicObject = LoadArg :self@0 + v9:BasicObject = LoadArg :s@1 + v10:BasicObject = LoadArg :beg@2 + v11:BasicObject = LoadArg :len@3 + Jump bb3(v8, v9, v10, v11) + bb3(v13:BasicObject, v14:BasicObject, v15:BasicObject, v16:BasicObject): + PatchPoint NoSingletonClass(String@0x1008) + PatchPoint MethodRedefined(String@0x1008, byteslice@0x1010, cme:0x1018) + v32:StringExact = GuardType v14, StringExact recompile + v33:Fixnum = GuardType v15, Fixnum + v34:Fixnum = GuardType v16, Fixnum + v35:StringExact|NilClass = StringByteslice v32, v33, v34 + CheckInterrupts + Return v35 + "); + } + + #[test] + fn test_do_not_optimize_string_byteslice_non_fixnum() { + eval(r#" + def test(s, beg, len) = s.byteslice(beg, len) + test("foo", 0.0, 1.0) + "#); + assert_snapshot!(hir_string("test"), @" + fn test@:2: + bb1(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + v2:CPtr = LoadSP + v3:BasicObject = LoadField v2, :s@0x1000 + v4:BasicObject = LoadField v2, :beg@0x1001 + v5:BasicObject = LoadField v2, :len@0x1002 + Jump bb3(v1, v3, v4, v5) + bb2(): + EntryPoint JIT(0) + v8:BasicObject = LoadArg :self@0 + v9:BasicObject = LoadArg :s@1 + v10:BasicObject = LoadArg :beg@2 + v11:BasicObject = LoadArg :len@3 + Jump bb3(v8, v9, v10, v11) + bb3(v13:BasicObject, v14:BasicObject, v15:BasicObject, v16:BasicObject): + PatchPoint NoSingletonClass(String@0x1008) + PatchPoint MethodRedefined(String@0x1008, byteslice@0x1010, cme:0x1018) + v32:StringExact = GuardType v14, StringExact recompile + v33:BasicObject = CCallVariadic v32, :String#byteslice@0x1040, v15, v16 + CheckInterrupts + Return v33 + "); + } + + #[test] + fn test_do_not_optimize_string_byteslice_one_arg() { + eval(r#" + def test(s, beg) = s.byteslice(beg) + test("foo", 0) + "#); + assert_snapshot!(hir_string("test"), @" + fn test@:2: + bb1(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + v2:CPtr = LoadSP + v3:BasicObject = LoadField v2, :s@0x1000 + v4:BasicObject = LoadField v2, :beg@0x1001 + Jump bb3(v1, v3, v4) + bb2(): + EntryPoint JIT(0) + v7:BasicObject = LoadArg :self@0 + v8:BasicObject = LoadArg :s@1 + v9:BasicObject = LoadArg :beg@2 + Jump bb3(v7, v8, v9) + bb3(v11:BasicObject, v12:BasicObject, v13:BasicObject): + PatchPoint NoSingletonClass(String@0x1008) + PatchPoint MethodRedefined(String@0x1008, byteslice@0x1010, cme:0x1018) + v28:StringExact = GuardType v12, StringExact recompile + v29:BasicObject = CCallVariadic v28, :String#byteslice@0x1040, v13 + CheckInterrupts + Return v29 + "); + } + + #[test] + fn test_do_not_optimize_string_byteslice_three_args() { + eval(r#" + def test(s, beg, len, extra) + s.byteslice(beg, len, extra) + rescue ArgumentError + nil + end + test("foo", 0, 1, 2) + "#); + assert_snapshot!(hir_string("test"), @" + fn test@:3: + bb1(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + v2:CPtr = LoadSP + v3:BasicObject = LoadField v2, :s@0x1000 + v4:BasicObject = LoadField v2, :beg@0x1001 + v5:BasicObject = LoadField v2, :len@0x1002 + v6:BasicObject = LoadField v2, :extra@0x1003 + Jump bb3(v1, v3, v4, v5, v6) + bb2(): + EntryPoint JIT(0) + v9:BasicObject = LoadArg :self@0 + v10:BasicObject = LoadArg :s@1 + v11:BasicObject = LoadArg :beg@2 + v12:BasicObject = LoadArg :len@3 + v13:BasicObject = LoadArg :extra@4 + Jump bb3(v9, v10, v11, v12, v13) + bb3(v15:BasicObject, v16:BasicObject, v17:BasicObject, v18:BasicObject, v19:BasicObject): + PatchPoint NoSingletonClass(String@0x1008) + PatchPoint MethodRedefined(String@0x1008, byteslice@0x1010, cme:0x1018) + v37:StringExact = GuardType v16, StringExact recompile + v38:BasicObject = CCallVariadic v37, :String#byteslice@0x1040, v17, v18, v19 + CheckInterrupts + Return v38 + "); + } + + #[test] + fn test_string_byteslice_result_may_be_nil() { + eval(r#" + def test(s, beg, len) = s.byteslice(beg, len).length + test("foo", 0, 1) + "#); + assert_snapshot!(hir_string("test"), @" + fn test@:2: + bb1(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + v2:CPtr = LoadSP + v3:BasicObject = LoadField v2, :s@0x1000 + v4:BasicObject = LoadField v2, :beg@0x1001 + v5:BasicObject = LoadField v2, :len@0x1002 + Jump bb3(v1, v3, v4, v5) + bb2(): + EntryPoint JIT(0) + v8:BasicObject = LoadArg :self@0 + v9:BasicObject = LoadArg :s@1 + v10:BasicObject = LoadArg :beg@2 + v11:BasicObject = LoadArg :len@3 + Jump bb3(v8, v9, v10, v11) + bb3(v13:BasicObject, v14:BasicObject, v15:BasicObject, v16:BasicObject): + PatchPoint NoSingletonClass(String@0x1008) + PatchPoint MethodRedefined(String@0x1008, byteslice@0x1010, cme:0x1018) + v35:StringExact = GuardType v14, StringExact recompile + v36:Fixnum = GuardType v15, Fixnum + v37:Fixnum = GuardType v16, Fixnum + v38:StringExact|NilClass = StringByteslice v35, v36, v37 + PatchPoint NoSingletonClass(String@0x1008) + PatchPoint MethodRedefined(String@0x1008, length@0x1040, cme:0x1048) + v42:StringExact = GuardType v38, StringExact recompile + v43:Fixnum = CCall v42, :String#length@0x1070 + CheckInterrupts + Return v43 + "); + } + #[test] fn test_elide_string_getbyte_fixnum() { eval(r#" @@ -17004,9 +17280,139 @@ mod hir_opt_tests { v4:BasicObject = LoadArg :self@0 Jump bb3(v4) bb3(v6:BasicObject): - v11:BasicObject = Send v6, :forwardable # SendFallbackReason: Complex argument passing + PatchPoint MethodRedefined(Object@0x1000, forwardable@0x1008, cme:0x1010) + v18:ObjectSubclass[class_exact*:Object@VALUE(0x1000)] = GuardType v6, ObjectSubclass[class_exact*:Object@VALUE(0x1000)] recompile + v19:BasicObject = SendDirect v18, 0x0, :forwardable (0x1038) CheckInterrupts - Return v11 + Return v19 + "); + } + + #[test] + fn call_method_forwardable_param_with_args() { + eval(" + def target(a, b, k:) = [a, b, k] + def forwardable(...) = target(...) + def call_forwardable = forwardable(1, 2, k: 3) + call_forwardable + "); + assert_snapshot!(hir_string("call_forwardable"), @" + fn call_forwardable@:4: + bb1(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + Jump bb3(v1) + bb2(): + EntryPoint JIT(0) + v4:BasicObject = LoadArg :self@0 + Jump bb3(v4) + bb3(v6:BasicObject): + v11:Fixnum[1] = Const Value(1) + v13:Fixnum[2] = Const Value(2) + v15:Fixnum[3] = Const Value(3) + PatchPoint MethodRedefined(Object@0x1000, forwardable@0x1008, cme:0x1010) + v24:ObjectSubclass[class_exact*:Object@VALUE(0x1000)] = GuardType v6, ObjectSubclass[class_exact*:Object@VALUE(0x1000)] recompile + v25:BasicObject = SendDirect v24, 0x0, :forwardable (0x1038), v11, v13, v15 + CheckInterrupts + Return v25 + "); + } + + // A literal block goes through a different path than `&block`: it stays a direct send, + // with the blockiseq (the second SendDirect operand, 0x0 without a block) passed along. + #[test] + fn call_method_forwardable_param_with_block_literal() { + eval(" + def target(a) = yield(a) + def forwardable(...) = target(...) + def call_forwardable = forwardable(1) { |v| v } + call_forwardable + "); + assert_snapshot!(hir_string("call_forwardable"), @" + fn call_forwardable@:4: + bb1(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + Jump bb3(v1) + bb2(): + EntryPoint JIT(0) + v4:BasicObject = LoadArg :self@0 + Jump bb3(v4) + bb3(v6:BasicObject): + v11:Fixnum[1] = Const Value(1) + PatchPoint MethodRedefined(Object@0x1000, forwardable@0x1008, cme:0x1010) + v20:ObjectSubclass[class_exact*:Object@VALUE(0x1000)] = GuardType v6, ObjectSubclass[class_exact*:Object@VALUE(0x1000)] recompile + v21:BasicObject = SendDirect v20, 0x1038, :forwardable (0x1058), v11 + CheckInterrupts + Return v21 + "); + } + + #[test] + fn call_method_forwardable_param_with_splat() { + eval(" + def forwardable(...) = itself(...) + def call_forwardable(args) = forwardable(*args) + call_forwardable([]) + "); + assert_snapshot!(hir_string("call_forwardable"), @" + fn call_forwardable@:3: + bb1(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + v2:CPtr = LoadSP + v3:BasicObject = LoadField v2, :args@0x1000 + Jump bb3(v1, v3) + bb2(): + EntryPoint JIT(0) + v6:BasicObject = LoadArg :self@0 + v7:BasicObject = LoadArg :args@1 + Jump bb3(v6, v7) + bb3(v9:BasicObject, v10:BasicObject): + v16:ArrayExact = ToArray v10 + v18:BasicObject = Send v9, :forwardable, v16 # SendFallbackReason: Complex argument passing + CheckInterrupts + Return v18 + "); + } + + #[test] + fn call_super_to_forwardable_param() { + eval(" + class SuperFwdBase + def run(...) = fin(...) + def fin(a, b) = [a, b] + end + class SuperFwdChild < SuperFwdBase + def run(a, b) = super(a, b) + end + SuperFwdChild.new.run(1, 2) + "); + assert_snapshot!(hir_string_proc("SuperFwdChild.instance_method(:run)"), @" + fn run@:7: + bb1(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + v2:CPtr = LoadSP + v3:BasicObject = LoadField v2, :a@0x1000 + v4:BasicObject = LoadField v2, :b@0x1001 + Jump bb3(v1, v3, v4) + bb2(): + EntryPoint JIT(0) + v7:BasicObject = LoadArg :self@0 + v8:BasicObject = LoadArg :a@1 + v9:BasicObject = LoadArg :b@2 + Jump bb3(v7, v8, v9) + bb3(v11:BasicObject, v12:BasicObject, v13:BasicObject): + PatchPoint MethodRedefined(SuperFwdBase@0x1008, run@0x1010, cme:0x1018) + v27:CPtr = GetEP 0 + v28:RubyValue = LoadField v27, :VM_ENV_DATA_INDEX_ME_CREF@0x1040 + v29:CallableMethodEntry[VALUE(0x1048)] = GuardBitEquals v28, Value(VALUE(0x1048)) + v30:RubyValue = LoadField v27, :VM_ENV_DATA_INDEX_SPECVAL@0x1050 + v31:FalseClass = GuardBitEquals v30, Value(false) + v32:BasicObject = SendDirect v11, 0x0, :run (0x1058), v12, v13 + CheckInterrupts + Return v32 "); } @@ -20856,7 +21262,10 @@ mod hir_opt_tests { #[test] fn test_trigger_guard_type_recompilation() { - set_max_versions(2); + // The first call transitions C's shape by defining @a, so the setivar shape + // guard misses once and already spends a version during the Fixnum phase. + // Leave room for one more version so the Float phase can recompile. + set_max_versions(3); set_inline_threshold(0); eval(" class C @@ -20961,7 +21370,7 @@ mod hir_opt_tests { v8:BasicObject = LoadArg :x@1 Jump bb3(v7, v8) bb3(v11:HeapBasicObject, v12:BasicObject): - v90:NilClass = Const Value(nil) + v94:NilClass = Const Value(nil) v17:Fixnum[1] = Const Value(1) PatchPoint SingleRactorMode v21:CShape = LoadField v11, :shape_id@0x1001 @@ -21002,9 +21411,11 @@ mod hir_opt_tests { v89:Float = FloatAdd v57, v44 Jump bb9(v89) bb13(): - v60:BasicObject = Send v12, :+, v44 # SendFallbackReason: Send: polymorphic call site - Jump bb9(v60) - bb9(v47:BasicObject): + PatchPoint MethodRedefined(Integer@0x1008, +@0x1010, cme:0x1018) + v92:Fixnum = GuardType v12, Fixnum recompile + v93:Fixnum = FixnumAdd v92, v44 + Jump bb9(v93) + bb9(v47:Float|Fixnum): PatchPoint SingleRactorMode v69:CShape = LoadField v11, :shape_id@0x1001 v70:CShape[0x1002] = Const CShape(0x1002) diff --git a/zjit/src/payload.rs b/zjit/src/payload.rs index b10e97324afeac..988b842990b347 100644 --- a/zjit/src/payload.rs +++ b/zjit/src/payload.rs @@ -3,7 +3,6 @@ use std::ptr::NonNull; use crate::codegen::IseqCallRef; use crate::stats::CompileError; use crate::{cruby::*, profile::IseqProfile, virtualmem::CodePtr}; -use crate::options::get_option; pub use crate::jit_frame::JITFrame; @@ -36,14 +35,6 @@ impl IseqPayload { self_is_heap_object: false, } } - - /// Profile counts are used for compilation policy. - /// When we deoptimize a method that can be recompiled, we need to update the count to collect more profiles. - /// Otherwise, we will generate the same code that was just deoptimized. - pub fn reset_profiles_remaining(&mut self, insn_idx: YarvInsnIdx) { - let num_profiles = get_option!(num_profiles); - self.profile.entry_mut(insn_idx).set_profiles_remaining(num_profiles); - } } /// JIT code version. When the same ISEQ is compiled with a different assumption, a new version is created. diff --git a/zjit/src/profile.rs b/zjit/src/profile.rs index f4a69d3afec3a0..9e1c62d98dbd01 100644 --- a/zjit/src/profile.rs +++ b/zjit/src/profile.rs @@ -408,12 +408,6 @@ pub struct ProfileEntry { profiles_remaining: NumProfiles, } -impl ProfileEntry { - pub fn set_profiles_remaining(&mut self, num_profiles: NumProfiles) { - self.profiles_remaining = num_profiles; - } -} - #[derive(Debug)] pub struct IseqProfile { /// Sparse storage of per-instruction profile data, sorted by instruction index. @@ -459,11 +453,6 @@ impl IseqProfile { .ok().map(|i| &self.entries[i]) } - /// Check if enough profiles have been gathered for this instruction. - pub fn done_profiling_at(&self, insn_idx: YarvInsnIdx) -> bool { - self.entry(insn_idx).map_or(false, |e| e.profiles_remaining == 0) - } - /// Get profiled operand types for a given instruction index pub fn get_operand_types(&self, insn_idx: YarvInsnIdx) -> Option<&[TypeDistribution]> { self.entry(insn_idx).map(|e| e.opnd_types.as_slice()).filter(|s| !s.is_empty())