diff --git a/common.mk b/common.mk index a43b9304f773c5..4407bc41fddec5 100644 --- a/common.mk +++ b/common.mk @@ -1687,7 +1687,7 @@ BUNDLER_SPECS = PREPARE_BUNDLER = $(TEST_RUNNABLE)-test-bundler-prepare test-bundler: $(TEST_RUNNABLE)-test-bundler yes-test-bundler: $(PREPARE_BUNDLER) - $(gnumake_recursive)$(XRUBY) \ + $(gnumake_recursive)$(XRUBY) --enable-gems \ -r./$(arch)-fake \ -r$(tooldir)/lib/_tmpdir \ -I$(srcdir)/spec/bundler -I$(srcdir)/spec/lib \ diff --git a/zjit/src/codegen.rs b/zjit/src/codegen.rs index 097257ddf85ede..d89d3900fc02b4 100644 --- a/zjit/src/codegen.rs +++ b/zjit/src/codegen.rs @@ -713,7 +713,6 @@ fn gen_insn(cb: &mut CodeBlock, jit: &mut JITState, asm: &mut Assembler, functio Insn::RefineType { val, .. } => opnd!(val), Insn::HasType { val, expected } => gen_has_type(jit, asm, opnd!(val), *expected), Insn::GuardType { val, guard_type, state } => gen_guard_type(jit, asm, opnd!(val), *guard_type, &function.frame_state(*state)), - Insn::GuardTypeNot { val, guard_type, state } => gen_guard_type_not(jit, asm, opnd!(val), *guard_type, &function.frame_state(*state)), &Insn::GuardBitEquals { val, expected, reason, state, recompile } => gen_guard_bit_equals(jit, asm, opnd!(val), expected, reason, recompile, &function.frame_state(state)), &Insn::GuardAnyBitSet { val, mask, reason, state, .. } => gen_guard_any_bit_set(jit, asm, opnd!(val), mask, reason, &function.frame_state(state)), &Insn::GuardNoBitsSet { val, mask, reason, state, .. } => gen_guard_no_bits_set(jit, asm, opnd!(val), mask, reason, &function.frame_state(state)), @@ -2641,45 +2640,6 @@ fn gen_guard_type(jit: &mut JITState, asm: &mut Assembler, val: lir::Opnd, guard val } -fn gen_guard_type_not(jit: &mut JITState, asm: &mut Assembler, val: lir::Opnd, guard_type: Type, state: &FrameState) -> lir::Opnd { - if guard_type.is_subtype(types::String) { - // We only exit if val *is* a String. Otherwise we fall through. - let hir_block_id = asm.current_block().hir_block_id; - let rpo_idx = asm.current_block().rpo_index; - - // Create continuation block upfront so early-out jumps can target it - let cont_block = asm.new_block(hir_block_id, false, rpo_idx); - let cont_edge = || Target::Block(lir::BranchEdge { target: cont_block, args: vec![] }); - - let side = side_exit(jit, state, GuardTypeNot(guard_type)); - - // Continue if special constant (not string) - asm.test(val, Opnd::UImm(RUBY_IMMEDIATE_MASK as u64)); - asm.jnz(jit, cont_edge()); - - // Continue if false (not string) - asm.cmp(val, Qfalse.into()); - asm.je(jit, cont_edge()); - - let val = asm.load_mem(val); - - let flags = asm.load(Opnd::mem(VALUE_BITS, val, RUBY_OFFSET_RBASIC_FLAGS)); - let tag = asm.and(flags, Opnd::UImm(RUBY_T_MASK as u64)); - asm.cmp(tag, Opnd::UImm(RUBY_T_STRING as u64)); - asm.je(jit, side); - - // Fall through to continuation block - asm.jmp(cont_edge()); - - asm.set_current_block(cont_block); - let label = jit.get_label(asm, cont_block, hir_block_id); - asm.write_label(label); - } else { - unimplemented!("unsupported type: {guard_type}"); - } - val -} - /// Compile an identity check with a side exit fn gen_guard_bit_equals(jit: &mut JITState, asm: &mut Assembler, val: lir::Opnd, expected: crate::hir::Const, reason: SideExitReason, recompile: Option, state: &FrameState) -> lir::Opnd { if matches!(reason, SideExitReason::GuardShape(_) ) { diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index 1d8358cbada039..6b2d9ee7e3c046 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -518,7 +518,6 @@ pub enum SideExitReason { FixnumMultOverflow, FixnumLShiftOverflow, GuardType(Type), - GuardTypeNot(Type), GuardShape(ShapeId), ExpandArray, GuardNotFrozen, @@ -628,7 +627,6 @@ impl std::fmt::Display for SideExitReason { SideExitReason::UnhandledNewarraySend(VM_OPT_NEWARRAY_SEND_INCLUDE_P) => write!(f, "UnhandledNewarraySend(INCLUDE_P)"), SideExitReason::UnhandledDuparraySend(method_id) => write!(f, "UnhandledDuparraySend({method_id})"), SideExitReason::GuardType(guard_type) => write!(f, "GuardType({guard_type})"), - SideExitReason::GuardTypeNot(guard_type) => write!(f, "GuardTypeNot({guard_type})"), SideExitReason::GuardNotShared => write!(f, "GuardNotShared"), SideExitReason::PatchPoint(invariant) => write!(f, "PatchPoint({invariant})"), _ => write!(f, "{self:?}"), @@ -1115,7 +1113,6 @@ pub enum Insn { /// Side-exit if val doesn't have the expected type. GuardType { val: InsnId, guard_type: Type, state: InsnId }, - GuardTypeNot { val: InsnId, guard_type: Type, state: InsnId }, /// Side-exit if val is not the expected Const. GuardBitEquals { val: InsnId, expected: Const, reason: SideExitReason, state: InsnId, recompile: Option }, /// Side-exit if (val & mask) == 0 @@ -1268,7 +1265,6 @@ macro_rules! for_each_operand_impl { | Insn::StringCopy { val, state, .. } | Insn::ObjectAlloc { val, state } | Insn::GuardType { val, state, .. } - | Insn::GuardTypeNot { val, state, .. } | Insn::GuardBitEquals { val, state, .. } | Insn::GuardAnyBitSet { val, state, .. } | Insn::GuardNoBitsSet { val, state, .. } @@ -1680,11 +1676,6 @@ impl Insn { if guard_type.is_subtype(types::Immediate) { abstract_heaps::Empty } else { abstract_heaps::Memory }, abstract_heaps::Control ), - Insn::GuardTypeNot { guard_type, .. } - => Effect::read_write( - if guard_type.is_subtype(types::Immediate) { abstract_heaps::Empty } else { abstract_heaps::Memory }, - abstract_heaps::Control - ), Insn::GuardBitEquals { .. } => Effect::read_write(abstract_heaps::Empty, abstract_heaps::Control), Insn::GuardAnyBitSet { .. } => Effect::read_write(abstract_heaps::Empty, abstract_heaps::Control), Insn::GuardNoBitsSet { .. } => Effect::read_write(abstract_heaps::Empty, abstract_heaps::Control), @@ -2075,7 +2066,6 @@ impl<'a> std::fmt::Display for InsnPrinter<'a> { Insn::GuardType { val, guard_type, .. } => { write!(f, "GuardType {val}, {}", guard_type.print(self.ptr_map)) }, Insn::RefineType { val, new_type, .. } => { write!(f, "RefineType {val}, {}", new_type.print(self.ptr_map)) }, Insn::HasType { val, expected, .. } => { write!(f, "HasType {val}, {}", expected.print(self.ptr_map)) }, - Insn::GuardTypeNot { val, guard_type, .. } => { write!(f, "GuardTypeNot {val}, {}", guard_type.print(self.ptr_map)) }, Insn::GuardBitEquals { val, expected, .. } => { write!(f, "GuardBitEquals {val}, {}", expected.print(self.ptr_map)) }, Insn::GuardAnyBitSet { val, mask, mask_name: Some(name), .. } => { write!(f, "GuardAnyBitSet {val}, {name}={}", mask.print(self.ptr_map)) }, Insn::GuardAnyBitSet { val, mask, .. } => { write!(f, "GuardAnyBitSet {val}, {}", mask.print(self.ptr_map)) }, @@ -2905,7 +2895,6 @@ impl Function { Insn::GuardType { val, guard_type, .. } => self.type_of(*val).intersection(*guard_type), Insn::RefineType { val, new_type, .. } => self.type_of(*val).intersection(*new_type), Insn::HasType { .. } => types::CBool, - Insn::GuardTypeNot { .. } => types::BasicObject, Insn::GuardBitEquals { val, expected, .. } => self.type_of(*val).intersection(Type::from_const(*expected)), Insn::GuardAnyBitSet { val, .. } => self.type_of(*val), Insn::GuardNoBitsSet { val, .. } => self.type_of(*val), @@ -3021,6 +3010,17 @@ impl Function { // Fill entry parameter types self.copy_param_types(); + // Assign `new_type` to `insn` if it differs from the recorded type. + // Returns `true` if a write actually happened, `false` if the type + // was already equal. + let set_type = |this: &mut Function, insn: InsnId, new_type: Type| -> bool { + if this.type_of(insn).bit_equal(new_type) { + return false; + } + this.insn_types[insn.0] = new_type; + true + }; + let mut reachable = BlockSet::with_capacity(self.blocks.len()); reachable.insert(self.entries_block); @@ -3030,7 +3030,8 @@ impl Function { let mut changed = false; for &block in &rpo { if !reachable.get(block) { continue; } - for &insn_id in &self.blocks[block.0].insns { + for i in 0..self.blocks[block.0].insns.len() { + let insn_id = self.blocks[block.0].insns[i]; // Instructions without output, including branch instructions, can't be targets // of make_equal_to, so we don't need find() here. let insn_type = match &self.insns[insn_id.0] { @@ -3038,9 +3039,13 @@ impl Function { assert!(!self.type_of(val).bit_equal(types::Empty)); if self.type_of(val).could_be(Type::from_cbool(true)) { reachable.insert(target); - for (idx, arg) in args.iter().enumerate() { + // Snapshot arg types before any param updates so phi-style + // updates happen in parallel (the args of a self-loop may name + // params of `target` itself). + let arg_types: Vec = args.iter().map(|a| self.type_of(*a)).collect(); + for (idx, arg_type) in arg_types.into_iter().enumerate() { let param = self.blocks[target.0].params[idx]; - self.insn_types[param.0] = self.type_of(param).union(self.type_of(*arg)); + changed |= set_type(self, param, self.type_of(param).union(arg_type)); } } continue; @@ -3049,18 +3054,20 @@ impl Function { assert!(!self.type_of(val).bit_equal(types::Empty)); if self.type_of(val).could_be(Type::from_cbool(false)) { reachable.insert(target); - for (idx, arg) in args.iter().enumerate() { + let arg_types: Vec = args.iter().map(|a| self.type_of(*a)).collect(); + for (idx, arg_type) in arg_types.into_iter().enumerate() { let param = self.blocks[target.0].params[idx]; - self.insn_types[param.0] = self.type_of(param).union(self.type_of(*arg)); + changed |= set_type(self, param, self.type_of(param).union(arg_type)); } } continue; } &Insn::Jump(BranchEdge { target, ref args }) => { reachable.insert(target); - for (idx, arg) in args.iter().enumerate() { + let arg_types: Vec = args.iter().map(|a| self.type_of(*a)).collect(); + for (idx, arg_type) in arg_types.into_iter().enumerate() { let param = self.blocks[target.0].params[idx]; - self.insn_types[param.0] = self.type_of(param).union(self.type_of(*arg)); + changed |= set_type(self, param, self.type_of(param).union(arg_type)); } continue; } @@ -3073,10 +3080,7 @@ impl Function { insn if insn.has_output() => self.infer_type(insn_id), _ => continue, }; - if !self.type_of(insn_id).bit_equal(insn_type) { - self.insn_types[insn_id.0] = insn_type; - changed = true; - } + changed |= set_type(self, insn_id, insn_type); } } if !changed { @@ -3089,7 +3093,6 @@ impl Function { let id = self.union_find.borrow().find_const(insn); match self.insns[id.0] { Insn::GuardType { val, .. } - | Insn::GuardTypeNot { val, .. } | Insn::GuardBitEquals { val, .. } | Insn::GuardAnyBitSet { val, .. } | Insn::GuardNoBitsSet { val, .. } => self.chase_insn(val), @@ -5845,7 +5848,6 @@ impl Function { | Insn::Throw { val, .. } | Insn::ObjToString { val, .. } | Insn::GuardType { val, .. } - | Insn::GuardTypeNot { val, .. } | Insn::ToArray { val, .. } | Insn::ToNewArray { val, .. } | Insn::Defined { v: val, .. } @@ -9107,6 +9109,53 @@ mod infer_tests { assert_bit_equal(function.type_of(param), types::Fixnum); } + #[test] + fn self_loop_param_rotation_reaches_full_union() { + // bb_entry: jump bb_loop(c1, c2, c3, c4) // 4 distinct types + // bb_loop(p1, p2, p3, p4): + // jump bb_loop(p2, p3, p4, p1) // 4-cycle rotation + // + // Every param transitively flows into every other across enough trips + // around the loop, so the fixpoint for every param is the full union + // of all four input types. The fixpoint loop must not exit while a + // branch arm is still widening a param's type. + let mut function = Function::new(std::ptr::null()); + let entry = function.entry_block; + let loop_block = function.new_block(0); + + let c1 = function.push_insn(entry, Insn::Const { val: Const::Value(Qtrue) }); + let c2 = function.push_insn(entry, Insn::Const { val: Const::Value(Qfalse) }); + let c3 = function.push_insn(entry, Insn::Const { val: Const::Value(Qnil) }); + let c4 = function.push_insn(entry, Insn::Const { val: Const::Value(VALUE::fixnum_from_usize(7)) }); + function.push_insn(entry, Insn::Jump(BranchEdge { + target: loop_block, + args: vec![c1, c2, c3, c4], + })); + + let p1 = function.push_insn(loop_block, Insn::Param); + let p2 = function.push_insn(loop_block, Insn::Param); + let p3 = function.push_insn(loop_block, Insn::Param); + let p4 = function.push_insn(loop_block, Insn::Param); + function.push_insn(loop_block, Insn::Jump(BranchEdge { + target: loop_block, + args: vec![p2, p3, p4, p1], + })); + + function.seal_entries(); + crate::cruby::with_rubyvm(|| { + function.infer_types(); + }); + + let full = types::TrueClass + .union(types::FalseClass) + .union(types::NilClass) + .union(types::Fixnum); + assert_bit_equal(function.type_of(p1), full); + assert_bit_equal(function.type_of(p2), full); + assert_bit_equal(function.type_of(p3), full); + assert_bit_equal(function.type_of(p4), full); + } + #[test] fn diamond_iffalse_merge_bool() { let mut function = Function::new(std::ptr::null()); diff --git a/zjit/src/stats.rs b/zjit/src/stats.rs index 412dd045bfcef1..522b74c48afcba 100644 --- a/zjit/src/stats.rs +++ b/zjit/src/stats.rs @@ -204,7 +204,6 @@ make_counters! { exit_fixnum_div_by_zero, exit_box_fixnum_overflow, exit_guard_type_failure, - exit_guard_type_not_failure, exit_guard_bit_equals_failure, exit_guard_int_equals_failure, exit_guard_shape_failure, @@ -599,7 +598,6 @@ pub fn side_exit_counter(reason: crate::hir::SideExitReason) -> Counter { FixnumDivByZero => exit_fixnum_div_by_zero, BoxFixnumOverflow => exit_box_fixnum_overflow, GuardType(_) => exit_guard_type_failure, - GuardTypeNot(_) => exit_guard_type_not_failure, GuardShape(_) => exit_guard_shape_failure, ExpandArray => exit_expandarray_failure, GuardNotFrozen => exit_guard_not_frozen_failure,