Skip to content

Commit 1d665ee

Browse files
XrXrXrXr
committed
vibe1
1 parent d7ef972 commit 1d665ee

1 file changed

Lines changed: 46 additions & 106 deletions

File tree

‎zjit/src/hir.rs‎

Lines changed: 46 additions & 106 deletions
Original file line numberDiff line numberDiff line change
@@ -6522,24 +6522,6 @@ impl ProfileOracle {
65226522
}
65236523
}
65246524

6525-
fn invalidates_locals(opcode: u32, operands: *const VALUE) -> bool {
6526-
match opcode {
6527-
// Control-flow is non-leaf in the interpreter because it can execute arbitrary code on
6528-
// interrupt. But in the JIT, we side-exit if there is a pending interrupt.
6529-
YARVINSN_jump
6530-
| YARVINSN_branchunless
6531-
| YARVINSN_branchif
6532-
| YARVINSN_branchnil
6533-
| YARVINSN_jump_without_ints
6534-
| YARVINSN_branchunless_without_ints
6535-
| YARVINSN_branchif_without_ints
6536-
| YARVINSN_branchnil_without_ints
6537-
| YARVINSN_leave => false,
6538-
// TODO(max): Read the invokebuiltin target from operands and determine if it's leaf
6539-
_ => unsafe { !rb_zjit_insn_leaf(opcode as i32, operands) }
6540-
}
6541-
}
6542-
65436525
/// The index of the self parameter in the HIR function
65446526
pub const SELF_PARAM_IDX: usize = 0;
65456527

