diff --git a/.github/auto_request_review.yml b/.github/auto_request_review.yml index 51e0e4db973ca7..814150c90e1da9 100644 --- a/.github/auto_request_review.yml +++ b/.github/auto_request_review.yml @@ -11,6 +11,7 @@ files: 'doc/zjit*': [team:jit] 'test/ruby/test_zjit*': [team:jit] 'defs/jit.mk': [team:jit] + 'tool/zjit_bisect.rb': [team:jit] # Skip github workflow files because the team don't necessarily need to review dependabot updates for GitHub Actions. It's noisy in notifications, and they're auto-merged anyway. options: ignore_draft: true diff --git a/prism/prism.c b/prism/prism.c index 0ebcae62f9e822..95e7d0905040b1 100644 --- a/prism/prism.c +++ b/prism/prism.c @@ -15764,7 +15764,7 @@ parse_return(pm_parser_t *parser, pm_node_t *node) { break; } } - if (in_sclass) { + if (in_sclass && parser->version >= PM_OPTIONS_VERSION_CRUBY_3_4) { pm_parser_err_node(parser, node, PM_ERR_RETURN_INVALID); } } diff --git a/test/prism/errors/3.3-3.3/circular_parameters.txt b/test/prism/errors/3.3-3.3/circular_parameters.txt new file mode 100644 index 00000000000000..ef9642b075aae9 --- /dev/null +++ b/test/prism/errors/3.3-3.3/circular_parameters.txt @@ -0,0 +1,12 @@ +def foo(bar = bar) = 42 + ^~~ circular argument reference - bar + +def foo(bar: bar) = 42 + ^~~ circular argument reference - bar + +proc { |foo = foo| } + ^~~ circular argument reference - foo + +proc { |foo: foo| } + ^~~ circular argument reference - foo + diff --git a/test/prism/errors/3.3-3.4/leading_logical.txt b/test/prism/errors/3.3-3.4/leading_logical.txt new file mode 100644 index 00000000000000..2a702e281d5efc --- /dev/null +++ b/test/prism/errors/3.3-3.4/leading_logical.txt @@ -0,0 +1,34 @@ +1 +&& 2 +^~ unexpected '&&', ignoring it +&& 3 +^~ unexpected '&&', ignoring it + +1 +|| 2 +^ unexpected '|', ignoring it + ^ unexpected '|', ignoring it +|| 3 +^ unexpected '|', ignoring it + ^ unexpected '|', ignoring it + +1 +and 2 +^~~ unexpected 'and', ignoring it +and 3 +^~~ unexpected 'and', ignoring it + +1 +or 2 +^~ unexpected 'or', ignoring it +or 3 +^~ unexpected 'or', ignoring it + +1 +and foo +^~~ unexpected 'and', ignoring it + +2 +or foo +^~ unexpected 'or', ignoring it + diff --git a/test/prism/errors/3.3-3.4/private_endless_method.txt b/test/prism/errors/3.3-3.4/private_endless_method.txt new file mode 100644 index 00000000000000..8aae5e0cd39035 --- /dev/null +++ b/test/prism/errors/3.3-3.4/private_endless_method.txt @@ -0,0 +1,3 @@ +private def foo = puts "Hello" + ^ unexpected string literal, expecting end-of-input + diff --git a/test/prism/errors/block_args_in_array_assignment.txt b/test/prism/errors/3.4/block_args_in_array_assignment.txt similarity index 100% rename from test/prism/errors/block_args_in_array_assignment.txt rename to test/prism/errors/3.4/block_args_in_array_assignment.txt diff --git a/test/prism/errors/dont_allow_return_inside_sclass_body.txt b/test/prism/errors/3.4/dont_allow_return_inside_sclass_body.txt similarity index 100% rename from test/prism/errors/dont_allow_return_inside_sclass_body.txt rename to test/prism/errors/3.4/dont_allow_return_inside_sclass_body.txt diff --git a/test/prism/errors/it_with_ordinary_parameter.txt b/test/prism/errors/3.4/it_with_ordinary_parameter.txt similarity index 100% rename from test/prism/errors/it_with_ordinary_parameter.txt rename to test/prism/errors/3.4/it_with_ordinary_parameter.txt diff --git a/test/prism/errors/keyword_args_in_array_assignment.txt b/test/prism/errors/3.4/keyword_args_in_array_assignment.txt similarity index 100% rename from test/prism/errors/keyword_args_in_array_assignment.txt rename to test/prism/errors/3.4/keyword_args_in_array_assignment.txt diff --git a/test/prism/errors_test.rb b/test/prism/errors_test.rb index 9dd4aea72865d9..9abed9265212d0 100644 --- a/test/prism/errors_test.rb +++ b/test/prism/errors_test.rb @@ -1,41 +1,19 @@ # frozen_string_literal: true +return if RUBY_VERSION < "3.3.0" + require_relative "test_helper" module Prism class ErrorsTest < TestCase base = File.expand_path("errors", __dir__) - filepaths = Dir["*.txt", base: base] - - if RUBY_VERSION < "3.0" - filepaths -= [ - "cannot_assign_to_a_reserved_numbered_parameter.txt", - "writing_numbered_parameter.txt", - "targeting_numbered_parameter.txt", - "defining_numbered_parameter.txt", - "defining_numbered_parameter_2.txt", - "numbered_parameters_in_block_arguments.txt", - "numbered_and_write.txt", - "numbered_or_write.txt", - "numbered_operator_write.txt" - ] - end - - if RUBY_VERSION < "3.4" - filepaths -= [ - "it_with_ordinary_parameter.txt", - "block_args_in_array_assignment.txt", - "keyword_args_in_array_assignment.txt" - ] - end - - if RUBY_VERSION < "3.4" || RUBY_RELEASE_DATE < "2024-07-24" - filepaths -= ["dont_allow_return_inside_sclass_body.txt"] - end + filepaths = Dir["**/*.txt", base: base] filepaths.each do |filepath| - define_method(:"test_#{File.basename(filepath, ".txt")}") do - assert_errors(File.join(base, filepath)) + ruby_versions_for(filepath).each do |version| + define_method(:"test_#{version}_#{File.basename(filepath, ".txt")}") do + assert_errors(File.join(base, filepath), version) + end end end @@ -86,38 +64,15 @@ def test_invalid_message_name assert_equal :"", Prism.parse_statement("+.@foo,+=foo").write_name end - def test_circular_parameters - source = <<~RUBY - def foo(bar = bar) = 42 - def foo(bar: bar) = 42 - proc { |foo = foo| } - proc { |foo: foo| } - RUBY - - source.each_line do |line| - assert_predicate Prism.parse(line, version: "3.3.0"), :failure? - assert_predicate Prism.parse(line), :success? - end - end - - def test_private_endless_method - source = <<~RUBY - private def foo = puts "Hello" - RUBY - - assert_predicate Prism.parse(source, version: "3.4"), :failure? - assert_predicate Prism.parse(source), :success? - end - private - def assert_errors(filepath) + def assert_errors(filepath, version) expected = File.read(filepath, binmode: true, external_encoding: Encoding::UTF_8) source = expected.lines.grep_v(/^\s*\^/).join.gsub(/\n*\z/, "") - refute_valid_syntax(source) + refute_valid_syntax(source) if current_major_minor == version - result = Prism.parse(source) + result = Prism.parse(source, version: version) errors = result.errors refute_empty errors, "Expected errors in #{filepath}" diff --git a/test/prism/fixtures/3.3-3.3/block_args_in_array_assignment.txt b/test/prism/fixtures/3.3-3.3/block_args_in_array_assignment.txt new file mode 100644 index 00000000000000..6d6b052681b5da --- /dev/null +++ b/test/prism/fixtures/3.3-3.3/block_args_in_array_assignment.txt @@ -0,0 +1 @@ +matrix[5, &block] = 8 diff --git a/test/prism/fixtures/it.txt b/test/prism/fixtures/3.3-3.3/it.txt similarity index 100% rename from test/prism/fixtures/it.txt rename to test/prism/fixtures/3.3-3.3/it.txt diff --git a/test/prism/fixtures/it_indirect_writes.txt b/test/prism/fixtures/3.3-3.3/it_indirect_writes.txt similarity index 100% rename from test/prism/fixtures/it_indirect_writes.txt rename to test/prism/fixtures/3.3-3.3/it_indirect_writes.txt diff --git a/test/prism/fixtures/it_read_and_assignment.txt b/test/prism/fixtures/3.3-3.3/it_read_and_assignment.txt similarity index 100% rename from test/prism/fixtures/it_read_and_assignment.txt rename to test/prism/fixtures/3.3-3.3/it_read_and_assignment.txt diff --git a/test/prism/fixtures/3.3-3.3/it_with_ordinary_parameter.txt b/test/prism/fixtures/3.3-3.3/it_with_ordinary_parameter.txt new file mode 100644 index 00000000000000..178b641e6b94ec --- /dev/null +++ b/test/prism/fixtures/3.3-3.3/it_with_ordinary_parameter.txt @@ -0,0 +1 @@ +proc { || it } diff --git a/test/prism/fixtures/3.3-3.3/keyword_args_in_array_assignment.txt b/test/prism/fixtures/3.3-3.3/keyword_args_in_array_assignment.txt new file mode 100644 index 00000000000000..88016c2afe8251 --- /dev/null +++ b/test/prism/fixtures/3.3-3.3/keyword_args_in_array_assignment.txt @@ -0,0 +1 @@ +matrix[5, axis: :y] = 8 diff --git a/test/prism/fixtures/3.3-3.3/return_in_sclass.txt b/test/prism/fixtures/3.3-3.3/return_in_sclass.txt new file mode 100644 index 00000000000000..f1fde5771afab2 --- /dev/null +++ b/test/prism/fixtures/3.3-3.3/return_in_sclass.txt @@ -0,0 +1 @@ +class << A; return; end diff --git a/test/prism/fixtures/3.4/circular_parameters.txt b/test/prism/fixtures/3.4/circular_parameters.txt new file mode 100644 index 00000000000000..11537023ada61a --- /dev/null +++ b/test/prism/fixtures/3.4/circular_parameters.txt @@ -0,0 +1,4 @@ +def foo(bar = bar) = 42 +def foo(bar: bar) = 42 +proc { |foo = foo| } +proc { |foo: foo| } diff --git a/test/prism/fixtures/3.4/it.txt b/test/prism/fixtures/3.4/it.txt new file mode 100644 index 00000000000000..5410b01e711a3a --- /dev/null +++ b/test/prism/fixtures/3.4/it.txt @@ -0,0 +1,5 @@ +x do + it +end + +-> { it } diff --git a/test/prism/fixtures/3.4/it_indirect_writes.txt b/test/prism/fixtures/3.4/it_indirect_writes.txt new file mode 100644 index 00000000000000..bb87e9483e2a75 --- /dev/null +++ b/test/prism/fixtures/3.4/it_indirect_writes.txt @@ -0,0 +1,23 @@ +tap { it += 1 } + +tap { it ||= 1 } + +tap { it &&= 1 } + +tap { it; it += 1 } + +tap { it; it ||= 1 } + +tap { it; it &&= 1 } + +tap { it += 1; it } + +tap { it ||= 1; it } + +tap { it &&= 1; it } + +tap { it; it += 1; it } + +tap { it; it ||= 1; it } + +tap { it; it &&= 1; it } diff --git a/test/prism/fixtures/3.4/it_read_and_assignment.txt b/test/prism/fixtures/3.4/it_read_and_assignment.txt new file mode 100644 index 00000000000000..2cceeb2a548710 --- /dev/null +++ b/test/prism/fixtures/3.4/it_read_and_assignment.txt @@ -0,0 +1 @@ +42.tap { p it; it = it; p it } diff --git a/test/prism/fixtures/endless_methods_command_call.txt b/test/prism/fixtures/3.5/endless_methods_command_call.txt similarity index 100% rename from test/prism/fixtures/endless_methods_command_call.txt rename to test/prism/fixtures/3.5/endless_methods_command_call.txt diff --git a/test/prism/fixtures/leading_logical.txt b/test/prism/fixtures/3.5/leading_logical.txt similarity index 100% rename from test/prism/fixtures/leading_logical.txt rename to test/prism/fixtures/3.5/leading_logical.txt diff --git a/test/prism/fixtures_test.rb b/test/prism/fixtures_test.rb index ddb6ffb40c6632..0f0577c10d5735 100644 --- a/test/prism/fixtures_test.rb +++ b/test/prism/fixtures_test.rb @@ -24,9 +24,19 @@ class FixturesTest < TestCase except << "whitequark/ruby_bug_19281.txt" end + if RUBY_VERSION < "3.4.0" + except << "3.4/circular_parameters.txt" + end + + # Valid only on Ruby 3.3 + except << "3.3-3.3/block_args_in_array_assignment.txt" + except << "3.3-3.3/it_with_ordinary_parameter.txt" + except << "3.3-3.3/keyword_args_in_array_assignment.txt" + except << "3.3-3.3/return_in_sclass.txt" + # Leaving these out until they are supported by parse.y. - except << "leading_logical.txt" - except << "endless_methods_command_call.txt" + except << "3.5/leading_logical.txt" + except << "3.5/endless_methods_command_call.txt" # https://bugs.ruby-lang.org/issues/21168#note-5 except << "command_method_call_2.txt" diff --git a/test/prism/lex_test.rb b/test/prism/lex_test.rb index 3a0da1a2d87d67..9682bf8a322c21 100644 --- a/test/prism/lex_test.rb +++ b/test/prism/lex_test.rb @@ -43,16 +43,16 @@ class LexTest < TestCase end # https://bugs.ruby-lang.org/issues/20925 - except << "leading_logical.txt" + except << "3.5/leading_logical.txt" # https://bugs.ruby-lang.org/issues/17398#note-12 - except << "endless_methods_command_call.txt" + except << "3.5/endless_methods_command_call.txt" # https://bugs.ruby-lang.org/issues/21168#note-5 except << "command_method_call_2.txt" - Fixture.each(except: except) do |fixture| - define_method(fixture.test_name) { assert_lex(fixture) } + Fixture.each_with_version(except: except) do |fixture, version| + define_method(fixture.test_name(version)) { assert_lex(fixture, version) } end def test_lex_file @@ -97,10 +97,12 @@ def test_parse_lex_file private - def assert_lex(fixture) + def assert_lex(fixture, version) + return unless current_major_minor == version + source = fixture.read - result = Prism.lex_compat(source) + result = Prism.lex_compat(source, version: version) assert_equal [], result.errors Prism.lex_ripper(source).zip(result.value).each do |(ripper, prism)| diff --git a/test/prism/locals_test.rb b/test/prism/locals_test.rb index 9a3224e8ef8843..439625b750a6f1 100644 --- a/test/prism/locals_test.rb +++ b/test/prism/locals_test.rb @@ -31,9 +31,15 @@ class LocalsTest < TestCase # CRuby is eliminating dead code. "whitequark/ruby_bug_10653.txt", + # Valid only on Ruby 3.3 + "3.3-3.3/block_args_in_array_assignment.txt", + "3.3-3.3/it_with_ordinary_parameter.txt", + "3.3-3.3/keyword_args_in_array_assignment.txt", + "3.3-3.3/return_in_sclass.txt", + # Leaving these out until they are supported by parse.y. - "leading_logical.txt", - "endless_methods_command_call.txt", + "3.5/leading_logical.txt", + "3.5/endless_methods_command_call.txt", "command_method_call_2.txt" ] diff --git a/test/prism/ruby/parser_test.rb b/test/prism/ruby/parser_test.rb index 10b5fca5eaefef..016fda91f03819 100644 --- a/test/prism/ruby/parser_test.rb +++ b/test/prism/ruby/parser_test.rb @@ -65,11 +65,14 @@ class ParserTest < TestCase # 1.. && 2 "ranges.txt", + # https://bugs.ruby-lang.org/issues/20478 + "3.4/circular_parameters.txt", + # Cannot yet handling leading logical operators. - "leading_logical.txt", + "3.5/leading_logical.txt", # Ruby >= 3.5 specific syntax - "endless_methods_command_call.txt", + "3.5/endless_methods_command_call.txt", # https://bugs.ruby-lang.org/issues/21168#note-5 "command_method_call_2.txt", @@ -165,9 +168,9 @@ def test_non_prism_builder_class_deprecated if RUBY_VERSION >= "3.3" def test_current_parser_for_current_ruby - major, minor, _patch = Gem::Version.new(RUBY_VERSION).segments + major, minor = current_major_minor.split(".") # Let's just hope there never is a Ruby 3.10 or similar - expected = major * 10 + minor + expected = major.to_i * 10 + minor.to_i assert_equal(expected, Translation::ParserCurrent.new.version) end end @@ -189,7 +192,7 @@ def test_invalid_syntax end def test_it_block_parameter_syntax - it_fixture_path = Pathname(__dir__).join("../../../test/prism/fixtures/it.txt") + it_fixture_path = Pathname(__dir__).join("../../../test/prism/fixtures/3.4/it.txt") buffer = Parser::Source::Buffer.new(it_fixture_path) buffer.source = it_fixture_path.read diff --git a/test/prism/ruby/ripper_test.rb b/test/prism/ruby/ripper_test.rb index 4916ec8d9de752..12c854aea660be 100644 --- a/test/prism/ruby/ripper_test.rb +++ b/test/prism/ruby/ripper_test.rb @@ -9,7 +9,7 @@ class RipperTest < TestCase # Skip these tests that Ripper is reporting the wrong results for. incorrect = [ # Not yet supported. - "leading_logical.txt", + "3.5/leading_logical.txt", # Ripper incorrectly attributes the block to the keyword. "seattlerb/block_break.txt", @@ -31,8 +31,16 @@ class RipperTest < TestCase # Ripper fails to understand some structures that span across heredocs. "spanning_heredoc.txt", + "3.3-3.3/block_args_in_array_assignment.txt", + "3.3-3.3/it_with_ordinary_parameter.txt", + "3.3-3.3/keyword_args_in_array_assignment.txt", + "3.3-3.3/return_in_sclass.txt", + + # https://bugs.ruby-lang.org/issues/20478 + "3.4/circular_parameters.txt", + # https://bugs.ruby-lang.org/issues/17398#note-12 - "endless_methods_command_call.txt", + "3.5/endless_methods_command_call.txt", # https://bugs.ruby-lang.org/issues/21168#note-5 "command_method_call_2.txt", diff --git a/test/prism/ruby/ruby_parser_test.rb b/test/prism/ruby/ruby_parser_test.rb index ec55e41967c6a0..7640ddaf1ca6c0 100644 --- a/test/prism/ruby/ruby_parser_test.rb +++ b/test/prism/ruby/ruby_parser_test.rb @@ -39,7 +39,6 @@ class RubyParserTest < TestCase "dos_endings.txt", "heredocs_with_fake_newlines.txt", "heredocs_with_ignored_newlines.txt", - "leading_logical.txt", "method_calls.txt", "methods.txt", "multi_write.txt", @@ -77,8 +76,15 @@ class RubyParserTest < TestCase "whitequark/ruby_bug_19281.txt", "whitequark/slash_newline_in_heredocs.txt", - # Ruby >= 3.5 specific syntax - "endless_methods_command_call.txt", + "3.3-3.3/block_args_in_array_assignment.txt", + "3.3-3.3/it_with_ordinary_parameter.txt", + "3.3-3.3/keyword_args_in_array_assignment.txt", + "3.3-3.3/return_in_sclass.txt", + + "3.4/circular_parameters.txt", + + "3.5/endless_methods_command_call.txt", + "3.5/leading_logical.txt", # https://bugs.ruby-lang.org/issues/21168#note-5 "command_method_call_2.txt", diff --git a/test/prism/snippets_test.rb b/test/prism/snippets_test.rb index 66802c5dc3a1be..3160442cc07653 100644 --- a/test/prism/snippets_test.rb +++ b/test/prism/snippets_test.rb @@ -18,24 +18,24 @@ class SnippetsTest < TestCase "whitequark/multiple_pattern_matches.txt" ] - Fixture.each(except: except) do |fixture| - define_method(fixture.test_name) { assert_snippets(fixture) } + Fixture.each_with_version(except: except) do |fixture, version| + define_method(fixture.test_name(version)) { assert_snippets(fixture, version) } end private # We test every snippet (separated by \n\n) in isolation to ensure the # parser does not try to read bytes further than the end of each snippet. - def assert_snippets(fixture) + def assert_snippets(fixture, version) fixture.read.split(/(?<=\S)\n\n(?=\S)/).each do |snippet| snippet = snippet.rstrip - result = Prism.parse(snippet, filepath: fixture.path) + result = Prism.parse(snippet, filepath: fixture.path, version: version) assert result.success? if !ENV["PRISM_BUILD_MINIMAL"] - dumped = Prism.dump(snippet, filepath: fixture.path) - assert_equal_nodes(result.value, Prism.load(snippet, dumped).value) + dumped = Prism.dump(snippet, filepath: fixture.path, version: version) + assert_equal_nodes(result.value, Prism.load(snippet, dumped, version: version).value) end end end diff --git a/test/prism/test_helper.rb b/test/prism/test_helper.rb index 0be9d1e7da4f36..84871722c9b6ad 100644 --- a/test/prism/test_helper.rb +++ b/test/prism/test_helper.rb @@ -58,8 +58,12 @@ def snapshot_path File.join(File.expand_path("../..", __dir__), "snapshots", path) end - def test_name - :"test_#{path}" + def test_name(version = nil) + if version + :"test_#{version}_#{path}" + else + :"test_#{path}" + end end def self.each(except: [], &block) @@ -68,6 +72,14 @@ def self.each(except: [], &block) paths.each { |path| yield Fixture.new(path) } end + def self.each_with_version(except: [], &block) + each(except: except) do |fixture| + TestCase.ruby_versions_for(fixture.path).each do |version| + yield fixture, version + end + end + end + def self.custom_base_path? ENV.key?("FIXTURE_BASE") end @@ -217,6 +229,37 @@ def self.windows? RbConfig::CONFIG["host_os"].match?(/bccwin|cygwin|djgpp|mingw|mswin|wince/i) end + # All versions that prism can parse + SYNTAX_VERSIONS = %w[3.3 3.4 3.5] + + # Returns an array of ruby versions that a given filepath should test against: + # test.txt # => all available versions + # 3.4/test.txt # => versions since 3.4 (inclusive) + # 3.4-4.2/test.txt # => verisions since 3.4 (inclusive) up to 4.2 (inclusive) + def self.ruby_versions_for(filepath) + return [ENV['SYNTAX_VERSION']] if ENV['SYNTAX_VERSION'] + + parts = filepath.split("/") + return SYNTAX_VERSIONS if parts.size == 1 + + version_start, version_stop = parts[0].split("-") + if version_stop + SYNTAX_VERSIONS[SYNTAX_VERSIONS.index(version_start)..SYNTAX_VERSIONS.index(version_stop)] + else + SYNTAX_VERSIONS[SYNTAX_VERSIONS.index(version_start)..] + end + end + + def current_major_minor + RUBY_VERSION.split(".")[0, 2].join(".") + end + + if RUBY_VERSION >= "3.3.0" + def test_all_syntax_versions_present + assert_include(SYNTAX_VERSIONS, current_major_minor) + end + end + private if RUBY_ENGINE == "ruby" && RubyVM::InstructionSequence.compile("").to_a[4][:parser] != :prism diff --git a/tool/zjit_bisect.rb b/tool/zjit_bisect.rb index 175bbe5feb34e2..997c572f513019 100755 --- a/tool/zjit_bisect.rb +++ b/tool/zjit_bisect.rb @@ -72,7 +72,31 @@ def run_bisect(command, items) bisect_impl(command, [], items) end +def add_zjit_options cmd + if RUBY == "make" + # Automatically detect that we're running a make command instead of a Ruby + # one. Pass the bisection options via RUN_OPTS/SPECOPTS instead. + zjit_opts = cmd.select { |arg| arg.start_with?("--zjit") } + run_opts_index = cmd.find_index { |arg| arg.start_with?("RUN_OPTS=") } + specopts_index = cmd.find_index { |arg| arg.start_with?("SPECOPTS=") } + if run_opts_index + run_opts = Shellwords.split(cmd[run_opts_index].delete_prefix("RUN_OPTS=")) + run_opts.concat(zjit_opts) + cmd[run_opts_index] = "RUN_OPTS=#{run_opts.shelljoin}" + elsif specopts_index + specopts = Shellwords.split(cmd[specopts_index].delete_prefix("SPECOPTS=")) + specopts.concat(zjit_opts) + cmd[specopts_index] = "SPECOPTS=#{specopts.shelljoin}" + else + raise "Expected RUN_OPTS or SPECOPTS to be present in make command" + end + cmd = cmd - zjit_opts + end + cmd +end + def run_ruby *cmd + cmd = add_zjit_options(cmd) pid = Process.spawn(*cmd, { in: :close, out: [File::NULL, File::RDWR], @@ -128,7 +152,7 @@ def run_with_jit_list(ruby, options, jit_list) file.puts(result) end puts "Run:" -command = [RUBY, "--zjit-allowed-iseqs=jitlist.txt", *OPTIONS].shelljoin -puts command +jitlist_path = File.expand_path("jitlist.txt") +puts add_zjit_options([RUBY, "--zjit-allowed-iseqs=#{jitlist_path}", *OPTIONS]).shelljoin puts "Reduced JIT list (available in jitlist.txt):" puts result diff --git a/zjit.rb b/zjit.rb index fdfe4ce9835a13..88c572849c7529 100644 --- a/zjit.rb +++ b/zjit.rb @@ -176,6 +176,7 @@ def stats_string :optimized_send_count, :iseq_optimized_send_count, :inline_cfunc_optimized_send_count, + :inline_iseq_optimized_send_count, :non_variadic_cfunc_optimized_send_count, :variadic_cfunc_optimized_send_count, ], buf:, stats:, right_align: true, base: :send_count) diff --git a/zjit/src/backend/arm64/mod.rs b/zjit/src/backend/arm64/mod.rs index 74b5210f0fb0a6..a6a5fc59582610 100644 --- a/zjit/src/backend/arm64/mod.rs +++ b/zjit/src/backend/arm64/mod.rs @@ -835,10 +835,8 @@ impl Assembler { } /// Emit a CBZ or CBNZ which branches when a register is zero or non-zero - fn emit_cmp_zero_jump(_cb: &mut CodeBlock, _reg: A64Opnd, _branch_if_zero: bool, target: Target) { - if let Target::Label(_) = target { - unimplemented!("this should be re-implemented with Label for side exits"); - /* + fn emit_cmp_zero_jump(cb: &mut CodeBlock, reg: A64Opnd, branch_if_zero: bool, target: Target) { + if let Target::CodePtr(dst_ptr) = target { let dst_addr = dst_ptr.as_offset(); let src_addr = cb.get_write_ptr().as_offset(); @@ -869,7 +867,6 @@ impl Assembler { emit_load_value(cb, Assembler::EMIT0_OPND, dst_addr); br(cb, Assembler::EMIT0_OPND); } - */ } else { unreachable!("We should only generate Joz/Jonz with side-exit targets"); } @@ -878,7 +875,9 @@ impl Assembler { /// Do the address calculation of `out_reg = base_reg + disp` fn load_effective_address(cb: &mut CodeBlock, out: A64Opnd, base_reg_no: u8, disp: i32) { let base_reg = A64Opnd::Reg(A64Reg { num_bits: 64, reg_no: base_reg_no }); - assert_ne!(31, out.unwrap_reg().reg_no, "Lea sp, [sp, #imm] not always encodable. Use add/sub instead."); + let out_reg_no = out.unwrap_reg().reg_no; + assert_ne!(31, out_reg_no, "Lea sp, [sp, #imm] not always encodable. Use add/sub instead."); + assert_ne!(base_reg_no, out_reg_no, "large displacement need a scratch register"); if ShiftedImmediate::try_from(disp.unsigned_abs() as u64).is_ok() { // Use ADD/SUB if the displacement fits @@ -1203,7 +1202,27 @@ impl Assembler { let &Opnd::Mem(Mem { num_bits: _, base: MemBase::Reg(base_reg_no), disp }) = opnd else { panic!("Unexpected Insn::Lea operand in arm64_emit: {opnd:?}"); }; - load_effective_address(cb, out.into(), base_reg_no, disp); + let out_reg_no = out.unwrap_reg().reg_no; + assert_ne!(31, out_reg_no, "Lea sp, [sp, #imm] not always encodable. Use add/sub instead."); + + let out = A64Opnd::from(out); + let base_reg = A64Opnd::Reg(A64Reg { num_bits: 64, reg_no: base_reg_no }); + if ShiftedImmediate::try_from(disp.unsigned_abs() as u64).is_ok() { + // Use ADD/SUB if the displacement fits + add(cb, out, base_reg, A64Opnd::new_imm(disp.into())); + } else { + // Use a scratch reg for `out += displacement` + let disp_reg = if out_reg_no == base_reg_no { + Self::EMIT0_OPND + } else { + out + }; + // Use add_extended() to interpret reg_no=31 as sp + // since the base register is never the zero register. + // Careful! Only the first two operands can refer to sp. + emit_load_value(cb, disp_reg, disp as u64); + add_extended(cb, out, base_reg, disp_reg); + } } Insn::LeaJumpTarget { out, target, .. } => { if let Target::Label(label_idx) = target { @@ -1806,6 +1825,25 @@ mod tests { assert_snapshot!(cb.hexdump(), @"e07b40b2e063208b000180d22000a0f2e063208b000083d2e063208be0230891e02308d1e0ff8292e063208b00ff9fd2c0ffbff2e0ffdff2e0fffff2e063208be08361b2e063208b"); } + #[test] + fn test_load_larg_disp_mem() { + let (mut asm, mut cb) = setup_asm(); + + let extended_ivars = asm.load(Opnd::mem(64, NATIVE_STACK_PTR, 0)); + let result = asm.load(Opnd::mem(VALUE_BITS, extended_ivars, 1000 * SIZEOF_VALUE_I32)); + asm.store(Opnd::mem(VALUE_BITS, NATIVE_STACK_PTR, 0), result); + + asm.compile_with_num_regs(&mut cb, 1); + assert_disasm_snapshot!(cb.disasm(), @r" + 0x0: ldur x0, [sp] + 0x4: mov x16, #0x1f40 + 0x8: add x0, x0, x16, uxtx + 0xc: ldur x0, [x0] + 0x10: stur x0, [sp] + "); + assert_snapshot!(cb.hexdump(), @"e00340f810e883d20060308b000040f8e00300f8"); + } + #[test] fn test_store() { let (mut asm, mut cb) = setup_asm(); diff --git a/zjit/src/backend/mod.rs b/zjit/src/backend/mod.rs index ee861f5bd3562a..635acbf60c1009 100644 --- a/zjit/src/backend/mod.rs +++ b/zjit/src/backend/mod.rs @@ -12,4 +12,7 @@ pub use x86_64 as current; #[cfg(target_arch = "aarch64")] pub use arm64 as current; +#[cfg(test)] +mod tests; + pub mod lir; diff --git a/zjit/src/backend/tests.rs b/zjit/src/backend/tests.rs index aca775edd66cb5..1b72777212cb23 100644 --- a/zjit/src/backend/tests.rs +++ b/zjit/src/backend/tests.rs @@ -1,19 +1,21 @@ #![cfg(test)] use crate::asm::CodeBlock; -use crate::backend::*; +use crate::backend::lir::*; use crate::cruby::*; -use crate::utils::c_callable; +use crate::codegen::c_callable; +use crate::options::rb_zjit_prepare_options; #[test] fn test_add() { - let mut asm = Assembler::new(0); + let mut asm = Assembler::new(); let out = asm.add(SP, Opnd::UImm(1)); let _ = asm.add(out, Opnd::UImm(2)); } #[test] fn test_alloc_regs() { - let mut asm = Assembler::new(0); + rb_zjit_prepare_options(); // for asm.alloc_regs + let mut asm = Assembler::new(); // Get the first output that we're going to reuse later. let out1 = asm.add(EC, Opnd::UImm(1)); @@ -36,7 +38,7 @@ fn test_alloc_regs() { let _ = asm.add(out3, Opnd::UImm(6)); // Here we're going to allocate the registers. - let result = asm.alloc_regs(Assembler::get_alloc_regs()); + let result = asm.alloc_regs(Assembler::get_alloc_regs()).unwrap(); // Now we're going to verify that the out field has been appropriately // updated for each of the instructions that needs it. @@ -62,8 +64,8 @@ fn test_alloc_regs() { fn setup_asm() -> (Assembler, CodeBlock) { return ( - Assembler::new(0), - CodeBlock::new_dummy(1024) + Assembler::new(), + CodeBlock::new_dummy() ); } @@ -86,6 +88,7 @@ fn test_compile() fn test_mov_mem2mem() { let (mut asm, mut cb) = setup_asm(); + rb_zjit_prepare_options(); // for asm_comment asm_comment!(asm, "check that comments work too"); asm.mov(Opnd::mem(64, SP, 0), Opnd::mem(64, SP, 8)); @@ -163,11 +166,10 @@ fn test_base_insn_out() ); // Load the pointer into a register - let ptr_reg = asm.load(Opnd::const_ptr(4351776248 as *const u8)); - let counter_opnd = Opnd::mem(64, ptr_reg, 0); + let ptr_opnd = Opnd::const_ptr(4351776248 as *const u8); // Increment and store the updated value - asm.incr_counter(counter_opnd, 1.into()); + asm.incr_counter(ptr_opnd, 1.into()); asm.compile_with_num_regs(&mut cb, 2); } @@ -180,6 +182,7 @@ fn test_c_call() } let (mut asm, mut cb) = setup_asm(); + rb_zjit_prepare_options(); // for asm.compile let ret_val = asm.ccall( dummy_c_fun as *const u8, @@ -189,17 +192,17 @@ fn test_c_call() // Make sure that the call's return value is usable asm.mov(Opnd::mem(64, SP, 0), ret_val); - asm.compile_with_num_regs(&mut cb, 1); + asm.compile(&mut cb).unwrap(); } #[test] fn test_alloc_ccall_regs() { - let mut asm = Assembler::new(0); + let mut asm = Assembler::new(); let out1 = asm.ccall(0 as *const u8, vec![]); let out2 = asm.ccall(0 as *const u8, vec![out1]); asm.mov(EC, out2); - let mut cb = CodeBlock::new_dummy(1024); - asm.compile_with_regs(&mut cb, None, Assembler::get_alloc_regs()); + let mut cb = CodeBlock::new_dummy(); + asm.compile_with_regs(&mut cb, Assembler::get_alloc_regs()).unwrap(); } #[test] @@ -220,7 +223,7 @@ fn test_jcc_label() let label = asm.new_label("foo"); asm.cmp(EC, EC); - asm.je(label); + asm.je(label.clone()); asm.write_label(label); asm.compile_with_num_regs(&mut cb, 1); @@ -281,31 +284,11 @@ fn test_bake_string() { asm.compile_with_num_regs(&mut cb, 0); } -#[test] -fn test_draining_iterator() { - let mut asm = Assembler::new(0); - - let _ = asm.load(Opnd::None); - asm.store(Opnd::None, Opnd::None); - let _ = asm.add(Opnd::None, Opnd::None); - - let mut iter = asm.into_draining_iter(); - - while let Some((index, insn)) = iter.next_unmapped() { - match index { - 0 => assert!(matches!(insn, Insn::Load { .. })), - 1 => assert!(matches!(insn, Insn::Store { .. })), - 2 => assert!(matches!(insn, Insn::Add { .. })), - _ => panic!("Unexpected instruction index"), - }; - } -} - #[test] fn test_cmp_8_bit() { let (mut asm, mut cb) = setup_asm(); let reg = Assembler::get_alloc_regs()[0]; - asm.cmp(Opnd::Reg(reg).with_num_bits(8).unwrap(), Opnd::UImm(RUBY_SYMBOL_FLAG as u64)); + asm.cmp(Opnd::Reg(reg).with_num_bits(8), Opnd::UImm(RUBY_SYMBOL_FLAG as u64)); asm.compile_with_num_regs(&mut cb, 1); } @@ -314,7 +297,8 @@ fn test_cmp_8_bit() { fn test_no_pos_marker_callback_when_compile_fails() { // When compilation fails (e.g. when out of memory), the code written out is malformed. // We don't want to invoke the pos_marker callbacks with positions of malformed code. - let mut asm = Assembler::new(0); + let mut asm = Assembler::new(); + rb_zjit_prepare_options(); // for asm.compile // Markers around code to exhaust memory limit let fail_if_called = |_code_ptr, _cb: &_| panic!("pos_marker callback should not be called"); @@ -324,6 +308,6 @@ fn test_no_pos_marker_callback_when_compile_fails() { asm.store(Opnd::mem(64, SP, 8), sum); asm.pos_marker(fail_if_called); - let cb = &mut CodeBlock::new_dummy(8); - assert!(asm.compile(cb, None).is_none(), "should fail due to tiny size limit"); + let cb = &mut CodeBlock::new_dummy_sized(8); + assert!(asm.compile(cb).is_err(), "should fail due to tiny size limit"); } diff --git a/zjit/src/codegen.rs b/zjit/src/codegen.rs index 402c500c8b17b9..50a7295bbe2672 100644 --- a/zjit/src/codegen.rs +++ b/zjit/src/codegen.rs @@ -365,6 +365,7 @@ fn gen_insn(cb: &mut CodeBlock, jit: &mut JITState, asm: &mut Assembler, functio Insn::StringConcat { strings, state, .. } if strings.is_empty() => return Err(*state), Insn::StringConcat { strings, state } => gen_string_concat(jit, asm, opnds!(strings), &function.frame_state(*state)), &Insn::StringGetbyteFixnum { string, index } => gen_string_getbyte_fixnum(asm, opnd!(string), opnd!(index)), + Insn::StringAppend { recv, other, state } => gen_string_append(jit, asm, opnd!(recv), opnd!(other), &function.frame_state(*state)), Insn::StringIntern { val, state } => gen_intern(asm, opnd!(val), &function.frame_state(*state)), Insn::ToRegexp { opt, values, state } => gen_toregexp(jit, asm, *opt, opnds!(values), &function.frame_state(*state)), Insn::Param => unreachable!("block.insns should not have Insn::Param"), @@ -2189,6 +2190,11 @@ fn gen_string_getbyte_fixnum(asm: &mut Assembler, string: Opnd, index: Opnd) -> asm_ccall!(asm, rb_str_getbyte, string, index) } +fn gen_string_append(jit: &mut JITState, asm: &mut Assembler, string: Opnd, val: Opnd, state: &FrameState) -> Opnd { + gen_prepare_non_leaf_call(jit, asm, state); + asm_ccall!(asm, rb_str_buf_append, string, val) +} + /// 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(); diff --git a/zjit/src/cruby_methods.rs b/zjit/src/cruby_methods.rs index 656ccab7817c91..9d3f5a756b4a47 100644 --- a/zjit/src/cruby_methods.rs +++ b/zjit/src/cruby_methods.rs @@ -192,6 +192,7 @@ pub fn init() -> Annotations { annotate!(rb_cString, "to_s", types::StringExact); annotate!(rb_cString, "getbyte", inline_string_getbyte); annotate!(rb_cString, "empty?", types::BoolExact, no_gc, leaf, elidable); + annotate!(rb_cString, "<<", inline_string_append); annotate!(rb_cModule, "name", types::StringExact.union(types::NilClass), no_gc, leaf, elidable); annotate!(rb_cModule, "===", types::BoolExact, no_gc, leaf); annotate!(rb_cArray, "length", types::Fixnum, no_gc, leaf, elidable); @@ -276,6 +277,20 @@ fn inline_string_getbyte(fun: &mut hir::Function, block: hir::BlockId, recv: hir None } +fn inline_string_append(fun: &mut hir::Function, block: hir::BlockId, recv: hir::InsnId, args: &[hir::InsnId], state: hir::InsnId) -> Option { + let &[other] = args else { return None; }; + // Inline only StringExact << String, which matches original type check from + // `vm_opt_ltlt`, which checks `RB_TYPE_P(obj, T_STRING)`. + if fun.likely_a(recv, types::StringExact, state) && fun.likely_a(other, types::String, state) { + let recv = fun.coerce_to(block, recv, types::StringExact, state); + let other = fun.coerce_to(block, other, types::String, state); + let _ = fun.push_insn(block, hir::Insn::StringAppend { recv, other, state }); + Some(recv) + } else { + None + } +} + fn inline_integer_succ(fun: &mut hir::Function, block: hir::BlockId, recv: hir::InsnId, args: &[hir::InsnId], state: hir::InsnId) -> Option { if !args.is_empty() { return None; } if fun.likely_a(recv, types::Fixnum, state) { diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index 68ed867e4ddd4d..e8a366ca6c45f2 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -560,6 +560,7 @@ pub enum Insn { StringConcat { strings: Vec, state: InsnId }, /// Call rb_str_getbyte with known-Fixnum index StringGetbyteFixnum { string: InsnId, index: InsnId }, + StringAppend { recv: InsnId, other: InsnId, state: InsnId }, /// Combine count stack values into a regexp ToRegexp { opt: usize, values: Vec, state: InsnId }, @@ -956,6 +957,9 @@ impl<'a> std::fmt::Display for InsnPrinter<'a> { Insn::StringGetbyteFixnum { string, index, .. } => { write!(f, "StringGetbyteFixnum {string}, {index}") } + Insn::StringAppend { recv, other, .. } => { + write!(f, "StringAppend {recv}, {other}") + } Insn::ToRegexp { values, opt, .. } => { write!(f, "ToRegexp")?; let mut prefix = " "; @@ -1354,6 +1358,72 @@ pub struct Function { profiles: Option, } +/// The kind of a value an ISEQ returns +enum IseqReturn { + Value(VALUE), + LocalVariable(u32), + Receiver, +} + +unsafe extern "C" { + fn rb_simple_iseq_p(iseq: IseqPtr) -> bool; +} + +/// Return the ISEQ's return value if it consists of one simple instruction and leave. +fn iseq_get_return_value(iseq: IseqPtr, captured_opnd: Option, ci_flags: u32) -> Option { + // Expect only two instructions and one possible operand + // NOTE: If an ISEQ has an optional keyword parameter with a default value that requires + // computation, the ISEQ will always have more than two instructions and won't be inlined. + let iseq_size = unsafe { get_iseq_encoded_size(iseq) }; + if !(2..=3).contains(&iseq_size) { + return None; + } + + // Get the first two instructions + let first_insn = iseq_opcode_at_idx(iseq, 0); + let second_insn = iseq_opcode_at_idx(iseq, insn_len(first_insn as usize)); + + // Extract the return value if known + if second_insn != YARVINSN_leave { + return None; + } + match first_insn { + YARVINSN_getlocal_WC_0 => { + // Accept only cases where only positional arguments are used by both the callee and the caller. + // Keyword arguments may be specified by the callee or the caller but not used. + if captured_opnd.is_some() + // Equivalent to `VM_CALL_ARGS_SIMPLE - VM_CALL_KWARG - has_block_iseq` + || ci_flags & ( + VM_CALL_ARGS_SPLAT + | VM_CALL_KW_SPLAT + | VM_CALL_ARGS_BLOCKARG + | VM_CALL_FORWARDING + ) != 0 + { + return None; + } + + let ep_offset = unsafe { *rb_iseq_pc_at_idx(iseq, 1) }.as_u32(); + let local_idx = ep_offset_to_local_idx(iseq, ep_offset); + + if unsafe { rb_simple_iseq_p(iseq) } { + return Some(IseqReturn::LocalVariable(local_idx.try_into().unwrap())); + } + + // TODO(max): Support only_kwparam case where the local_idx is a positional parameter + + return None; + } + YARVINSN_putnil => Some(IseqReturn::Value(Qnil)), + YARVINSN_putobject => Some(IseqReturn::Value(unsafe { *rb_iseq_pc_at_idx(iseq, 1) })), + YARVINSN_putobject_INT2FIX_0_ => Some(IseqReturn::Value(VALUE::fixnum_from_usize(0))), + YARVINSN_putobject_INT2FIX_1_ => Some(IseqReturn::Value(VALUE::fixnum_from_usize(1))), + // We don't support invokeblock for now. Such ISEQs are likely not used by blocks anyway. + YARVINSN_putself if captured_opnd.is_none() => Some(IseqReturn::Receiver), + _ => None, + } +} + impl Function { fn new(iseq: *const rb_iseq_t) -> Function { Function { @@ -1518,6 +1588,7 @@ impl Function { &StringIntern { val, state } => StringIntern { val: find!(val), state: find!(state) }, &StringConcat { ref strings, state } => StringConcat { strings: find_vec!(strings), state: find!(state) }, &StringGetbyteFixnum { string, index } => StringGetbyteFixnum { string: find!(string), index: find!(index) }, + &StringAppend { recv, other, state } => StringAppend { recv: find!(recv), other: find!(other), state: find!(state) }, &ToRegexp { opt, ref values, state } => ToRegexp { opt, values: find_vec!(values), state }, &Test { val } => Test { val: find!(val) }, &IsNil { val } => IsNil { val: find!(val) }, @@ -1712,6 +1783,7 @@ impl Function { Insn::StringIntern { .. } => types::Symbol, Insn::StringConcat { .. } => types::StringExact, Insn::StringGetbyteFixnum { .. } => types::Fixnum.union(types::NilClass), + Insn::StringAppend { .. } => types::StringExact, Insn::ToRegexp { .. } => types::RegexpExact, Insn::NewArray { .. } => types::ArrayExact, Insn::ArrayDup { .. } => types::ArrayExact, @@ -2337,6 +2409,46 @@ impl Function { self.infer_types(); } + fn inline(&mut self) { + for block in self.rpo() { + let old_insns = std::mem::take(&mut self.blocks[block.0].insns); + assert!(self.blocks[block.0].insns.is_empty()); + for insn_id in old_insns { + match self.find(insn_id) { + // Reject block ISEQs to avoid autosplat and other block parameter complications. + Insn::SendWithoutBlockDirect { recv, iseq, cd, args, .. } => { + let call_info = unsafe { (*cd).ci }; + let ci_flags = unsafe { vm_ci_flag(call_info) }; + // .send call is not currently supported for builtins + if ci_flags & VM_CALL_OPT_SEND != 0 { + self.push_insn_id(block, insn_id); continue; + } + let Some(value) = iseq_get_return_value(iseq, None, ci_flags) else { + self.push_insn_id(block, insn_id); continue; + }; + match value { + IseqReturn::LocalVariable(idx) => { + self.push_insn(block, Insn::IncrCounter(Counter::inline_iseq_optimized_send_count)); + self.make_equal_to(insn_id, args[idx as usize]); + } + IseqReturn::Value(value) => { + self.push_insn(block, Insn::IncrCounter(Counter::inline_iseq_optimized_send_count)); + let replacement = self.push_insn(block, Insn::Const { val: Const::Value(value) }); + self.make_equal_to(insn_id, replacement); + } + IseqReturn::Receiver => { + self.push_insn(block, Insn::IncrCounter(Counter::inline_iseq_optimized_send_count)); + self.make_equal_to(insn_id, recv); + } + } + } + _ => { self.push_insn_id(block, insn_id); } + } + } + } + self.infer_types(); + } + fn optimize_getivar(&mut self) { for block in self.rpo() { let old_insns = std::mem::take(&mut self.blocks[block.0].insns); @@ -2925,6 +3037,11 @@ impl Function { worklist.push_back(string); worklist.push_back(index); } + &Insn::StringAppend { recv, other, state } => { + worklist.push_back(recv); + worklist.push_back(other); + worklist.push_back(state); + } &Insn::ToRegexp { ref values, state, .. } => { worklist.extend(values); worklist.push_back(state); @@ -3197,6 +3314,8 @@ impl Function { // Function is assumed to have types inferred already self.type_specialize(); #[cfg(debug_assertions)] self.assert_validates(); + self.inline(); + #[cfg(debug_assertions)] self.assert_validates(); self.optimize_getivar(); #[cfg(debug_assertions)] self.assert_validates(); self.optimize_c_calls(); @@ -8969,15 +9088,14 @@ mod opt_tests { #[test] fn test_optimize_top_level_call_into_send_direct() { eval(" - def foo - end + def foo = [] def test foo end test; test "); assert_snapshot!(hir_string("test"), @r" - fn test@:5: + fn test@:4: bb0(): EntryPoint interpreter v1:BasicObject = LoadSelf @@ -9025,8 +9143,7 @@ mod opt_tests { #[test] fn test_optimize_private_top_level_call() { eval(" - def foo - end + def foo = [] private :foo def test foo @@ -9034,7 +9151,7 @@ mod opt_tests { test; test "); assert_snapshot!(hir_string("test"), @r" - fn test@:6: + fn test@:5: bb0(): EntryPoint interpreter v1:BasicObject = LoadSelf @@ -9083,15 +9200,14 @@ mod opt_tests { #[test] fn test_optimize_top_level_call_with_args_into_send_direct() { eval(" - def foo a, b - end + def foo(a, b) = [] def test foo 1, 2 end test; test "); assert_snapshot!(hir_string("test"), @r" - fn test@:5: + fn test@:4: bb0(): EntryPoint interpreter v1:BasicObject = LoadSelf @@ -9114,10 +9230,8 @@ mod opt_tests { #[test] fn test_optimize_top_level_sends_into_send_direct() { eval(" - def foo - end - def bar - end + def foo = [] + def bar = [] def test foo bar @@ -9125,7 +9239,7 @@ mod opt_tests { test; test "); assert_snapshot!(hir_string("test"), @r" - fn test@:7: + fn test@:5: bb0(): EntryPoint interpreter v1:BasicObject = LoadSelf @@ -10645,9 +10759,7 @@ mod opt_tests { fn test_send_direct_to_instance_method() { eval(" class C - def foo - 3 - end + def foo = [] end def test(c) = c.foo @@ -10657,7 +10769,7 @@ mod opt_tests { "); assert_snapshot!(hir_string("test"), @r" - fn test@:8: + fn test@:6: bb0(): EntryPoint interpreter v1:BasicObject = LoadSelf @@ -12086,7 +12198,7 @@ mod opt_tests { fn test_dont_optimize_array_aref_if_redefined() { eval(r##" class Array - def [](index); end + def [](index) = [] end def test = [4,5,6].freeze[10] "##); @@ -12115,7 +12227,7 @@ mod opt_tests { fn test_dont_optimize_array_max_if_redefined() { eval(r##" class Array - def max = 10 + def max = [] end def test = [4,5,6].max "##); @@ -12211,14 +12323,15 @@ mod opt_tests { PatchPoint MethodRedefined(Object@0x1000, zero@0x1008, cme:0x1010) PatchPoint NoSingletonClass(Object@0x1000) v22:HeapObject[class_exact*:Object@VALUE(0x1000)] = GuardType v6, HeapObject[class_exact*:Object@VALUE(0x1000)] - v23:BasicObject = SendWithoutBlockDirect v22, :zero (0x1038) + IncrCounter inline_iseq_optimized_send_count + v30:StaticSymbol[:b] = Const Value(VALUE(0x1038)) PatchPoint SingleRactorMode PatchPoint MethodRedefined(Object@0x1000, one@0x1040, cme:0x1048) PatchPoint NoSingletonClass(Object@0x1000) v27:HeapObject[class_exact*:Object@VALUE(0x1000)] = GuardType v6, HeapObject[class_exact*:Object@VALUE(0x1000)] - v28:BasicObject = SendWithoutBlockDirect v27, :one (0x1038), v23 + IncrCounter inline_iseq_optimized_send_count CheckInterrupts - Return v28 + Return v30 "); } @@ -12296,9 +12409,9 @@ mod opt_tests { v12:Fixnum[100] = Const Value(100) PatchPoint MethodRedefined(Class@0x1010, identity@0x1018, cme:0x1020) PatchPoint NoSingletonClass(Class@0x1010) - v25:BasicObject = SendWithoutBlockDirect v22, :identity (0x1048), v12 + IncrCounter inline_iseq_optimized_send_count CheckInterrupts - Return v25 + Return v12 "); } @@ -12786,9 +12899,10 @@ mod opt_tests { PatchPoint MethodRedefined(Object@0x1000, foo@0x1008, cme:0x1010) PatchPoint NoSingletonClass(Object@0x1000) v19:HeapObject[class_exact*:Object@VALUE(0x1000)] = GuardType v6, HeapObject[class_exact*:Object@VALUE(0x1000)] - v20:BasicObject = SendWithoutBlockDirect v19, :foo (0x1038) + IncrCounter inline_iseq_optimized_send_count + v22:NilClass = Const Value(nil) CheckInterrupts - Return v20 + Return v22 "); } @@ -13919,6 +14033,122 @@ mod opt_tests { "); } + #[test] + fn test_optimize_string_append() { + eval(r#" + def test(x, y) = x << y + test("iron", "fish") + "#); + assert_snapshot!(hir_string("test"), @r" + fn test@:2: + bb0(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + v2:BasicObject = GetLocal l0, SP@5 + v3:BasicObject = GetLocal l0, SP@4 + Jump bb2(v1, v2, v3) + bb1(v6:BasicObject, v7:BasicObject, v8:BasicObject): + EntryPoint JIT(0) + Jump bb2(v6, v7, v8) + bb2(v10:BasicObject, v11:BasicObject, v12:BasicObject): + PatchPoint MethodRedefined(String@0x1000, <<@0x1008, cme:0x1010) + PatchPoint NoSingletonClass(String@0x1000) + v28:StringExact = GuardType v11, StringExact + v29:String = GuardType v12, String + v30:StringExact = StringAppend v28, v29 + IncrCounter inline_cfunc_optimized_send_count + CheckInterrupts + Return v28 + "); + } + + // TODO: This should be inlined just as in the interpreter + #[test] + fn test_optimize_string_append_non_string() { + eval(r#" + def test(x, y) = x << y + test("iron", 4) + "#); + assert_snapshot!(hir_string("test"), @r" + fn test@:2: + bb0(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + v2:BasicObject = GetLocal l0, SP@5 + v3:BasicObject = GetLocal l0, SP@4 + Jump bb2(v1, v2, v3) + bb1(v6:BasicObject, v7:BasicObject, v8:BasicObject): + EntryPoint JIT(0) + Jump bb2(v6, v7, v8) + bb2(v10:BasicObject, v11:BasicObject, v12:BasicObject): + PatchPoint MethodRedefined(String@0x1000, <<@0x1008, cme:0x1010) + PatchPoint NoSingletonClass(String@0x1000) + v28:StringExact = GuardType v11, StringExact + v29:BasicObject = CCallWithFrame <<@0x1038, v28, v12 + CheckInterrupts + Return v29 + "); + } + + // TODO: Should be optimized, but is waiting on String#== inlining + #[test] + fn test_optimize_string_append_string_subclass() { + eval(r#" + class MyString < String + end + def test(x, y) = x << y + test("iron", MyString.new) + "#); + assert_snapshot!(hir_string("test"), @r" + fn test@:4: + bb0(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + v2:BasicObject = GetLocal l0, SP@5 + v3:BasicObject = GetLocal l0, SP@4 + Jump bb2(v1, v2, v3) + bb1(v6:BasicObject, v7:BasicObject, v8:BasicObject): + EntryPoint JIT(0) + Jump bb2(v6, v7, v8) + bb2(v10:BasicObject, v11:BasicObject, v12:BasicObject): + PatchPoint MethodRedefined(String@0x1000, <<@0x1008, cme:0x1010) + PatchPoint NoSingletonClass(String@0x1000) + v28:StringExact = GuardType v11, StringExact + v29:BasicObject = CCallWithFrame <<@0x1038, v28, v12 + CheckInterrupts + Return v29 + "); + } + + #[test] + fn test_do_not_optimize_string_subclass_append_string() { + eval(r#" + class MyString < String + end + def test(x, y) = x << y + test(MyString.new, "iron") + "#); + assert_snapshot!(hir_string("test"), @r" + fn test@:4: + bb0(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + v2:BasicObject = GetLocal l0, SP@5 + v3:BasicObject = GetLocal l0, SP@4 + Jump bb2(v1, v2, v3) + bb1(v6:BasicObject, v7:BasicObject, v8:BasicObject): + EntryPoint JIT(0) + Jump bb2(v6, v7, v8) + bb2(v10:BasicObject, v11:BasicObject, v12:BasicObject): + PatchPoint MethodRedefined(MyString@0x1000, <<@0x1008, cme:0x1010) + PatchPoint NoSingletonClass(MyString@0x1000) + v28:HeapObject[class_exact:MyString] = GuardType v11, HeapObject[class_exact:MyString] + v29:BasicObject = CCallWithFrame <<@0x1038, v28, v12 + CheckInterrupts + Return v29 + "); + } + #[test] fn test_dont_inline_integer_succ_with_args() { eval(" @@ -14490,4 +14720,299 @@ mod opt_tests { Return v25 "); } + + #[test] + fn test_inline_send_without_block_direct_putself() { + eval(r#" + def callee = self + def test = callee + test + "#); + assert_snapshot!(hir_string("test"), @r" + fn test@:3: + bb0(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + Jump bb2(v1) + bb1(v4:BasicObject): + EntryPoint JIT(0) + Jump bb2(v4) + bb2(v6:BasicObject): + PatchPoint MethodRedefined(Object@0x1000, callee@0x1008, cme:0x1010) + PatchPoint NoSingletonClass(Object@0x1000) + v19:HeapObject[class_exact*:Object@VALUE(0x1000)] = GuardType v6, HeapObject[class_exact*:Object@VALUE(0x1000)] + IncrCounter inline_iseq_optimized_send_count + CheckInterrupts + Return v19 + "); + } + + #[test] + fn test_inline_send_without_block_direct_putobject_string() { + eval(r#" + # frozen_string_literal: true + def callee = "abc" + def test = callee + test + "#); + assert_snapshot!(hir_string("test"), @r" + fn test@:4: + bb0(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + Jump bb2(v1) + bb1(v4:BasicObject): + EntryPoint JIT(0) + Jump bb2(v4) + bb2(v6:BasicObject): + PatchPoint MethodRedefined(Object@0x1000, callee@0x1008, cme:0x1010) + PatchPoint NoSingletonClass(Object@0x1000) + v19:HeapObject[class_exact*:Object@VALUE(0x1000)] = GuardType v6, HeapObject[class_exact*:Object@VALUE(0x1000)] + IncrCounter inline_iseq_optimized_send_count + v22:StringExact[VALUE(0x1038)] = Const Value(VALUE(0x1038)) + CheckInterrupts + Return v22 + "); + } + + #[test] + fn test_inline_send_without_block_direct_putnil() { + eval(r#" + def callee = nil + def test = callee + test + "#); + assert_snapshot!(hir_string("test"), @r" + fn test@:3: + bb0(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + Jump bb2(v1) + bb1(v4:BasicObject): + EntryPoint JIT(0) + Jump bb2(v4) + bb2(v6:BasicObject): + PatchPoint MethodRedefined(Object@0x1000, callee@0x1008, cme:0x1010) + PatchPoint NoSingletonClass(Object@0x1000) + v19:HeapObject[class_exact*:Object@VALUE(0x1000)] = GuardType v6, HeapObject[class_exact*:Object@VALUE(0x1000)] + IncrCounter inline_iseq_optimized_send_count + v22:NilClass = Const Value(nil) + CheckInterrupts + Return v22 + "); + } + + #[test] + fn test_inline_send_without_block_direct_putobject_true() { + eval(r#" + def callee = true + def test = callee + test + "#); + assert_snapshot!(hir_string("test"), @r" + fn test@:3: + bb0(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + Jump bb2(v1) + bb1(v4:BasicObject): + EntryPoint JIT(0) + Jump bb2(v4) + bb2(v6:BasicObject): + PatchPoint MethodRedefined(Object@0x1000, callee@0x1008, cme:0x1010) + PatchPoint NoSingletonClass(Object@0x1000) + v19:HeapObject[class_exact*:Object@VALUE(0x1000)] = GuardType v6, HeapObject[class_exact*:Object@VALUE(0x1000)] + IncrCounter inline_iseq_optimized_send_count + v22:TrueClass = Const Value(true) + CheckInterrupts + Return v22 + "); + } + + #[test] + fn test_inline_send_without_block_direct_putobject_false() { + eval(r#" + def callee = false + def test = callee + test + "#); + assert_snapshot!(hir_string("test"), @r" + fn test@:3: + bb0(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + Jump bb2(v1) + bb1(v4:BasicObject): + EntryPoint JIT(0) + Jump bb2(v4) + bb2(v6:BasicObject): + PatchPoint MethodRedefined(Object@0x1000, callee@0x1008, cme:0x1010) + PatchPoint NoSingletonClass(Object@0x1000) + v19:HeapObject[class_exact*:Object@VALUE(0x1000)] = GuardType v6, HeapObject[class_exact*:Object@VALUE(0x1000)] + IncrCounter inline_iseq_optimized_send_count + v22:FalseClass = Const Value(false) + CheckInterrupts + Return v22 + "); + } + + #[test] + fn test_inline_send_without_block_direct_putobject_zero() { + eval(r#" + def callee = 0 + def test = callee + test + "#); + assert_snapshot!(hir_string("test"), @r" + fn test@:3: + bb0(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + Jump bb2(v1) + bb1(v4:BasicObject): + EntryPoint JIT(0) + Jump bb2(v4) + bb2(v6:BasicObject): + PatchPoint MethodRedefined(Object@0x1000, callee@0x1008, cme:0x1010) + PatchPoint NoSingletonClass(Object@0x1000) + v19:HeapObject[class_exact*:Object@VALUE(0x1000)] = GuardType v6, HeapObject[class_exact*:Object@VALUE(0x1000)] + IncrCounter inline_iseq_optimized_send_count + v22:Fixnum[0] = Const Value(0) + CheckInterrupts + Return v22 + "); + } + + #[test] + fn test_inline_send_without_block_direct_putobject_one() { + eval(r#" + def callee = 1 + def test = callee + test + "#); + assert_snapshot!(hir_string("test"), @r" + fn test@:3: + bb0(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + Jump bb2(v1) + bb1(v4:BasicObject): + EntryPoint JIT(0) + Jump bb2(v4) + bb2(v6:BasicObject): + PatchPoint MethodRedefined(Object@0x1000, callee@0x1008, cme:0x1010) + PatchPoint NoSingletonClass(Object@0x1000) + v19:HeapObject[class_exact*:Object@VALUE(0x1000)] = GuardType v6, HeapObject[class_exact*:Object@VALUE(0x1000)] + IncrCounter inline_iseq_optimized_send_count + v22:Fixnum[1] = Const Value(1) + CheckInterrupts + Return v22 + "); + } + + #[test] + fn test_inline_send_without_block_direct_parameter() { + eval(r#" + def callee(x) = x + def test = callee 3 + test + "#); + assert_snapshot!(hir_string("test"), @r" + fn test@:3: + bb0(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + Jump bb2(v1) + bb1(v4:BasicObject): + EntryPoint JIT(0) + Jump bb2(v4) + bb2(v6:BasicObject): + v10:Fixnum[3] = Const Value(3) + PatchPoint MethodRedefined(Object@0x1000, callee@0x1008, cme:0x1010) + PatchPoint NoSingletonClass(Object@0x1000) + v20:HeapObject[class_exact*:Object@VALUE(0x1000)] = GuardType v6, HeapObject[class_exact*:Object@VALUE(0x1000)] + IncrCounter inline_iseq_optimized_send_count + CheckInterrupts + Return v10 + "); + } + + #[test] + fn test_inline_send_without_block_direct_last_parameter() { + eval(r#" + def callee(x, y, z) = z + def test = callee 1, 2, 3 + test + "#); + assert_snapshot!(hir_string("test"), @r" + fn test@:3: + bb0(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + Jump bb2(v1) + bb1(v4:BasicObject): + EntryPoint JIT(0) + Jump bb2(v4) + bb2(v6:BasicObject): + v10:Fixnum[1] = Const Value(1) + v11:Fixnum[2] = Const Value(2) + v12:Fixnum[3] = Const Value(3) + PatchPoint MethodRedefined(Object@0x1000, callee@0x1008, cme:0x1010) + PatchPoint NoSingletonClass(Object@0x1000) + v22:HeapObject[class_exact*:Object@VALUE(0x1000)] = GuardType v6, HeapObject[class_exact*:Object@VALUE(0x1000)] + IncrCounter inline_iseq_optimized_send_count + CheckInterrupts + Return v12 + "); + } + + #[test] + fn test_inline_symbol_to_sym() { + eval(r#" + def test(o) = o.to_sym + test :foo + "#); + assert_snapshot!(hir_string("test"), @r" + fn test@:2: + bb0(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + v2:BasicObject = GetLocal l0, SP@4 + Jump bb2(v1, v2) + bb1(v5:BasicObject, v6:BasicObject): + EntryPoint JIT(0) + Jump bb2(v5, v6) + bb2(v8:BasicObject, v9:BasicObject): + PatchPoint MethodRedefined(Symbol@0x1000, to_sym@0x1008, cme:0x1010) + v21:StaticSymbol = GuardType v9, StaticSymbol + IncrCounter inline_iseq_optimized_send_count + CheckInterrupts + Return v21 + "); + } + + #[test] + fn test_inline_integer_to_i() { + eval(r#" + def test(o) = o.to_i + test 5 + "#); + assert_snapshot!(hir_string("test"), @r" + fn test@:2: + bb0(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + v2:BasicObject = GetLocal l0, SP@4 + Jump bb2(v1, v2) + bb1(v5:BasicObject, v6:BasicObject): + EntryPoint JIT(0) + Jump bb2(v5, v6) + bb2(v8:BasicObject, v9:BasicObject): + PatchPoint MethodRedefined(Integer@0x1000, to_i@0x1008, cme:0x1010) + v21:Fixnum = GuardType v9, Fixnum + IncrCounter inline_iseq_optimized_send_count + CheckInterrupts + Return v21 + "); + } } diff --git a/zjit/src/stats.rs b/zjit/src/stats.rs index 913a72fa5646ff..4dd87d269ad4e7 100644 --- a/zjit/src/stats.rs +++ b/zjit/src/stats.rs @@ -179,6 +179,7 @@ make_counters! { optimized_send { iseq_optimized_send_count, inline_cfunc_optimized_send_count, + inline_iseq_optimized_send_count, non_variadic_cfunc_optimized_send_count, variadic_cfunc_optimized_send_count, }