From a7a0c36b2002c65ceafca592a4837c261426ab62 Mon Sep 17 00:00:00 2001 From: Max Bernstein Date: Thu, 22 Jan 2026 11:41:23 -0500 Subject: [PATCH 1/3] ZJIT: Clean up partial SSI (#15929) After Kokubun requested named unions, I realized we don't actually need a `Type::subtract` function. They were only used for the ad-hoc unions. Also, add a test that is illustrative of what we can get from this partial SSI. --- zjit/src/hir.rs | 10 +++---- zjit/src/hir/opt_tests.rs | 49 ++++++++++++++++++++++++++++++ zjit/src/hir/tests.rs | 63 +++++++++++++++++++++++++++++++++++++++ zjit/src/hir_type/mod.rs | 60 ------------------------------------- 4 files changed, 117 insertions(+), 65 deletions(-) diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index b4f78c025d3aa9..18649b98000cca 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -6293,7 +6293,7 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result { let test_id = fun.push_insn(block, Insn::Test { val }); let target_idx = insn_idx_at_offset(insn_idx, offset); let target = insn_idx_to_block[&target_idx]; - let nil_false_type = types::NilClass.union(types::FalseClass); + let nil_false_type = types::Falsy; let nil_false = fun.push_insn(block, Insn::RefineType { val, new_type: nil_false_type }); let mut iffalse_state = state.clone(); iffalse_state.replace(val, nil_false); @@ -6301,7 +6301,7 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result { val: test_id, target: BranchEdge { target, args: iffalse_state.as_args(self_param) } }); - let not_nil_false_type = types::BasicObject.subtract(types::NilClass).subtract(types::FalseClass); + let not_nil_false_type = types::Truthy; let not_nil_false = fun.push_insn(block, Insn::RefineType { val, new_type: not_nil_false_type }); state.replace(val, not_nil_false); queue.push_back((state.clone(), target, target_idx, local_inval)); @@ -6313,7 +6313,7 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result { let test_id = fun.push_insn(block, Insn::Test { val }); let target_idx = insn_idx_at_offset(insn_idx, offset); let target = insn_idx_to_block[&target_idx]; - let not_nil_false_type = types::BasicObject.subtract(types::NilClass).subtract(types::FalseClass); + let not_nil_false_type = types::Truthy; let not_nil_false = fun.push_insn(block, Insn::RefineType { val, new_type: not_nil_false_type }); let mut iftrue_state = state.clone(); iftrue_state.replace(val, not_nil_false); @@ -6321,7 +6321,7 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result { val: test_id, target: BranchEdge { target, args: iftrue_state.as_args(self_param) } }); - let nil_false_type = types::NilClass.union(types::FalseClass); + let nil_false_type = types::Falsy; let nil_false = fun.push_insn(block, Insn::RefineType { val, new_type: nil_false_type }); state.replace(val, nil_false); queue.push_back((state.clone(), target, target_idx, local_inval)); @@ -6340,7 +6340,7 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result { val: test_id, target: BranchEdge { target, args: iftrue_state.as_args(self_param) } }); - let new_type = types::BasicObject.subtract(types::NilClass); + let new_type = types::NotNil; let not_nil = fun.push_insn(block, Insn::RefineType { val, new_type }); state.replace(val, not_nil); queue.push_back((state.clone(), target, target_idx, local_inval)); diff --git a/zjit/src/hir/opt_tests.rs b/zjit/src/hir/opt_tests.rs index 0a42652993894f..dcfcc1e1361241 100644 --- a/zjit/src/hir/opt_tests.rs +++ b/zjit/src/hir/opt_tests.rs @@ -11319,4 +11319,53 @@ mod hir_opt_tests { Return v31 "); } + + #[test] + fn test_infer_truthiness_from_branch() { + eval(" + def test(x) + if x + if x + if x + 3 + else + 4 + end + else + 5 + end + else + 6 + end + end + "); + assert_snapshot!(hir_string("test"), @r" + fn test@:3: + bb0(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + v2:BasicObject = GetLocal :x, l0, SP@4 + Jump bb2(v1, v2) + bb1(v5:BasicObject, v6:BasicObject): + EntryPoint JIT(0) + Jump bb2(v5, v6) + bb2(v8:BasicObject, v9:BasicObject): + CheckInterrupts + v15:CBool = Test v9 + v16:Falsy = RefineType v9, Falsy + IfFalse v15, bb5(v8, v16) + v18:Truthy = RefineType v9, Truthy + CheckInterrupts + v26:Truthy = RefineType v18, Truthy + CheckInterrupts + v34:Truthy = RefineType v26, Truthy + v37:Fixnum[3] = Const Value(3) + CheckInterrupts + Return v37 + bb5(v42:BasicObject, v43:Falsy): + v47:Fixnum[6] = Const Value(6) + CheckInterrupts + Return v47 + "); + } } diff --git a/zjit/src/hir/tests.rs b/zjit/src/hir/tests.rs index 44082ce908757d..56f1928f1fa753 100644 --- a/zjit/src/hir/tests.rs +++ b/zjit/src/hir/tests.rs @@ -3222,6 +3222,69 @@ pub mod hir_build_tests { "); } + #[test] + fn test_infer_truthiness_from_branch() { + eval(" + def test(x) + if x + if x + if x + 3 + else + 4 + end + else + 5 + end + else + 6 + end + end + "); + assert_snapshot!(hir_string("test"), @r" + fn test@:3: + bb0(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + v2:BasicObject = GetLocal :x, l0, SP@4 + Jump bb2(v1, v2) + bb1(v5:BasicObject, v6:BasicObject): + EntryPoint JIT(0) + Jump bb2(v5, v6) + bb2(v8:BasicObject, v9:BasicObject): + CheckInterrupts + v15:CBool = Test v9 + v16:Falsy = RefineType v9, Falsy + IfFalse v15, bb5(v8, v16) + v18:Truthy = RefineType v9, Truthy + CheckInterrupts + v23:CBool[true] = Test v18 + v24 = RefineType v18, Falsy + IfFalse v23, bb4(v8, v24) + v26:Truthy = RefineType v18, Truthy + CheckInterrupts + v31:CBool[true] = Test v26 + v32 = RefineType v26, Falsy + IfFalse v31, bb3(v8, v32) + v34:Truthy = RefineType v26, Truthy + v37:Fixnum[3] = Const Value(3) + CheckInterrupts + Return v37 + bb5(v42:BasicObject, v43:Falsy): + v47:Fixnum[6] = Const Value(6) + CheckInterrupts + Return v47 + bb4(v52, v53): + v57 = Const Value(5) + CheckInterrupts + Return v57 + bb3(v62, v63): + v67 = Const Value(4) + CheckInterrupts + Return v67 + "); + } + #[test] fn test_invokebuiltin_delegate_annotated() { assert_contains_opcode("Float", YARVINSN_opt_invokebuiltin_delegate_leave); diff --git a/zjit/src/hir_type/mod.rs b/zjit/src/hir_type/mod.rs index 1f7526915c2f16..cc6a208bcd413e 100644 --- a/zjit/src/hir_type/mod.rs +++ b/zjit/src/hir_type/mod.rs @@ -453,25 +453,6 @@ impl Type { types::Empty } - /// Subtract `other` from `self`, preserving specialization if possible. - pub fn subtract(&self, other: Type) -> Type { - // If self is a subtype of other, the result is empty (no negative types). - if self.is_subtype(other) { return types::Empty; } - // Self is not a subtype of other. That means either: - // * Their type bits do not overlap at all (eg Int vs String) - // * Their type bits overlap but self's specialization is not a subtype of other's (eg - // Fixnum[5] vs Fixnum[4]) - // Check for the latter case, returning self unchanged if so. - if !self.spec_is_subtype_of(other) { - return *self; - } - // Now self is either a supertype of other (eg Object vs String or Fixnum vs Fixnum[5]) or - // their type bits do not overlap at all (eg Int vs String). - // Just subtract the bits and keep self's specialization. - let bits = self.bits & !other.bits; - Type { bits, spec: self.spec } - } - pub fn could_be(&self, other: Type) -> bool { !self.intersection(other).bit_equal(types::Empty) } @@ -1079,45 +1060,4 @@ mod tests { assert!(!types::CBool.has_value(Const::CBool(true))); assert!(!types::CShape.has_value(Const::CShape(crate::cruby::ShapeId(0x1234)))); } - - #[test] - fn test_subtract_with_superset_returns_empty() { - let left = types::NilClass; - let right = types::BasicObject; - let result = left.subtract(right); - assert_bit_equal(result, types::Empty); - } - - #[test] - fn test_subtract_with_subset_removes_bits() { - let left = types::BasicObject; - let right = types::NilClass; - let result = left.subtract(right); - assert_subtype(result, types::BasicObject); - assert_not_subtype(types::NilClass, result); - } - - #[test] - fn test_subtract_with_no_overlap_returns_self() { - let left = types::Fixnum; - let right = types::StringExact; - let result = left.subtract(right); - assert_bit_equal(result, left); - } - - #[test] - fn test_subtract_with_no_specialization_overlap_returns_self() { - let left = Type::fixnum(4); - let right = Type::fixnum(5); - let result = left.subtract(right); - assert_bit_equal(result, left); - } - - #[test] - fn test_subtract_with_specialization_subset_removes_specialization() { - let left = types::Fixnum; - let right = Type::fixnum(42); - let result = left.subtract(right); - assert_bit_equal(result, types::Fixnum); - } } From fd7bf518a6cf5c73ab957c4c19a3e974fa6b9fbe Mon Sep 17 00:00:00 2001 From: Max Bernstein Date: Thu, 22 Jan 2026 12:35:11 -0500 Subject: [PATCH 2/3] ZJIT: Make sure to add a LIR basic block in compile failure entrypoint (#15932) We need to add a dummy block for this stub otherwise it won't be able to push any instructions. Without this, `--zjit-stats` is broken. --- zjit/src/codegen.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/zjit/src/codegen.rs b/zjit/src/codegen.rs index 870fe7584a1fee..1729d4404143a6 100644 --- a/zjit/src/codegen.rs +++ b/zjit/src/codegen.rs @@ -2899,6 +2899,7 @@ fn gen_string_append_codepoint(jit: &mut JITState, asm: &mut Assembler, string: /// Generate a JIT entry that just increments exit_compilation_failure and exits fn gen_compile_error_counter(cb: &mut CodeBlock, compile_error: &CompileError) -> Result { let mut asm = Assembler::new(); + asm.new_block_without_id(); gen_incr_counter(&mut asm, exit_compile_error); gen_incr_counter(&mut asm, exit_counter_for_compile_error(compile_error)); asm.cret(Qundef.into()); From 253bfd7d09b78171f7c30444bade28aed458a067 Mon Sep 17 00:00:00 2001 From: Earlopain <14981592+Earlopain@users.noreply.github.com> Date: Thu, 22 Jan 2026 19:20:28 +0100 Subject: [PATCH 3/3] Check for folder in `sync_default_gems` (#15933) It was not clear to me that you have to do anything for this command to work. Previous versions (for example on the 3.4 branch) had this check but it got lost along the way. Without this when the folder doesn't exist, you get this error (after it deleted all the files): ``` $ ./tool/sync_default_gems.rb syntax_suggest Sync ruby/syntax_suggest ./tool/sync_default_gems.rb:464:in 'SyncDefaultGems.check_prerelease_version': undefined method 'version' for nil (NoMethodError) puts "#{gem}-#{spec.version} is not latest version of rubygems.org" if spec.version.to_s != latest_version ^^^^^^^^ from ./tool/sync_default_gems.rb:436:in 'SyncDefaultGems.sync_default_gems' from ./tool/sync_default_gems.rb:942:in '' from ./tool/sync_default_gems.rb:10:in '
' ``` Now you get ``` $ ./tool/sync_default_gems.rb syntax_suggest Sync ruby/syntax_suggest Expected '../ruby/syntax_suggest' (/home/earlopain/Documents/ruby/syntax_suggest) to be a directory, but it didn't exist. ``` This was changed in https://github.com/ruby/ruby/commit/b722631b481314023b9fa2f3fd16fa9ab0b4bf9c Since then, `sync_lib` is unused, delete it --- tool/sync_default_gems.rb | 24 ++++-------------------- 1 file changed, 4 insertions(+), 20 deletions(-) diff --git a/tool/sync_default_gems.rb b/tool/sync_default_gems.rb index 14d7a3893deae6..477cc755462d2c 100755 --- a/tool/sync_default_gems.rb +++ b/tool/sync_default_gems.rb @@ -398,6 +398,10 @@ def sync_default_gems(gem) upstream = File.join("..", "..", config.upstream) + unless File.exist?(upstream) + abort %[Expected '#{upstream}' (#{File.expand_path("#{upstream}")}) to be a directory, but it didn't exist.] + end + config.mappings.each do |src, dst| rm_rf(dst) end @@ -798,26 +802,6 @@ def sync_default_gems_with_commits(gem, ranges, edit: nil) return true end - def sync_lib(repo, upstream = nil) - unless upstream and File.directory?(upstream) or File.directory?(upstream = "../#{repo}") - abort %[Expected '#{upstream}' \(#{File.expand_path("#{upstream}")}\) to be a directory, but it wasn't.] - end - rm_rf(["lib/#{repo}.rb", "lib/#{repo}/*", "test/test_#{repo}.rb"]) - cp_r(Dir.glob("#{upstream}/lib/*"), "lib") - tests = if File.directory?("test/#{repo}") - "test/#{repo}" - else - "test/test_#{repo}.rb" - end - cp_r("#{upstream}/#{tests}", "test") if File.exist?("#{upstream}/#{tests}") - gemspec = if File.directory?("lib/#{repo}") - "lib/#{repo}/#{repo}.gemspec" - else - "lib/#{repo}.gemspec" - end - cp_r("#{upstream}/#{repo}.gemspec", "#{gemspec}") - end - def update_default_gems(gem, release: false) config = REPOSITORIES[gem] author, repository = config.upstream.split('/')