Skip to content

Commit 13e3979

Browse files
committed
ZJIT: Revert worklist-based type inference
ruby#16122 (c272297) worked for maximal SSA but does not work for "normal" SSA. This is because it used information propagating across block args/params as a proxy for tracking changes in dependent blocks. To do this properly we need to move to something like SCCP. In the meantime, go back to looping over RPO repeatedly.
1 parent f145d76 commit 13e3979

2 files changed

Lines changed: 71 additions & 67 deletions

File tree

zjit/src/codegen_tests.rs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5610,3 +5610,25 @@ fn test_load_immediates_into_registers_before_masking() {
56105610
test
56115611
"#), @"true");
56125612
}
5613+
5614+
#[test]
5615+
fn test_loop_terminates() {
5616+
set_call_threshold(3);
5617+
// Previous worklist-based type inference only worked for maximal SSA. This is a regression
5618+
// test for hanging.
5619+
assert_snapshot!(inspect(r#"
5620+
class TheClass
5621+
def set_value_loop
5622+
i = 0
5623+
while i < 10
5624+
@levar ||= i
5625+
i += 1
5626+
end
5627+
end
5628+
end
5629+
5630+
3.times do |i|
5631+
TheClass.new.set_value_loop
5632+
end
5633+
"#), @"3");
5634+
}

zjit/src/hir.rs

Lines changed: 49 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -3204,82 +3204,64 @@ impl Function {
32043204
self.copy_param_types();
32053205

32063206
let mut reachable = BlockSet::with_capacity(self.blocks.len());
3207-
3208-
// Maintain both a worklist and a fast membership check to avoid linear search
3209-
let mut worklist: VecDeque<BlockId> = VecDeque::with_capacity(self.blocks.len());
3210-
let mut in_worklist = BlockSet::with_capacity(self.blocks.len());
3211-
macro_rules! worklist_add {
3212-
($block:expr) => {
3213-
if in_worklist.insert($block) {
3214-
worklist.push_back($block);
3215-
}
3216-
};
3217-
}
3218-
32193207
reachable.insert(self.entries_block);
3220-
worklist_add!(self.entries_block);
3221-
3222-
// Helper to propagate types along a branch edge and enqueue the target if anything changed
3223-
macro_rules! enqueue {
3224-
($self:ident, $target:expr) => {
3225-
let newly_reachable = reachable.insert($target.target);
3226-
let mut target_changed = newly_reachable;
3227-
for (idx, arg) in $target.args.iter().enumerate() {
3228-
let arg = self.union_find.borrow().find_const(*arg);
3229-
let param = $self.blocks[$target.target.0].params[idx];
3230-
let param = self.union_find.borrow().find_const(param);
3231-
let new = self.insn_types[param.0].union(self.insn_types[arg.0]);
3232-
if !self.insn_types[param.0].bit_equal(new) {
3233-
self.insn_types[param.0] = new;
3234-
target_changed = true;
3235-
}
3236-
}
3237-
if target_changed {
3238-
worklist_add!($target.target);
3239-
}
3240-
};
3241-
}
32423208

3243-
// Walk the graph, computing types until worklist is empty
3244-
while let Some(block) = worklist.pop_front() {
3245-
in_worklist.remove(block);
3246-
if !reachable.get(block) { continue; }
3247-
for insn_id in &self.blocks[block.0].insns {
3248-
let insn_id = self.union_find.borrow().find_const(*insn_id);
3249-
let insn_type = match &self.insns[insn_id.0] {
3250-
&Insn::IfTrue { val, ref target } => {
3251-
assert!(!self.type_of(val).bit_equal(types::Empty));
3252-
if self.type_of(val).could_be(Type::from_cbool(true)) {
3253-
enqueue!(self, target);
3209+
// Walk the graph, computing types until fixpoint
3210+
let rpo = self.rpo();
3211+
loop {
3212+
let mut changed = false;
3213+
for &block in &rpo {
3214+
if !reachable.get(block) { continue; }
3215+
for &insn_id in &self.blocks[block.0].insns {
3216+
let insn_type = match &self.insns[insn_id.0] {
3217+
&Insn::IfTrue { val, target: BranchEdge { target, ref args } } => {
3218+
assert!(!self.type_of(val).bit_equal(types::Empty));
3219+
if self.type_of(val).could_be(Type::from_cbool(true)) {
3220+
reachable.insert(target);
3221+
for (idx, arg) in args.iter().enumerate() {
3222+
let param = self.blocks[target.0].params[idx];
3223+
self.insn_types[param.0] = self.type_of(param).union(self.type_of(*arg));
3224+
}
3225+
}
3226+
continue;
32543227
}
3255-
continue;
3256-
}
3257-
&Insn::IfFalse { val, ref target } => {
3258-
assert!(!self.type_of(val).bit_equal(types::Empty));
3259-
if self.type_of(val).could_be(Type::from_cbool(false)) {
3260-
enqueue!(self, target);
3228+
&Insn::IfFalse { val, target: BranchEdge { target, ref args } } => {
3229+
assert!(!self.type_of(val).bit_equal(types::Empty));
3230+
if self.type_of(val).could_be(Type::from_cbool(false)) {
3231+
reachable.insert(target);
3232+
for (idx, arg) in args.iter().enumerate() {
3233+
let param = self.blocks[target.0].params[idx];
3234+
self.insn_types[param.0] = self.type_of(param).union(self.type_of(*arg));
3235+
}
3236+
}
3237+
continue;
32613238
}
3262-
continue;
3263-
}
3264-
&Insn::Jump(ref target) => {
3265-
enqueue!(self, target);
3266-
continue;
3267-
}
3268-
&Insn::Entries { ref targets } => {
3269-
for target in targets {
3270-
if reachable.insert(*target) {
3271-
worklist_add!(*target);
3239+
&Insn::Jump(BranchEdge { target, ref args }) => {
3240+
reachable.insert(target);
3241+
for (idx, arg) in args.iter().enumerate() {
3242+
let param = self.blocks[target.0].params[idx];
3243+
self.insn_types[param.0] = self.type_of(param).union(self.type_of(*arg));
32723244
}
3245+
continue;
32733246
}
3274-
continue;
3247+
Insn::Entries { targets } => {
3248+
for &target in targets {
3249+
reachable.insert(target);
3250+
}
3251+
continue;
3252+
}
3253+
insn if insn.has_output() => self.infer_type(insn_id),
3254+
_ => continue,
3255+
};
3256+
if !self.type_of(insn_id).bit_equal(insn_type) {
3257+
self.insn_types[insn_id.0] = insn_type;
3258+
changed = true;
32753259
}
3276-
insn if insn.has_output() => self.infer_type(insn_id),
3277-
_ => continue,
3278-
};
3279-
if !self.type_of(insn_id).bit_equal(insn_type) {
3280-
self.insn_types[insn_id.0] = insn_type;
32813260
}
32823261
}
3262+
if !changed {
3263+
break;
3264+
}
32833265
}
32843266
}
32853267

0 commit comments

Comments
 (0)