Skip to content

Commit f0c7367

Browse files
XrXrXrXr
committed
Revert "ZJIT: Clean up branching in polymorphic opt_send_without_block HIR construction (ruby#16634)"
This reverts commit 77b803c. There is a new NULL dereference in tool/test/testunit/test_sorting.rb that seem to have started with this commit.
1 parent 25ca647 commit f0c7367

File tree

2 files changed

+170
-134
lines changed

2 files changed

+170
-134
lines changed

‎zjit/src/hir.rs‎

Lines changed: 72 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -7793,45 +7793,81 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result<Function, ParseError> {
77937793
break;
77947794
}
77957795

7796-
let branch_insn_idx = exit_state.insn_idx as u32;
7797-
let args = state.stack_pop_n(argc as usize)?;
7798-
let recv = state.stack_pop()?;
7799-
7800-
if let Some(summary) = fun.polymorphic_summary(&profiles, recv, exit_state.insn_idx) {
7801-
let join_block = insn_idx_to_block.get(&insn_idx).copied().unwrap_or_else(|| fun.new_block(insn_idx));
7802-
let join_param = fun.push_insn(join_block, Insn::Param);
7803-
// Dedup by expected type so immediate/heap variants
7804-
// under the same Ruby class can still get separate branches.
7805-
let mut seen_types = Vec::with_capacity(summary.buckets().len());
7806-
for &profiled_type in summary.buckets() {
7807-
if profiled_type.is_empty() { break; }
7808-
let expected = Type::from_profiled_type(profiled_type);
7809-
if seen_types.iter().any(|ty: &Type| ty.bit_equal(expected)) {
7810-
continue;
7796+
{
7797+
fn new_branch_block(
7798+
fun: &mut Function,
7799+
cd: *const rb_call_data,
7800+
argc: usize,
7801+
opcode: u32,
7802+
new_type: Type,
7803+
insn_idx: u32,
7804+
exit_state: &FrameState,
7805+
locals_count: usize,
7806+
stack_count: usize,
7807+
join_block: BlockId,
7808+
) -> BlockId {
7809+
let block = fun.new_block(insn_idx);
7810+
let self_param = fun.push_insn(block, Insn::Param);
7811+
let mut state = exit_state.clone();
7812+
state.locals.clear();
7813+
state.stack.clear();
7814+
state.locals.extend((0..locals_count).map(|_| fun.push_insn(block, Insn::Param)));
7815+
state.stack.extend((0..stack_count).map(|_| fun.push_insn(block, Insn::Param)));
7816+
let snapshot = fun.push_insn(block, Insn::Snapshot { state: state.clone() });
7817+
let args = state.stack_pop_n(argc).unwrap();
7818+
let recv = state.stack_pop().unwrap();
7819+
let refined_recv = fun.push_insn(block, Insn::RefineType { val: recv, new_type });
7820+
state.replace(recv, refined_recv);
7821+
let send = fun.push_insn(block, Insn::Send { recv: refined_recv, cd, block: None, args, state: snapshot, reason: Uncategorized(opcode) });
7822+
state.stack_push(send);
7823+
fun.push_insn(block, Insn::Jump(BranchEdge { target: join_block, args: state.as_args(self_param) }));
7824+
block
7825+
}
7826+
let branch_insn_idx = exit_state.insn_idx as u32;
7827+
let locals_count = state.locals.len();
7828+
let stack_count = state.stack.len();
7829+
let recv = state.stack_topn(argc as usize)?; // args are on top
7830+
let entry_args = state.as_args(self_param);
7831+
if let Some(summary) = fun.polymorphic_summary(&profiles, recv, exit_state.insn_idx) {
7832+
let join_block = insn_idx_to_block.get(&insn_idx).copied().unwrap_or_else(|| fun.new_block(insn_idx));
7833+
// Dedup by expected type so immediate/heap variants
7834+
// under the same Ruby class can still get separate branches.
7835+
let mut seen_types = Vec::with_capacity(summary.buckets().len());
7836+
for &profiled_type in summary.buckets() {
7837+
if profiled_type.is_empty() { break; }
7838+
let expected = Type::from_profiled_type(profiled_type);
7839+
if seen_types.iter().any(|ty: &Type| ty.bit_equal(expected)) {
7840+
continue;
7841+
}
7842+
seen_types.push(expected);
7843+
let has_type = fun.push_insn(block, Insn::HasType { val: recv, expected });
7844+
let iftrue_block =
7845+
new_branch_block(&mut fun, cd, argc as usize, opcode, expected, branch_insn_idx, &exit_state, locals_count, stack_count, join_block);
7846+
let target = BranchEdge { target: iftrue_block, args: entry_args.clone() };
7847+
fun.push_insn(block, Insn::IfTrue { val: has_type, target });
78117848
}
7812-
seen_types.push(expected);
7813-
let has_type = fun.push_insn(block, Insn::HasType { val: recv, expected });
7814-
let iftrue_block = fun.new_block(branch_insn_idx);
7815-
let target = BranchEdge { target: iftrue_block, args: vec![] };
7816-
fun.push_insn(block, Insn::IfTrue { val: has_type, target });
7817-
7818-
let refined_recv = fun.push_insn(iftrue_block, Insn::RefineType { val: recv, new_type: expected });
7819-
let send = fun.push_insn(iftrue_block, Insn::Send { recv: refined_recv, cd, block: None, args: args.clone(), state: exit_id, reason: Uncategorized(opcode) });
7820-
fun.push_insn(iftrue_block, Insn::Jump(BranchEdge { target: join_block, args: vec![send] }));
7849+
// Continue compilation from the join block at the next instruction.
7850+
// Make a copy of the current state without the args (pop the receiver
7851+
// and push the result) because we just use the locals/stack sizes to
7852+
// make the right number of Params
7853+
let mut join_state = state.clone();
7854+
join_state.stack_pop_n(argc as usize)?;
7855+
queue.push_back((join_state, join_block, insn_idx, local_inval));
7856+
// In the fallthrough case, do a generic interpreter send and then join.
7857+
let args = state.stack_pop_n(argc as usize)?;
7858+
let recv = state.stack_pop()?;
7859+
let reason = SendWithoutBlockPolymorphicFallback;
7860+
let send = fun.push_insn(block, Insn::Send { recv, cd, block: None, args, state: exit_id, reason });
7861+
state.stack_push(send);
7862+
fun.push_insn(block, Insn::Jump(BranchEdge { target: join_block, args: state.as_args(self_param) }));
7863+
break; // End the block
78217864
}
7822-
// In the fallthrough case, do a generic interpreter send and then join.
7823-
let reason = SendWithoutBlockPolymorphicFallback;
7824-
let send = fun.push_insn(block, Insn::Send { recv, cd, block: None, args, state: exit_id, reason });
7825-
fun.push_insn(block, Insn::Jump(BranchEdge { target: join_block, args: vec![send] }));
7826-
7827-
// Continue compilation in the join_block
7828-
block = join_block;
7829-
state.stack_push(join_param);
7830-
} else {
7831-
// Maybe monomorphic; handled in type_specialize
7832-
let send = fun.push_insn(block, Insn::Send { recv, cd, block: None, args, state: exit_id, reason: Uncategorized(opcode) });
7833-
state.stack_push(send);
78347865
}
7866+
7867+
let args = state.stack_pop_n(argc as usize)?;
7868+
let recv = state.stack_pop()?;
7869+
let send = fun.push_insn(block, Insn::Send { recv, cd, block: None, args, state: exit_id, reason: Uncategorized(opcode) });
7870+
state.stack_push(send);
78357871
}
78367872
YARVINSN_send => {
78377873
let cd: *const rb_call_data = get_arg(pc, 0).as_ptr();

0 commit comments

Comments
 (0)