Skip to content

Commit f6adf1c

Browse files
committed
ZJIT: Use a slice for ISeq opt_table for quick one-off access
This gets rid of the copying of the entire opt_table in function_stub_hit() since it only reads one element. HIR builder still does a copy and that's unchanged. It needs to read the whole table.
1 parent 34c94b3 commit f6adf1c

File tree

3 files changed

+27
-24
lines changed

3 files changed

+27
-24
lines changed

zjit/src/codegen.rs

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3076,11 +3076,15 @@ c_callable! {
30763076
unsafe { Rc::increment_strong_count(iseq_call_ptr as *const IseqCall); }
30773077
let iseq_call = unsafe { Rc::from_raw(iseq_call_ptr as *const IseqCall) };
30783078
let iseq = iseq_call.iseq.get();
3079-
let entry_insn_idxs = crate::hir::jit_entry_insns(iseq);
3079+
let params = unsafe { iseq.params() };
3080+
let entry_idx = iseq_call.jit_entry_idx.to_usize();
3081+
let entry_insn_idx = params.opt_table_slice().get(entry_idx)
3082+
.unwrap_or_else(|| panic!("function_stub: opt_table out of bounds. {params:#?}, entry_idx={entry_idx}"))
3083+
.as_u32();
30803084
// gen_push_frame() doesn't set PC or ISEQ, so we need to set them before exit.
30813085
// function_stub_hit_body() may allocate and call gc_validate_pc(), so we always set PC and ISEQ.
30823086
// Clear jit_return so the interpreter reads cfp->pc and cfp->iseq directly.
3083-
let pc = unsafe { rb_iseq_pc_at_idx(iseq, entry_insn_idxs[iseq_call.jit_entry_idx.to_usize()]) };
3087+
let pc = unsafe { rb_iseq_pc_at_idx(iseq, entry_insn_idx) };
30843088
unsafe { rb_set_cfp_pc(cfp, pc) };
30853089
unsafe { (*cfp)._iseq = iseq };
30863090
unsafe { (*cfp).jit_return = std::ptr::null_mut() };
@@ -3501,7 +3505,7 @@ impl Assembler {
35013505

35023506
/// Store info about a JIT entry point
35033507
pub struct JITEntry {
3504-
/// Index that corresponds to [crate::hir::jit_entry_insns]
3508+
/// Index that corresponds to an entry in [crate::cruby::IseqParameters::opt_table_slice]
35053509
jit_entry_idx: usize,
35063510
/// Position where the entry point starts
35073511
start_addr: Cell<Option<CodePtr>>,
@@ -3524,7 +3528,7 @@ pub struct IseqCall {
35243528
/// Callee ISEQ that start_addr jumps to
35253529
pub iseq: Cell<IseqPtr>,
35263530

3527-
/// Index that corresponds to [crate::hir::jit_entry_insns]
3531+
/// Index that corresponds to an entry in [crate::cruby::IseqParameters::opt_table_slice]
35283532
jit_entry_idx: u16,
35293533

35303534
/// Argument count passing to the HIR function

zjit/src/cruby.rs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -738,6 +738,24 @@ impl IseqAccess for IseqPtr {
738738
}
739739
}
740740

741+
impl IseqParameters {
742+
/// The `opt_table` is a mapping where `opt_table[number_of_optional_parameters_filled]`
743+
/// gives the YARV entry point of ISeq as an index of the iseq_encoded array.
744+
/// This method gives over the table that additionally works when `opt_num==0`,
745+
/// when the table is stored as `NULL` and implicit.
746+
/// The table stores the indexes as raw VALUE integers; they are not tagged as fixnum.
747+
pub fn opt_table_slice(&self) -> &[VALUE] {
748+
let opt_num: usize = self.opt_num.try_into().expect("ISeq opt_num should always >=0");
749+
if opt_num > 0 {
750+
// The table has size=opt_num+1 because opt_table[opt_num] is valid (all optionals filled)
751+
unsafe { std::slice::from_raw_parts(self.opt_table, opt_num + 1) }
752+
} else {
753+
// The ISeq entry point is index 0 when there are no optional parameters
754+
&[VALUE(0)]
755+
}
756+
}
757+
}
758+
741759
impl From<IseqPtr> for VALUE {
742760
/// For `.into()` convenience
743761
fn from(iseq: IseqPtr) -> Self {

zjit/src/hir.rs

Lines changed: 1 addition & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -6743,25 +6743,6 @@ fn insn_idx_at_offset(idx: u32, offset: i64) -> u32 {
67436743
((idx as isize) + (offset as isize)) as u32
67446744
}
67456745

6746-
/// List of insn_idx that starts a JIT entry block
6747-
pub fn jit_entry_insns(iseq: IseqPtr) -> Vec<u32> {
6748-
// TODO(alan): Make an iterator type for this instead of copying all of the opt_table each call
6749-
let params = unsafe { iseq.params() };
6750-
let opt_num = params.opt_num;
6751-
if opt_num > 0 {
6752-
let mut result = vec![];
6753-
6754-
let opt_table = params.opt_table; // `opt_num + 1` entries
6755-
for opt_idx in 0..=opt_num as isize {
6756-
let insn_idx = unsafe { opt_table.offset(opt_idx).read().as_u32() };
6757-
result.push(insn_idx);
6758-
}
6759-
result
6760-
} else {
6761-
vec![0]
6762-
}
6763-
}
6764-
67656746
struct BytecodeInfo {
67666747
jump_targets: Vec<u32>,
67676748
}
@@ -6921,7 +6902,7 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result<Function, ParseError> {
69216902
fun.was_invalidated_for_singleton_class_creation = payload.was_invalidated_for_singleton_class_creation;
69226903

69236904
// Compute a map of PC->Block by finding jump targets
6924-
let jit_entry_insns = jit_entry_insns(iseq);
6905+
let jit_entry_insns = unsafe { iseq.params() }.opt_table_slice().iter().copied().map(VALUE::as_u32).collect::<Vec<_>>();
69256906
let BytecodeInfo { jump_targets } = compute_bytecode_info(iseq, &jit_entry_insns);
69266907

69276908
// Make all empty basic blocks. The ordering of the BBs matters for getting fallthrough jumps

0 commit comments

Comments
 (0)