@@ -6597,13 +6579,13 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result<Function, ParseError> {
65976579
// TODO(max): Basic block arguments at edges
65986580
let mut queue = VecDeque::new();
65996581
for &insn_idx in jit_entry_insns.iter() {
6600-
queue.push_back((FrameState::new(iseq), insn_idx_to_block[&insn_idx], /*insn_idx=*/insn_idx, /*local_inval=*/false));
6582+
queue.push_back((FrameState::new(iseq), insn_idx_to_block[&insn_idx], /*insn_idx=*/insn_idx));
66016583
}
66026584

66036585
// Keep compiling blocks until the queue becomes empty
66046586
let mut visited = HashSet::new();
66056587
let iseq_size = unsafe { get_iseq_encoded_size(iseq) };
6606-
while let Some((incoming_state, mut block, mut insn_idx, mut local_inval)) = queue.pop_front() {
6588+
while let Some((incoming_state, mut block, mut insn_idx)) = queue.pop_front() {
66076589
// Compile each block only once
66086590
if visited.contains(&block) { continue; }
66096591
visited.insert(block);
@@ -6720,11 +6702,6 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result<Function, ParseError> {
67206702
profiles.profile_stack(&exit_state, stack_offset);
67216703
}
67226704

6723-
// Flag a future getlocal/setlocal to add a patch point if this instruction is not leaf.
6724-
if invalidates_locals(opcode, unsafe { pc.offset(1) }) {
6725-
local_inval = true;
6726-
}
6727-
67286705
// We add NoTracePoint patch points before every instruction that could be affected by TracePoint.
67296706
// This ensures that if TracePoint is enabled, we can exit the generated code as fast as possible.
67306707
unsafe extern "C" {
@@ -6990,18 +6967,9 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result<Function, ParseError> {
69906967
let ep_offset = get_arg(pc, 0).as_u32();
69916968
let index = get_arg(pc, 1).as_u64();
69926969
let index: u8 = index.try_into().map_err(|_| ParseError::MalformedIseq(insn_idx))?;
6993-
// Use FrameState to get kw_bits when possible, just like getlocal_WC_0.
6994-
let val = if !local_inval {
6995-
state.getlocal(ep_offset)
6996-
} else if ep_escaped {
6997-
let ep = fun.push_insn(block, Insn::GetEP { level: 0 });
6998-
fun.get_local_from_ep(block, ep, ep_offset, 0, types::BasicObject)
6999-
} else {
7000-
let exit_id = fun.push_insn(block, Insn::Snapshot { state: exit_state.without_locals() });
7001-
fun.push_insn(block, Insn::PatchPoint { invariant: Invariant::NoEPEscape(iseq), state: exit_id });
7002-
local_inval = false;
7003-
state.getlocal(ep_offset)
7004-
};
6970+
let ep = fun.push_insn(block, Insn::GetEP { level: 0 });
6971+
let val = fun.get_local_from_ep(block, ep, ep_offset, 0, types::BasicObject);
6972+
state.setlocal(ep_offset, val); // keep FrameState in sync for side-exit spilling
70056973
state.stack_push(fun.push_insn(block, Insn::FixnumBitCheck { val, index }));
70066974
}
70076975
YARVINSN_checkmatch => {
@@ -7067,7 +7035,7 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result<Function, ParseError> {
70677035
let not_nil_false_type = types::Truthy;
70687036
let not_nil_false = fun.push_insn(block, Insn::RefineType { val, new_type: not_nil_false_type });
70697037
state.replace(val, not_nil_false);
7070-
queue.push_back((state.clone(), target, target_idx, local_inval));
7038+
queue.push_back((state.clone(), target, target_idx));
70717039
}
70727040
YARVINSN_branchif | YARVINSN_branchif_without_ints => {
70737041
if opcode == YARVINSN_branchif {
@@ -7089,7 +7057,7 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result<Function, ParseError> {
70897057
let nil_false_type = types::Falsy;
70907058
let nil_false = fun.push_insn(block, Insn::RefineType { val, new_type: nil_false_type });
70917059
state.replace(val, nil_false);
7092-
queue.push_back((state.clone(), target, target_idx, local_inval));
7060+
queue.push_back((state.clone(), target, target_idx));
70937061
}
70947062
YARVINSN_branchnil | YARVINSN_branchnil_without_ints => {
70957063
if opcode == YARVINSN_branchnil {
@@ -7110,7 +7078,7 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result<Function, ParseError> {
71107078
let new_type = types::NotNil;
71117079
let not_nil = fun.push_insn(block, Insn::RefineType { val, new_type });
71127080
state.replace(val, not_nil);
7113-
queue.push_back((state.clone(), target, target_idx, local_inval));
7081+
queue.push_back((state.clone(), target, target_idx));
71147082
}
71157083
YARVINSN_opt_case_dispatch => {
71167084
// TODO: Some keys are visible at compile time, so in the future we can
@@ -7136,7 +7104,7 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result<Function, ParseError> {
71367104
val: test_id,
71377105
target: BranchEdge { target, args: state.as_args(self_param) }
71387106
});
7139-
queue.push_back((state.clone(), target, target_idx, local_inval));
7107+
queue.push_back((state.clone(), target, target_idx));
71407108

71417109
// Move on to the fast path
71427110
let insn_id = fun.push_insn(block, Insn::ObjectAlloc { val, state: exit_id });
@@ -7153,50 +7121,21 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result<Function, ParseError> {
71537121
let _branch_id = fun.push_insn(block, Insn::Jump(
71547122
BranchEdge { target, args: state.as_args(self_param) }
71557123
));
7156-
queue.push_back((state.clone(), target, target_idx, local_inval));
7124+
queue.push_back((state.clone(), target, target_idx));
71577125
break; // Don't enqueue the next block as a successor
71587126
}
71597127
YARVINSN_getlocal_WC_0 => {
71607128
let ep_offset = get_arg(pc, 0).as_u32();
7161-
if !local_inval {
7162-
// The FrameState is the source of truth for locals until invalidated.
7163-
// In case of JIT-to-JIT send locals might never end up in EP memory.
7164-
let val = state.getlocal(ep_offset);
7165-
state.stack_push(val);
7166-
} else if ep_escaped {
7167-
// Read the local using EP
7168-
let ep = fun.push_insn(block, Insn::GetEP { level: 0 });
7169-
let val = fun.get_local_from_ep(block, ep, ep_offset, 0, types::BasicObject);
7170-
state.setlocal(ep_offset, val); // remember the result to spill on side-exits
7171-
state.stack_push(val);
7172-
} else {
7173-
assert!(local_inval); // if check above
7174-
// There has been some non-leaf call since JIT entry or the last patch point,
7175-
// so add a patch point to make sure locals have not been escaped.
7176-
let exit_id = fun.push_insn(block, Insn::Snapshot { state: exit_state.without_locals() }); // skip spilling locals
7177-
fun.push_insn(block, Insn::PatchPoint { invariant: Invariant::NoEPEscape(iseq), state: exit_id });
7178-
local_inval = false;
7179-
7180-
// Read the local from FrameState
7181-
let val = state.getlocal(ep_offset);
7182-
state.stack_push(val);
7183-
}
7129+
let ep = fun.push_insn(block, Insn::GetEP { level: 0 });
7130+
let val = fun.get_local_from_ep(block, ep, ep_offset, 0, types::BasicObject);
7131+
state.setlocal(ep_offset, val); // keep FrameState in sync for side-exit spilling
7132+
state.stack_push(val);
71847133
}
71857134
YARVINSN_setlocal_WC_0 => {
71867135
let ep_offset = get_arg(pc, 0).as_u32();
71877136
let val = state.stack_pop()?;
7188-
if ep_escaped {
7189-
// Write the local using EP
7190-
fun.push_insn(block, Insn::SetLocal { val, ep_offset, level: 0 });
7191-
} else if local_inval {
7192-
// If there has been any non-leaf call since JIT entry or the last patch point,
7193-
// add a patch point to make sure locals have not been escaped.
7194-
let exit_id = fun.push_insn(block, Insn::Snapshot { state: exit_state.without_locals() }); // skip spilling locals
7195-
fun.push_insn(block, Insn::PatchPoint { invariant: Invariant::NoEPEscape(iseq), state: exit_id });
7196-
local_inval = false;
7197-
}
7198-
// Write the local into FrameState
7199-
state.setlocal(ep_offset, val);
7137+
fun.push_insn(block, Insn::SetLocal { val, ep_offset, level: 0 });
7138+
state.setlocal(ep_offset, val); // keep FrameState in sync for side-exit spilling
72007139
}
72017140
YARVINSN_getlocal_WC_1 => {
72027141
let ep_offset = get_arg(pc, 0).as_u32();
@@ -7210,23 +7149,21 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result<Function, ParseError> {
72107149
YARVINSN_getlocal => {
72117150
let ep_offset = get_arg(pc, 0).as_u32();
72127151
let level = get_arg(pc, 1).as_u32();
7213-
if level == 0 && !local_inval {
7214-
// Same optimization as getlocal_WC_0: use FrameState
7215-
let val = state.getlocal(ep_offset);
7216-
state.stack_push(val);
7217-
} else {
7218-
let ep = fun.push_insn(block, Insn::GetEP { level });
7219-
let val = fun.get_local_from_ep(block, ep, ep_offset, level, types::BasicObject);
7220-
if level == 0 {
7221-
state.setlocal(ep_offset, val);
7222-
}
7223-
state.stack_push(val);
7152+
let ep = fun.push_insn(block, Insn::GetEP { level });
7153+
let val = fun.get_local_from_ep(block, ep, ep_offset, level, types::BasicObject);
7154+
if level == 0 {
7155+
state.setlocal(ep_offset, val);
72247156
}
7157+
state.stack_push(val);
72257158
}
72267159
YARVINSN_setlocal => {
72277160
let ep_offset = get_arg(pc, 0).as_u32();
72287161
let level = get_arg(pc, 1).as_u32();
7229-
fun.push_insn(block, Insn::SetLocal { val: state.stack_pop()?, ep_offset, level });
7162+
let val = state.stack_pop()?;
7163+
fun.push_insn(block, Insn::SetLocal { val, ep_offset, level });
7164+
if level == 0 {
7165+
state.setlocal(ep_offset, val);
7166+
}
72307167
}
72317168
YARVINSN_setblockparam => {
72327169
let ep_offset = get_arg(pc, 0).as_u32();
@@ -7735,7 +7672,7 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result<Function, ParseError> {
77357672
// make the right number of Params
77367673
let mut join_state = state.clone();
77377674
join_state.stack_pop_n(argc as usize)?;
7738-
queue.push_back((join_state, join_block, insn_idx, local_inval));
7675+
queue.push_back((join_state, join_block, insn_idx));
77397676
// In the fallthrough case, do a generic interpreter send and then join.
77407677
let args = state.stack_pop_n(argc as usize)?;
77417678
let recv = state.stack_pop()?;
@@ -8262,7 +8199,7 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result<Function, ParseError> {
82628199
if insn_idx_to_block.contains_key(&insn_idx) {
82638200
let target = insn_idx_to_block[&insn_idx];
82648201
fun.push_insn(block, Insn::Jump(BranchEdge { target, args: state.as_args(self_param) }));
8265-
queue.push_back((state, target, insn_idx, local_inval));
8202+
queue.push_back((state, target, insn_idx));
82668203
break; // End the block
82678204
}
82688205
}
@@ -8411,32 +8348,35 @@ fn compile_jit_entry_state(fun: &mut Function, jit_entry_block: BlockId, jit_ent
84118348
let mut entry_state = FrameState::new(iseq);
84128349
let mut ep: Option<InsnId> = None;
84138350
for local_idx in 0..num_locals(iseq) {
8414-
if (lead_num + passed_opt_num..lead_num + opt_num).contains(&local_idx) {
8351+
let ep_offset = local_idx_to_ep_offset(iseq, local_idx);
8352+
let ep_offset_u32 = u32::try_from(ep_offset)
8353+
.unwrap_or_else(|_| panic!("Could not convert ep_offset {ep_offset} to u32"));
8354+
let val = if (lead_num + passed_opt_num..lead_num + opt_num).contains(&local_idx) {
84158355
// Omitted optionals are locals, so they start as nils before their code run
8416-
entry_state.locals.push(fun.push_insn(jit_entry_block, Insn::Const { val: Const::Value(Qnil) }));
8356+
fun.push_insn(jit_entry_block, Insn::Const { val: Const::Value(Qnil) })
84178357
} else if Some(local_idx) == kw_bits_idx {
84188358
// Read the kw_bits value written by the caller to the callee frame.
84198359
// This tells us which optional keywords were NOT provided and need their defaults evaluated.
84208360
// Note: The caller writes kw_bits to memory via gen_send_iseq_direct but does NOT pass it
84218361
// as a C argument, so we must read it from EP memory rather than Param.
8422-
let ep_offset = local_idx_to_ep_offset(iseq, local_idx);
8423-
let ep_offset_u32 = u32::try_from(ep_offset)
8424-
.unwrap_or_else(|_| panic!("Could not convert ep_offset {ep_offset} to u32"));
84258362
let ep = *ep.get_or_insert_with(|| fun.push_insn(jit_entry_block, Insn::GetEP { level: 0 }));
8426-
entry_state.locals.push(fun.get_local_from_ep(
8427-
jit_entry_block,
8428-
ep,
8429-
ep_offset_u32,
8430-
0,
8431-
types::BasicObject,
8432-
));
8363+
fun.get_local_from_ep(jit_entry_block, ep, ep_offset_u32, 0, types::BasicObject)
84338364
} else if local_idx < param_size {
84348365
let id = unsafe { rb_zjit_local_id(iseq, local_idx.try_into().unwrap()) };
8435-
entry_state.locals.push(fun.push_insn(jit_entry_block, Insn::LoadArg { idx: arg_idx, id, val_type: types::BasicObject }));
8366+
let v = fun.push_insn(jit_entry_block, Insn::LoadArg { idx: arg_idx, id, val_type: types::BasicObject });
84368367
arg_idx += 1;
8368+
v
84378369
} else {
8438-
entry_state.locals.push(fun.push_insn(jit_entry_block, Insn::Const { val: Const::Value(Qnil) }));
8439-
}
8370+
fun.push_insn(jit_entry_block, Insn::Const { val: Const::Value(Qnil) })
8371+
};
8372+
// The JIT-to-JIT calling convention passes params in registers (via LoadArg), so EP memory
8373+
// is not eagerly populated. Spill every local to its EP slot here so subsequent getlocal/
8374+
// setlocal instructions, which always go through memory, see the correct values.
8375+
// kw_bits already lives in EP memory, so skip rewriting it.
8376+
if Some(local_idx) != kw_bits_idx {
8377+
fun.push_insn(jit_entry_block, Insn::SetLocal { val, ep_offset: ep_offset_u32, level: 0 });
8378+
}
8379+
entry_state.locals.push(val);
84408380
}
84418381
(self_param, entry_state)
84428382
}

0 commit comments

Comments
 (0)