From edca81a1bb72a9dc54a37766d2c80790dec13884 Mon Sep 17 00:00:00 2001 From: Abrar Habib <86024593+dddictionary@users.noreply.github.com> Date: Tue, 9 Dec 2025 07:41:09 -0500 Subject: [PATCH] ZJIT: Add codegen for FixnumDiv (#15452) Fixes https://github.com/Shopify/ruby/issues/902 This pull request adds code generation for dividing fixnums. Testing confirms the normal case, flooring, and side-exiting on division by zero. --- jit.c | 6 ++++++ test/ruby/test_zjit.rb | 31 +++++++++++++++++++++++++++++++ yjit.c | 6 ------ yjit/bindgen/src/main.rs | 2 +- yjit/src/cruby.rs | 2 +- yjit/src/cruby_bindings.inc.rs | 2 +- zjit/bindgen/src/main.rs | 1 + zjit/src/codegen.rs | 12 +++++++++++- zjit/src/cruby_bindings.inc.rs | 1 + zjit/src/hir.rs | 1 + zjit/src/stats.rs | 2 ++ 11 files changed, 56 insertions(+), 10 deletions(-) diff --git a/jit.c b/jit.c index ae592b2205..ff44ac5b2e 100644 --- a/jit.c +++ b/jit.c @@ -761,6 +761,12 @@ rb_jit_fix_mod_fix(VALUE recv, VALUE obj) return rb_fix_mod_fix(recv, obj); } +VALUE +rb_jit_fix_div_fix(VALUE recv, VALUE obj) +{ + return rb_fix_div_fix(recv, obj); +} + // YJIT/ZJIT need this function to never allocate and never raise VALUE rb_yarv_str_eql_internal(VALUE str1, VALUE str2) diff --git a/test/ruby/test_zjit.rb b/test/ruby/test_zjit.rb index 03d2461fa0..cc5143aee5 100644 --- a/test/ruby/test_zjit.rb +++ b/test/ruby/test_zjit.rb @@ -960,6 +960,37 @@ class TestZJIT < Test::Unit::TestCase }, call_threshold: 2, insns: [:opt_mult] end + def test_fixnum_div + assert_compiles '12', %q{ + C = 48 + def test(n) = C / n + test(4) + test(4) + }, call_threshold: 2, insns: [:opt_div] + end + + def test_fixnum_floor + assert_compiles '0', %q{ + C = 3 + def test(n) = C / n + test(4) + test(4) + }, call_threshold: 2, insns: [:opt_div] + end + + def test_fixnum_div_zero + assert_runs '"divided by 0"', %q{ + def test(n) + n / 0 + rescue ZeroDivisionError => e + e.message + end + + test(0) + test(0) + }, call_threshold: 2, insns: [:opt_div] + end + def test_opt_not assert_compiles('[true, true, false]', <<~RUBY, insns: [:opt_not]) def test(obj) = !obj diff --git a/yjit.c b/yjit.c index 16debf6eca..6d909a0da6 100644 --- a/yjit.c +++ b/yjit.c @@ -292,12 +292,6 @@ rb_yjit_rb_ary_subseq_length(VALUE ary, long beg) return rb_ary_subseq(ary, beg, len); } -VALUE -rb_yjit_fix_div_fix(VALUE recv, VALUE obj) -{ - return rb_fix_div_fix(recv, obj); -} - // Return non-zero when `obj` is an array and its last item is a // `ruby2_keywords` hash. We don't support this kind of splat. size_t diff --git a/yjit/bindgen/src/main.rs b/yjit/bindgen/src/main.rs index 8d11b4216e..06c475f3c8 100644 --- a/yjit/bindgen/src/main.rs +++ b/yjit/bindgen/src/main.rs @@ -368,7 +368,7 @@ fn main() { .allowlist_function("rb_str_neq_internal") .allowlist_function("rb_yarv_ary_entry_internal") .allowlist_function("rb_yjit_ruby2_keywords_splat_p") - .allowlist_function("rb_yjit_fix_div_fix") + .allowlist_function("rb_jit_fix_div_fix") .allowlist_function("rb_jit_fix_mod_fix") .allowlist_function("rb_FL_TEST") .allowlist_function("rb_FL_TEST_RAW") diff --git a/yjit/src/cruby.rs b/yjit/src/cruby.rs index 5562f73be2..6e6a1810c6 100644 --- a/yjit/src/cruby.rs +++ b/yjit/src/cruby.rs @@ -197,7 +197,7 @@ pub use rb_get_cikw_keywords_idx as get_cikw_keywords_idx; pub use rb_get_call_data_ci as get_call_data_ci; pub use rb_yarv_str_eql_internal as rb_str_eql_internal; pub use rb_yarv_ary_entry_internal as rb_ary_entry_internal; -pub use rb_yjit_fix_div_fix as rb_fix_div_fix; +pub use rb_jit_fix_div_fix as rb_fix_div_fix; pub use rb_jit_fix_mod_fix as rb_fix_mod_fix; pub use rb_FL_TEST as FL_TEST; pub use rb_FL_TEST_RAW as FL_TEST_RAW; diff --git a/yjit/src/cruby_bindings.inc.rs b/yjit/src/cruby_bindings.inc.rs index b9e5fb3ab1..d2347671bb 100644 --- a/yjit/src/cruby_bindings.inc.rs +++ b/yjit/src/cruby_bindings.inc.rs @@ -1174,7 +1174,6 @@ extern "C" { pub fn rb_str_neq_internal(str1: VALUE, str2: VALUE) -> VALUE; pub fn rb_ary_unshift_m(argc: ::std::os::raw::c_int, argv: *mut VALUE, ary: VALUE) -> VALUE; pub fn rb_yjit_rb_ary_subseq_length(ary: VALUE, beg: ::std::os::raw::c_long) -> VALUE; - pub fn rb_yjit_fix_div_fix(recv: VALUE, obj: VALUE) -> VALUE; pub fn rb_yjit_ruby2_keywords_splat_p(obj: VALUE) -> usize; pub fn rb_yjit_splat_varg_checks( sp: *mut VALUE, @@ -1312,6 +1311,7 @@ extern "C" { end: *mut ::std::os::raw::c_void, ); pub fn rb_jit_fix_mod_fix(recv: VALUE, obj: VALUE) -> VALUE; + pub fn rb_jit_fix_div_fix(recv: VALUE, obj: VALUE) -> VALUE; pub fn rb_yarv_str_eql_internal(str1: VALUE, str2: VALUE) -> VALUE; pub fn rb_jit_str_concat_codepoint(str_: VALUE, codepoint: VALUE); } diff --git a/zjit/bindgen/src/main.rs b/zjit/bindgen/src/main.rs index 30e8514974..7b968d14bb 100644 --- a/zjit/bindgen/src/main.rs +++ b/zjit/bindgen/src/main.rs @@ -279,6 +279,7 @@ fn main() { .allowlist_function("rb_jit_mark_unused") .allowlist_function("rb_jit_get_page_size") .allowlist_function("rb_jit_array_len") + .allowlist_function("rb_jit_fix_div_fix") .allowlist_function("rb_jit_iseq_builtin_attrs") .allowlist_function("rb_jit_str_concat_codepoint") .allowlist_function("rb_zjit_iseq_inspect") diff --git a/zjit/src/codegen.rs b/zjit/src/codegen.rs index 73c092f720..642991a438 100644 --- a/zjit/src/codegen.rs +++ b/zjit/src/codegen.rs @@ -396,6 +396,7 @@ fn gen_insn(cb: &mut CodeBlock, jit: &mut JITState, asm: &mut Assembler, functio Insn::FixnumAdd { left, right, state } => gen_fixnum_add(jit, asm, opnd!(left), opnd!(right), &function.frame_state(*state)), Insn::FixnumSub { left, right, state } => gen_fixnum_sub(jit, asm, opnd!(left), opnd!(right), &function.frame_state(*state)), Insn::FixnumMult { left, right, state } => gen_fixnum_mult(jit, asm, opnd!(left), opnd!(right), &function.frame_state(*state)), + Insn::FixnumDiv { left, right, state } => gen_fixnum_div(jit, asm, opnd!(left), opnd!(right), &function.frame_state(*state)), Insn::FixnumEq { left, right } => gen_fixnum_eq(asm, opnd!(left), opnd!(right)), Insn::FixnumNeq { left, right } => gen_fixnum_neq(asm, opnd!(left), opnd!(right)), Insn::FixnumLt { left, right } => gen_fixnum_lt(asm, opnd!(left), opnd!(right)), @@ -484,7 +485,6 @@ fn gen_insn(cb: &mut CodeBlock, jit: &mut JITState, asm: &mut Assembler, functio Insn::ArrayHash { elements, state } => gen_opt_newarray_hash(jit, asm, opnds!(elements), &function.frame_state(*state)), &Insn::IsA { val, class } => gen_is_a(asm, opnd!(val), opnd!(class)), &Insn::ArrayMax { state, .. } - | &Insn::FixnumDiv { state, .. } | &Insn::Throw { state, .. } => return Err(state), }; @@ -1704,6 +1704,16 @@ fn gen_fixnum_mult(jit: &mut JITState, asm: &mut Assembler, left: lir::Opnd, rig asm.add(out_val, Opnd::UImm(1)) } +/// Compile Fixnum / Fixnum +fn gen_fixnum_div(jit: &mut JITState, asm: &mut Assembler, left: lir::Opnd, right: lir::Opnd, state: &FrameState) -> lir::Opnd { + gen_prepare_leaf_call_with_gc(asm, state); + + // Side exit if rhs is 0 + asm.cmp(right, Opnd::from(VALUE::fixnum_from_usize(0))); + asm.je(side_exit(jit, state, FixnumDivByZero)); + asm_ccall!(asm, rb_jit_fix_div_fix, left, right) +} + /// Compile Fixnum == Fixnum fn gen_fixnum_eq(asm: &mut Assembler, left: lir::Opnd, right: lir::Opnd) -> lir::Opnd { asm.cmp(left, right); diff --git a/zjit/src/cruby_bindings.inc.rs b/zjit/src/cruby_bindings.inc.rs index aed35c3c63..c436e20087 100644 --- a/zjit/src/cruby_bindings.inc.rs +++ b/zjit/src/cruby_bindings.inc.rs @@ -2198,6 +2198,7 @@ unsafe extern "C" { start: *mut ::std::os::raw::c_void, end: *mut ::std::os::raw::c_void, ); + pub fn rb_jit_fix_div_fix(recv: VALUE, obj: VALUE) -> VALUE; pub fn rb_yarv_str_eql_internal(str1: VALUE, str2: VALUE) -> VALUE; pub fn rb_jit_str_concat_codepoint(str_: VALUE, codepoint: VALUE); pub fn rb_jit_shape_capacity(shape_id: shape_id_t) -> attr_index_t; diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index 4323b145fe..abf48e04d6 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -501,6 +501,7 @@ pub enum SideExitReason { BlockParamProxyNotIseqOrIfunc, StackOverflow, FixnumModByZero, + FixnumDivByZero, BoxFixnumOverflow, } diff --git a/zjit/src/stats.rs b/zjit/src/stats.rs index 7a076b7cda..2272404b69 100644 --- a/zjit/src/stats.rs +++ b/zjit/src/stats.rs @@ -183,6 +183,7 @@ make_counters! { exit_fixnum_mult_overflow, exit_fixnum_lshift_overflow, exit_fixnum_mod_by_zero, + exit_fixnum_div_by_zero, exit_box_fixnum_overflow, exit_guard_type_failure, exit_guard_type_not_failure, @@ -492,6 +493,7 @@ pub fn side_exit_counter(reason: crate::hir::SideExitReason) -> Counter { FixnumMultOverflow => exit_fixnum_mult_overflow, FixnumLShiftOverflow => exit_fixnum_lshift_overflow, FixnumModByZero => exit_fixnum_mod_by_zero, + FixnumDivByZero => exit_fixnum_div_by_zero, BoxFixnumOverflow => exit_box_fixnum_overflow, GuardType(_) => exit_guard_type_failure, GuardTypeNot(_) => exit_guard_type_not_failure,