diff --git a/.github/workflows/zjit-macos.yml b/.github/workflows/zjit-macos.yml index 3ff9076d57bca0..d441d22842e1c4 100644 --- a/.github/workflows/zjit-macos.yml +++ b/.github/workflows/zjit-macos.yml @@ -47,7 +47,7 @@ jobs: specopts: '-T --zjit-disable-hir-opt -T --zjit-call-threshold=1' configure: '--enable-zjit=dev' - - test_task: 'zjit-check' # zjit-test + quick feedback of test_zjit.rb + - test_task: 'zjit-check' # zjit-test + quick feedback of test_zjit_cli.rb configure: '--enable-yjit=dev --enable-zjit' rust_version: "1.85.0" diff --git a/.github/workflows/zjit-ubuntu.yml b/.github/workflows/zjit-ubuntu.yml index 223712af7cb87d..412167da4ed1ba 100644 --- a/.github/workflows/zjit-ubuntu.yml +++ b/.github/workflows/zjit-ubuntu.yml @@ -95,7 +95,7 @@ jobs: configure: '--enable-zjit=dev' run_opts: '--zjit-inline-threshold=0 --zjit-call-threshold=1' - - test_task: 'zjit-check' # zjit-test + quick feedback of test_zjit.rb + - test_task: 'zjit-check' # zjit-test + quick feedback of test_zjit_cli.rb configure: '--enable-yjit --enable-zjit=dev' rust_version: '1.85.0' diff --git a/bignum.c b/bignum.c index 088e8c49b55024..89b35dd1e1275c 100644 --- a/bignum.c +++ b/bignum.c @@ -7029,6 +7029,23 @@ rb_big_bit_length(VALUE big) INTEGER_PACK_LSWORD_FIRST|INTEGER_PACK_NATIVE_BYTE_ORDER); } +VALUE +rb_big_bit_count(VALUE big) +{ + if (BIGNUM_NEGATIVE_P(big)) + rb_raise(rb_eArgError, "bit_count is undefined for negative integers"); + + BDIGIT *ds = BDIGITS(big); + size_t n = BIGNUM_LEN(big); + size_t count = 0; + + while (n--) { + count += rb_popcount64((uint64_t)ds[n]); + } + + return SIZET2NUM(count); +} + VALUE rb_big_odd_p(VALUE num) { diff --git a/doc/jit/zjit.md b/doc/jit/zjit.md index 0f1cb595203d29..fc69905749468b 100644 --- a/doc/jit/zjit.md +++ b/doc/jit/zjit.md @@ -287,28 +287,28 @@ cd zjit && cargo insta review Test changes will be reviewed alongside code changes. -### Running integration tests +### Running smoke tests -This command runs Ruby execution tests. +Runs both `make zjit-test` and `test/ruby/test_zjit_cli.rb`: ```bash -make test-all TESTS="test/ruby/test_zjit.rb" -``` - -You can also run a single test case by matching the method name: - -```bash -make test-all TESTS="test/ruby/test_zjit.rb -n TestZJIT#test_putobject" +make zjit-check ``` -### Running all tests +### Integration testing -Runs both `make zjit-test` and `test/ruby/test_zjit.rb`: +A thorough test run will want to enable ZJIT on Ruby workloads. Some +test suites written in Ruby in this repository accepts the `RUN_OPTS` +Make macro for passing ruby(1) command-line arguments: ```bash -make zjit-check +make test-all RUN_OPTS='--zjit-call-threshold=2' +make btest RUN_OPTS='--zjit-call-threshold=1' ``` +See [Testing Ruby](rdoc-ref:contributing/testing_ruby.md) for a list of +test suites. + ## Statistics Collection ZJIT provides detailed statistics about JIT compilation and execution behavior. diff --git a/include/ruby/internal/intern/class.h b/include/ruby/internal/intern/class.h index 357af5d176bfcb..63a2bc09017a81 100644 --- a/include/ruby/internal/intern/class.h +++ b/include/ruby/internal/intern/class.h @@ -174,19 +174,6 @@ VALUE rb_mod_include_p(VALUE child, VALUE parent); */ VALUE rb_mod_ancestors(VALUE mod); -/** - * Queries the class's descendants. This routine gathers classes that are - * subclasses of the given class (or subclasses of those subclasses, etc.), - * returning an array of classes that have the given class as an ancestor. - * The returned array does not include the given class or singleton classes. - * - * @param[in] klass A class. - * @return An array of classes where `klass` is an ancestor. - * - * @internal - */ -VALUE rb_class_descendants(VALUE klass); - /** * Queries the class's direct descendants. This routine gathers classes that are * direct subclasses of the given class, diff --git a/internal/bignum.h b/internal/bignum.h index 7389a17c747e15..08f7c85b578775 100644 --- a/internal/bignum.h +++ b/internal/bignum.h @@ -123,6 +123,7 @@ VALUE rb_big_aref2(VALUE num, VALUE beg, VALUE len); VALUE rb_big_abs(VALUE x); VALUE rb_big_size_m(VALUE big); VALUE rb_big_bit_length(VALUE big); +VALUE rb_big_bit_count(VALUE big); VALUE rb_big_remainder(VALUE x, VALUE y); VALUE rb_big_gt(VALUE x, VALUE y); VALUE rb_big_ge(VALUE x, VALUE y); diff --git a/internal/numeric.h b/internal/numeric.h index a24432df1e6450..ed1f6035239d94 100644 --- a/internal/numeric.h +++ b/internal/numeric.h @@ -126,6 +126,7 @@ VALUE rb_int_even_p(VALUE num); VALUE rb_int_odd_p(VALUE num); VALUE rb_int_abs(VALUE num); VALUE rb_int_bit_length(VALUE num); +VALUE rb_int_bit_count(VALUE num); VALUE rb_int_uminus(VALUE num); VALUE rb_int_comp(VALUE num); diff --git a/numeric.c b/numeric.c index 90270617c18843..f80a419bafb974 100644 --- a/numeric.c +++ b/numeric.c @@ -5732,6 +5732,49 @@ rb_int_bit_length(VALUE num) return Qnil; } +static VALUE +rb_fix_bit_count(VALUE fix) +{ + long v = FIX2LONG(fix); + if (v < 0) + rb_raise(rb_eArgError, "bit_count is undefined for negative integers"); + return LONG2FIX(rb_popcount_intptr((uintptr_t)v)); +} + +/* + * call-seq: + * bit_count -> integer + * + * Returns the number of set bits (bits equal to 1) in the binary + * representation of +self+, also known as the population count or + * Hamming weight. + * + * 0.bit_count # => 0 + * 1.bit_count # => 1 + * 7.bit_count # => 3 + * 0b10101.bit_count # => 3 + * 255.bit_count # => 8 + * (2**1000).bit_count # => 1 + * (2**1000-1).bit_count # => 1000 + * + * Raises an exception if +self+ is negative. + * + * (-1).bit_count # Raises ArgumentError + * + */ + +VALUE +rb_int_bit_count(VALUE num) +{ + if (FIXNUM_P(num)) { + return rb_fix_bit_count(num); + } + else if (RB_BIGNUM_TYPE_P(num)) { + return rb_big_bit_count(num); + } + UNREACHABLE_RETURN(Qnil); +} + static VALUE rb_fix_digits(VALUE fix, long base) { @@ -6610,6 +6653,7 @@ Init_Numeric(void) rb_define_method(rb_cInteger, ">>", rb_int_rshift, 1); rb_define_method(rb_cInteger, "digits", rb_int_digits, -1); + rb_define_method(rb_cInteger, "bit_count", rb_int_bit_count, 0); #define fix_to_s_static(n) do { \ VALUE lit = rb_fstring_literal(#n); \ diff --git a/spec/ruby/core/integer/bit_count_spec.rb b/spec/ruby/core/integer/bit_count_spec.rb new file mode 100644 index 00000000000000..6fa06154aae588 --- /dev/null +++ b/spec/ruby/core/integer/bit_count_spec.rb @@ -0,0 +1,31 @@ +require_relative '../../spec_helper' + +ruby_version_is "4.1" do + describe "Integer#bit_count" do + it "returns the number of set bits in the binary representation" do + 0.bit_count.should == 0 + 1.bit_count.should == 1 + 2.bit_count.should == 1 + 3.bit_count.should == 2 + 7.bit_count.should == 3 + 0b10101.bit_count.should == 3 + 0xff.bit_count.should == 8 + 0x100.bit_count.should == 1 + fixnum_max.bit_count.should == fixnum_max.to_s(2).count("1") + + (2**1000).bit_count.should == 1 + (2**1000 - 1).bit_count.should == 1000 + (2**1000 + 2**500).bit_count.should == 2 + (2**64 - 1).bit_count.should == 64 + (2**10000 - 1).bit_count.should == 10000 + end + + it "raises an ArgumentError for a negative number" do + -> { -1.bit_count }.should raise_error(ArgumentError) + -> { -19.bit_count }.should raise_error(ArgumentError) + -> { fixnum_min.bit_count }.should raise_error(ArgumentError) + -> { (-2**1000).bit_count }.should raise_error(ArgumentError) + -> { (-2**1000 - 1).bit_count }.should raise_error(ArgumentError) + end + end +end diff --git a/test/ruby/test_integer.rb b/test/ruby/test_integer.rb index c3d9d311c84049..7753bce20c559d 100644 --- a/test/ruby/test_integer.rb +++ b/test/ruby/test_integer.rb @@ -632,6 +632,41 @@ def test_bit_length } end + def test_bit_count + assert_equal(0, 0.bit_count) + assert_equal(1, 1.bit_count) + assert_equal(1, 2.bit_count) + assert_equal(2, 3.bit_count) + assert_equal(3, 7.bit_count) + assert_equal(3, 0b10101.bit_count) + assert_equal(8, 0xff.bit_count) + assert_equal(1, 0x100.bit_count) + + # Fixnum boundaries + assert_equal(1, (2**30).bit_count) + assert_equal(1, (2**62).bit_count) + + # Bignum + assert_equal(1, (2**64).bit_count) + assert_equal(64, (2**64-1).bit_count) + assert_equal(1, (2**1000).bit_count) + assert_equal(1000, (2**1000-1).bit_count) + assert_equal(2, (2**1000 + 2**500).bit_count) + + 0.upto(1000) {|i| + n = 2**i + assert_equal(1, n.bit_count, "(#{n}).bit_count") + assert_equal(i, (n-1).bit_count, "(#{n-1}).bit_count") + assert_equal(2, (n+1).bit_count, "(#{n+1}).bit_count") if i >= 1 + } + end + + def test_bit_count_for_negative_numbers + assert_raise(ArgumentError) { -1.bit_count } + assert_raise(ArgumentError) { -19.bit_count } + assert_raise(ArgumentError) { (-2**1000).bit_count } + end + def test_digits assert_equal([0], 0.digits) assert_equal([1], 1.digits) diff --git a/test/ruby/test_zjit.rb b/test/ruby/test_zjit_cli.rb similarity index 72% rename from test/ruby/test_zjit.rb rename to test/ruby/test_zjit_cli.rb index 18baf3832abb72..ad7e7a72d9d3c2 100644 --- a/test/ruby/test_zjit.rb +++ b/test/ruby/test_zjit_cli.rb @@ -1,19 +1,22 @@ # frozen_string_literal: true # # This set of tests can be run with: -# make test-all TESTS=test/ruby/test_zjit.rb # -# Instead of adding new tests here, you should probably -# be adding tests that run under the Rust test harness, -# say, in `codegen_tests.rs`. It parallelizes better and -# allows for easy inspection of VM internal states. +# make test/ruby/test_zjit_cli.rb +# +# Test ZJIT through the command-line interface. This relatively heavy-weight +# way to test is only necessary when exercising command-line options, stats, +# environment variables, or process-level behavior. Tests that only exercise +# codegen should be added to the Rust test harness (`codegen_tests.rs`) +# instead: it parallelizes better and allows for easy inspection of VM internal +# states. require 'test/unit' require 'envutil' require_relative '../lib/jit_support' return unless JITSupport.zjit_supported? -class TestZJIT < Test::Unit::TestCase +class TestZJITCLI < Test::Unit::TestCase def test_enabled assert_runs 'false', <<~RUBY, zjit: false RubyVM::ZJIT.enabled? @@ -192,13 +195,6 @@ def test }, insns: [:opt_new], call_threshold: 2 end - def test_uncached_getconstant_path - assert_compiles RUBY_COPYRIGHT.dump, %q{ - def test = RUBY_COPYRIGHT - test - }, call_threshold: 1, insns: [:opt_getconstant_path] - end - def test_getconstant_path_autoload # A constant-referencing expression can run arbitrary code through Kernel#autoload. Dir.mktmpdir('autoload') do |tmpdir| @@ -213,22 +209,6 @@ def test = X end end - def test_send_backtrace - backtrace = [ - "-e:2:in 'Object#jit_frame1'", - "-e:3:in 'Object#entry'", - "-e:5:in 'block in
'", - "-e:6:in '
'", - ] - assert_compiles backtrace.inspect, %q{ - def jit_frame2 = caller # 1 - def jit_frame1 = jit_frame2 # 2 - def entry = jit_frame1 # 3 - entry # profile send # 4 - entry # 5 - }, call_threshold: 2 - end - # tool/ruby_vm/views/*.erb relies on the zjit instructions a) being contiguous and # b) being reliably ordered after all the other instructions. def test_instruction_order @@ -305,105 +285,6 @@ def test = 1 }, stats: true end - def test_zjit_option_uses_array_each_in_ruby - omit 'ZJIT wrongly compiles Array#each, so it is disabled for now' - assert_runs '""', %q{ - Array.instance_method(:each).source_location&.first - } - end - - def test_line_tracepoint_on_c_method - assert_compiles '"[[:line, true]]"', %q{ - events = [] - events.instance_variable_set( - :@tp, - TracePoint.new(:line) { |tp| events << [tp.event, tp.lineno] if tp.path == __FILE__ } - ) - def events.to_str - @tp.enable; '' - end - - # Stay in generated code while enabling tracing - def events.compiled(obj) - String(obj) - @tp.disable; __LINE__ - end - - line = events.compiled(events) - events[0][-1] = (events[0][-1] == line) - - events.to_s # can't dump events as it's a singleton object AND it has a TracePoint instance variable, which also can't be dumped - } - end - - def test_targeted_line_tracepoint_in_c_method_call - assert_compiles '"[true]"', %q{ - events = [] - events.instance_variable_set(:@tp, TracePoint.new(:line) { |tp| events << tp.lineno }) - def events.to_str - @tp.enable(target: method(:compiled)) - '' - end - - # Stay in generated code while enabling tracing - def events.compiled(obj) - String(obj) - __LINE__ - end - - line = events.compiled(events) - events[0] = (events[0] == line) - - events.to_s # can't dump events as it's a singleton object AND it has a TracePoint instance variable, which also can't be dumped - } - end - - def test_regression_cfp_sp_set_correctly_before_leaf_gc_call - assert_compiles ':ok', %q{ - def check(l, r) - return 1 unless l - 1 + check(*l) + check(*r) - end - - def tree(depth) - # This duparray is our leaf-gc target. - return [nil, nil] unless depth > 0 - - # Modify the local and pass it to the following calls. - depth -= 1 - [tree(depth), tree(depth)] - end - - def test - GC.stress = true - 2.times do - t = tree(11) - check(*t) - end - :ok - end - - test - }, call_threshold: 14, num_profiles: 5 - end - - def test_regression_gc_stress_with_lazy_block_code - assert_compiles ':ok', %q{ - def allocate_array - [1, 2, 3] - end - - begin - GC.stress = true - allocate_array - allocate_array - :ok - ensure - GC.stress = false - end - } - end - def test_exit_tracing # Smoke test: --zjit-trace-exits writes a Fuchsia trace (.fxt) file to /tmp assert_compiles('true', <<~RUBY, extra_args: ['--zjit-trace-exits']) @@ -423,35 +304,6 @@ def array.itself = :not_itself RUBY end - def test_send_no_profiles_with_disabled_specialized_instruction - # Regression test: when specialized_instruction is disabled (as power_assert does), - # eval'd code uses `send` instead of `opt_send_without_block`, producing SendNoProfiles. - # The `times` call with a literal block is the SendNoProfiles send whose exit profiling - # triggers recompilation of `run`. After recompilation, `make`'s eval("proc { }") crashes - # in vm_make_env_each because the caller frame's EP[-1] (specval) has a stale value. - assert_runs ':ok', <<~RUBY - RubyVM::InstructionSequence.compile_option = { specialized_instruction: false } - eval <<~'INNERRUBY' - def make = eval("proc { }") - def run(n) = n.times { make } - INNERRUBY - run(6) - :ok - RUBY - end - - def test_float_arithmetic - assert_compiles '4.0', 'def test = 1.5 + 2.5; test' - assert_compiles '6.0', 'def test = 2.0 * 3.0; test' - assert_compiles '1.5', 'def test = 3.5 - 2.0; test' - assert_compiles '2.5', 'def test = 5.0 / 2.0; test' - assert_compiles '4.5', 'def test = 1.5 * 3; test' # Float * Fixnum - assert_compiles 'true', 'def test = (Float::NAN + 1.0).nan?; test' - assert_compiles 'Infinity', 'def test = Float::INFINITY * 2.0; test' - assert_compiles '3', 'def test = 3.7.to_i; test' - assert_compiles '-2', 'def test = (-2.9).to_i; test' - end - private # Assert that every method call in `test_script` can be compiled by ZJIT diff --git a/zjit/src/codegen_tests.rs b/zjit/src/codegen_tests.rs index 8ecfdf98e39994..0d93b31e232090 100644 --- a/zjit/src/codegen_tests.rs +++ b/zjit/src/codegen_tests.rs @@ -6865,3 +6865,170 @@ fn test_getlocal_level_zero_after_setlocal_wc_0() { test "#), @"2"); } + +#[test] +fn test_uncached_getconstant_path() { + set_call_threshold(1); + eval(" + def test = RUBY_COPYRIGHT + test + "); + assert_contains_opcode("test", YARVINSN_opt_getconstant_path); + // RUBY_COPYRIGHT is version-dependent, so compare against its runtime value + // rather than a fixed snapshot. + assert_eq!(assert_compiles_allowing_exits("test"), inspect("RUBY_COPYRIGHT")); +} + +#[test] +fn test_line_tracepoint_on_c_method() { + set_call_threshold(1); + eval("nil"); // boot the VM before assert_compiles_allowing_exits touches ZJITState + assert_snapshot!(assert_compiles_allowing_exits(r#" + events = [] + events.instance_variable_set( + :@tp, + TracePoint.new(:line) { |tp| events << [tp.event, tp.lineno] if tp.path == __FILE__ } + ) + def events.to_str + @tp.enable; '' + end + + # Stay in generated code while enabling tracing + def events.compiled(obj) + String(obj) + @tp.disable; __LINE__ + end + + line = events.compiled(events) + events[0][-1] = (events[0][-1] == line) + + events.to_s # can't dump events as it's a singleton object AND it has a TracePoint instance variable, which also can't be dumped + "#), @r#""[[:line, true]]""#); +} + +#[test] +fn test_targeted_line_tracepoint_in_c_method_call() { + set_call_threshold(1); + eval("nil"); // boot the VM before assert_compiles_allowing_exits touches ZJITState + assert_snapshot!(assert_compiles_allowing_exits(r#" + events = [] + events.instance_variable_set(:@tp, TracePoint.new(:line) { |tp| events << tp.lineno }) + def events.to_str + @tp.enable(target: method(:compiled)) + '' + end + + # Stay in generated code while enabling tracing + def events.compiled(obj) + String(obj) + __LINE__ + end + + line = events.compiled(events) + events[0] = (events[0] == line) + + events.to_s # can't dump events as it's a singleton object AND it has a TracePoint instance variable, which also can't be dumped + "#), @r#""[true]""#); +} + +#[test] +fn test_regression_cfp_sp_set_correctly_before_leaf_gc_call() { + set_call_threshold(14); + eval("nil"); // boot the VM before assert_compiles_allowing_exits touches ZJITState + assert_snapshot!(assert_compiles_allowing_exits(r#" + def check(l, r) + return 1 unless l + 1 + check(*l) + check(*r) + end + + def tree(depth) + # This duparray is our leaf-gc target. + return [nil, nil] unless depth > 0 + + # Modify the local and pass it to the following calls. + depth -= 1 + [tree(depth), tree(depth)] + end + + def test + GC.stress = true + 2.times do + t = tree(11) + check(*t) + end + :ok + end + + test + "#), @":ok"); +} + +#[test] +fn test_regression_gc_stress_with_lazy_block_code() { + eval("nil"); // boot the VM before assert_compiles_allowing_exits touches ZJITState + assert_snapshot!(assert_compiles_allowing_exits(r#" + def allocate_array + [1, 2, 3] + end + + begin + GC.stress = true + allocate_array + allocate_array + :ok + ensure + GC.stress = false + end + "#), @":ok"); +} + +#[test] +fn test_float_arithmetic() { + set_call_threshold(1); + eval("nil"); // boot the VM before assert_compiles_allowing_exits touches ZJITState + assert_snapshot!(assert_compiles_allowing_exits("def test = 1.5 + 2.5; test"), @"4.0"); + assert_snapshot!(assert_compiles_allowing_exits("def test = 2.0 * 3.0; test"), @"6.0"); + assert_snapshot!(assert_compiles_allowing_exits("def test = 3.5 - 2.0; test"), @"1.5"); + assert_snapshot!(assert_compiles_allowing_exits("def test = 5.0 / 2.0; test"), @"2.5"); + assert_snapshot!(assert_compiles_allowing_exits("def test = 1.5 * 3; test"), @"4.5"); // Float * Fixnum + assert_snapshot!(assert_compiles_allowing_exits("def test = (Float::NAN + 1.0).nan?; test"), @"true"); + assert_snapshot!(assert_compiles_allowing_exits("def test = Float::INFINITY * 2.0; test"), @"Infinity"); + assert_snapshot!(assert_compiles_allowing_exits("def test = 3.7.to_i; test"), @"3"); + assert_snapshot!(assert_compiles_allowing_exits("def test = (-2.9).to_i; test"), @"-2"); +} + +#[test] +fn test_send_backtrace() { + eval("nil"); // boot the VM before assert_compiles_allowing_exits touches ZJITState + assert_snapshot!(assert_compiles_allowing_exits(r#" + def jit_frame2 = caller # 1 + def jit_frame1 = jit_frame2 # 2 + def entry = jit_frame1 # 3 + entry # profile send # 4 + entry # 5 + "#), @r#"[":3:in 'Object#jit_frame1'", ":4:in 'Object#entry'", ":6:in ''", "-e:in 'RubyVM::InstructionSequence#eval'"]"#); +} + +// Regression test: when specialized_instruction is disabled (as power_assert does), +// eval'd code uses `send` instead of `opt_send_without_block`, producing SendNoProfiles. +// The `times` call with a literal block is the SendNoProfiles send whose exit profiling +// triggers recompilation of `run`. After recompilation, `make`'s eval("proc { }") crashes +// in vm_make_env_each because the caller frame's EP[-1] (specval) has a stale value. +#[test] +fn test_send_no_profiles_with_disabled_specialized_instruction() { + set_call_threshold(1); + assert_snapshot!(inspect(r#" + RubyVM::InstructionSequence.compile_option = { specialized_instruction: false } + eval <<~'INNERRUBY' + def make = eval("proc { }") + def run(n) = n.times { make } + INNERRUBY + run(6) + :ok + "#), @":ok"); +} + +#[test] +fn test_array_each_is_defined_in_ruby() { + assert_snapshot!(inspect("Array.instance_method(:each).source_location&.first"), @r#""""#); +} diff --git a/zjit/src/cruby_methods.rs b/zjit/src/cruby_methods.rs index 4ff3eb759199be..2004d1beedaf27 100644 --- a/zjit/src/cruby_methods.rs +++ b/zjit/src/cruby_methods.rs @@ -656,14 +656,22 @@ fn try_inline_float_op(fun: &mut hir::Function, block: hir::BlockId, f: &dyn Fn( if fun.likely_a(recv, types::Flonum, state) && (fun.likely_a(other, types::Flonum, state) || fun.likely_a(other, types::Fixnum, state)) { - let recv = fun.coerce_to(block, recv, types::Flonum, state); + let recv = coerce_float_op_operand(fun, block, recv, types::Flonum, state); let other_type = if fun.likely_a(other, types::Flonum, state) { types::Flonum } else { types::Fixnum }; - let other = fun.coerce_to(block, other, other_type, state); + let other = coerce_float_op_operand(fun, block, other, other_type, state); return Some(fun.push_insn(block, f(recv, other))); } None } +fn coerce_float_op_operand(fun: &mut hir::Function, block: hir::BlockId, val: hir::InsnId, guard_type: Type, state: hir::InsnId) -> hir::InsnId { + if fun.is_a(val, guard_type) { + val + } else { + fun.guard_type_recompile(block, val, guard_type, state, hir::Recompile) + } +} + fn inline_float_plus(fun: &mut hir::Function, block: hir::BlockId, recv: hir::InsnId, args: &[hir::InsnId], state: hir::InsnId) -> Option { let &[other] = args else { return None; }; try_inline_float_op(fun, block, &|recv, other| hir::Insn::FloatAdd { recv, other, state }, BOP_PLUS, recv, other, state) diff --git a/zjit/src/hir/opt_tests.rs b/zjit/src/hir/opt_tests.rs index 2d7e47b8d9c8bd..b723d720812906 100644 --- a/zjit/src/hir/opt_tests.rs +++ b/zjit/src/hir/opt_tests.rs @@ -17618,7 +17618,7 @@ mod hir_opt_tests { bb3(v11:BasicObject, v12:BasicObject, v13:BasicObject): PatchPoint MethodRedefined(Float@0x1008, +@0x1010, cme:0x1018) v28:Flonum = GuardType v12, Flonum recompile - v29:Flonum = GuardType v13, Flonum + v29:Flonum = GuardType v13, Flonum recompile v30:Float = FloatAdd v28, v29 CheckInterrupts Return v30 @@ -17649,7 +17649,7 @@ mod hir_opt_tests { bb3(v11:BasicObject, v12:BasicObject, v13:BasicObject): PatchPoint MethodRedefined(Float@0x1008, *@0x1010, cme:0x1018) v28:Flonum = GuardType v12, Flonum recompile - v29:Flonum = GuardType v13, Flonum + v29:Flonum = GuardType v13, Flonum recompile v30:Float = FloatMul v28, v29 CheckInterrupts Return v30 @@ -17680,7 +17680,7 @@ mod hir_opt_tests { bb3(v11:BasicObject, v12:BasicObject, v13:BasicObject): PatchPoint MethodRedefined(Float@0x1008, -@0x1010, cme:0x1018) v28:Flonum = GuardType v12, Flonum recompile - v29:Flonum = GuardType v13, Flonum + v29:Flonum = GuardType v13, Flonum recompile v30:Float = FloatSub v28, v29 CheckInterrupts Return v30 @@ -17711,7 +17711,7 @@ mod hir_opt_tests { bb3(v11:BasicObject, v12:BasicObject, v13:BasicObject): PatchPoint MethodRedefined(Float@0x1008, /@0x1010, cme:0x1018) v28:Flonum = GuardType v12, Flonum recompile - v29:Flonum = GuardType v13, Flonum + v29:Flonum = GuardType v13, Flonum recompile v30:Float = FloatDiv v28, v29 CheckInterrupts Return v30 @@ -17770,10 +17770,156 @@ mod hir_opt_tests { bb3(v11:BasicObject, v12:BasicObject, v13:BasicObject): PatchPoint MethodRedefined(Float@0x1008, *@0x1010, cme:0x1018) v28:Flonum = GuardType v12, Flonum recompile - v29:Fixnum = GuardType v13, Fixnum + v29:Fixnum = GuardType v13, Fixnum recompile + v30:Float = FloatMul v28, v29 + CheckInterrupts + Return v30 + "); + } + + #[test] + fn test_float_mul_recompile_stops_inlining_heap_float() { + set_max_versions(2); + eval(r#" + def test_float_mul_recompile(a, b) = a * b + + 30.times { test_float_mul_recompile(1.5, 2.5) } + "#); + + let intermediate_hir = hir_string("test_float_mul_recompile"); + assert!(intermediate_hir.contains("FloatMul"), "{intermediate_hir}"); + + eval(r#" + 30.times { test_float_mul_recompile(1.5, -0.0) } + "#); + + let final_hir = hir_string("test_float_mul_recompile"); + assert!(final_hir.contains("CCallWithFrame"), "{final_hir}"); + assert!(!final_hir.contains("FloatMul"), "{final_hir}"); + assert_snapshot!(format!("{intermediate_hir}\n{final_hir}"), @" + fn test_float_mul_recompile@:2: + bb1(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + v2:CPtr = LoadSP + v3:BasicObject = LoadField v2, :a@0x1000 + v4:BasicObject = LoadField v2, :b@0x1001 + Jump bb3(v1, v3, v4) + bb2(): + EntryPoint JIT(0) + v7:BasicObject = LoadArg :self@0 + v8:BasicObject = LoadArg :a@1 + v9:BasicObject = LoadArg :b@2 + Jump bb3(v7, v8, v9) + bb3(v11:BasicObject, v12:BasicObject, v13:BasicObject): + PatchPoint MethodRedefined(Float@0x1008, *@0x1010, cme:0x1018) + v28:Flonum = GuardType v12, Flonum recompile + v29:Flonum = GuardType v13, Flonum recompile v30:Float = FloatMul v28, v29 CheckInterrupts Return v30 + + fn test_float_mul_recompile@:2: + bb1(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + v2:CPtr = LoadSP + v3:BasicObject = LoadField v2, :a@0x1000 + v4:BasicObject = LoadField v2, :b@0x1001 + Jump bb3(v1, v3, v4) + bb2(): + EntryPoint JIT(0) + v7:BasicObject = LoadArg :self@0 + v8:BasicObject = LoadArg :a@1 + v9:BasicObject = LoadArg :b@2 + Jump bb3(v7, v8, v9) + bb3(v11:BasicObject, v12:BasicObject, v13:BasicObject): + PatchPoint MethodRedefined(Float@0x1008, *@0x1010, cme:0x1018) + v28:Flonum = GuardType v12, Flonum recompile + v29:BasicObject = CCallWithFrame v28, :Float#*@0x1040, v13 + CheckInterrupts + Return v29 + "); + } + + #[test] + fn test_float_mul_recompile_stops_inlining_heap_float_receiver() { + set_max_versions(2); + eval(r#" + def test_float_mul_recompile(a, b) = a * b + + 30.times { test_float_mul_recompile(1.5, 2.5) } + "#); + + let intermediate_hir = hir_string("test_float_mul_recompile"); + assert!(intermediate_hir.contains("FloatMul"), "{intermediate_hir}"); + + eval(r#" + 30.times { test_float_mul_recompile(-0.0, 1.5) } + "#); + + let final_hir = hir_string("test_float_mul_recompile"); + assert!(final_hir.contains("CCallWithFrame"), "{final_hir}"); + assert!(!final_hir.contains("FloatMul"), "{final_hir}"); + assert_snapshot!(format!("{intermediate_hir}\n{final_hir}"), @" + fn test_float_mul_recompile@:2: + bb1(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + v2:CPtr = LoadSP + v3:BasicObject = LoadField v2, :a@0x1000 + v4:BasicObject = LoadField v2, :b@0x1001 + Jump bb3(v1, v3, v4) + bb2(): + EntryPoint JIT(0) + v7:BasicObject = LoadArg :self@0 + v8:BasicObject = LoadArg :a@1 + v9:BasicObject = LoadArg :b@2 + Jump bb3(v7, v8, v9) + bb3(v11:BasicObject, v12:BasicObject, v13:BasicObject): + PatchPoint MethodRedefined(Float@0x1008, *@0x1010, cme:0x1018) + v28:Flonum = GuardType v12, Flonum recompile + v29:Flonum = GuardType v13, Flonum recompile + v30:Float = FloatMul v28, v29 + CheckInterrupts + Return v30 + + fn test_float_mul_recompile@:2: + bb1(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + v2:CPtr = LoadSP + v3:BasicObject = LoadField v2, :a@0x1000 + v4:BasicObject = LoadField v2, :b@0x1001 + Jump bb3(v1, v3, v4) + bb2(): + EntryPoint JIT(0) + v7:BasicObject = LoadArg :self@0 + v8:BasicObject = LoadArg :a@1 + v9:BasicObject = LoadArg :b@2 + Jump bb3(v7, v8, v9) + bb3(v11:BasicObject, v12:BasicObject, v13:BasicObject): + v21:CBool = HasType v12, HeapFloat + CondBranch v21, bb5(), bb6() + bb5(): + v24:HeapFloat = RefineType v12, HeapFloat + PatchPoint MethodRedefined(Float@0x1008, *@0x1010, cme:0x1018) + v42:BasicObject = CCallWithFrame v24, :Float#*@0x1040, v13 + Jump bb4(v42) + bb6(): + v27:CBool = HasType v12, Flonum + CondBranch v27, bb7(), bb8() + bb7(): + v30:Flonum = RefineType v12, Flonum + PatchPoint MethodRedefined(Float@0x1008, *@0x1010, cme:0x1018) + v45:BasicObject = CCallWithFrame v30, :Float#*@0x1040, v13 + Jump bb4(v45) + bb8(): + v33:BasicObject = Send v12, :*, v13 # SendFallbackReason: SendWithoutBlock: polymorphic fallback + Jump bb4(v33) + bb4(v20:BasicObject): + CheckInterrupts + Return v20 "); } diff --git a/zjit/zjit.mk b/zjit/zjit.mk index dad4ece932cc56..7a98bdc6d53a63 100644 --- a/zjit/zjit.mk +++ b/zjit/zjit.mk @@ -51,7 +51,7 @@ update-zjit-bench: .PHONY: zjit-check zjit-check: $(MAKE) zjit-test - $(MAKE) test-all TESTS='$(top_srcdir)/test/ruby/test_zjit.rb' + $(MAKE) test-all TESTS='$(top_srcdir)/test/ruby/test_zjit_cli.rb' ZJIT_BINDGEN_DIFF_OPTS =