Skip to content

Commit 5571212

Browse files
tekknolagitekknolagi
authored
ZJIT: Remove specialized instructions (ruby#16034)
* ZJIT: Fix codegen for GuardSuperMethodEntry We need to register the VALUE in the generated code so that it is visible to the GC. This may be the cause of a crash in the test suite that Alan found. * ZJIT: Use LoadField+GuardBitEquals instead of GuardSuperMethodEntry We have modular instructions. Use them. * ZJIT: Use LoadField instead of GetBlockHandler We have modular instructions. Use them. * ZJIT: Remove unused HIR instructions We don't use GuardSuperMethodEntry, GuardShape, or GetBlockHandler anymore. * ZJIT: Replace GuardNotFrozen with LoadField+GuardNoBitsSet * ZJIT: Replace GuardNotShared with LoadField+GuardNoBitsSet * ZJIT: Remove unused HIR instructions * ZJIT: Remove comment
1 parent 540ea72 commit 5571212

5 files changed

Lines changed: 101 additions & 176 deletions

File tree

‎zjit/src/codegen.rs‎

Lines changed: 0 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -538,12 +538,8 @@ fn gen_insn(cb: &mut CodeBlock, jit: &mut JITState, asm: &mut Assembler, functio
538538
&Insn::GuardBitEquals { val, expected, reason, state } => gen_guard_bit_equals(jit, asm, opnd!(val), expected, reason, &function.frame_state(state)),
539539
&Insn::GuardAnyBitSet { val, mask, reason, state } => gen_guard_any_bit_set(jit, asm, opnd!(val), mask, reason, &function.frame_state(state)),
540540
&Insn::GuardNoBitsSet { val, mask, reason, state } => gen_guard_no_bits_set(jit, asm, opnd!(val), mask, reason, &function.frame_state(state)),
541-
Insn::GuardNotFrozen { recv, state } => gen_guard_not_frozen(jit, asm, opnd!(recv), &function.frame_state(*state)),
542-
Insn::GuardNotShared { recv, state } => gen_guard_not_shared(jit, asm, opnd!(recv), &function.frame_state(*state)),
543541
&Insn::GuardLess { left, right, state } => gen_guard_less(jit, asm, opnd!(left), opnd!(right), &function.frame_state(state)),
544542
&Insn::GuardGreaterEq { left, right, state } => gen_guard_greater_eq(jit, asm, opnd!(left), opnd!(right), &function.frame_state(state)),
545-
&Insn::GuardSuperMethodEntry { lep, cme, state } => no_output!(gen_guard_super_method_entry(jit, asm, opnd!(lep), cme, &function.frame_state(state))),
546-
Insn::GetBlockHandler { lep } => gen_get_block_handler(asm, opnd!(lep)),
547543
Insn::PatchPoint { invariant, state } => no_output!(gen_patch_point(jit, asm, invariant, &function.frame_state(*state))),
548544
Insn::CCall { cfunc, recv, args, name, return_type: _, elidable: _ } => gen_ccall(asm, *cfunc, *name, opnd!(recv), opnds!(args)),
549545
// Give up CCallWithFrame for 7+ args since asm.ccall() supports at most 6 args (recv + args).
@@ -585,7 +581,6 @@ fn gen_insn(cb: &mut CodeBlock, jit: &mut JITState, asm: &mut Assembler, functio
585581
&Insn::ToArray { val, state } => { gen_to_array(jit, asm, opnd!(val), &function.frame_state(state)) },
586582
&Insn::DefinedIvar { self_val, id, pushval, .. } => { gen_defined_ivar(asm, opnd!(self_val), id, pushval) },
587583
&Insn::ArrayExtend { left, right, state } => { no_output!(gen_array_extend(jit, asm, opnd!(left), opnd!(right), &function.frame_state(state))) },
588-
&Insn::GuardShape { val, shape, state } => gen_guard_shape(jit, asm, opnd!(val), shape, &function.frame_state(state)),
589584
Insn::LoadPC => gen_load_pc(asm),
590585
Insn::LoadEC => gen_load_ec(),
591586
&Insn::GetEP { level } => gen_get_ep(asm, level),
@@ -795,25 +790,6 @@ fn gen_getblockparam(jit: &mut JITState, asm: &mut Assembler, ep_offset: u32, le
795790
asm.load(Opnd::mem(VALUE_BITS, ep, offset))
796791
}
797792

798-
fn gen_guard_not_frozen(jit: &JITState, asm: &mut Assembler, recv: Opnd, state: &FrameState) -> Opnd {
799-
let recv = asm.load(recv);
800-
// It's a heap object, so check the frozen flag
801-
let flags = asm.load(Opnd::mem(64, recv, RUBY_OFFSET_RBASIC_FLAGS));
802-
asm.test(flags, (RUBY_FL_FREEZE as u64).into());
803-
// Side-exit if frozen
804-
asm.jnz(side_exit(jit, state, GuardNotFrozen));
805-
recv
806-
}
807-
808-
fn gen_guard_not_shared(jit: &JITState, asm: &mut Assembler, recv: Opnd, state: &FrameState) -> Opnd {
809-
let recv = asm.load(recv);
810-
// It's a heap object, so check the shared flag
811-
let flags = asm.load(Opnd::mem(VALUE_BITS, recv, RUBY_OFFSET_RBASIC_FLAGS));
812-
asm.test(flags, (RUBY_ELTS_SHARED as u64).into());
813-
asm.jnz(side_exit(jit, state, SideExitReason::GuardNotShared));
814-
recv
815-
}
816-
817793
fn gen_guard_less(jit: &JITState, asm: &mut Assembler, left: Opnd, right: Opnd, state: &FrameState) -> Opnd {
818794
asm.cmp(left, right);
819795
asm.jge(side_exit(jit, state, SideExitReason::GuardLess));
@@ -826,28 +802,6 @@ fn gen_guard_greater_eq(jit: &JITState, asm: &mut Assembler, left: Opnd, right:
826802
left
827803
}
828804

829-
/// Guard that the method entry at ep[VM_ENV_DATA_INDEX_ME_CREF] matches the expected CME.
830-
/// This ensures we're calling super from the expected method context.
831-
fn gen_guard_super_method_entry(
832-
jit: &JITState,
833-
asm: &mut Assembler,
834-
lep: Opnd,
835-
cme: *const rb_callable_method_entry_t,
836-
state: &FrameState,
837-
) {
838-
asm_comment!(asm, "guard super method entry");
839-
let ep_me_opnd = Opnd::mem(64, lep, SIZEOF_VALUE_I32 * VM_ENV_DATA_INDEX_ME_CREF);
840-
let ep_me = asm.load(ep_me_opnd);
841-
asm.cmp(ep_me, Opnd::UImm(cme as u64));
842-
asm.jne(side_exit(jit, state, SideExitReason::GuardSuperMethodEntry));
843-
}
844-
845-
/// Get the block handler from ep[VM_ENV_DATA_INDEX_SPECVAL] at the local EP (LEP).
846-
fn gen_get_block_handler(asm: &mut Assembler, lep: Opnd) -> Opnd {
847-
asm_comment!(asm, "get block handler from LEP");
848-
asm.load(Opnd::mem(64, lep, SIZEOF_VALUE_I32 * VM_ENV_DATA_INDEX_SPECVAL))
849-
}
850-
851805
fn gen_get_constant_path(jit: &JITState, asm: &mut Assembler, ic: *const iseq_inline_constant_cache, state: &FrameState) -> Opnd {
852806
unsafe extern "C" {
853807
fn rb_vm_opt_getconstant_path(ec: EcPtr, cfp: CfpPtr, ic: *const iseq_inline_constant_cache) -> VALUE;
@@ -1243,16 +1197,6 @@ fn gen_array_extend(jit: &mut JITState, asm: &mut Assembler, left: Opnd, right:
12431197
asm_ccall!(asm, rb_ary_concat, left, right);
12441198
}
12451199

1246-
fn gen_guard_shape(jit: &mut JITState, asm: &mut Assembler, val: Opnd, shape: ShapeId, state: &FrameState) -> Opnd {
1247-
gen_incr_counter(asm, Counter::guard_shape_count);
1248-
let shape_id_offset = unsafe { rb_shape_id_offset() };
1249-
let val = asm.load(val);
1250-
let shape_opnd = Opnd::mem(SHAPE_ID_NUM_BITS as u8, val, shape_id_offset);
1251-
asm.cmp(shape_opnd, Opnd::UImm(shape.0 as u64));
1252-
asm.jne(side_exit(jit, state, SideExitReason::GuardShape(shape)));
1253-
val
1254-
}
1255-
12561200
fn gen_load_pc(asm: &mut Assembler) -> Opnd {
12571201
asm.load(Opnd::mem(64, CFP, RUBY_OFFSET_CFP_PC))
12581202
}

‎zjit/src/cruby.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1398,6 +1398,9 @@ pub(crate) mod ids {
13981398
name: _shape_id
13991399
name: _env_data_index_flags
14001400
name: _env_data_index_specval
1401+
name: _ep_method_entry
1402+
name: _ep_specval
1403+
name: _rbasic_flags
14011404
}
14021405

14031406
/// Get an CRuby `ID` to an interned string, e.g. a particular method name.

‎zjit/src/cruby_methods.rs‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -351,8 +351,8 @@ fn inline_array_aset(fun: &mut hir::Function, block: hir::BlockId, recv: hir::In
351351
{
352352
let recv = fun.coerce_to(block, recv, types::ArrayExact, state);
353353
let index = fun.coerce_to(block, index, types::Fixnum, state);
354-
let recv = fun.push_insn(block, hir::Insn::GuardNotFrozen { recv, state });
355-
let recv = fun.push_insn(block, hir::Insn::GuardNotShared { recv, state });
354+
fun.guard_not_frozen(block, recv, state);
355+
fun.guard_not_shared(block, recv, state);
356356

357357
// Bounds check: unbox Fixnum index and guard 0 <= idx < length.
358358
let index = fun.push_insn(block, hir::Insn::UnboxFixnum { val: index });
@@ -381,9 +381,8 @@ fn inline_array_push(fun: &mut hir::Function, block: hir::BlockId, recv: hir::In
381381
fn inline_array_pop(fun: &mut hir::Function, block: hir::BlockId, recv: hir::InsnId, args: &[hir::InsnId], state: hir::InsnId) -> Option<hir::InsnId> {
382382
// Only inline the case of no arguments.
383383
let &[] = args else { return None; };
384-
// We know that all Array are HeapObject, so no need to insert a GuardType(HeapObject).
385-
let arr = fun.push_insn(block, hir::Insn::GuardNotFrozen { recv, state });
386-
Some(fun.push_insn(block, hir::Insn::ArrayPop { array: arr, state }))
384+
fun.guard_not_shared(block, recv, state);
385+
Some(fun.push_insn(block, hir::Insn::ArrayPop { array: recv, state }))
387386
}
388387

389388
fn inline_hash_aref(fun: &mut hir::Function, block: hir::BlockId, recv: hir::InsnId, args: &[hir::InsnId], state: hir::InsnId) -> Option<hir::InsnId> {
@@ -476,7 +475,7 @@ fn inline_string_setbyte(fun: &mut hir::Function, block: hir::BlockId, recv: hir
476475
let zero = fun.push_insn(block, hir::Insn::Const { val: hir::Const::CInt64(0) });
477476
let _ = fun.push_insn(block, hir::Insn::GuardGreaterEq { left: unboxed_index, right: zero, state });
478477
// We know that all String are HeapObject, so no need to insert a GuardType(HeapObject).
479-
let recv = fun.push_insn(block, hir::Insn::GuardNotFrozen { recv, state });
478+
fun.guard_not_frozen(block, recv, state);
480479
let _ = fun.push_insn(block, hir::Insn::StringSetbyteFixnum { string: recv, index, value });
481480
// String#setbyte returns the fixnum provided as its `value` argument back to the caller.
482481
Some(value)

0 commit comments

Comments
 (0)