Skip to content

Commit 3522eaa

Browse files
tekknolagitekknolagi
committed
ZJIT: Clean up getblockparam/getblockparamproxy to thread SSA values directly
1 parent 9c827a4 commit 3522eaa

3 files changed

Lines changed: 432 additions & 495 deletions

File tree

‎zjit/src/hir.rs‎

Lines changed: 47 additions & 110 deletions
Original file line numberDiff line numberDiff line change
@@ -2610,24 +2610,6 @@ impl Function {
26102610
id
26112611
}
26122612

2613-
fn new_branch_block(
2614-
&mut self,
2615-
insn_idx: u32,
2616-
exit_state: &FrameState,
2617-
locals_count: usize,
2618-
stack_count: usize,
2619-
) -> (BlockId, InsnId, FrameState, InsnId) {
2620-
let block = self.new_block(insn_idx);
2621-
let self_param = self.push_insn(block, Insn::Param);
2622-
let mut state = exit_state.clone();
2623-
state.locals.clear();
2624-
state.stack.clear();
2625-
state.locals.extend((0..locals_count).map(|_| self.push_insn(block, Insn::Param)));
2626-
state.stack.extend((0..stack_count).map(|_| self.push_insn(block, Insn::Param)));
2627-
let snapshot = self.push_insn(block, Insn::Snapshot { state: state.clone() });
2628-
(block, self_param, state, snapshot)
2629-
}
2630-
26312613
fn remove_block(&mut self, block_id: BlockId) {
26322614
if BlockId(self.blocks.len() - 1) != block_id {
26332615
panic!("Can only remove the last block");
@@ -7587,44 +7569,41 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result<Function, ParseError> {
75877569
None
75887570
};
75897571

7590-
let locals_count = state.locals.len();
7591-
let stack_count = state.stack.len();
7592-
let entry_args = state.as_args(self_param);
7593-
75947572
// `getblockparamproxy` has two semantic paths:
75957573
// - modified: return the already-materialized block local from EP
75967574
// - unmodified: inspect the block handler and produce proxy/nil
7597-
let (modified_block, modified_self_param, mut modified_state, ..) =
7598-
fun.new_branch_block(branch_insn_idx, &exit_state, locals_count, stack_count);
7599-
let (unmodified_block, unmodified_self_param, mut unmodified_state, unmodified_exit_id) =
7600-
fun.new_branch_block(branch_insn_idx, &exit_state, locals_count, stack_count);
7575+
let modified_block = fun.new_block(branch_insn_idx);
7576+
let unmodified_block = fun.new_block(branch_insn_idx);
76017577
let join_block = insn_idx_to_block.get(&insn_idx).copied().unwrap_or_else(|| fun.new_block(insn_idx));
7578+
let join_result = fun.push_insn(join_block, Insn::Param);
7579+
let join_local = if level == 0 { Some(fun.push_insn(join_block, Insn::Param)) } else { None };
76027580

76037581
let ep = fun.push_insn(block, Insn::GetEP { level });
76047582
let is_modified = fun.push_insn(block, Insn::IsBlockParamModified { ep });
76057583

7606-
fun.push_insn(block, Insn::IfTrue { val: is_modified, target: BranchEdge { target: modified_block, args: entry_args.clone() }});
7607-
fun.push_insn(block, Insn::Jump(BranchEdge { target: unmodified_block, args: entry_args }));
7584+
fun.push_insn(block, Insn::IfTrue { val: is_modified, target: BranchEdge { target: modified_block, args: vec![] }});
7585+
fun.push_insn(block, Insn::Jump(BranchEdge { target: unmodified_block, args: vec![] }));
76087586

7609-
// Push modified block: load the block local via EP.
7587+
// Modified block: load the block local via EP.
76107588
let ep = fun.push_insn(modified_block, Insn::GetEP { level });
76117589
let modified_val = fun.get_local_from_ep(modified_block, ep, ep_offset, level, types::BasicObject);
7612-
if level == 0 {
7613-
modified_state.setlocal(ep_offset, modified_val);
7614-
}
7615-
modified_state.stack_push(modified_val);
7616-
fun.push_insn(modified_block, Insn::Jump(BranchEdge { target: join_block, args: modified_state.as_args(modified_self_param) }));
7590+
let mut modified_args = vec![modified_val];
7591+
if level == 0 { modified_args.push(modified_val); }
7592+
fun.push_insn(modified_block, Insn::Jump(BranchEdge { target: join_block, args: modified_args }));
76177593

7618-
// Push unmodified block: inspect the current block handler to
7594+
// Unmodified block: inspect the current block handler to
76197595
// decide whether this path returns `nil` or `BlockParamProxy`.
76207596
let ep = fun.push_insn(unmodified_block, Insn::GetEP { level });
76217597
let block_handler = fun.push_insn(unmodified_block, Insn::LoadField { recv: ep, id: ID!(_env_data_index_specval), offset: SIZEOF_VALUE_I32 * VM_ENV_DATA_INDEX_SPECVAL, return_type: types::CInt64 });
7598+
let original_local = if level == 0 { Some(state.getlocal(ep_offset)) } else { None };
76227599

76237600
match profiled_block_type {
76247601
Some(ty) if ty.nil_p() => {
7625-
fun.push_insn(unmodified_block, Insn::GuardBitEquals { val: block_handler, expected: Const::CInt64(VM_BLOCK_HANDLER_NONE.into()), reason: SideExitReason::BlockParamProxyNotNil, state: unmodified_exit_id });
7626-
unmodified_state.stack_push(fun.push_insn(unmodified_block, Insn::Const { val: Const::Value(Qnil) }));
7627-
fun.push_insn(unmodified_block, Insn::Jump(BranchEdge { target: join_block, args: unmodified_state.as_args(unmodified_self_param) }));
7602+
fun.push_insn(unmodified_block, Insn::GuardBitEquals { val: block_handler, expected: Const::CInt64(VM_BLOCK_HANDLER_NONE.into()), reason: SideExitReason::BlockParamProxyNotNil, state: exit_id });
7603+
let nil_val = fun.push_insn(unmodified_block, Insn::Const { val: Const::Value(Qnil) });
7604+
let mut unmodified_args = vec![nil_val];
7605+
if let Some(local) = original_local { unmodified_args.push(local); }
7606+
fun.push_insn(unmodified_block, Insn::Jump(BranchEdge { target: join_block, args: unmodified_args }));
76287607
}
76297608
_ => {
76307609
// This handles two cases which are nearly identical
@@ -7635,107 +7614,65 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result<Function, ParseError> {
76357614
const _: () = assert!(RUBY_SYMBOL_FLAG & 1 == 0, "guard below rejects symbol block handlers");
76367615

76377616
// Bail out if the block handler is neither ISEQ nor ifunc
7638-
fun.push_insn(unmodified_block, Insn::GuardAnyBitSet { val: block_handler, mask: Const::CUInt64(0x1), mask_name: None, reason: SideExitReason::BlockParamProxyNotIseqOrIfunc, state: unmodified_exit_id });
7617+
fun.push_insn(unmodified_block, Insn::GuardAnyBitSet { val: block_handler, mask: Const::CUInt64(0x1), mask_name: None, reason: SideExitReason::BlockParamProxyNotIseqOrIfunc, state: exit_id });
76397618
// TODO(Shopify/ruby#753): GC root, so we should be able to avoid unnecessary GC tracing
7640-
unmodified_state.stack_push(fun.push_insn(unmodified_block, Insn::Const { val: Const::Value(unsafe { rb_block_param_proxy }) }));
7641-
fun.push_insn(unmodified_block, Insn::Jump(BranchEdge { target: join_block, args: unmodified_state.as_args(unmodified_self_param) }));
7619+
let proxy_val = fun.push_insn(unmodified_block, Insn::Const { val: Const::Value(unsafe { rb_block_param_proxy }) });
7620+
let mut unmodified_args = vec![proxy_val];
7621+
if let Some(local) = original_local { unmodified_args.push(local); }
7622+
fun.push_insn(unmodified_block, Insn::Jump(BranchEdge { target: join_block, args: unmodified_args }));
76427623
}
76437624
}
76447625

7645-
// Continue compilation from the merged continuation block at the next
7646-
// instruction.
7647-
queue.push_back((unmodified_state, join_block, insn_idx, local_inval));
7648-
break;
7626+
// Continue from the join block
7627+
if let Some(local_param) = join_local {
7628+
state.setlocal(ep_offset, local_param);
7629+
}
7630+
state.stack_push(join_result);
7631+
block = join_block;
76497632
}
76507633
YARVINSN_getblockparam => {
7651-
fn finish_getblockparam_branch(
7652-
fun: &mut Function,
7653-
block: BlockId,
7654-
self_param: InsnId,
7655-
state: &mut FrameState,
7656-
join_block: BlockId,
7657-
ep_offset: u32,
7658-
level: u32,
7659-
val: InsnId,
7660-
) {
7661-
if level == 0 {
7662-
state.setlocal(ep_offset, val);
7663-
}
7664-
state.stack_push(val);
7665-
fun.push_insn(block, Insn::Jump(BranchEdge {
7666-
target: join_block,
7667-
args: state.as_args(self_param),
7668-
}));
7669-
}
7670-
76717634
let ep_offset = get_arg(pc, 0).as_u32();
76727635
let level = get_arg(pc, 1).as_u32();
76737636
let branch_insn_idx = exit_state.insn_idx as u32;
76747637

76757638
// If the block param is already a Proc (modified), read it from EP.
76767639
// Otherwise, convert it to a Proc and store it to EP.
7640+
let modified_block = fun.new_block(branch_insn_idx);
7641+
let unmodified_block = fun.new_block(branch_insn_idx);
7642+
let join_block = insn_idx_to_block.get(&insn_idx).copied().unwrap_or_else(|| fun.new_block(insn_idx));
7643+
let join_param = fun.push_insn(join_block, Insn::Param);
7644+
76777645
let ep = fun.push_insn(block, Insn::GetEP { level });
76787646
let is_modified = fun.push_insn(block, Insn::IsBlockParamModified { ep });
76797647

7680-
let locals_count = state.locals.len();
7681-
let stack_count = state.stack.len();
7682-
let entry_args = state.as_args(self_param);
7683-
7684-
// Set up branch and join blocks.
7685-
let (modified_block, modified_self_param, mut modified_state, ..) =
7686-
fun.new_branch_block(branch_insn_idx, &exit_state, locals_count, stack_count);
7687-
let (unmodified_block, unmodified_self_param, mut unmodified_state, unmodified_exit_id) =
7688-
fun.new_branch_block(branch_insn_idx, &exit_state, locals_count, stack_count);
7689-
let join_block = insn_idx_to_block.get(&insn_idx).copied().unwrap_or_else(|| fun.new_block(insn_idx));
7690-
76917648
fun.push_insn(block, Insn::IfTrue {
76927649
val: is_modified,
7693-
target: BranchEdge { target: modified_block, args: entry_args.clone() },
7650+
target: BranchEdge { target: modified_block, args: vec![] },
76947651
});
76957652
fun.push_insn(block, Insn::Jump(BranchEdge {
76967653
target: unmodified_block,
7697-
args: entry_args,
7654+
args: vec![],
76987655
}));
76997656

7700-
// Push modified block: read Proc from EP.
7657+
// Modified block: read Proc from EP.
77017658
let ep = fun.push_insn(modified_block, Insn::GetEP { level });
7702-
let modified_val = fun.get_local_from_ep(modified_block,
7703-
ep,
7704-
ep_offset,
7705-
level,
7706-
types::BasicObject,
7707-
);
7708-
finish_getblockparam_branch(
7709-
&mut fun,
7710-
modified_block,
7711-
modified_self_param,
7712-
&mut modified_state,
7713-
join_block,
7714-
ep_offset,
7715-
level,
7716-
modified_val,
7717-
);
7659+
let modified_val = fun.get_local_from_ep(modified_block, ep, ep_offset, level, types::BasicObject);
7660+
fun.push_insn(modified_block, Insn::Jump(BranchEdge { target: join_block, args: vec![modified_val] }));
77187661

7719-
// Push unmodified block: convert block handler to Proc.
7662+
// Unmodified block: convert block handler to Proc.
77207663
let unmodified_val = fun.push_insn(unmodified_block, Insn::GetBlockParam {
77217664
ep_offset,
77227665
level,
7723-
state: unmodified_exit_id,
7666+
state: exit_id,
77247667
});
7725-
finish_getblockparam_branch(
7726-
&mut fun,
7727-
unmodified_block,
7728-
unmodified_self_param,
7729-
&mut unmodified_state,
7730-
join_block,
7731-
ep_offset,
7732-
level,
7733-
unmodified_val,
7734-
);
7668+
fun.push_insn(unmodified_block, Insn::Jump(BranchEdge { target: join_block, args: vec![unmodified_val] }));
77357669

7736-
// Continue compilation from the join block at the next instruction.
7737-
queue.push_back((unmodified_state, join_block, insn_idx, local_inval));
7738-
break;
7670+
// Continue from the join block
7671+
if level == 0 {
7672+
state.setlocal(ep_offset, join_param);
7673+
}
7674+
state.stack_push(join_param);
7675+
block = join_block;
77397676
}
77407677
YARVINSN_pop => { state.stack_pop()?; }
77417678
YARVINSN_dup => { state.stack_push(state.stack_top()?); }

0 commit comments

Comments
 (0)