From 6e2906f60da20d6cd057aa8ad4b84f8c988406d9 Mon Sep 17 00:00:00 2001 From: Max Bernstein Date: Fri, 21 Nov 2025 13:43:52 -0500 Subject: [PATCH] ZJIT: Don't make GuardNotFrozen consider immediates --- zjit/src/codegen.rs | 10 +--------- zjit/src/cruby_methods.rs | 2 ++ zjit/src/hir.rs | 8 ++++++-- 3 files changed, 9 insertions(+), 11 deletions(-) diff --git a/zjit/src/codegen.rs b/zjit/src/codegen.rs index 944f9504a2..3300219ccd 100644 --- a/zjit/src/codegen.rs +++ b/zjit/src/codegen.rs @@ -654,20 +654,12 @@ fn gen_guard_block_param_proxy(jit: &JITState, asm: &mut Assembler, level: u32, } fn gen_guard_not_frozen(jit: &JITState, asm: &mut Assembler, recv: Opnd, state: &FrameState) -> Opnd { - let side_exit = side_exit(jit, state, GuardNotFrozen); let recv = asm.load(recv); - // Side-exit if recv is false - assert_eq!(Qfalse.as_i64(), 0); - asm.test(recv, recv); - asm.jz(side_exit.clone()); - // Side-exit if recv is immediate - asm.test(recv, (RUBY_IMMEDIATE_MASK as u64).into()); - asm.jnz(side_exit.clone()); // It's a heap object, so check the frozen flag let flags = asm.load(Opnd::mem(64, recv, RUBY_OFFSET_RBASIC_FLAGS)); asm.test(flags, (RUBY_FL_FREEZE as u64).into()); // Side-exit if frozen - asm.jnz(side_exit.clone()); + asm.jnz(side_exit(jit, state, GuardNotFrozen)); recv } diff --git a/zjit/src/cruby_methods.rs b/zjit/src/cruby_methods.rs index 6d09b5e5a7..23c05e6d58 100644 --- a/zjit/src/cruby_methods.rs +++ b/zjit/src/cruby_methods.rs @@ -330,6 +330,7 @@ fn inline_array_push(fun: &mut hir::Function, block: hir::BlockId, recv: hir::In fn inline_array_pop(fun: &mut hir::Function, block: hir::BlockId, recv: hir::InsnId, args: &[hir::InsnId], state: hir::InsnId) -> Option { // Only inline the case of no arguments. let &[] = args else { return None; }; + // We know that all Array are HeapObject, so no need to insert a GuardType(HeapObject). let arr = fun.push_insn(block, hir::Insn::GuardNotFrozen { recv, state }); Some(fun.push_insn(block, hir::Insn::ArrayPop { array: arr, state })) } @@ -391,6 +392,7 @@ fn inline_string_setbyte(fun: &mut hir::Function, block: hir::BlockId, recv: hir let unboxed_index = fun.push_insn(block, hir::Insn::GuardLess { left: unboxed_index, right: len, state }); let zero = fun.push_insn(block, hir::Insn::Const { val: hir::Const::CInt64(0) }); let _ = fun.push_insn(block, hir::Insn::GuardGreaterEq { left: unboxed_index, right: zero, state }); + // We know that all String are HeapObject, so no need to insert a GuardType(HeapObject). let recv = fun.push_insn(block, hir::Insn::GuardNotFrozen { recv, state }); let _ = fun.push_insn(block, hir::Insn::StringSetbyteFixnum { string: recv, index, value }); // String#setbyte returns the fixnum provided as its `value` argument back to the caller. diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index 3a69dd6610..d6ccf9160e 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -903,7 +903,8 @@ pub enum Insn { /// Side-exit if the block param has been modified or the block handler for the frame /// is neither ISEQ nor ifunc, which makes it incompatible with rb_block_param_proxy. GuardBlockParamProxy { level: u32, state: InsnId }, - /// Side-exit if val is frozen. + /// Side-exit if val is frozen. Does *not* check if the val is an immediate; assumes that it is + /// a heap object. GuardNotFrozen { recv: InsnId, state: InsnId }, /// Side-exit if left is not greater than or equal to right (both operands are C long). GuardGreaterEq { left: InsnId, right: InsnId, state: InsnId }, @@ -2553,6 +2554,7 @@ impl Function { // // No need for a GuardShape. if let OptimizedMethodType::StructAset = opt_type { + // We know that all Struct are HeapObject, so no need to insert a GuardType(HeapObject). recv = self.push_insn(block, Insn::GuardNotFrozen { recv, state }); } @@ -4150,7 +4152,6 @@ impl Function { | Insn::IsNil { val } | Insn::IsMethodCfunc { val, .. } | Insn::GuardShape { val, .. } - | Insn::GuardNotFrozen { recv: val, .. } | Insn::SetGlobal { val, .. } | Insn::SetLocal { val, .. } | Insn::SetClassVar { val, .. } @@ -4169,6 +4170,9 @@ impl Function { | Insn::DefinedIvar { self_val: val, .. } => { self.assert_subtype(insn_id, val, types::BasicObject) } + Insn::GuardNotFrozen { recv, .. } => { + self.assert_subtype(insn_id, recv, types::HeapBasicObject) + } // Instructions with 2 Ruby object operands Insn::SetIvar { self_val: left, val: right, .. } | Insn::NewRange { low: left, high: right, .. }