diff options
34 files changed, 262 insertions, 198 deletions
@@ -337,7 +337,8 @@ rb_gc_shutdown_call_finalizer_p(VALUE obj) { switch (BUILTIN_TYPE(obj)) { case T_DATA: - if (!ruby_free_at_exit_p() && (!DATA_PTR(obj) || !RDATA(obj)->dfree)) return false; + if (!ruby_free_at_exit_p() && !DATA_PTR(obj)) return false; + if (!ruby_free_at_exit_p() && !RTYPEDDATA_P(obj) && !RDATA(obj)->dfree) return false; if (rb_obj_is_thread(obj)) return false; if (rb_obj_is_mutex(obj)) return false; if (rb_obj_is_fiber(obj)) return false; @@ -1142,14 +1143,16 @@ rb_data_object_wrap(VALUE klass, void *datap, RUBY_DATA_FUNC dmark, RUBY_DATA_FU { RUBY_ASSERT_ALWAYS(dfree != (RUBY_DATA_FUNC)1); if (klass) rb_data_object_check(klass); - VALUE obj = rb_newobj(GET_EC(), klass, T_DATA, ROOT_SHAPE_ID, !dmark, sizeof(struct RTypedData)); + VALUE obj = rb_newobj(GET_EC(), klass, T_DATA, ROOT_SHAPE_ID, !dmark, sizeof(struct RData)); rb_gc_register_pinning_obj(obj); struct RData *data = (struct RData *)obj; + data->fields_obj = 0; + data->_reserved = 0; + data->data = datap; data->dmark = dmark; data->dfree = dfree; - data->data = datap; return obj; } @@ -3512,9 +3515,7 @@ rb_gc_mark_children(void *objspace, VALUE obj) bool typed_data = RTYPEDDATA_P(obj); void *const ptr = typed_data ? RTYPEDDATA_GET_DATA(obj) : DATA_PTR(obj); - if (typed_data) { - gc_mark_internal(RTYPEDDATA(obj)->fields_obj); - } + gc_mark_internal(RTYPEDDATA(obj)->fields_obj); if (ptr) { if (typed_data && gc_declarative_marking_p(RTYPEDDATA_TYPE(obj))) { @@ -4462,9 +4463,7 @@ rb_gc_update_object_references(void *objspace, VALUE obj) bool typed_data = RTYPEDDATA_P(obj); void *const ptr = typed_data ? RTYPEDDATA_GET_DATA(obj) : DATA_PTR(obj); - if (typed_data) { - UPDATE_IF_MOVED(objspace, RTYPEDDATA(obj)->fields_obj); - } + UPDATE_IF_MOVED(objspace, RTYPEDDATA(obj)->fields_obj); if (ptr) { if (typed_data && gc_declarative_marking_p(RTYPEDDATA_TYPE(obj))) { diff --git a/include/ruby/internal/abi.h b/include/ruby/internal/abi.h index e6d1fa7e8f..0c99d93bf9 100644 --- a/include/ruby/internal/abi.h +++ b/include/ruby/internal/abi.h @@ -24,7 +24,7 @@ * In released versions of Ruby, this number is not defined since teeny * versions of Ruby should guarantee ABI compatibility. */ -#define RUBY_ABI_VERSION 1 +#define RUBY_ABI_VERSION 2 /* Windows does not support weak symbols so ruby_abi_version will not exist * in the shared library. */ diff --git a/include/ruby/internal/core/rdata.h b/include/ruby/internal/core/rdata.h index cee5e7b5ea..5ebeb2473e 100644 --- a/include/ruby/internal/core/rdata.h +++ b/include/ruby/internal/core/rdata.h @@ -116,12 +116,24 @@ typedef void (*RUBY_DATA_FUNC)(void*); * @shyouhei tried to add RBIMPL_ATTR_DEPRECATED for this type but that yielded * too many warnings in the core. Maybe we want to retry later... Just add * deprecated document for now. + * + * RData shares its initial fields with struct ::RTypedData so the VM can handle + * per-object fields without checking whether a T_DATA object is typed or legacy. */ struct RData { /** Basic part, including flags and class. */ struct RBasic basic; + /** @internal Direct reference to the slots that hold instance variables, if any. */ + VALUE fields_obj; + + /** @internal Padding where ::RTypedData stores its type, so both structs place data at the same offset. */ + VALUE _reserved; + + /** Pointer to the actual C level struct that you want to wrap. */ + void *data; + /** * This function is called when the object is experiencing GC marks. If it * contains references to other Ruby objects, you need to mark them also. @@ -141,12 +153,6 @@ struct RData { * impossible at that moment (that is why GC runs). */ RUBY_DATA_FUNC dfree; - - /** Pointer to the actual C level struct that you want to wrap. - * This is after dmark and dfree to allow DATA_PTR to continue to work for - * both RData and non-embedded RTypedData. - */ - void *data; }; RBIMPL_SYMBOL_EXPORT_BEGIN() diff --git a/include/ruby/internal/core/rtypeddata.h b/include/ruby/internal/core/rtypeddata.h index 22bf46eb03..0b189c7ef8 100644 --- a/include/ruby/internal/core/rtypeddata.h +++ b/include/ruby/internal/core/rtypeddata.h @@ -405,6 +405,7 @@ struct RTypedData { }; #if !defined(__cplusplus) || __cplusplus >= 201103L +RBIMPL_STATIC_ASSERT(fields_obj_in_rdata, offsetof(struct RData, fields_obj) == offsetof(struct RTypedData, fields_obj)); RBIMPL_STATIC_ASSERT(data_in_rtypeddata, offsetof(struct RData, data) == offsetof(struct RTypedData, data)); #endif @@ -34,8 +34,8 @@ enum jit_bindgen_constants { // Field offset for prime classext's fields_obj from a class pointer RCLASS_OFFSET_PRIME_FIELDS_OBJ = offsetof(struct RClass_and_rb_classext_t, classext.fields_obj), - // Field offset for fields_obj in RTypedData - RTYPEDDATA_OFFSET_FIELDS_OBJ = offsetof(struct RTypedData, fields_obj), + // Field offset for fields_obj in T_DATA + TDATA_OFFSET_FIELDS_OBJ = offsetof(struct RTypedData, fields_obj), // Field offsets for the RString struct RUBY_OFFSET_RSTRING_LEN = offsetof(struct RString, len), @@ -568,7 +568,7 @@ rb_jit_class_fields_embedded_p(VALUE klass) } bool -rb_jit_typed_data_fields_embedded_p(VALUE obj) +rb_jit_data_fields_embedded_p(VALUE obj) { VALUE fields_obj = RTYPEDDATA(obj)->fields_obj; return !fields_obj || !FL_TEST_RAW(fields_obj, OBJ_FIELD_HEAP); diff --git a/lib/rubygems/package.rb b/lib/rubygems/package.rb index d36b6dfb5e..7e41b18f66 100644 --- a/lib/rubygems/package.rb +++ b/lib/rubygems/package.rb @@ -743,9 +743,11 @@ EOM if Gem.win_platform? # Create a symlink and fallback to copy the file or directory on Windows, # where symlink creation needs special privileges in form of the Developer Mode. + # JRuby on Windows raises TypeError from the wincode path-conversion helper + # when it cannot create the symlink, so fall back to copy in that case too. def create_symlink(old_name, new_name) File.symlink(old_name, new_name) - rescue Errno::EACCES + rescue Errno::EACCES, TypeError from = File.expand_path(old_name, File.dirname(new_name)) FileUtils.cp_r(from, new_name) end @@ -428,8 +428,7 @@ rb_obj_using_gen_fields_table_p(VALUE obj) { switch (BUILTIN_TYPE(obj)) { case T_DATA: - if (RTYPEDDATA_P(obj)) return false; - break; + return false; case T_STRUCT: if (!FL_TEST_RAW(obj, RSTRUCT_GEN_FIELDS)) return false; diff --git a/spec/bundler/realworld/fixtures/warbler/Gemfile b/spec/bundler/realworld/fixtures/warbler/Gemfile index 07dc301d03..5687bbd975 100644 --- a/spec/bundler/realworld/fixtures/warbler/Gemfile +++ b/spec/bundler/realworld/fixtures/warbler/Gemfile @@ -4,4 +4,4 @@ source "https://rubygems.org" gem "demo", path: "./demo" gem "jruby-jars", "~> 10.0" -gem "warbler", github: "https://github.com/jruby/warbler/pull/557" +gem "warbler", "~> 2.1" diff --git a/spec/bundler/realworld/fixtures/warbler/Gemfile.lock b/spec/bundler/realworld/fixtures/warbler/Gemfile.lock index 2f2deea994..05f3bc4e3f 100644 --- a/spec/bundler/realworld/fixtures/warbler/Gemfile.lock +++ b/spec/bundler/realworld/fixtures/warbler/Gemfile.lock @@ -1,15 +1,3 @@ -GIT - remote: https://github.com/jruby/warbler.git - revision: 3a3a89e9a055ab1badb4e6fee860e8617b4acfe1 - ref: refs/pull/557/head - specs: - warbler (2.0.5) - jruby-jars (>= 9.0.0) - jruby-rack (>= 1.1.1, < 1.3) - rake (>= 13.0.3) - rexml (~> 3.0) - rubyzip (>= 1.0.0) - PATH remote: demo specs: @@ -19,10 +7,18 @@ GEM remote: https://rubygems.org/ specs: jruby-jars (10.0.0.1) - jruby-rack (1.2.2) + jruby-rack (1.2.7) + ostruct (0.6.3) rake (13.3.0) rexml (3.4.2) - rubyzip (2.4.1) + rubyzip (3.3.0) + warbler (2.1.0) + jruby-jars (>= 9.4, < 10.1) + jruby-rack (>= 1.2.3, < 1.3) + ostruct (~> 0.6.2) + rake (~> 13.0, >= 13.0.3) + rexml (~> 3.0) + rubyzip (>= 3.0.0) PLATFORMS arm64-darwin @@ -33,7 +29,7 @@ PLATFORMS DEPENDENCIES demo! jruby-jars (~> 10.0) - warbler! + warbler (~> 2.1) BUNDLED WITH 4.1.0.dev diff --git a/test/rubygems/helper.rb b/test/rubygems/helper.rb index 9ddc79e6e8..2411dbc649 100644 --- a/test/rubygems/helper.rb +++ b/test/rubygems/helper.rb @@ -38,6 +38,7 @@ require "test/unit" require "fileutils" require "pathname" require "pp" +require "rubygems/installer" require "rubygems/package" require "shellwords" require "tmpdir" @@ -45,6 +46,24 @@ require "rubygems/vendor/uri/lib/uri" require "zlib" require_relative "mock_gem_ui" +# JRuby on Windows raises TypeError inside File.symlink (the wincode helper +# trips on a nil path), so any test that exercises Gem::Installer's symlink +# branch fails to even install the gem. Real users hit the wrapper branch via +# `gem install` (DependencyInstaller passes wrappers: true), so mirror that +# default for direct Gem::Installer.at callers in the test suite. +if Gem.win_platform? && Gem.java_platform? + module Gem::InstallerDefaultWrappersOnJRubyWindows + def at(path, options = {}) + super(path, { wrappers: true }.merge(options)) + end + + def for_spec(spec, options = {}) + super(spec, { wrappers: true }.merge(options)) + end + end + Gem::Installer.singleton_class.prepend(Gem::InstallerDefaultWrappersOnJRubyWindows) +end + module Gem ## # Allows setting the gem path searcher. @@ -1268,11 +1287,13 @@ Also, a list: if @@symlink_supported.nil? begin File.symlink(File.join(@tempdir, "a"), File.join(@tempdir, "b")) + File.readlink(File.join(@tempdir, "b")) rescue NotImplementedError, SystemCallError @@symlink_supported = false else - File.unlink(File.join(@tempdir, "b")) @@symlink_supported = true + ensure + File.unlink(File.join(@tempdir, "b")) if File.symlink?(File.join(@tempdir, "b")) end end @@symlink_supported diff --git a/test/rubygems/test_gem.rb b/test/rubygems/test_gem.rb index f3d9ef95d6..b9a4cf1ce0 100644 --- a/test/rubygems/test_gem.rb +++ b/test/rubygems/test_gem.rb @@ -150,6 +150,8 @@ class TestGem < Gem::TestCase end def assert_self_install_permissions(format_executable: false, data_mode: 0o640) + omit "FileUtils.install signature differs on JRuby/Windows" if Gem.win_platform? && Gem.java_platform? + mask = Gem.win_platform? ? 0o700 : 0o777 options = { dir_mode: 0o500, @@ -199,7 +201,8 @@ class TestGem < Gem::TestCase end assert_equal(expected, result) ensure - File.chmod(0o755, *Dir.glob(@gemhome + "/gems/**/")) + files = Dir.glob(@gemhome + "/gems/**/") + File.chmod(0o755, *files) unless files.empty? end def test_require_missing diff --git a/test/rubygems/test_gem_commands_open_command.rb b/test/rubygems/test_gem_commands_open_command.rb index d9e518048c..addc7427e2 100644 --- a/test/rubygems/test_gem_commands_open_command.rb +++ b/test/rubygems/test_gem_commands_open_command.rb @@ -21,6 +21,8 @@ class TestGemCommandsOpenCommand < Gem::TestCase end def test_execute + omit "JRuby on Windows spawns the editor with a different cwd" if Gem.win_platform? && Gem.java_platform? + @cmd.options[:args] = %w[foo] @cmd.options[:editor] = (ruby_with_rubygems_in_load_path + ["-e", "puts(ARGV,Dir.pwd)", "--"]).join(" ") diff --git a/test/rubygems/test_gem_installer.rb b/test/rubygems/test_gem_installer.rb index 4cdc9479f7..bf7a4a8dfc 100644 --- a/test/rubygems/test_gem_installer.rb +++ b/test/rubygems/test_gem_installer.rb @@ -689,8 +689,11 @@ class TestGemInstaller < Gem::InstallerTestCase def test_generate_bin_symlink_win32 old_win_platform = Gem.win_platform? - Gem.win_platform = true old_alt_separator = File::ALT_SEPARATOR + + omit "JRuby on Windows still creates the symlink so the wrapper branch is not exercised" if Gem.win_platform? && Gem.java_platform? + + Gem.win_platform = true File.__send__(:remove_const, :ALT_SEPARATOR) File.const_set(:ALT_SEPARATOR, "\\") @@ -743,6 +746,8 @@ class TestGemInstaller < Gem::InstallerTestCase end def test_generate_bin_with_dangling_symlink + omit "JRuby on Windows still creates the symlink so the wrapper branch is not exercised" if Gem.win_platform? && Gem.java_platform? + gem_with_dangling_symlink = File.expand_path("packages/ascii_binder-0.1.10.1.gem", __dir__) installer = Gem::Installer.at( diff --git a/test/rubygems/test_gem_path_support.rb b/test/rubygems/test_gem_path_support.rb index 8720bcf858..c5181496c0 100644 --- a/test/rubygems/test_gem_path_support.rb +++ b/test/rubygems/test_gem_path_support.rb @@ -121,14 +121,12 @@ class TestGemPathSupport < Gem::TestCase end def test_gem_paths_do_not_contain_symlinks + pend "symlinks not supported" unless symlink_supported? + dir = "#{@tempdir}/realgemdir" symlink = "#{@tempdir}/symdir" Dir.mkdir dir - begin - File.symlink(dir, symlink) - rescue NotImplementedError, SystemCallError - pend "symlinks not supported" - end + File.symlink(dir, symlink) not_existing = "#{@tempdir}/does_not_exist" path = "#{symlink}#{File::PATH_SEPARATOR}#{not_existing}" diff --git a/test/rubygems/test_gem_safe_marshal.rb b/test/rubygems/test_gem_safe_marshal.rb index bd15f4f796..7e3a046c4e 100644 --- a/test/rubygems/test_gem_safe_marshal.rb +++ b/test/rubygems/test_gem_safe_marshal.rb @@ -252,6 +252,8 @@ class TestGemSafeMarshal < Gem::TestCase end def test_hash_with_compare_by_identity + pend "Marshal.dump of a compare_by_identity Hash emits an unexpected ivar on jruby" if RUBY_ENGINE == "jruby" + with_const(Gem::SafeMarshal, :PERMITTED_CLASSES, %w[Hash]) do assert_safe_load_as Hash.new.compare_by_identity.tap {|h| h[+"a"] = 1 diff --git a/test/rubygems/test_gem_source_git.rb b/test/rubygems/test_gem_source_git.rb index fef79a0743..b7b2c52f9a 100644 --- a/test/rubygems/test_gem_source_git.rb +++ b/test/rubygems/test_gem_source_git.rb @@ -65,6 +65,8 @@ class TestGemSourceGit < Gem::TestCase end def test_checkout_submodules + omit "JRuby on Windows hits git submodule path differences" if Gem.win_platform? && Gem.java_platform? + # We need to allow to checkout submodules with file:// protocol # CVE-2022-39253 # https://lore.kernel.org/lkml/xmqq4jw1uku5.fsf@gitster.g/ diff --git a/test/rubygems/test_gem_specification.rb b/test/rubygems/test_gem_specification.rb index 52ae977313..79be0c996d 100644 --- a/test/rubygems/test_gem_specification.rb +++ b/test/rubygems/test_gem_specification.rb @@ -817,7 +817,7 @@ dependencies: [] write_file full_path do |io| io.write @a2.to_ruby_for_cache end - rescue Errno::EINVAL + rescue Errno::EINVAL, Errno::EACCES pend "cannot create '#{full_path}' on this platform" end @@ -836,7 +836,7 @@ dependencies: [] write_file full_path do |io| io.write @a2.to_ruby_for_cache end - rescue Errno::EINVAL + rescue Errno::EINVAL, Errno::EACCES pend "cannot create '#{full_path}' on this platform" end @@ -855,7 +855,7 @@ dependencies: [] write_file full_path do |io| io.write @a2.to_ruby_for_cache end - rescue Errno::EINVAL + rescue Errno::EINVAL, Errno::EACCES pend "cannot create '#{full_path}' on this platform" end diff --git a/test/rubygems/test_project_sanity.rb b/test/rubygems/test_project_sanity.rb index 8f23b2d8c0..3b08d1ec7b 100644 --- a/test/rubygems/test_project_sanity.rb +++ b/test/rubygems/test_project_sanity.rb @@ -12,6 +12,7 @@ class TestGemProjectSanity < Gem::TestCase def test_manifest_is_up_to_date pend unless File.exist?("#{root}/Rakefile") + omit "JRuby on Windows cannot exec the bin/rake shebang" if Gem.win_platform? && Gem.java_platform? rake = "#{root}/bin/rake" _, status = Open3.capture2e(rake, "check_manifest") @@ -37,6 +38,8 @@ class TestGemProjectSanity < Gem::TestCase end def test_require_rubygems_package + omit "JRuby on Windows fails to spawn ruby --disable-gems here" if Gem.win_platform? && Gem.java_platform? + err, status = Open3.capture2e(*ruby_with_rubygems_in_load_path, "--disable-gems", "-e", "require \"rubygems/package\"") assert status.success?, err diff --git a/test/rubygems/test_require.rb b/test/rubygems/test_require.rb index e5f9d7bed2..db86a30905 100644 --- a/test/rubygems/test_require.rb +++ b/test/rubygems/test_require.rb @@ -476,6 +476,7 @@ class TestGemRequire < Gem::TestCase def test_realworld_default_gem omit "this test can't work under ruby-core setup" if ruby_repo? + omit "JRuby on Windows does not register json as a default gem the same way" if Gem.win_platform? && Gem.java_platform? cmd = <<-RUBY $stderr = $stdout @@ -786,6 +787,8 @@ class TestGemRequire < Gem::TestCase end def test_require_does_not_crash_when_utilizing_bundler_version_finder + omit "JRuby on Windows hits a different require path" if Gem.win_platform? && Gem.java_platform? + a1 = util_spec "a", "1.1", { "bundler" => ">= 0" } a2 = util_spec "a", "1.2", { "bundler" => ">= 0" } b1 = util_spec "bundler", "2.3.7" diff --git a/test/rubygems/test_rubygems.rb b/test/rubygems/test_rubygems.rb index ec195b65cd..6566b5981e 100644 --- a/test/rubygems/test_rubygems.rb +++ b/test/rubygems/test_rubygems.rb @@ -10,6 +10,7 @@ class GemTest < Gem::TestCase def test_operating_system_other_exceptions pend "does not apply to truffleruby" if RUBY_ENGINE == "truffleruby" + omit "JRuby on Windows loads a different operating_system defaults file" if Gem.win_platform? && Gem.java_platform? path = util_install_operating_system_rb <<-RUBY intentionally_not_implemented_method diff --git a/variable.c b/variable.c index 4d2988a603..857d870413 100644 --- a/variable.c +++ b/variable.c @@ -1284,11 +1284,8 @@ rb_obj_fields(VALUE obj, ID field_name) if (rb_obj_shape_has_fields(obj)) { switch (BUILTIN_TYPE(obj)) { case T_DATA: - if (LIKELY(RTYPEDDATA_P(obj))) { - fields_obj = RTYPEDDATA(obj)->fields_obj; - break; - } - goto generic_fields; + fields_obj = RTYPEDDATA(obj)->fields_obj; + break; case T_STRUCT: if (LIKELY(!FL_TEST_RAW(obj, RSTRUCT_GEN_FIELDS))) { fields_obj = RSTRUCT_FIELDS_OBJ(obj); @@ -1321,11 +1318,8 @@ rb_free_generic_ivar(VALUE obj) st_data_t key = (st_data_t)obj, value; switch (BUILTIN_TYPE(obj)) { case T_DATA: - if (LIKELY(RTYPEDDATA_P(obj))) { - RB_OBJ_WRITE(obj, &RTYPEDDATA(obj)->fields_obj, 0); - break; - } - goto generic_fields; + RB_OBJ_WRITE(obj, &RTYPEDDATA(obj)->fields_obj, 0); + break; case T_STRUCT: if (LIKELY(!FL_TEST_RAW(obj, RSTRUCT_GEN_FIELDS))) { RSTRUCT_SET_FIELDS_OBJ(obj, 0); @@ -1371,11 +1365,8 @@ rb_obj_set_fields(VALUE obj, VALUE fields_obj, ID field_name, VALUE original_fie if (fields_obj != original_fields_obj) { switch (BUILTIN_TYPE(obj)) { case T_DATA: - if (LIKELY(RTYPEDDATA_P(obj))) { - RB_OBJ_WRITE(obj, &RTYPEDDATA(obj)->fields_obj, fields_obj); - break; - } - goto generic_fields; + RB_OBJ_WRITE(obj, &RTYPEDDATA(obj)->fields_obj, fields_obj); + break; case T_STRUCT: if (LIKELY(!FL_TEST_RAW(obj, RSTRUCT_GEN_FIELDS))) { RSTRUCT_SET_FIELDS_OBJ(obj, fields_obj); diff --git a/yjit/src/cruby_bindings.inc.rs b/yjit/src/cruby_bindings.inc.rs index ef7f6c0a9e..ae372711d7 100644 --- a/yjit/src/cruby_bindings.inc.rs +++ b/yjit/src/cruby_bindings.inc.rs @@ -986,7 +986,7 @@ pub type rb_seq_param_keyword_struct = pub const ROBJECT_OFFSET_AS_HEAP_FIELDS: jit_bindgen_constants = 16; pub const ROBJECT_OFFSET_AS_ARY: jit_bindgen_constants = 16; pub const RCLASS_OFFSET_PRIME_FIELDS_OBJ: jit_bindgen_constants = 40; -pub const RTYPEDDATA_OFFSET_FIELDS_OBJ: jit_bindgen_constants = 16; +pub const TDATA_OFFSET_FIELDS_OBJ: jit_bindgen_constants = 16; pub const RUBY_OFFSET_RSTRING_LEN: jit_bindgen_constants = 16; pub const RB_SHAPE_FLAG_SHIFT: jit_bindgen_constants = 32; pub const RUBY_OFFSET_EC_CFP: jit_bindgen_constants = 16; diff --git a/zjit/bindgen/src/main.rs b/zjit/bindgen/src/main.rs index 32b75117c8..59daf0b92f 100644 --- a/zjit/bindgen/src/main.rs +++ b/zjit/bindgen/src/main.rs @@ -321,8 +321,7 @@ fn main() { .allowlist_function("rb_jit_shape_complex_p") .allowlist_function("rb_jit_multi_ractor_p") .allowlist_function("rb_jit_class_fields_embedded_p") - .allowlist_function("rb_jit_typed_data_p") - .allowlist_function("rb_jit_typed_data_fields_embedded_p") + .allowlist_function("rb_jit_data_fields_embedded_p") .allowlist_function("rb_jit_vm_lock_then_barrier") .allowlist_function("rb_jit_vm_unlock") .allowlist_function("rb_jit_for_each_iseq") diff --git a/zjit/src/backend/lir.rs b/zjit/src/backend/lir.rs index b29f832000..25fa0cc151 100644 --- a/zjit/src/backend/lir.rs +++ b/zjit/src/backend/lir.rs @@ -203,7 +203,7 @@ pub use crate::backend::current::{ mem_base_reg, Reg, EC, CFP, SP, - NATIVE_STACK_PTR, NATIVE_BASE_PTR, + NATIVE_BASE_PTR, C_ARG_OPNDS, C_RET_OPND, }; @@ -1406,10 +1406,9 @@ pub struct Assembler { /// On `compile`, it also disables the backend's use of them. pub(super) accept_scratch_reg: bool, - /// The Assembler can use NATIVE_BASE_PTR + stack_base_idx as the - /// first stack slot in case it needs to allocate memory. This is - /// equal to the number of spilled basic block arguments. - pub(super) stack_base_idx: usize, + /// The maximum number of stack slots that have been reserved + /// by Assembler::alloc_stack(). + pub stack_base_idx: usize, /// If Some, the next ccall should verify its leafness leaf_ccall_stack_size: Option<usize>, @@ -3603,6 +3602,7 @@ pub(crate) use asm_ccall; mod tests { use super::*; use insta::assert_snapshot; + use crate::backend::current::NATIVE_STACK_PTR; fn scratch_reg() -> Opnd { Assembler::new_with_scratch_reg().1 diff --git a/zjit/src/codegen.rs b/zjit/src/codegen.rs index fbf62a5c81..7c893a72fe 100644 --- a/zjit/src/codegen.rs +++ b/zjit/src/codegen.rs @@ -19,13 +19,17 @@ use crate::state::ZJITState; use crate::stats::{CompileError, exit_counter_for_compile_error, exit_counter_for_unhandled_hir_insn, incr_counter, incr_counter_by, send_fallback_counter, send_fallback_counter_for_method_type, send_fallback_counter_for_super_method_type, send_fallback_counter_ptr_for_opcode, send_without_block_fallback_counter_for_method_type, send_without_block_fallback_counter_for_optimized_method_type}; use crate::stats::{counter_ptr, with_time_stat, trace_compile_phase, Counter, Counter::{compile_time_ns, exit_compile_error}}; use crate::{asm::CodeBlock, cruby::*, options::debug, virtualmem::CodePtr}; -use crate::backend::lir::{self, Assembler, C_ARG_OPNDS, C_RET_OPND, CFP, EC, NATIVE_BASE_PTR, NATIVE_STACK_PTR, Opnd, SP, SideExit, SideExitRecompile, Target, asm_ccall, asm_comment}; +use crate::backend::lir::{self, Assembler, C_ARG_OPNDS, C_RET_OPND, CFP, EC, NATIVE_BASE_PTR, Opnd, SP, SideExit, SideExitRecompile, Target, asm_ccall, asm_comment}; use crate::hir::{iseq_to_hir, BlockId, Invariant, RangeType, SideExitReason::{self, *}, SpecialBackrefSymbol, SpecialObjectType}; use crate::hir::{BlockHandler, Const, FieldName, FrameState, Function, Insn, InsnId, Recompile, SendFallbackReason}; use crate::hir_type::{types, Type}; use crate::options::{get_option, PerfMap}; use crate::cast::IntoUsize; +/// The number of stack slots used for JITFrame. gen_save_pc_for_gc() writes +/// JITFrame into this number of slots at the bottom of the native stack. +const JIT_FRAME_SIZE: usize = 1; + /// Default maximum number of compiled versions per ISEQ. const DEFAULT_MAX_VERSIONS: usize = 2; @@ -66,17 +70,22 @@ struct JITState { /// ISEQ calls that need to be compiled later iseq_calls: Vec<IseqCallRef>, + + /// The number of native stack slots reserved for JITFrame. + /// gen_save_pc_for_gc() writes JITFrame into the allocated space. + jit_frame_size: usize, } impl JITState { /// Create a new JITState instance - fn new(version: IseqVersionRef, num_insns: usize, num_blocks: usize) -> Self { + fn new(version: IseqVersionRef, num_insns: usize, num_blocks: usize, jit_frame_size: usize) -> Self { JITState { version, opnds: vec![None; num_insns], labels: vec![None; num_blocks], jit_entries: Vec::default(), iseq_calls: Vec::default(), + jit_frame_size, } } @@ -374,9 +383,8 @@ fn gen_iseq_body(cb: &mut CodeBlock, iseq: IseqPtr, mut version: IseqVersionRef, /// Compile a function fn gen_function(cb: &mut CodeBlock, iseq: IseqPtr, version: IseqVersionRef, function: &Function) -> Result<(IseqCodePtrs, Vec<CodePtr>, Vec<IseqCallRef>), CompileError> { let (mut jit, asm) = trace_compile_phase("codegen", || { - let num_spilled_params = max_num_params(function).saturating_sub(ALLOC_REGS.len()); - let mut jit = JITState::new(version, function.num_insns(), function.num_blocks()); - let mut asm = Assembler::new_with_stack_slots(num_spilled_params + 1); // +1 for JITFrame + let mut jit = JITState::new(version, function.num_insns(), function.num_blocks(), JIT_FRAME_SIZE); + let mut asm = Assembler::new_with_stack_slots(JIT_FRAME_SIZE); // Mapping from HIR block IDs to LIR block IDs. // This is is a one-to-one mapping from HIR to LIR blocks used for finding @@ -583,7 +591,7 @@ fn gen_insn(cb: &mut CodeBlock, jit: &mut JITState, asm: &mut Assembler, functio gen_const_uint32(val.0) } Insn::Const { .. } => panic!("Unexpected Const in gen_insn: {insn}"), - Insn::NewArray { elements, state } => gen_new_array(asm, opnds!(elements), &function.frame_state(*state)), + Insn::NewArray { elements, state } => gen_new_array(jit, asm, opnds!(elements), &function.frame_state(*state)), Insn::NewHash { elements, state } => gen_new_hash(jit, asm, opnds!(elements), &function.frame_state(*state)), Insn::NewRange { low, high, flag, state } => gen_new_range(jit, asm, opnd!(low), opnd!(high), *flag, &function.frame_state(*state)), Insn::NewRangeFixnum { low, high, flag, state } => gen_new_range_fixnum(asm, opnd!(low), opnd!(high), *flag, &function.frame_state(*state)), @@ -1139,10 +1147,9 @@ fn gen_ccall_variadic( asm.mov(CFP, new_cfp); asm.store(Opnd::mem(64, EC, RUBY_OFFSET_EC_CFP as i32), CFP); - let argv_ptr = gen_push_opnds(asm, &args); + let argv_ptr = gen_push_opnds(jit, asm, &args); asm.count_call_to(&name.contents_lossy()); let result = asm.ccall(cfunc, vec![args.len().into(), argv_ptr, recv]); - gen_pop_opnds(asm, &args); asm_comment!(asm, "pop C frame"); let new_cfp = asm.add(CFP, RUBY_SIZEOF_CONTROL_FRAME.into()); @@ -1699,7 +1706,7 @@ fn gen_invokeblock_ifunc( gen_prepare_non_leaf_call(jit, asm, state); // Push args to memory so we can pass argv pointer - let argv_ptr = gen_push_opnds(asm, &args); + let argv_ptr = gen_push_opnds(jit, asm, &args); // Untag the block handler to get the captured block pointer // captured = block_handler & ~0x3 @@ -1715,9 +1722,7 @@ fn gen_invokeblock_ifunc( argv: *const VALUE, ) -> VALUE; } - let result = asm_ccall!(asm, rb_vm_yield_with_cfunc, EC, captured, (args.len() as i64).into(), argv_ptr); - gen_pop_opnds(asm, &args); - result + asm_ccall!(asm, rb_vm_yield_with_cfunc, EC, captured, (args.len() as i64).into(), argv_ptr) } fn gen_invokeproc( @@ -1732,9 +1737,9 @@ fn gen_invokeproc( asm_comment!(asm, "call invokeproc"); - let argv_ptr = gen_push_opnds(asm, &args); + let argv_ptr = gen_push_opnds(jit, asm, &args); let kw_splat_opnd = Opnd::Imm(i64::from(kw_splat)); - let result = asm_ccall!( + asm_ccall!( asm, rb_optimized_call, recv, @@ -1743,10 +1748,7 @@ fn gen_invokeproc( argv_ptr, kw_splat_opnd, VM_BLOCK_HANDLER_NONE.into() - ); - gen_pop_opnds(asm, &args); - - result + ) } /// Compile a dynamic dispatch for `super` @@ -1820,6 +1822,7 @@ fn gen_array_dup( /// Compile a new array instruction fn gen_new_array( + jit: &JITState, asm: &mut Assembler, elements: Vec<Opnd>, state: &FrameState, @@ -1831,10 +1834,8 @@ fn gen_new_array( if elements.is_empty() { asm_ccall!(asm, rb_ec_ary_new_from_values, EC, 0i64.into(), Opnd::UImm(0)) } else { - let argv = gen_push_opnds(asm, &elements); - let new_array = asm_ccall!(asm, rb_ec_ary_new_from_values, EC, num.into(), argv); - gen_pop_opnds(asm, &elements); - new_array + let argv = gen_push_opnds(jit, asm, &elements); + asm_ccall!(asm, rb_ec_ary_new_from_values, EC, num.into(), argv) } } @@ -2117,10 +2118,8 @@ fn gen_new_hash( } else { gen_prepare_non_leaf_call(jit, asm, state); - let argv = gen_push_opnds(asm, &elements); - let hash = asm_ccall!(asm, rb_hash_new_with_bulk_insert, elements.len().into(), argv); - gen_pop_opnds(asm, &elements); - hash + let argv = gen_push_opnds(jit, asm, &elements); + asm_ccall!(asm, rb_hash_new_with_bulk_insert, elements.len().into(), argv) } } @@ -2552,7 +2551,7 @@ fn gen_guard_type(jit: &mut JITState, asm: &mut Assembler, val: lir::Opnd, guard asm.cmp(klass, Opnd::Value(expected_class)); asm.jne(jit, side_exit); - } else if guard_type.is_subtype(types::TypedTData) { + } else if guard_type.is_subtype(types::TData) { let side = side_exit(jit, state, GuardType(guard_type)); // Check special constant @@ -2563,11 +2562,11 @@ fn gen_guard_type(jit: &mut JITState, asm: &mut Assembler, val: lir::Opnd, guard asm.cmp(val, Qfalse.into()); asm.je(jit, side.clone()); - // Check the builtin type and RUBY_TYPED_FL_IS_TYPED_DATA with mask and compare + // Check the T_DATA builtin type. let val = asm.load_mem(val); let flags = asm.load(Opnd::mem(VALUE_BITS, val, RUBY_OFFSET_RBASIC_FLAGS)); - let mask = RUBY_T_MASK.to_usize() | RUBY_TYPED_FL_IS_TYPED_DATA.to_usize(); - let expected = RUBY_T_DATA.to_usize() | RUBY_TYPED_FL_IS_TYPED_DATA.to_usize(); + let mask = RUBY_T_MASK.to_usize(); + let expected = RUBY_T_DATA.to_usize(); let masked = asm.and(flags, mask.into()); asm.cmp(masked, expected.into()); asm.jne(jit, side); @@ -2986,20 +2985,6 @@ fn build_side_exit(jit: &JITState, state: &FrameState) -> SideExit { } } -/// Returne the maximum number of arguments for a block in a given function -fn max_num_params(function: &Function) -> usize { - let reverse_post_order = function.reverse_post_order(); - reverse_post_order - .iter() - .filter(|&&block_id| function.is_entry_block(block_id)) - .map(|&block_id| { - let block = function.block(block_id); - block.params().len() - }) - .max() - .unwrap_or(0) -} - #[cfg(target_arch = "x86_64")] macro_rules! c_callable { ($(#[$outer:meta])* @@ -3362,21 +3347,18 @@ pub fn gen_exit_trampoline_with_counter(cb: &mut CodeBlock, exit_trampoline: Cod }) } -fn gen_push_opnds(asm: &mut Assembler, opnds: &[Opnd]) -> lir::Opnd { - let n = opnds.len(); - let allocation_size = aligned_stack_bytes(n); - - // Bump the stack pointer to reserve the space for opnds - if n != 0 { - asm_comment!(asm, "allocate {} bytes on C stack for {} values", allocation_size, n); - asm.sub_into(NATIVE_STACK_PTR, allocation_size.into()); +/// Reserve native stack space and write operands into it. +fn gen_push_opnds(jit: &JITState, asm: &mut Assembler, opnds: &[Opnd]) -> lir::Opnd { + let argv = if opnds.len() > 0 { + // Make sure the Assembler will reserve a sufficient stack size for given opnds + asm_comment!(asm, "allocate space on C stack for {} values", opnds.len()); + asm.alloc_stack(jit, opnds.len()) } else { asm_comment!(asm, "no opnds to allocate"); - } + Opnd::UImm(0) + }; - // Load NATIVE_STACK_PTR to get the address of a returned array - // to allow the backend to move it for its own use. - let argv = asm.load(NATIVE_STACK_PTR); + // Write operands into stack slots allocated by asm.alloc_stack() for (idx, &opnd) in opnds.iter().enumerate() { asm.mov(Opnd::mem(VALUE_BITS, argv, idx as i32 * SIZEOF_VALUE_I32), opnd); } @@ -3384,35 +3366,18 @@ fn gen_push_opnds(asm: &mut Assembler, opnds: &[Opnd]) -> lir::Opnd { argv } -fn gen_pop_opnds(asm: &mut Assembler, opnds: &[Opnd]) { - if opnds.is_empty() { - asm_comment!(asm, "no opnds to restore"); - return - } - - asm_comment!(asm, "restore C stack pointer"); - let allocation_size = aligned_stack_bytes(opnds.len()); - asm.add_into(NATIVE_STACK_PTR, allocation_size.into()); -} - fn gen_toregexp(jit: &mut JITState, asm: &mut Assembler, opt: usize, values: Vec<Opnd>, state: &FrameState) -> Opnd { gen_prepare_non_leaf_call(jit, asm, state); - let first_opnd_ptr = gen_push_opnds(asm, &values); - let result = asm_ccall!(asm, rb_reg_new_from_values, values.len().into(), first_opnd_ptr, opt.into()); - gen_pop_opnds(asm, &values); - - result + let first_opnd_ptr = gen_push_opnds(jit, asm, &values); + asm_ccall!(asm, rb_reg_new_from_values, values.len().into(), first_opnd_ptr, opt.into()) } fn gen_string_concat(jit: &mut JITState, asm: &mut Assembler, strings: Vec<Opnd>, state: &FrameState) -> Opnd { gen_prepare_non_leaf_call(jit, asm, state); - let first_string_ptr = gen_push_opnds(asm, &strings); - let result = asm_ccall!(asm, rb_str_concat_literals, strings.len().into(), first_string_ptr); - gen_pop_opnds(asm, &strings); - - result + let first_string_ptr = gen_push_opnds(jit, asm, &strings); + asm_ccall!(asm, rb_str_concat_literals, strings.len().into(), first_string_ptr) } // Generate RSTRING_PTR @@ -3484,6 +3449,33 @@ fn aligned_stack_bytes(num_slots: usize) -> usize { } impl Assembler { + /// Allocate stack space on top of the stack slots reserved for JITFrame, + /// and return a pointer to the allocated space. + fn alloc_stack(&mut self, jit: &JITState, stack_size: usize) -> Opnd { + let total_stack_size = jit.jit_frame_size + stack_size; + self.stack_base_idx = self.stack_base_idx.max(total_stack_size); + // high addr + // +------------------------+ + // | return address | + // +------------------------+ + // | previous frame pointer | <- NATIVE_BASE_PTR == cfp->jit_return + // +------------------------+ + // | JITFrame pointer | <- jit.jit_frame_size, read by CFP_ZJIT_FRAME(cfp) + // +------------------------+ + // | opnds.last() | + // +------------------------+ + // | ... | + // +------------------------+ + // | opnds.first() | <- pointer returned by alloc_stack() + // +------------------------+ + // | register spill slots | if any + // +------------------------+ + // | FrameSetup align slot | if needed + // +------------------------+ + // low addr + self.sub(NATIVE_BASE_PTR, (SIZEOF_VALUE * total_stack_size).into()) + } + /// Emits a load for memory based operands and returns a vreg, /// otherwise returns recv. fn load_mem(&mut self, recv: Opnd) -> Opnd { diff --git a/zjit/src/codegen_tests.rs b/zjit/src/codegen_tests.rs index fd6367b7a3..b099371718 100644 --- a/zjit/src/codegen_tests.rs +++ b/zjit/src/codegen_tests.rs @@ -26,6 +26,7 @@ fn test_breakpoint_hir_codegen() { IseqVersion::new(iseq), function.num_insns(), function.num_blocks(), + 0, ); let mut asm = Assembler::new(); asm.new_block_without_id("test"); diff --git a/zjit/src/cruby.rs b/zjit/src/cruby.rs index 14db7c57d7..eb3241b0a2 100644 --- a/zjit/src/cruby.rs +++ b/zjit/src/cruby.rs @@ -608,15 +608,13 @@ impl VALUE { unsafe { rb_jit_class_fields_embedded_p(self) } } - /// Typed `T_DATA` made from `TypedData_Make_Struct()` (e.g. Thread, ARGF) - pub fn typed_data_p(self) -> bool { + pub fn data_p(self) -> bool { !self.special_const_p() && - self.builtin_type() == RUBY_T_DATA && - 0 != (self.builtin_flags() & RUBY_TYPED_FL_IS_TYPED_DATA.to_usize()) + self.builtin_type() == RUBY_T_DATA } - pub fn typed_data_fields_embedded_p(self) -> bool { - unsafe { rb_jit_typed_data_fields_embedded_p(self) } + pub fn data_fields_embedded_p(self) -> bool { + unsafe { rb_jit_data_fields_embedded_p(self) } } pub fn as_fixnum(self) -> i64 { diff --git a/zjit/src/cruby_bindings.inc.rs b/zjit/src/cruby_bindings.inc.rs index 21a2f6bbb4..dad29087be 100644 --- a/zjit/src/cruby_bindings.inc.rs +++ b/zjit/src/cruby_bindings.inc.rs @@ -1929,7 +1929,7 @@ pub type zjit_struct_offsets = u32; pub const ROBJECT_OFFSET_AS_HEAP_FIELDS: jit_bindgen_constants = 16; pub const ROBJECT_OFFSET_AS_ARY: jit_bindgen_constants = 16; pub const RCLASS_OFFSET_PRIME_FIELDS_OBJ: jit_bindgen_constants = 40; -pub const RTYPEDDATA_OFFSET_FIELDS_OBJ: jit_bindgen_constants = 16; +pub const TDATA_OFFSET_FIELDS_OBJ: jit_bindgen_constants = 16; pub const RUBY_OFFSET_RSTRING_LEN: jit_bindgen_constants = 16; pub const RB_SHAPE_FLAG_SHIFT: jit_bindgen_constants = 32; pub const RUBY_OFFSET_EC_CFP: jit_bindgen_constants = 16; @@ -2294,7 +2294,7 @@ unsafe extern "C" { pub fn rb_jit_shape_complex_p(shape_id: shape_id_t) -> bool; pub fn rb_jit_multi_ractor_p() -> bool; pub fn rb_jit_class_fields_embedded_p(klass: VALUE) -> bool; - pub fn rb_jit_typed_data_fields_embedded_p(obj: VALUE) -> bool; + pub fn rb_jit_data_fields_embedded_p(obj: VALUE) -> bool; pub fn rb_jit_vm_lock_then_barrier( recursive_lock_level: *mut ::std::os::raw::c_uint, file: *const ::std::os::raw::c_char, diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index 02ef9e0a1f..86078f4ac8 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -4147,7 +4147,6 @@ impl Function { if let Some(replacement) = (props.inline)(self, tmp_block, recv, &args, state) { // Copy contents of tmp_block to block assert_ne!(block, tmp_block); - emit_super_call_guards(self, block, super_cme, current_cme, mid, state); let insns = std::mem::take(&mut self.blocks[tmp_block.0].insns); self.blocks[block.0].insns.extend(insns); self.count(block, Counter::inline_cfunc_optimized_send_count); @@ -4336,8 +4335,8 @@ impl Function { self.push_insn(block, Insn::GuardType { val: recv, guard_type: types::Class, state }) } else if recv_type.flags().is_t_module() { self.push_insn(block, Insn::GuardType { val: recv, guard_type: types::Module, state }) - } else if recv_type.flags().is_typed_data() { - self.push_insn(block, Insn::GuardType { val: recv, guard_type: types::TypedTData, state }) + } else if recv_type.flags().is_t_data() { + self.push_insn(block, Insn::GuardType { val: recv, guard_type: types::TData, state }) } else { // HeapBasicObject is wider than T_OBJECT, but shapes for T_OBJECTs are in a pool of // its own and are guaranteed to be different from shapes of any other T_* types. So @@ -4373,11 +4372,10 @@ impl Function { }); return self.load_ivar_from_fields(block, fields_obj, recv_type.flags().is_fields_embedded(), id, ivar_index); } - if recv_type.flags().is_typed_data() { - // Typed T_DATA: load from fields_obj at fixed offset in RTypedData + if recv_type.flags().is_t_data() { let fields_obj = self.push_insn(block, Insn::LoadField { recv: self_val, id: FieldName::fields_obj, - offset: RTYPEDDATA_OFFSET_FIELDS_OBJ as i32, + offset: TDATA_OFFSET_FIELDS_OBJ as i32, return_type: types::RubyValue, }); return self.load_ivar_from_fields(block, fields_obj, recv_type.flags().is_fields_embedded(), id, ivar_index); @@ -5318,6 +5316,14 @@ impl Function { Insn::Test { val } if self.type_of(val).is_known_truthy() => { self.new_insn(Insn::Const { val: Const::CBool(true) }) } + Insn::Test { val: test_val } => { + if let Insn::BoxBool { val: bool_val } = self.find(test_val) { + self.make_equal_to(insn_id, bool_val); + continue; + } else { + insn_id + } + } Insn::CondBranch { val, if_true, .. } if self.is_a(val, Type::from_cbool(true)) => { self.new_insn(Insn::Jump(if_true)) } diff --git a/zjit/src/hir/opt_tests.rs b/zjit/src/hir/opt_tests.rs index 4a83b40e38..74c0af710e 100644 --- a/zjit/src/hir/opt_tests.rs +++ b/zjit/src/hir/opt_tests.rs @@ -7912,8 +7912,8 @@ mod hir_opt_tests { } #[test] - fn test_optimize_getivar_on_typed_data() { - // Thread is typed T_DATA: uses LoadField chain via RTypedData fields_obj + fn test_optimize_getivar_on_t_data() { + // T_DATA uses fields_obj for instance variables. eval(" class C < Thread def test = @a @@ -7936,7 +7936,7 @@ mod hir_opt_tests { Jump bb3(v4) bb3(v6:BasicObject): PatchPoint SingleRactorMode - v17:TypedTData = GuardType v6, TypedTData + v17:TData = GuardType v6, TData v18:CShape = LoadField v17, :shape_id@0x1000 v19:CShape[0x1001] = GuardBitEquals v18, CShape(0x1001) recompile v20:RubyValue = LoadField v17, :fields_obj@0x1002 @@ -7947,8 +7947,8 @@ mod hir_opt_tests { } #[test] - fn test_optimize_getivar_on_typed_data_complex_fields() { - // Typed T_DATA with enough ivars to force heap field storage + fn test_optimize_getivar_on_t_data_complex_fields() { + // T_DATA with enough ivars to force heap field storage eval(" class C < Thread def test = @var1000 @@ -8260,7 +8260,7 @@ mod hir_opt_tests { } #[test] - fn test_getivar_polymorphic_t_class_and_typed_data() { + fn test_getivar_polymorphic_t_class_and_t_data() { set_call_threshold(3); eval(r#" module Reader @@ -8295,7 +8295,7 @@ mod hir_opt_tests { PatchPoint SingleRactorMode v11:HeapBasicObject = GuardType v6, HeapBasicObject v12:CUInt64 = LoadField v11, :RBASIC_FLAGS@0x1000 - v14:CUInt64[0xffffffff0000005f] = Const CUInt64(0xffffffff0000005f) + v14:CUInt64[0xffffffff0000001f] = Const CUInt64(0xffffffff0000001f) v15:CPtr[CPtr(0x1001)] = Const CPtr(0x1001) v16 = RefineType v15, CUInt64 v17:CInt64 = IntAnd v12, v14 @@ -10010,11 +10010,6 @@ mod hir_opt_tests { v20:CallableMethodEntry[VALUE(0x1040)] = GuardBitEquals v19, Value(VALUE(0x1040)) v21:RubyValue = LoadField v18, :VM_ENV_DATA_INDEX_SPECVAL@0x1048 v22:FalseClass = GuardBitEquals v21, Value(false) - v30:CPtr = GetEP 0 - v31:RubyValue = LoadField v30, :VM_ENV_DATA_INDEX_ME_CREF@0x1038 - v32:CallableMethodEntry[VALUE(0x1040)] = GuardBitEquals v31, Value(VALUE(0x1040)) - v33:RubyValue = LoadField v30, :VM_ENV_DATA_INDEX_SPECVAL@0x1048 - v34:FalseClass = GuardBitEquals v33, Value(false) v23:Array = GuardType v6, Array v24:CUInt64 = LoadField v23, :RBASIC_FLAGS@0x1049 v25:CUInt64 = GuardNoBitsSet v24, RUBY_FL_FREEZE=CUInt64(2048) @@ -10054,11 +10049,6 @@ mod hir_opt_tests { v25:CallableMethodEntry[VALUE(0x1048)] = GuardBitEquals v24, Value(VALUE(0x1048)) v26:RubyValue = LoadField v23, :VM_ENV_DATA_INDEX_SPECVAL@0x1050 v27:FalseClass = GuardBitEquals v26, Value(false) - v38:CPtr = GetEP 0 - v39:RubyValue = LoadField v38, :VM_ENV_DATA_INDEX_ME_CREF@0x1040 - v40:CallableMethodEntry[VALUE(0x1048)] = GuardBitEquals v39, Value(VALUE(0x1048)) - v41:RubyValue = LoadField v38, :VM_ENV_DATA_INDEX_SPECVAL@0x1050 - v42:FalseClass = GuardBitEquals v41, Value(false) v28:Array = GuardType v9, Array v29:Fixnum = GuardType v10, Fixnum v30:CInt64 = UnboxFixnum v29 @@ -16604,4 +16594,49 @@ mod hir_opt_tests { Return v27 "); } + + #[test] + fn test_elide_test_of_box_bool() { + eval(r#" + def test(a, b) + if a == b + 3 + else + 4 + end + end + test(:a, :b) + "#); + assert_snapshot!(hir_string("test"), @" + fn test@<compiled>:3: + 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(Symbol@0x1008, ==@0x1010, cme:0x1018) + v48:StaticSymbol = GuardType v12, StaticSymbol + v49:CBool = IsBitEqual v48, v13 + v50:BoolExact = BoxBool v49 + CheckInterrupts + CondBranch v49, bb5(), bb4(v11, v48, v13) + bb5(): + v29:Fixnum[3] = Const Value(3) + CheckInterrupts + Return v29 + bb4(v34:BasicObject, v35:StaticSymbol, v36:BasicObject): + v40:Fixnum[4] = Const Value(4) + CheckInterrupts + Return v40 + "); + } } diff --git a/zjit/src/hir_type/gen_hir_type.rb b/zjit/src/hir_type/gen_hir_type.rb index 41f96a7a82..2eb5ca1932 100644 --- a/zjit/src/hir_type/gen_hir_type.rb +++ b/zjit/src/hir_type/gen_hir_type.rb @@ -134,10 +134,9 @@ nil_exact = final_type "NilClass", c_name: "rb_cNilClass" true_exact = final_type "TrueClass", c_name: "rb_cTrueClass" false_exact = final_type "FalseClass", c_name: "rb_cFalseClass" -# Typed T_DATA objects (RTYPEDDATA_P). These have a distinct memory layout -# for field access (fields_obj at a fixed offset in RTypedData). These -# don't have a common class ancestor below BasicObject. -basic_object.subtype "TypedTData" +# T_DATA objects have a distinct memory layout for field access and don't have a +# common class ancestor below BasicObject. +basic_object.subtype "TData" # Build the cvalue object universe. This is for C-level types that may be # passed around when calling into the Ruby VM or after some strength reduction diff --git a/zjit/src/hir_type/hir_type.inc.rs b/zjit/src/hir_type/hir_type.inc.rs index f4c3bf3489..1dae344b36 100644 --- a/zjit/src/hir_type/hir_type.inc.rs +++ b/zjit/src/hir_type/hir_type.inc.rs @@ -4,7 +4,7 @@ mod bits { pub const Array: u64 = ArrayExact | ArraySubclass; pub const ArrayExact: u64 = 1u64 << 0; pub const ArraySubclass: u64 = 1u64 << 1; - pub const BasicObject: u64 = BasicObjectExact | BasicObjectSubclass | Object | TypedTData; + pub const BasicObject: u64 = BasicObjectExact | BasicObjectSubclass | Object | TData; pub const BasicObjectExact: u64 = 1u64 << 2; pub const BasicObjectSubclass: u64 = 1u64 << 3; pub const Bignum: u64 = 1u64 << 4; @@ -76,7 +76,7 @@ mod bits { pub const Symbol: u64 = DynamicSymbol | StaticSymbol; pub const TrueClass: u64 = 1u64 << 45; pub const Truthy: u64 = BasicObject & !Falsy; - pub const TypedTData: u64 = 1u64 << 46; + pub const TData: u64 = 1u64 << 46; pub const Undef: u64 = 1u64 << 47; pub const AllBitPatterns: [(&str, u64); 78] = [ ("Any", Any), @@ -87,7 +87,7 @@ mod bits { ("NotNil", NotNil), ("Truthy", Truthy), ("HeapBasicObject", HeapBasicObject), - ("TypedTData", TypedTData), + ("TData", TData), ("Object", Object), ("BuiltinExact", BuiltinExact), ("BoolExact", BoolExact), @@ -238,7 +238,7 @@ pub mod types { pub const Symbol: Type = Type::from_bits(bits::Symbol); pub const TrueClass: Type = Type::from_bits(bits::TrueClass); pub const Truthy: Type = Type::from_bits(bits::Truthy); - pub const TypedTData: Type = Type::from_bits(bits::TypedTData); + pub const TData: Type = Type::from_bits(bits::TData); pub const Undef: Type = Type::from_bits(bits::Undef); pub const ExactBitsAndClass: [(u64, *const VALUE); 17] = [ (bits::ObjectExact, &raw const crate::cruby::rb_cObject), diff --git a/zjit/src/hir_type/mod.rs b/zjit/src/hir_type/mod.rs index cdfc15a935..6aa10dea55 100644 --- a/zjit/src/hir_type/mod.rs +++ b/zjit/src/hir_type/mod.rs @@ -214,7 +214,7 @@ impl Type { else if val.class_of() == unsafe { rb_cSymbol } { bits::DynamicSymbol } else if let Some(bits) = Self::bits_from_exact_class(val.class_of()) { bits } else if let Some(bits) = Self::bits_from_subclass(val.class_of()) { bits } - else if val.typed_data_p() { bits::TypedTData } + else if val.data_p() { bits::TData } else { unreachable!("Class {} is not a subclass of BasicObject! Don't know what to do.", get_class_name(val.class_of())) @@ -445,7 +445,7 @@ impl Type { } else if self.bit_equal(types::Hash) { Some(cruby::RUBY_T_HASH) } else { - // Note that types::TypedTData is narrower than T_DATA, so not here. + // T_DATA uses a specialized guard, so not here. None } } diff --git a/zjit/src/profile.rs b/zjit/src/profile.rs index 08803c4570..000c424da4 100644 --- a/zjit/src/profile.rs +++ b/zjit/src/profile.rs @@ -219,8 +219,8 @@ impl Flags { const IS_T_CLASS: u32 = 1 << 6; /// Object is a T_MODULE const IS_T_MODULE: u32 = 1 << 7; - /// Object is a typed T_DATA (RTYPEDDATA_P) - const IS_TYPED_DATA: u32 = 1 << 8; + /// Object is a T_DATA + const IS_T_DATA: u32 = 1 << 8; pub fn none() -> Self { Self(Self::NONE) } @@ -233,7 +233,7 @@ impl Flags { pub fn is_fields_embedded(self) -> bool { (self.0 & Self::IS_FIELDS_EMBEDDED) != 0 } pub fn is_t_class(self) -> bool { (self.0 & Self::IS_T_CLASS) != 0 } pub fn is_t_module(self) -> bool { (self.0 & Self::IS_T_MODULE) != 0 } - pub fn is_typed_data(self) -> bool { (self.0 & Self::IS_TYPED_DATA) != 0 } + pub fn is_t_data(self) -> bool { (self.0 & Self::IS_T_DATA) != 0 } } /// opt_send_without_block/opt_plus/... should store: @@ -322,9 +322,9 @@ impl ProfiledType { flags.0 |= Flags::IS_FIELDS_EMBEDDED; } } - if obj.typed_data_p() { - flags.0 |= Flags::IS_TYPED_DATA; - if obj.typed_data_fields_embedded_p() { + if obj.data_p() { + flags.0 |= Flags::IS_T_DATA; + if obj.data_fields_embedded_p() { flags.0 |= Flags::IS_FIELDS_EMBEDDED; } } @@ -365,8 +365,8 @@ impl ProfiledType { (RUBY_T_CLASS, RUBY_T_MASK) } else if self.flags().is_t_module() { (RUBY_T_MODULE, RUBY_T_MASK) - } else if self.flags().is_typed_data() { - (RUBY_T_DATA | RUBY_TYPED_FL_IS_TYPED_DATA, RUBY_T_MASK | RUBY_TYPED_FL_IS_TYPED_DATA) + } else if self.flags().is_t_data() { + (RUBY_T_DATA, RUBY_T_MASK) } else { (0, 0) }; |