diff --git a/Cargo.toml b/Cargo.toml index ec2ce880ca4c48..521129d92d2863 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,15 @@ -# Using Cargo's workspace feature to build all the Rust code in -# into a single package. -# TODO(alan) notes about rust version requirements. Undecided yet. +# This is the root Cargo [workspace](https://doc.rust-lang.org/cargo/reference/workspaces.html) +# and the root package for all the rust code that are statically linked into ruby. Rust tooling +# limitations means all Rust code need to share a single archive library (staticlib) at the +# integration point with non-rust code. (See rustlang/rust#44322 and #104707 for a taste of +# the linking challenges.) +# +# Do not add required dependencies. This is a policy that helps downstream consumers and give +# us tight control over what we ship. All of the optional dependencies are used exclusively +# during development. +# +# Release builds avoid Cargo entirely because offline builds can fail even when none of the +# optional dependencies are built (rust-lang/cargo#10352). [workspace] members = ["zjit", "yjit", "jit"] diff --git a/bootstraptest/test_ractor.rb b/bootstraptest/test_ractor.rb index 13c4652d3760a8..0b7a43272c3ca9 100644 --- a/bootstraptest/test_ractor.rb +++ b/bootstraptest/test_ractor.rb @@ -1661,7 +1661,7 @@ def hello = nil } # check experimental warning -assert_match /\Atest_ractor\.rb:1:\s+warning:\s+Ractor is experimental/, %q{ +assert_match /\Atest_ractor\.rb:1:\s+warning:\s+Ractor API is experimental/, %q{ Warning[:experimental] = $VERBOSE = true STDERR.reopen(STDOUT) eval("Ractor.new{}.value", nil, "test_ractor.rb", 1) diff --git a/common.mk b/common.mk index 35c23159800160..50abfeab8a4df6 100644 --- a/common.mk +++ b/common.mk @@ -268,9 +268,8 @@ MAKE_LINK = $(MINIRUBY) -rfileutils -e "include FileUtils::Verbose" \ # For release builds YJIT_RUSTC_ARGS = --crate-name=yjit \ - --crate-type=staticlib \ + $(JIT_RUST_FLAGS) \ --edition=2021 \ - --cfg 'feature="stats_allocator"' \ -g \ -C lto=thin \ -C opt-level=3 \ @@ -279,9 +278,8 @@ YJIT_RUSTC_ARGS = --crate-name=yjit \ '$(top_srcdir)/yjit/src/lib.rs' ZJIT_RUSTC_ARGS = --crate-name=zjit \ - --crate-type=staticlib \ + $(JIT_RUST_FLAGS) \ --edition=2024 \ - --cfg 'feature="stats_allocator"' \ -g \ -C lto=thin \ -C opt-level=3 \ diff --git a/configure.ac b/configure.ac index 14b0234ef0f9aa..ef4b76e5376883 100644 --- a/configure.ac +++ b/configure.ac @@ -3896,7 +3896,6 @@ AC_SUBST(INSTALL_STATIC_LIBRARY) [begin]_group "JIT section" && { AC_CHECK_PROG(RUSTC, [rustc], [rustc], [no]) dnl no ac_tool_prefix -AC_CHECK_TOOL(CARGO, [cargo], [no]) dnl check if rustc is recent enough to build YJIT (rustc >= 1.58.0) JIT_RUSTC_OK=no @@ -3963,11 +3962,7 @@ AC_ARG_ENABLE(zjit, # 1.85.0 is the first stable version that supports the 2024 edition. AS_IF([test "$RUSTC" != "no" && echo "#[cfg(target_arch = \"$JIT_TARGET_ARCH\")] fn main() {}" | $RUSTC - --edition=2024 --emit asm=/dev/null 2>/dev/null], - AS_IF([test "$gnumake" = "yes" -a \( "$YJIT_SUPPORT" = "no" -o "$CARGO" != "no" \)], [ - # When only building ZJIT, we don't need cargo; it's required for YJIT+ZJIT build. - # Assume that if rustc is new enough, then cargo is also. - # TODO(alan): Get rid of dependency on cargo in YJIT+ZJIT build. Cargo's offline mode - # still too unreliable: https://github.com/rust-lang/cargo/issues/10352 + AS_IF([test "$gnumake" = "yes"], [ rb_zjit_build_possible=yes ]) ) @@ -4053,36 +4048,49 @@ AS_CASE(["${ZJIT_SUPPORT}"], AC_DEFINE(USE_ZJIT, 0) ]) -# if YJIT+ZJIT release build, or any build that requires Cargo -AS_IF([test x"$JIT_CARGO_SUPPORT" != "xno" -o \( x"$YJIT_SUPPORT" != "xno" -a x"$ZJIT_SUPPORT" != "xno" \)], [ - AS_IF([test x"$CARGO" = "xno"], - AC_MSG_ERROR([this build configuration requires cargo. Installation instructions available at https://www.rust-lang.org/tools/install])) - - YJIT_LIBS= - ZJIT_LIBS= - - # There's more processing below to get the feature set for the - # top-level crate, so capture at this point for feature set of - # just the zjit crate. - ZJIT_TEST_FEATURES="${rb_cargo_features}" +JIT_RUST_FLAGS='--crate-type=staticlib --cfg feature=\"stats_allocator\"' +RLIB_DIR= +AS_CASE(["$JIT_CARGO_SUPPORT:$YJIT_SUPPORT:$ZJIT_SUPPORT"], + [no:yes:yes], [ # release build of YJIT+ZJIT + YJIT_LIBS= + ZJIT_LIBS= + JIT_RUST_FLAGS="--crate-type=rlib" + RLIB_DIR="target/release" + RUST_LIB="target/release/libruby.a" + ], + [no:*], [], + [*], [ # JIT_CARGO_SUPPORT not "no" -- cargo required. + AC_CHECK_TOOL(CARGO, [cargo], [no]) + AS_IF([test x"$CARGO" = "xno"], + AC_MSG_ERROR([this build configuration requires cargo. Installation instructions available at https://www.rust-lang.org/tools/install])) + + YJIT_LIBS= + ZJIT_LIBS= + + # There's more processing below to get the feature set for the + # top-level crate, so capture at this point for feature set of + # just the zjit crate. + ZJIT_TEST_FEATURES="${rb_cargo_features}" + + AS_IF([test x"${YJIT_SUPPORT}" != x"no"], [ + rb_cargo_features="$rb_cargo_features,yjit" + ]) + AS_IF([test x"${ZJIT_SUPPORT}" != x"no"], [ + AC_SUBST(ZJIT_TEST_FEATURES) + rb_cargo_features="$rb_cargo_features,zjit" + ]) + # if YJIT and ZJIT release mode + AS_IF([test "${YJIT_SUPPORT}:${ZJIT_SUPPORT}" = "yes:yes"], [ + JIT_CARGO_SUPPORT=release + ]) + CARGO_BUILD_ARGS="--profile ${JIT_CARGO_SUPPORT} --features ${rb_cargo_features}" + AS_IF([test "${JIT_CARGO_SUPPORT}" = "dev"], [ + RUST_LIB="target/debug/libruby.a" + ], [ + RUST_LIB="target/${JIT_CARGO_SUPPORT}/libruby.a" + ]) + ], - AS_IF([test x"${YJIT_SUPPORT}" != x"no"], [ - rb_cargo_features="$rb_cargo_features,yjit" - ]) - AS_IF([test x"${ZJIT_SUPPORT}" != x"no"], [ - AC_SUBST(ZJIT_TEST_FEATURES) - rb_cargo_features="$rb_cargo_features,zjit" - ]) - # if YJIT and ZJIT release mode - AS_IF([test "${YJIT_SUPPORT}:${ZJIT_SUPPORT}" = "yes:yes"], [ - JIT_CARGO_SUPPORT=release - ]) - CARGO_BUILD_ARGS="--profile ${JIT_CARGO_SUPPORT} --features ${rb_cargo_features}" - AS_IF([test "${JIT_CARGO_SUPPORT}" = "dev"], [ - RUST_LIB="target/debug/libruby.a" - ], [ - RUST_LIB="target/${JIT_CARGO_SUPPORT}/libruby.a" - ]) ]) # In case either we're linking rust code @@ -4098,6 +4106,7 @@ AS_IF([test -n "$RUST_LIB"], [ dnl These variables end up in ::RbConfig::CONFIG AC_SUBST(RUSTC)dnl Rust compiler command +AC_SUBST(JIT_RUST_FLAGS)dnl the common rustc flags for JIT crates such as zjit AC_SUBST(CARGO)dnl Cargo command for Rust builds AC_SUBST(CARGO_BUILD_ARGS)dnl for selecting Rust build profiles AC_SUBST(YJIT_SUPPORT)dnl what flavor of YJIT the Ruby build includes @@ -4108,6 +4117,7 @@ AC_SUBST(ZJIT_LIBS)dnl path to the .a library of ZJIT AC_SUBST(ZJIT_OBJ)dnl for optionally building the C parts of ZJIT AC_SUBST(JIT_OBJ)dnl for optionally building C glue code for Rust FFI AC_SUBST(RUST_LIB)dnl path to the rust .a library that contains either or both JITs +AC_SUBST(RLIB_DIR)dnl subpath of build directory for .rlib files AC_SUBST(JIT_CARGO_SUPPORT)dnl "no" or the cargo profile of the rust code } diff --git a/defs/jit.mk b/defs/jit.mk index a537d803002856..e893098ca26486 100644 --- a/defs/jit.mk +++ b/defs/jit.mk @@ -1,11 +1,5 @@ # Make recipes that deal with the rust code of YJIT and ZJIT. -# Because of Cargo cache, if the actual binary is not changed from the -# previous build, the mtime is preserved as the cached file. -# This means the target is not updated actually, and it will need to -# rebuild at the next build. -RUST_LIB_TOUCH = touch $@ - ifneq ($(JIT_CARGO_SUPPORT),no) # Show Cargo progress when doing `make V=1` @@ -13,6 +7,12 @@ CARGO_VERBOSE_0 = -q CARGO_VERBOSE_1 = CARGO_VERBOSE = $(CARGO_VERBOSE_$(V)) +# Because of Cargo cache, if the actual binary is not changed from the +# previous build, the mtime is preserved as the cached file. +# This means the target is not updated actually, and it will need to +# rebuild at the next build. +RUST_LIB_TOUCH = touch $@ + # NOTE: MACOSX_DEPLOYMENT_TARGET to match `rustc --print deployment-target` to avoid the warning below. # ld: warning: object file (target/debug/libjit.a()) was built for # newer macOS version (15.2) than being linked (15.0) @@ -30,7 +30,32 @@ $(RUST_LIB): $(srcdir)/ruby.rs MACOSX_DEPLOYMENT_TARGET=11.0 \ $(CARGO) $(CARGO_VERBOSE) build --manifest-path '$(top_srcdir)/Cargo.toml' $(CARGO_BUILD_ARGS) $(RUST_LIB_TOUCH) -endif +else ifneq ($(strip $(RLIB_DIR)),) # combo build + +$(RUST_LIB): $(srcdir)/ruby.rs + $(ECHO) 'building $(@F)' + $(Q) $(RUSTC) --edition=2024 \ + '-L$(@D)' \ + --extern=yjit \ + --extern=zjit \ + --crate-type=staticlib \ + --cfg 'feature="yjit"' \ + --cfg 'feature="zjit"' \ + '--out-dir=$(@D)' \ + '$(top_srcdir)/ruby.rs' + +# Absolute path to avoid VPATH ambiguity +JIT_RLIB = $(TOP_BUILD_DIR)/$(RLIB_DIR)/libjit.rlib +$(YJIT_RLIB): $(JIT_RLIB) +$(ZJIT_RLIB): $(JIT_RLIB) +$(JIT_RLIB): + $(ECHO) 'building $(@F)' + $(Q) $(RUSTC) --crate-name=jit \ + --edition=2024 \ + $(JIT_RUST_FLAGS) \ + '--out-dir=$(@D)' \ + '$(top_srcdir)/jit/src/lib.rs' +endif # ifneq ($(JIT_CARGO_SUPPORT),no) RUST_LIB_SYMBOLS = $(RUST_LIB:.a=).symbols $(RUST_LIBOBJ): $(RUST_LIB) diff --git a/ext/-test-/tracepoint/tracepoint.c b/ext/-test-/tracepoint/tracepoint.c index 7f7aa246628858..001d9513b29fb2 100644 --- a/ext/-test-/tracepoint/tracepoint.c +++ b/ext/-test-/tracepoint/tracepoint.c @@ -86,25 +86,6 @@ tracepoint_specify_normal_and_internal_events(VALUE self) return Qnil; /* should not be reached */ } -int rb_objspace_internal_object_p(VALUE obj); - -static void -on_newobj_event(VALUE tpval, void *data) -{ - VALUE obj = rb_tracearg_object(rb_tracearg_from_tracepoint(tpval)); - if (!rb_objspace_internal_object_p(obj)) rb_obj_id(obj); -} - -static VALUE -add_object_id(RB_UNUSED_VAR(VALUE _)) -{ - VALUE tp = rb_tracepoint_new(0, RUBY_INTERNAL_EVENT_NEWOBJ, on_newobj_event, NULL); - rb_tracepoint_enable(tp); - rb_yield(Qnil); - rb_tracepoint_disable(tp); - return Qnil; -} - void Init_gc_hook(VALUE); void @@ -114,5 +95,4 @@ Init_tracepoint(void) Init_gc_hook(tp_mBug); rb_define_module_function(tp_mBug, "tracepoint_track_objspace_events", tracepoint_track_objspace_events, 0); rb_define_module_function(tp_mBug, "tracepoint_specify_normal_and_internal_events", tracepoint_specify_normal_and_internal_events, 0); - rb_define_singleton_method(tp_mBug, "tracepoint_add_object_id", add_object_id, 0); } diff --git a/gc/default/default.c b/gc/default/default.c index a03fda859cf4ca..6a2035dccc9c99 100644 --- a/gc/default/default.c +++ b/gc/default/default.c @@ -316,7 +316,7 @@ int ruby_rgengc_debug; #endif #ifndef GC_DEBUG_STRESS_TO_CLASS -# define GC_DEBUG_STRESS_TO_CLASS 1 +# define GC_DEBUG_STRESS_TO_CLASS RUBY_DEBUG #endif typedef enum { @@ -9428,6 +9428,7 @@ rb_gc_impl_after_fork(void *objspace_ptr, rb_pid_t pid) VALUE rb_ident_hash_new_with_size(st_index_t size); +#if GC_DEBUG_STRESS_TO_CLASS /* * call-seq: * GC.add_stress_to_class(class[, ...]) @@ -9477,6 +9478,7 @@ rb_gcdebug_remove_stress_to_class(int argc, VALUE *argv, VALUE self) return Qnil; } +#endif void * rb_gc_impl_objspace_alloc(void) @@ -9582,10 +9584,10 @@ rb_gc_impl_init(void) rb_define_singleton_method(rb_mGC, "verify_compaction_references", rb_f_notimplement, -1); } - if (GC_DEBUG_STRESS_TO_CLASS) { - rb_define_singleton_method(rb_mGC, "add_stress_to_class", rb_gcdebug_add_stress_to_class, -1); - rb_define_singleton_method(rb_mGC, "remove_stress_to_class", rb_gcdebug_remove_stress_to_class, -1); - } +#if GC_DEBUG_STRESS_TO_CLASS + rb_define_singleton_method(rb_mGC, "add_stress_to_class", rb_gcdebug_add_stress_to_class, -1); + rb_define_singleton_method(rb_mGC, "remove_stress_to_class", rb_gcdebug_remove_stress_to_class, -1); +#endif /* internal methods */ rb_define_singleton_method(rb_mGC, "verify_internal_consistency", gc_verify_internal_consistency_m, 0); diff --git a/ractor.rb b/ractor.rb index d992f0a04702c9..70ce1a9fffecb9 100644 --- a/ractor.rb +++ b/ractor.rb @@ -230,8 +230,8 @@ def self.new(*args, name: nil, &block) b = block # TODO: builtin bug raise ArgumentError, "must be called with a block" unless block if __builtin_cexpr!("RBOOL(ruby_single_main_ractor)") - Kernel.warn("Ractor is experimental, and the behavior may change in future versions of Ruby! " \ - "Also there are many implementation issues.", uplevel: 0, category: :experimental) + Kernel.warn("Ractor API is experimental and may change in future versions of Ruby.", + uplevel: 0, category: :experimental) end loc = caller_locations(1, 1).first loc = "#{loc.path}:#{loc.lineno}" diff --git a/template/Makefile.in b/template/Makefile.in index 7bc40b9d019604..fd41ddca9d90d5 100644 --- a/template/Makefile.in +++ b/template/Makefile.in @@ -114,6 +114,8 @@ JIT_CARGO_SUPPORT=@JIT_CARGO_SUPPORT@ CARGO_TARGET_DIR=@abs_top_builddir@/target CARGO_BUILD_ARGS=@CARGO_BUILD_ARGS@ ZJIT_TEST_FEATURES=@ZJIT_TEST_FEATURES@ +JIT_RUST_FLAGS=@JIT_RUST_FLAGS@ +RLIB_DIR=@RLIB_DIR@ RUST_LIB=@RUST_LIB@ RUST_LIBOBJ = $(RUST_LIB:.a=.@OBJEXT@) LDFLAGS = @STATIC@ $(CFLAGS) @LDFLAGS@ diff --git a/test/-ext-/tracepoint/test_tracepoint.rb b/test/-ext-/tracepoint/test_tracepoint.rb index 2256f58bc710b0..debddd83d043fe 100644 --- a/test/-ext-/tracepoint/test_tracepoint.rb +++ b/test/-ext-/tracepoint/test_tracepoint.rb @@ -82,24 +82,6 @@ def run(hook) end end - def test_tracepoint_add_object_id - Bug.tracepoint_add_object_id do - klass = Struct.new - 2.times { klass.new } - - klass = Struct.new(:a) - 2.times { klass.new } - - klass = Struct.new(:a, :b, :c) - 2.times { klass.new } - - 2.times { Set.new } # To test T_DATA / TypedData RUBY_TYPED_EMBEDDABLE - 2.times { Proc.new { } } # To test T_DATA / TypedData non embeddable - - 2.times { Object.new } - end - end - def test_teardown_with_active_GC_end_hook assert_separately([], 'require("-test-/tracepoint"); Bug.after_gc_exit_hook = proc {}; GC.start') end diff --git a/yjit/Cargo.toml b/yjit/Cargo.toml index e2f1d84ffd3d8c..d3124e608c6a3c 100644 --- a/yjit/Cargo.toml +++ b/yjit/Cargo.toml @@ -10,8 +10,8 @@ rust-version = "1.58.0" # Minimally supported rust version publish = false # Don't publish to crates.io [dependencies] -# No required dependencies to simplify build process. TODO: Link to yet to be -# written rationale. Optional For development and testing purposes +# No required dependencies to simplify build process. +# Optional For development and testing purposes. capstone = { version = "0.13.0", optional = true } jit = { version = "0.1.0", path = "../jit" } diff --git a/yjit/yjit.mk b/yjit/yjit.mk index e019e4a08cf7b8..22d256fee78c14 100644 --- a/yjit/yjit.mk +++ b/yjit/yjit.mk @@ -9,12 +9,6 @@ YJIT_SRC_FILES = $(wildcard \ $(top_srcdir)/jit/src/lib.rs \ ) -# Because of Cargo cache, if the actual binary is not changed from the -# previous build, the mtime is preserved as the cached file. -# This means the target is not updated actually, and it will need to -# rebuild at the next build. -YJIT_LIB_TOUCH = touch $@ - # Absolute path to match RUST_LIB rules to avoid picking # the "target" dir in the source directory through VPATH. BUILD_YJIT_LIBS = $(TOP_BUILD_DIR)/$(YJIT_LIBS) @@ -24,9 +18,17 @@ ifneq ($(strip $(YJIT_LIBS)),) yjit-libs: $(BUILD_YJIT_LIBS) $(BUILD_YJIT_LIBS): $(YJIT_SRC_FILES) $(ECHO) 'building Rust YJIT (release mode)' - +$(Q) $(RUSTC) $(YJIT_RUSTC_ARGS) - $(YJIT_LIB_TOUCH) -endif + $(Q) $(RUSTC) $(YJIT_RUSTC_ARGS) +else ifneq ($(strip $(RLIB_DIR)),) # combo build +# Absolute path to avoid VPATH ambiguity +YJIT_RLIB = $(TOP_BUILD_DIR)/$(RLIB_DIR)/libyjit.rlib + +$(YJIT_RLIB): $(YJIT_SRC_FILES) + $(ECHO) 'building $(@F)' + $(Q) $(RUSTC) '-L$(@D)' --extern=jit $(YJIT_RUSTC_ARGS) + +$(RUST_LIB): $(YJIT_RLIB) +endif # ifneq ($(strip $(YJIT_LIBS)),) ifneq ($(YJIT_SUPPORT),no) $(RUST_LIB): $(YJIT_SRC_FILES) diff --git a/zjit.c b/zjit.c index 05fb3e1f028ff6..cb5a01734f9721 100644 --- a/zjit.c +++ b/zjit.c @@ -23,14 +23,6 @@ #include "ruby/debug.h" #include "internal/cont.h" -// For mmapp(), sysconf() -#ifndef _WIN32 -#include -#include -#endif - -#include - // This build config impacts the pointer tagging scheme and we only want to // support one scheme for simplicity. STATIC_ASSERT(pointer_tagging_scheme, USE_FLONUM); diff --git a/zjit/Cargo.toml b/zjit/Cargo.toml index c97c845a6eff7f..ef656c5252027c 100644 --- a/zjit/Cargo.toml +++ b/zjit/Cargo.toml @@ -6,8 +6,8 @@ rust-version = "1.85.0" # Minimally supported rust version publish = false # Don't publish to crates.io [dependencies] -# No required dependencies to simplify build process. TODO: Link to yet to be -# written rationale. Optional For development and testing purposes +# No required dependencies to simplify build process. +# Optional For development and testing purposes. capstone = { version = "0.13.0", optional = true } jit = { version = "0.1.0", path = "../jit" } diff --git a/zjit/src/asm/arm64/arg/sf.rs b/zjit/src/asm/arm64/arg/sf.rs index c2fd33302c1ef8..b6091821e906fb 100644 --- a/zjit/src/asm/arm64/arg/sf.rs +++ b/zjit/src/asm/arm64/arg/sf.rs @@ -13,7 +13,7 @@ impl From for Sf { match num_bits { 64 => Sf::Sf64, 32 => Sf::Sf32, - _ => panic!("Invalid number of bits: {}", num_bits) + _ => panic!("Invalid number of bits: {num_bits}"), } } } diff --git a/zjit/src/asm/arm64/inst/atomic.rs b/zjit/src/asm/arm64/inst/atomic.rs index dce9affedf8838..0917a4fd1c17ea 100644 --- a/zjit/src/asm/arm64/inst/atomic.rs +++ b/zjit/src/asm/arm64/inst/atomic.rs @@ -14,7 +14,7 @@ impl From for Size { match num_bits { 64 => Size::Size64, 32 => Size::Size32, - _ => panic!("Invalid number of bits: {}", num_bits) + _ => panic!("Invalid number of bits: {num_bits}"), } } } diff --git a/zjit/src/asm/arm64/inst/load_literal.rs b/zjit/src/asm/arm64/inst/load_literal.rs index a429c0fb53d082..37b5f3c7a7546b 100644 --- a/zjit/src/asm/arm64/inst/load_literal.rs +++ b/zjit/src/asm/arm64/inst/load_literal.rs @@ -15,7 +15,7 @@ impl From for Opc { match num_bits { 64 => Opc::Size64, 32 => Opc::Size32, - _ => panic!("Invalid number of bits: {}", num_bits) + _ => panic!("Invalid number of bits: {num_bits}"), } } } diff --git a/zjit/src/asm/arm64/inst/load_register.rs b/zjit/src/asm/arm64/inst/load_register.rs index 3d94e8da1f8f26..80813ffc870a42 100644 --- a/zjit/src/asm/arm64/inst/load_register.rs +++ b/zjit/src/asm/arm64/inst/load_register.rs @@ -25,7 +25,7 @@ impl From for Size { match num_bits { 64 => Size::Size64, 32 => Size::Size32, - _ => panic!("Invalid number of bits: {}", num_bits) + _ => panic!("Invalid number of bits: {num_bits}"), } } } diff --git a/zjit/src/asm/arm64/inst/load_store.rs b/zjit/src/asm/arm64/inst/load_store.rs index e78edd56752aae..d38e851ed73e30 100644 --- a/zjit/src/asm/arm64/inst/load_store.rs +++ b/zjit/src/asm/arm64/inst/load_store.rs @@ -15,7 +15,7 @@ impl From for Size { match num_bits { 64 => Size::Size64, 32 => Size::Size32, - _ => panic!("Invalid number of bits: {}", num_bits) + _ => panic!("Invalid number of bits: {num_bits}"), } } } diff --git a/zjit/src/asm/arm64/inst/load_store_exclusive.rs b/zjit/src/asm/arm64/inst/load_store_exclusive.rs index 1106b4cb372eb1..30cb663bdb9c67 100644 --- a/zjit/src/asm/arm64/inst/load_store_exclusive.rs +++ b/zjit/src/asm/arm64/inst/load_store_exclusive.rs @@ -17,7 +17,7 @@ impl From for Size { match num_bits { 64 => Size::Size64, 32 => Size::Size32, - _ => panic!("Invalid number of bits: {}", num_bits) + _ => panic!("Invalid number of bits: {num_bits}"), } } } diff --git a/zjit/src/asm/arm64/inst/mov.rs b/zjit/src/asm/arm64/inst/mov.rs index 70814938056f15..e9f9091713107b 100644 --- a/zjit/src/asm/arm64/inst/mov.rs +++ b/zjit/src/asm/arm64/inst/mov.rs @@ -27,7 +27,7 @@ impl From for Hw { 16 => Hw::LSL16, 32 => Hw::LSL32, 48 => Hw::LSL48, - _ => panic!("Invalid value for shift: {}", shift) + _ => panic!("Invalid value for shift: {shift}"), } } } diff --git a/zjit/src/asm/arm64/inst/reg_pair.rs b/zjit/src/asm/arm64/inst/reg_pair.rs index 9bffcd847925c1..39a44c24167370 100644 --- a/zjit/src/asm/arm64/inst/reg_pair.rs +++ b/zjit/src/asm/arm64/inst/reg_pair.rs @@ -26,7 +26,7 @@ impl From for Opc { match num_bits { 64 => Opc::Opc64, 32 => Opc::Opc32, - _ => panic!("Invalid number of bits: {}", num_bits) + _ => panic!("Invalid number of bits: {num_bits}"), } } } diff --git a/zjit/src/asm/arm64/inst/test_bit.rs b/zjit/src/asm/arm64/inst/test_bit.rs index f7aeca70fda8af..45f0c2317ec974 100644 --- a/zjit/src/asm/arm64/inst/test_bit.rs +++ b/zjit/src/asm/arm64/inst/test_bit.rs @@ -17,7 +17,7 @@ impl From for B5 { match bit_num { 0..=31 => B5::B532, 32..=63 => B5::B564, - _ => panic!("Invalid bit number: {}", bit_num) + _ => panic!("Invalid bit number: {bit_num}"), } } } diff --git a/zjit/src/asm/arm64/mod.rs b/zjit/src/asm/arm64/mod.rs index 4094d101fbc32c..c30520cd693f14 100644 --- a/zjit/src/asm/arm64/mod.rs +++ b/zjit/src/asm/arm64/mod.rs @@ -241,7 +241,7 @@ pub fn asr(cb: &mut CodeBlock, rd: A64Opnd, rn: A64Opnd, shift: A64Opnd) { SBFM::asr(rd.reg_no, rn.reg_no, shift.try_into().unwrap(), rd.num_bits).into() }, - _ => panic!("Invalid operand combination to asr instruction: asr {:?}, {:?}, {:?}", rd, rn, shift), + _ => panic!("Invalid operand combination to asr instruction: asr {rd:?}, {rn:?}, {shift:?}"), }; cb.write_bytes(&bytes); diff --git a/zjit/src/asm/mod.rs b/zjit/src/asm/mod.rs index 9b792f5f376325..90496245295f88 100644 --- a/zjit/src/asm/mod.rs +++ b/zjit/src/asm/mod.rs @@ -354,7 +354,7 @@ impl fmt::LowerHex for CodeBlock { for pos in 0..self.write_pos { let mem_block = &*self.mem_block.borrow(); let byte = unsafe { mem_block.start_ptr().raw_ptr(mem_block).add(pos).read() }; - fmtr.write_fmt(format_args!("{:02x}", byte))?; + fmtr.write_fmt(format_args!("{byte:02x}"))?; } Ok(()) } diff --git a/zjit/src/backend/arm64/mod.rs b/zjit/src/backend/arm64/mod.rs index 78fb69b0b04d5b..55a65e3ea6161e 100644 --- a/zjit/src/backend/arm64/mod.rs +++ b/zjit/src/backend/arm64/mod.rs @@ -903,8 +903,8 @@ impl Assembler { } } } - &mut Insn::PatchPoint { ref target, invariant, payload } => { - split_patch_point(asm, target, invariant, payload); + &mut Insn::PatchPoint { ref target, invariant, version } => { + split_patch_point(asm, target, invariant, version); } _ => { asm.push_insn(insn); @@ -1311,7 +1311,7 @@ impl Assembler { 64 | 32 => stur(cb, src, dest.into()), 16 => sturh(cb, src, dest.into()), 8 => sturb(cb, src, dest.into()), - num_bits => panic!("unexpected dest num_bits: {} (src: {:?}, dest: {:?})", num_bits, src, dest), + num_bits => panic!("unexpected dest num_bits: {num_bits} (src: {src:?}, dest: {dest:?})"), } }, Insn::Load { opnd, out } | @@ -1331,7 +1331,7 @@ impl Assembler { 64 | 32 => ldur(cb, out.into(), opnd.into()), 16 => ldurh(cb, out.into(), opnd.into()), 8 => ldurb(cb, out.into(), opnd.into()), - num_bits => panic!("unexpected num_bits: {}", num_bits) + num_bits => panic!("unexpected num_bits: {num_bits}"), }; }, Opnd::Value(value) => { diff --git a/zjit/src/backend/lir.rs b/zjit/src/backend/lir.rs index d7c6740d13a1e4..f4ed3d7cb78ce6 100644 --- a/zjit/src/backend/lir.rs +++ b/zjit/src/backend/lir.rs @@ -9,7 +9,7 @@ use crate::cruby::{Qundef, RUBY_OFFSET_CFP_PC, RUBY_OFFSET_CFP_SP, SIZEOF_VALUE_ use crate::hir::{Invariant, SideExitReason}; use crate::options::{TraceExits, debug, get_option}; use crate::cruby::VALUE; -use crate::payload::IseqPayload; +use crate::payload::IseqVersionRef; use crate::stats::{exit_counter_ptr, exit_counter_ptr_for_opcode, side_exit_counter, CompileError}; use crate::virtualmem::CodePtr; use crate::asm::{CodeBlock, Label}; @@ -531,7 +531,7 @@ pub enum Insn { Or { left: Opnd, right: Opnd, out: Opnd }, /// Patch point that will be rewritten to a jump to a side exit on invalidation. - PatchPoint { target: Target, invariant: Invariant, payload: *mut IseqPayload }, + PatchPoint { target: Target, invariant: Invariant, version: IseqVersionRef }, /// Make sure the last PatchPoint has enough space to insert a jump. /// We insert this instruction at the end of each block so that the jump @@ -2358,8 +2358,8 @@ impl Assembler { out } - pub fn patch_point(&mut self, target: Target, invariant: Invariant, payload: *mut IseqPayload) { - self.push_insn(Insn::PatchPoint { target, invariant, payload }); + pub fn patch_point(&mut self, target: Target, invariant: Invariant, version: IseqVersionRef) { + self.push_insn(Insn::PatchPoint { target, invariant, version }); } pub fn pad_patch_point(&mut self) { diff --git a/zjit/src/backend/x86_64/mod.rs b/zjit/src/backend/x86_64/mod.rs index 5e975e1bd03ccf..9f780617cc4ac5 100644 --- a/zjit/src/backend/x86_64/mod.rs +++ b/zjit/src/backend/x86_64/mod.rs @@ -640,8 +640,8 @@ impl Assembler { }; asm.store(dest, src); } - &mut Insn::PatchPoint { ref target, invariant, payload } => { - split_patch_point(asm, target, invariant, payload); + &mut Insn::PatchPoint { ref target, invariant, version } => { + split_patch_point(asm, target, invariant, version); } _ => { asm.push_insn(insn); diff --git a/zjit/src/codegen.rs b/zjit/src/codegen.rs index 05b704d66a4bb4..dbcffb04273a75 100644 --- a/zjit/src/codegen.rs +++ b/zjit/src/codegen.rs @@ -13,7 +13,7 @@ use crate::invariants::{ track_single_ractor_assumption, track_stable_constant_names_assumption, track_no_singleton_class_assumption }; use crate::gc::append_gc_offsets; -use crate::payload::{get_or_create_iseq_payload, get_or_create_iseq_payload_ptr, IseqCodePtrs, IseqPayload, IseqStatus}; +use crate::payload::{get_or_create_iseq_payload, IseqCodePtrs, IseqVersion, IseqVersionRef, IseqStatus}; 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_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, Counter, Counter::{compile_time_ns, exit_compile_error}}; @@ -25,6 +25,9 @@ use crate::hir_type::{types, Type}; use crate::options::get_option; use crate::cast::IntoUsize; +/// At the moment, we support recompiling each ISEQ only once. +pub const MAX_ISEQ_VERSIONS: usize = 2; + /// Sentinel program counter stored in C frames when runtime checks are enabled. const PC_POISON: Option<*const VALUE> = if cfg!(feature = "runtime_checks") { Some(usize::MAX as *const VALUE) @@ -37,6 +40,9 @@ struct JITState { /// Instruction sequence for the method being compiled iseq: IseqPtr, + /// ISEQ version that is being compiled, which will be used by PatchPoint + version: IseqVersionRef, + /// Low-level IR Operands indexed by High-level IR's Instruction ID opnds: Vec>, @@ -52,9 +58,10 @@ struct JITState { impl JITState { /// Create a new JITState instance - fn new(iseq: IseqPtr, num_insns: usize, num_blocks: usize) -> Self { + fn new(iseq: IseqPtr, version: IseqVersionRef, num_insns: usize, num_blocks: usize) -> Self { JITState { iseq, + version, opnds: vec![None; num_insns], labels: vec![None; num_blocks], jit_entries: Vec::default(), @@ -134,11 +141,10 @@ fn gen_iseq_entry_point(cb: &mut CodeBlock, iseq: IseqPtr, jit_exception: bool) } /// Stub a branch for a JIT-to-JIT call -fn gen_iseq_call(cb: &mut CodeBlock, caller_iseq: IseqPtr, iseq_call: &IseqCallRef) -> Result<(), CompileError> { +pub fn gen_iseq_call(cb: &mut CodeBlock, iseq_call: &IseqCallRef) -> Result<(), CompileError> { // Compile a function stub let stub_ptr = gen_function_stub(cb, iseq_call.clone()).inspect_err(|err| { - debug!("{err:?}: gen_function_stub failed: {} -> {}", - iseq_get_location(caller_iseq, 0), iseq_get_location(iseq_call.iseq.get(), 0)); + debug!("{err:?}: gen_function_stub failed: {}", iseq_get_location(iseq_call.iseq.get(), 0)); })?; // Update the JIT-to-JIT call to call the stub @@ -160,7 +166,7 @@ fn register_with_perf(iseq_name: String, start_ptr: usize, code_size: usize) { return; }; let mut file = std::io::BufWriter::new(file); - let Ok(_) = writeln!(file, "{:#x} {:#x} zjit::{}", start_ptr, code_size, iseq_name) else { + let Ok(_) = writeln!(file, "{start_ptr:#x} {code_size:#x} zjit::{iseq_name}") else { debug!("Failed to write {iseq_name} to perf map file: {perf_map}"); return; }; @@ -197,29 +203,36 @@ pub fn gen_entry_trampoline(cb: &mut CodeBlock) -> Result fn gen_iseq(cb: &mut CodeBlock, iseq: IseqPtr, function: Option<&Function>) -> Result { // Return an existing pointer if it's already compiled let payload = get_or_create_iseq_payload(iseq); - match &payload.status { - IseqStatus::Compiled(code_ptrs) => return Ok(code_ptrs.clone()), - IseqStatus::CantCompile(err) => return Err(err.clone()), - IseqStatus::NotCompiled => {}, + let last_status = payload.versions.last().map(|version| &unsafe { version.as_ref() }.status); + match last_status { + Some(IseqStatus::Compiled(code_ptrs)) => return Ok(code_ptrs.clone()), + Some(IseqStatus::CantCompile(err)) => return Err(err.clone()), + _ => {}, + } + // If the ISEQ already hax MAX_ISEQ_VERSIONS, do not compile a new version. + if payload.versions.len() == MAX_ISEQ_VERSIONS { + return Err(CompileError::IseqVersionLimitReached); } // Compile the ISEQ - let code_ptrs = gen_iseq_body(cb, iseq, function, payload); + let mut version = IseqVersion::new(iseq); + let code_ptrs = gen_iseq_body(cb, iseq, version, function); match &code_ptrs { Ok(code_ptrs) => { - payload.status = IseqStatus::Compiled(code_ptrs.clone()); + unsafe { version.as_mut() }.status = IseqStatus::Compiled(code_ptrs.clone()); incr_counter!(compiled_iseq_count); } Err(err) => { - payload.status = IseqStatus::CantCompile(err.clone()); + unsafe { version.as_mut() }.status = IseqStatus::CantCompile(err.clone()); incr_counter!(failed_iseq_count); } } + payload.versions.push(version); code_ptrs } /// Compile an ISEQ into machine code -fn gen_iseq_body(cb: &mut CodeBlock, iseq: IseqPtr, function: Option<&Function>, payload: &mut IseqPayload) -> Result { +fn gen_iseq_body(cb: &mut CodeBlock, iseq: IseqPtr, mut version: IseqVersionRef, function: Option<&Function>) -> Result { // If we ran out of code region, we shouldn't attempt to generate new code. if cb.has_dropped_bytes() { return Err(CompileError::OutOfMemory); @@ -232,23 +245,23 @@ fn gen_iseq_body(cb: &mut CodeBlock, iseq: IseqPtr, function: Option<&Function>, }; // Compile the High-level IR - let (iseq_code_ptrs, gc_offsets, iseq_calls) = gen_function(cb, iseq, function)?; + let (iseq_code_ptrs, gc_offsets, iseq_calls) = gen_function(cb, iseq, version, function)?; // Stub callee ISEQs for JIT-to-JIT calls for iseq_call in iseq_calls.iter() { - gen_iseq_call(cb, iseq, iseq_call)?; + gen_iseq_call(cb, iseq_call)?; } // Prepare for GC - payload.iseq_calls.extend(iseq_calls); - append_gc_offsets(iseq, &gc_offsets); + unsafe { version.as_mut() }.outgoing.extend(iseq_calls); + append_gc_offsets(iseq, version, &gc_offsets); Ok(iseq_code_ptrs) } /// Compile a function -fn gen_function(cb: &mut CodeBlock, iseq: IseqPtr, function: &Function) -> Result<(IseqCodePtrs, Vec, Vec), CompileError> { +fn gen_function(cb: &mut CodeBlock, iseq: IseqPtr, version: IseqVersionRef, function: &Function) -> Result<(IseqCodePtrs, Vec, Vec), CompileError> { let num_spilled_params = max_num_params(function).saturating_sub(ALLOC_REGS.len()); - let mut jit = JITState::new(iseq, function.num_insns(), function.num_blocks()); + let mut jit = JITState::new(iseq, version, function.num_insns(), function.num_blocks()); let mut asm = Assembler::new_with_stack_slots(num_spilled_params); // Compile each basic block @@ -724,17 +737,16 @@ fn gen_invokebuiltin(jit: &JITState, asm: &mut Assembler, state: &FrameState, bf /// Record a patch point that should be invalidated on a given invariant fn gen_patch_point(jit: &mut JITState, asm: &mut Assembler, invariant: &Invariant, state: &FrameState) { - let payload_ptr = get_or_create_iseq_payload_ptr(jit.iseq); let invariant = *invariant; let exit = build_side_exit(jit, state); // Let compile_exits compile a side exit. Let scratch_split lower it with split_patch_point. - asm.patch_point(Target::SideExit { exit, reason: PatchPoint(invariant) }, invariant, payload_ptr); + asm.patch_point(Target::SideExit { exit, reason: PatchPoint(invariant) }, invariant, jit.version); } /// This is used by scratch_split to lower PatchPoint into PadPatchPoint and PosMarker. /// It's called at scratch_split so that we can use the Label after side-exit deduplication in compile_exits. -pub fn split_patch_point(asm: &mut Assembler, target: &Target, invariant: Invariant, payload_ptr: *mut IseqPayload) { +pub fn split_patch_point(asm: &mut Assembler, target: &Target, invariant: Invariant, version: IseqVersionRef) { let Target::Label(exit_label) = *target else { unreachable!("PatchPoint's target should have been lowered to Target::Label by compile_exits: {target:?}"); }; @@ -747,25 +759,25 @@ pub fn split_patch_point(asm: &mut Assembler, target: &Target, invariant: Invari let side_exit_ptr = cb.resolve_label(exit_label); match invariant { Invariant::BOPRedefined { klass, bop } => { - track_bop_assumption(klass, bop, code_ptr, side_exit_ptr, payload_ptr); + track_bop_assumption(klass, bop, code_ptr, side_exit_ptr, version); } Invariant::MethodRedefined { klass: _, method: _, cme } => { - track_cme_assumption(cme, code_ptr, side_exit_ptr, payload_ptr); + track_cme_assumption(cme, code_ptr, side_exit_ptr, version); } Invariant::StableConstantNames { idlist } => { - track_stable_constant_names_assumption(idlist, code_ptr, side_exit_ptr, payload_ptr); + track_stable_constant_names_assumption(idlist, code_ptr, side_exit_ptr, version); } Invariant::NoTracePoint => { - track_no_trace_point_assumption(code_ptr, side_exit_ptr, payload_ptr); + track_no_trace_point_assumption(code_ptr, side_exit_ptr, version); } Invariant::NoEPEscape(iseq) => { - track_no_ep_escape_assumption(iseq, code_ptr, side_exit_ptr, payload_ptr); + track_no_ep_escape_assumption(iseq, code_ptr, side_exit_ptr, version); } Invariant::SingleRactorMode => { - track_single_ractor_assumption(code_ptr, side_exit_ptr, payload_ptr); + track_single_ractor_assumption(code_ptr, side_exit_ptr, version); } Invariant::NoSingletonClass { klass } => { - track_no_singleton_class_assumption(klass, code_ptr, side_exit_ptr, payload_ptr); + track_no_singleton_class_assumption(klass, code_ptr, side_exit_ptr, version); } } }); @@ -2019,7 +2031,7 @@ fn gen_guard_bit_equals(jit: &mut JITState, asm: &mut Assembler, val: lir::Opnd, let expected_opnd: Opnd = match expected { crate::hir::Const::Value(v) => { Opnd::Value(v) } crate::hir::Const::CInt64(v) => { v.into() } - _ => panic!("gen_guard_bit_equals: unexpected hir::Const {:?}", expected), + _ => panic!("gen_guard_bit_equals: unexpected hir::Const {expected:?}"), }; asm.cmp(val, expected_opnd); asm.jnz(side_exit(jit, state, GuardBitEquals(expected))); @@ -2383,8 +2395,9 @@ c_callable! { // code path can be made read-only. But you still need the check as is while holding the VM lock in any case. let cb = ZJITState::get_code_block(); let payload = get_or_create_iseq_payload(iseq); - let compile_error = match &payload.status { - IseqStatus::CantCompile(err) => Some(err), + let last_status = payload.versions.last().map(|version| &unsafe { version.as_ref() }.status); + let compile_error = match last_status { + Some(IseqStatus::CantCompile(err)) => Some(err), _ if cb.has_dropped_bytes() => Some(&CompileError::OutOfMemory), _ => None, }; @@ -2398,7 +2411,15 @@ c_callable! { // Otherwise, attempt to compile the ISEQ. We have to mark_all_executable() beyond this point. let code_ptr = with_time_stat(compile_time_ns, || function_stub_hit_body(cb, &iseq_call)); + if code_ptr.is_ok() { + if let Some(version) = payload.versions.last_mut() { + unsafe { version.as_mut() }.incoming.push(iseq_call); + } + } let code_ptr = code_ptr.unwrap_or_else(|compile_error| { + // We'll use this Rc again, so increment the ref count decremented by from_raw. + unsafe { Rc::increment_strong_count(iseq_call_ptr as *const IseqCall); } + prepare_for_exit(iseq, cfp, sp, &compile_error); ZJITState::get_exit_trampoline_with_counter() }); @@ -2715,3 +2736,37 @@ impl IseqCall { }); } } + +#[cfg(test)] +mod tests { + use crate::codegen::MAX_ISEQ_VERSIONS; + use crate::cruby::test_utils::*; + use crate::payload::*; + + #[test] + fn test_max_iseq_versions() { + eval(&format!(" + TEST = -1 + def test = TEST + + # compile and invalidate MAX+1 times + i = 0 + while i < {MAX_ISEQ_VERSIONS} + 1 + test; test # compile a version + + Object.send(:remove_const, :TEST) + TEST = i + + i += 1 + end + ")); + + // It should not exceed MAX_ISEQ_VERSIONS + let iseq = get_method_iseq("self", "test"); + let payload = get_or_create_iseq_payload(iseq); + assert_eq!(payload.versions.len(), MAX_ISEQ_VERSIONS); + + // The last call should not discard the JIT code + assert!(matches!(unsafe { payload.versions.last().unwrap().as_ref() }.status, IseqStatus::Compiled(_))); + } +} diff --git a/zjit/src/cruby.rs b/zjit/src/cruby.rs index e653602874df77..2bd44d21443b8d 100644 --- a/zjit/src/cruby.rs +++ b/zjit/src/cruby.rs @@ -986,7 +986,7 @@ pub fn rb_bug_panic_hook() { // You may also use ZJIT_RB_BUG=1 to trigger this on dev builds. if release_build || env::var("ZJIT_RB_BUG").is_ok() { // Abort with rb_bug(). It has a length limit on the message. - let panic_message = &format!("{}", panic_info)[..]; + let panic_message = &format!("{panic_info}")[..]; let len = std::cmp::min(0x100, panic_message.len()) as c_int; unsafe { rb_bug(b"ZJIT: %*s\0".as_ref().as_ptr() as *const c_char, len, panic_message.as_ptr()); } } else { diff --git a/zjit/src/gc.rs b/zjit/src/gc.rs index 93a9b10e562c4a..48d841a1048c39 100644 --- a/zjit/src/gc.rs +++ b/zjit/src/gc.rs @@ -1,8 +1,9 @@ //! This module is responsible for marking/moving objects on GC. +use std::ptr::null; use std::{ffi::c_void, ops::Range}; use crate::{cruby::*, state::ZJITState, stats::with_time_stat, virtualmem::CodePtr}; -use crate::payload::{IseqPayload, get_or_create_iseq_payload, payload_ptr_as_mut}; +use crate::payload::{IseqPayload, IseqVersionRef, get_or_create_iseq_payload}; use crate::stats::Counter::gc_time_ns; use crate::state::gc_mark_raw_samples; @@ -50,7 +51,13 @@ pub extern "C" fn rb_zjit_iseq_free(iseq: IseqPtr) { if !ZJITState::has_instance() { return; } + // TODO(Shopify/ruby#682): Free `IseqPayload` + let payload = get_or_create_iseq_payload(iseq); + for version in payload.versions.iter_mut() { + unsafe { version.as_mut() }.iseq = null(); + } + let invariants = ZJITState::get_invariants(); invariants.forget_iseq(iseq); } @@ -93,14 +100,16 @@ fn iseq_mark(payload: &IseqPayload) { // Mark objects baked in JIT code let cb = ZJITState::get_code_block(); - for &offset in payload.gc_offsets.iter() { - let value_ptr: *const u8 = offset.raw_ptr(cb); - // Creating an unaligned pointer is well defined unlike in C. - let value_ptr = value_ptr as *const VALUE; - - unsafe { - let object = value_ptr.read_unaligned(); - rb_gc_mark_movable(object); + for version in payload.versions.iter() { + for &offset in unsafe { version.as_ref() }.gc_offsets.iter() { + let value_ptr: *const u8 = offset.raw_ptr(cb); + // Creating an unaligned pointer is well defined unlike in C. + let value_ptr = value_ptr as *const VALUE; + + unsafe { + let object = value_ptr.read_unaligned(); + rb_gc_mark_movable(object); + } } } } @@ -115,8 +124,26 @@ fn iseq_update_references(payload: &mut IseqPayload) { } }); - // Move ISEQ references in IseqCall - for iseq_call in payload.iseq_calls.iter_mut() { + for &version in payload.versions.iter() { + iseq_version_update_references(version); + } +} + +fn iseq_version_update_references(mut version: IseqVersionRef) { + // Move ISEQ in the payload + unsafe { version.as_mut() }.iseq = unsafe { rb_gc_location(version.as_ref().iseq.into()) }.as_iseq(); + + // Move ISEQ references in incoming IseqCalls + for iseq_call in unsafe { version.as_mut() }.incoming.iter_mut() { + let old_iseq = iseq_call.iseq.get(); + let new_iseq = unsafe { rb_gc_location(VALUE(old_iseq as usize)) }.0 as IseqPtr; + if old_iseq != new_iseq { + iseq_call.iseq.set(new_iseq); + } + } + + // Move ISEQ references in outgoing IseqCalls + for iseq_call in unsafe { version.as_mut() }.outgoing.iter_mut() { let old_iseq = iseq_call.iseq.get(); let new_iseq = unsafe { rb_gc_location(VALUE(old_iseq as usize)) }.0 as IseqPtr; if old_iseq != new_iseq { @@ -126,7 +153,7 @@ fn iseq_update_references(payload: &mut IseqPayload) { // Move objects baked in JIT code let cb = ZJITState::get_code_block(); - for &offset in payload.gc_offsets.iter() { + for &offset in unsafe { version.as_ref() }.gc_offsets.iter() { let value_ptr: *const u8 = offset.raw_ptr(cb); // Creating an unaligned pointer is well defined unlike in C. let value_ptr = value_ptr as *const VALUE; @@ -146,9 +173,8 @@ fn iseq_update_references(payload: &mut IseqPayload) { } /// Append a set of gc_offsets to the iseq's payload -pub fn append_gc_offsets(iseq: IseqPtr, offsets: &Vec) { - let payload = get_or_create_iseq_payload(iseq); - payload.gc_offsets.extend(offsets); +pub fn append_gc_offsets(iseq: IseqPtr, mut version: IseqVersionRef, offsets: &Vec) { + unsafe { version.as_mut() }.gc_offsets.extend(offsets); // Call writebarrier on each newly added value let cb = ZJITState::get_code_block(); @@ -166,9 +192,8 @@ pub fn append_gc_offsets(iseq: IseqPtr, offsets: &Vec) { /// We do this when invalidation rewrites some code with a jump instruction /// and GC offsets are corrupted by the rewrite, assuming no on-stack code /// will step into the instruction with the GC offsets after invalidation. -pub fn remove_gc_offsets(payload_ptr: *mut IseqPayload, removed_range: &Range) { - let payload = payload_ptr_as_mut(payload_ptr); - payload.gc_offsets.retain(|&gc_offset| { +pub fn remove_gc_offsets(mut version: IseqVersionRef, removed_range: &Range) { + unsafe { version.as_mut() }.gc_offsets.retain(|&gc_offset| { let offset_range = gc_offset..(gc_offset.add_bytes(SIZEOF_VALUE)); !ranges_overlap(&offset_range, removed_range) }); diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index 1771f960c3f952..bc6f1fb5f564fa 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -176,7 +176,7 @@ impl From for SpecialObjectType { VM_SPECIAL_OBJECT_VMCORE => SpecialObjectType::VMCore, VM_SPECIAL_OBJECT_CBASE => SpecialObjectType::CBase, VM_SPECIAL_OBJECT_CONST_BASE => SpecialObjectType::ConstBase, - _ => panic!("Invalid special object type: {}", value), + _ => panic!("Invalid special object type: {value}"), } } } @@ -337,7 +337,7 @@ impl std::fmt::Display for RangeType { impl std::fmt::Debug for RangeType { fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { - write!(f, "{}", self) + write!(f, "{self}") } } @@ -346,7 +346,7 @@ impl From for RangeType { match flag { 0 => RangeType::Inclusive, 1 => RangeType::Exclusive, - _ => panic!("Invalid range flag: {}", flag), + _ => panic!("Invalid range flag: {flag}"), } } } @@ -369,7 +369,7 @@ impl TryFrom for SpecialBackrefSymbol { '`' => Ok(SpecialBackrefSymbol::PreMatch), '\'' => Ok(SpecialBackrefSymbol::PostMatch), '+' => Ok(SpecialBackrefSymbol::LastGroup), - c => Err(format!("invalid backref symbol: '{}'", c)), + c => Err(format!("invalid backref symbol: '{c}'")), } } } @@ -574,7 +574,7 @@ impl std::fmt::Display for SideExitReason { SideExitReason::UnhandledNewarraySend(VM_OPT_NEWARRAY_SEND_PACK) => write!(f, "UnhandledNewarraySend(PACK)"), SideExitReason::UnhandledNewarraySend(VM_OPT_NEWARRAY_SEND_PACK_BUFFER) => write!(f, "UnhandledNewarraySend(PACK_BUFFER)"), SideExitReason::UnhandledNewarraySend(VM_OPT_NEWARRAY_SEND_INCLUDE_P) => write!(f, "UnhandledNewarraySend(INCLUDE_P)"), - SideExitReason::UnhandledDuparraySend(method_id) => write!(f, "UnhandledDuparraySend({})", method_id), + SideExitReason::UnhandledDuparraySend(method_id) => write!(f, "UnhandledDuparraySend({method_id})"), SideExitReason::GuardType(guard_type) => write!(f, "GuardType({guard_type})"), SideExitReason::GuardTypeNot(guard_type) => write!(f, "GuardTypeNot({guard_type})"), SideExitReason::GuardBitEquals(value) => write!(f, "GuardBitEquals({})", value.print(&PtrPrintMap::identity())), @@ -3572,10 +3572,10 @@ impl Function { // rb_zjit_singleton_class_p also checks if it's a class if unsafe { rb_zjit_singleton_class_p(class) } { let class_name = get_class_name(unsafe { rb_class_attached_object(class) }); - format!("{}.{}", class_name, method_name) + format!("{class_name}.{method_name}") } else { let class_name = get_class_name(class); - format!("{}#{}", class_name, method_name) + format!("{class_name}#{method_name}") } } @@ -3682,6 +3682,19 @@ impl Function { // Don't bother re-inferring the type of val; we already know it. continue; } + Insn::LoadField { recv, offset, return_type, .. } if return_type.is_subtype(types::BasicObject) && + u32::try_from(offset).is_ok() => { + let offset = (offset as u32).to_usize(); + let recv_type = self.type_of(recv); + match recv_type.ruby_object() { + Some(recv_obj) if recv_obj.is_frozen() => { + let recv_ptr = recv_obj.as_ptr() as *const VALUE; + let val = unsafe { recv_ptr.byte_add(offset).read() }; + self.new_insn(Insn::Const { val: Const::Value(val) }) + } + _ => insn_id, + } + } Insn::AnyToString { str, .. } if self.is_a(str, types::String) => { self.make_equal_to(insn_id, str); // Don't bother re-inferring the type of str; we already know it. @@ -4372,7 +4385,7 @@ impl Function { .insert("name", function_name) .insert("passes", passes) .build(); - writeln!(file, "{}", json).unwrap(); + writeln!(file, "{json}").unwrap(); } /// Validates the following: @@ -4500,7 +4513,7 @@ impl Function { fn assert_subtype(&self, user: InsnId, operand: InsnId, expected: Type) -> Result<(), ValidationError> { let actual = self.type_of(operand); if !actual.is_subtype(expected) { - return Err(ValidationError::MismatchedOperandType(user, operand, format!("{}", expected), format!("{}", actual))); + return Err(ValidationError::MismatchedOperandType(user, operand, format!("{expected}"), format!("{actual}"))); } Ok(()) } diff --git a/zjit/src/hir/opt_tests.rs b/zjit/src/hir/opt_tests.rs index 2e20c8fe8a73e2..62ea7c11b0c672 100644 --- a/zjit/src/hir/opt_tests.rs +++ b/zjit/src/hir/opt_tests.rs @@ -9501,4 +9501,435 @@ mod hir_opt_tests { Return v65 "); } + + #[test] + fn test_fold_load_field_frozen_constant_object() { + // Basic case: frozen constant object with attr_accessor + eval(" + class TestFrozen + attr_accessor :a + def initialize + @a = 1 + end + end + + FROZEN_OBJ = TestFrozen.new.freeze + + def test = FROZEN_OBJ.a + test + test + "); + assert_snapshot!(hir_string("test"), @r" + fn test@:11: + bb0(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + Jump bb2(v1) + bb1(v4:BasicObject): + EntryPoint JIT(0) + Jump bb2(v4) + bb2(v6:BasicObject): + PatchPoint SingleRactorMode + PatchPoint StableConstantNames(0x1000, FROZEN_OBJ) + v20:HeapObject[VALUE(0x1008)] = Const Value(VALUE(0x1008)) + PatchPoint MethodRedefined(TestFrozen@0x1010, a@0x1018, cme:0x1020) + PatchPoint NoSingletonClass(TestFrozen@0x1010) + v25:HeapObject[VALUE(0x1008)] = GuardShape v20, 0x1048 + v27:Fixnum[1] = Const Value(1) + CheckInterrupts + Return v27 + "); + } + + #[test] + fn test_fold_load_field_frozen_multiple_ivars() { + // Frozen object with multiple instance variables + eval(" + class TestMultiIvars + attr_accessor :a, :b, :c + def initialize + @a = 10 + @b = 20 + @c = 30 + end + end + + MULTI_FROZEN = TestMultiIvars.new.freeze + + def test = MULTI_FROZEN.b + test + test + "); + assert_snapshot!(hir_string("test"), @r" + fn test@:13: + bb0(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + Jump bb2(v1) + bb1(v4:BasicObject): + EntryPoint JIT(0) + Jump bb2(v4) + bb2(v6:BasicObject): + PatchPoint SingleRactorMode + PatchPoint StableConstantNames(0x1000, MULTI_FROZEN) + v20:HeapObject[VALUE(0x1008)] = Const Value(VALUE(0x1008)) + PatchPoint MethodRedefined(TestMultiIvars@0x1010, b@0x1018, cme:0x1020) + PatchPoint NoSingletonClass(TestMultiIvars@0x1010) + v25:HeapObject[VALUE(0x1008)] = GuardShape v20, 0x1048 + v27:Fixnum[20] = Const Value(20) + CheckInterrupts + Return v27 + "); + } + + #[test] + fn test_fold_load_field_frozen_string_value() { + // Frozen object with a string ivar + eval(r#" + class TestFrozenStr + attr_accessor :name + def initialize + @name = "hello" + end + end + + FROZEN_STR = TestFrozenStr.new.freeze + + def test = FROZEN_STR.name + test + test + "#); + assert_snapshot!(hir_string("test"), @r" + fn test@:11: + bb0(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + Jump bb2(v1) + bb1(v4:BasicObject): + EntryPoint JIT(0) + Jump bb2(v4) + bb2(v6:BasicObject): + PatchPoint SingleRactorMode + PatchPoint StableConstantNames(0x1000, FROZEN_STR) + v20:HeapObject[VALUE(0x1008)] = Const Value(VALUE(0x1008)) + PatchPoint MethodRedefined(TestFrozenStr@0x1010, name@0x1018, cme:0x1020) + PatchPoint NoSingletonClass(TestFrozenStr@0x1010) + v25:HeapObject[VALUE(0x1008)] = GuardShape v20, 0x1048 + v27:StringExact[VALUE(0x1050)] = Const Value(VALUE(0x1050)) + CheckInterrupts + Return v27 + "); + } + + #[test] + fn test_fold_load_field_frozen_nil_value() { + // Frozen object with nil ivar + eval(" + class TestFrozenNil + attr_accessor :value + def initialize + @value = nil + end + end + + FROZEN_NIL = TestFrozenNil.new.freeze + + def test = FROZEN_NIL.value + test + test + "); + assert_snapshot!(hir_string("test"), @r" + fn test@:11: + bb0(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + Jump bb2(v1) + bb1(v4:BasicObject): + EntryPoint JIT(0) + Jump bb2(v4) + bb2(v6:BasicObject): + PatchPoint SingleRactorMode + PatchPoint StableConstantNames(0x1000, FROZEN_NIL) + v20:HeapObject[VALUE(0x1008)] = Const Value(VALUE(0x1008)) + PatchPoint MethodRedefined(TestFrozenNil@0x1010, value@0x1018, cme:0x1020) + PatchPoint NoSingletonClass(TestFrozenNil@0x1010) + v25:HeapObject[VALUE(0x1008)] = GuardShape v20, 0x1048 + v27:NilClass = Const Value(nil) + CheckInterrupts + Return v27 + "); + } + + #[test] + fn test_no_fold_load_field_unfrozen_object() { + // Non-frozen object should NOT be folded + eval(" + class TestUnfrozen + attr_accessor :a + def initialize + @a = 1 + end + end + + UNFROZEN_OBJ = TestUnfrozen.new + + def test = UNFROZEN_OBJ.a + test + test + "); + assert_snapshot!(hir_string("test"), @r" + fn test@:11: + bb0(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + Jump bb2(v1) + bb1(v4:BasicObject): + EntryPoint JIT(0) + Jump bb2(v4) + bb2(v6:BasicObject): + PatchPoint SingleRactorMode + PatchPoint StableConstantNames(0x1000, UNFROZEN_OBJ) + v20:HeapObject[VALUE(0x1008)] = Const Value(VALUE(0x1008)) + PatchPoint MethodRedefined(TestUnfrozen@0x1010, a@0x1018, cme:0x1020) + PatchPoint NoSingletonClass(TestUnfrozen@0x1010) + v25:HeapObject[VALUE(0x1008)] = GuardShape v20, 0x1048 + v26:BasicObject = LoadField v25, :@a@0x1049 + CheckInterrupts + Return v26 + "); + } + + #[test] + fn test_fold_load_field_frozen_with_attr_reader() { + // Using attr_reader instead of attr_accessor + eval(" + class TestAttrReader + attr_reader :value + def initialize(v) + @value = v + end + end + + FROZEN_READER = TestAttrReader.new(42).freeze + + def test = FROZEN_READER.value + test + test + "); + assert_snapshot!(hir_string("test"), @r" + fn test@:11: + bb0(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + Jump bb2(v1) + bb1(v4:BasicObject): + EntryPoint JIT(0) + Jump bb2(v4) + bb2(v6:BasicObject): + PatchPoint SingleRactorMode + PatchPoint StableConstantNames(0x1000, FROZEN_READER) + v20:HeapObject[VALUE(0x1008)] = Const Value(VALUE(0x1008)) + PatchPoint MethodRedefined(TestAttrReader@0x1010, value@0x1018, cme:0x1020) + PatchPoint NoSingletonClass(TestAttrReader@0x1010) + v25:HeapObject[VALUE(0x1008)] = GuardShape v20, 0x1048 + v27:Fixnum[42] = Const Value(42) + CheckInterrupts + Return v27 + "); + } + + #[test] + fn test_fold_load_field_frozen_symbol_value() { + // Frozen object with a symbol ivar + eval(" + class TestFrozenSym + attr_accessor :sym + def initialize + @sym = :hello + end + end + + FROZEN_SYM = TestFrozenSym.new.freeze + + def test = FROZEN_SYM.sym + test + test + "); + assert_snapshot!(hir_string("test"), @r" + fn test@:11: + bb0(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + Jump bb2(v1) + bb1(v4:BasicObject): + EntryPoint JIT(0) + Jump bb2(v4) + bb2(v6:BasicObject): + PatchPoint SingleRactorMode + PatchPoint StableConstantNames(0x1000, FROZEN_SYM) + v20:HeapObject[VALUE(0x1008)] = Const Value(VALUE(0x1008)) + PatchPoint MethodRedefined(TestFrozenSym@0x1010, sym@0x1018, cme:0x1020) + PatchPoint NoSingletonClass(TestFrozenSym@0x1010) + v25:HeapObject[VALUE(0x1008)] = GuardShape v20, 0x1048 + v27:StaticSymbol[:hello] = Const Value(VALUE(0x1050)) + CheckInterrupts + Return v27 + "); + } + + #[test] + fn test_fold_load_field_frozen_true_false() { + // Frozen object with boolean ivars + eval(" + class TestFrozenBool + attr_accessor :flag + def initialize + @flag = true + end + end + + FROZEN_TRUE = TestFrozenBool.new.freeze + + def test = FROZEN_TRUE.flag + test + test + "); + assert_snapshot!(hir_string("test"), @r" + fn test@:11: + bb0(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + Jump bb2(v1) + bb1(v4:BasicObject): + EntryPoint JIT(0) + Jump bb2(v4) + bb2(v6:BasicObject): + PatchPoint SingleRactorMode + PatchPoint StableConstantNames(0x1000, FROZEN_TRUE) + v20:HeapObject[VALUE(0x1008)] = Const Value(VALUE(0x1008)) + PatchPoint MethodRedefined(TestFrozenBool@0x1010, flag@0x1018, cme:0x1020) + PatchPoint NoSingletonClass(TestFrozenBool@0x1010) + v25:HeapObject[VALUE(0x1008)] = GuardShape v20, 0x1048 + v27:TrueClass = Const Value(true) + CheckInterrupts + Return v27 + "); + } + + #[test] + fn test_no_fold_load_field_dynamic_receiver() { + // Dynamic receiver (not a constant) should NOT be folded even if object is frozen + eval(" + class TestDynamic + attr_accessor :val + def initialize + @val = 99 + end + end + + def test(obj) = obj.val + o = TestDynamic.new.freeze + test o + test o + "); + assert_snapshot!(hir_string("test"), @r" + fn test@:9: + bb0(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + v2:BasicObject = GetLocal :obj, 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(TestDynamic@0x1000, val@0x1008, cme:0x1010) + PatchPoint NoSingletonClass(TestDynamic@0x1000) + v21:HeapObject[class_exact:TestDynamic] = GuardType v9, HeapObject[class_exact:TestDynamic] + v24:HeapObject[class_exact:TestDynamic] = GuardShape v21, 0x1038 + v25:BasicObject = LoadField v24, :@val@0x1039 + CheckInterrupts + Return v25 + "); + } + + #[test] + fn test_fold_load_field_frozen_nested_access() { + // Accessing multiple fields from frozen constant in sequence + eval(" + class TestNestedAccess + attr_accessor :x, :y + def initialize + @x = 100 + @y = 200 + end + end + + NESTED_FROZEN = TestNestedAccess.new.freeze + + def test = NESTED_FROZEN.x + NESTED_FROZEN.y + test + test + "); + assert_snapshot!(hir_string("test"), @r" + fn test@:12: + bb0(): + EntryPoint interpreter + v1:BasicObject = LoadSelf + Jump bb2(v1) + bb1(v4:BasicObject): + EntryPoint JIT(0) + Jump bb2(v4) + bb2(v6:BasicObject): + PatchPoint SingleRactorMode + PatchPoint StableConstantNames(0x1000, NESTED_FROZEN) + v28:HeapObject[VALUE(0x1008)] = Const Value(VALUE(0x1008)) + PatchPoint MethodRedefined(TestNestedAccess@0x1010, x@0x1018, cme:0x1020) + PatchPoint NoSingletonClass(TestNestedAccess@0x1010) + v39:HeapObject[VALUE(0x1008)] = GuardShape v28, 0x1048 + v50:Fixnum[100] = Const Value(100) + PatchPoint SingleRactorMode + PatchPoint StableConstantNames(0x1050, NESTED_FROZEN) + v34:HeapObject[VALUE(0x1008)] = Const Value(VALUE(0x1008)) + PatchPoint MethodRedefined(TestNestedAccess@0x1010, y@0x1058, cme:0x1060) + PatchPoint NoSingletonClass(TestNestedAccess@0x1010) + v42:HeapObject[VALUE(0x1008)] = GuardShape v34, 0x1048 + v51:Fixnum[200] = Const Value(200) + PatchPoint MethodRedefined(Integer@0x1088, +@0x1090, cme:0x1098) + v52:Fixnum[300] = Const Value(300) + IncrCounter inline_cfunc_optimized_send_count + CheckInterrupts + Return v52 + "); + } + + #[test] + fn test_dont_fold_load_field_with_primitive_return_type() { + eval(r#" + S = "abc".freeze + def test = S.bytesize + 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 SingleRactorMode + PatchPoint StableConstantNames(0x1000, S) + v20:StringExact[VALUE(0x1008)] = Const Value(VALUE(0x1008)) + PatchPoint MethodRedefined(String@0x1010, bytesize@0x1018, cme:0x1020) + PatchPoint NoSingletonClass(String@0x1010) + v24:CInt64 = LoadField v20, :len@0x1048 + v25:Fixnum = BoxFixnum v24 + IncrCounter inline_cfunc_optimized_send_count + CheckInterrupts + Return v25 + "); + } } diff --git a/zjit/src/hir_type/mod.rs b/zjit/src/hir_type/mod.rs index a7ffcd1b77e882..206d94f42bdb64 100644 --- a/zjit/src/hir_type/mod.rs +++ b/zjit/src/hir_type/mod.rs @@ -96,7 +96,7 @@ fn write_spec(f: &mut std::fmt::Formatter, printer: &TypePrinter) -> std::fmt::R Specialization::Int(val) if ty.is_subtype(types::CUInt8) => write!(f, "[{}]", val & u8::MAX as u64), Specialization::Int(val) if ty.is_subtype(types::CUInt16) => write!(f, "[{}]", val & u16::MAX as u64), Specialization::Int(val) if ty.is_subtype(types::CUInt32) => write!(f, "[{}]", val & u32::MAX as u64), - Specialization::Int(val) if ty.is_subtype(types::CUInt64) => write!(f, "[{}]", val), + Specialization::Int(val) if ty.is_subtype(types::CUInt64) => write!(f, "[{val}]"), Specialization::Int(val) if ty.is_subtype(types::CPtr) => write!(f, "[{}]", Const::CPtr(val as *const u8).print(printer.ptr_map)), Specialization::Int(val) => write!(f, "[{val}]"), Specialization::Double(val) => write!(f, "[{val}]"), diff --git a/zjit/src/invariants.rs b/zjit/src/invariants.rs index b6f5abe58bcd8e..749e0a9e1d38d9 100644 --- a/zjit/src/invariants.rs +++ b/zjit/src/invariants.rs @@ -3,7 +3,9 @@ use std::{collections::{HashMap, HashSet}, mem}; use crate::{backend::lir::{Assembler, asm_comment}, cruby::{ID, IseqPtr, RedefinitionFlag, VALUE, iseq_name, rb_callable_method_entry_t, rb_gc_location, ruby_basic_operators, src_loc, with_vm_lock}, hir::Invariant, options::debug, state::{ZJITState, zjit_enabled_p}, virtualmem::CodePtr}; -use crate::payload::IseqPayload; +use crate::payload::{IseqVersionRef, IseqStatus, get_or_create_iseq_payload}; +use crate::codegen::{MAX_ISEQ_VERSIONS, gen_iseq_call}; +use crate::cruby::{rb_iseq_reset_jit_func, iseq_get_location}; use crate::stats::with_time_stat; use crate::stats::Counter::invalidation_time_ns; use crate::gc::remove_gc_offsets; @@ -19,7 +21,25 @@ macro_rules! compile_patch_points { asm.compile(cb).expect("can write existing code"); }); // Stop marking GC offsets corrupted by the jump instruction - remove_gc_offsets(patch_point.payload_ptr, &written_range); + remove_gc_offsets(patch_point.version, &written_range); + + // If the ISEQ doesn't have max versions, invalidate this version. + let mut version = patch_point.version; + let iseq = unsafe { version.as_ref() }.iseq; + if !iseq.is_null() { + let payload = get_or_create_iseq_payload(iseq); + if unsafe { version.as_ref() }.status != IseqStatus::Invalidated && payload.versions.len() < MAX_ISEQ_VERSIONS { + unsafe { version.as_mut() }.status = IseqStatus::Invalidated; + unsafe { rb_iseq_reset_jit_func(version.as_ref().iseq) }; + + // Recompile JIT-to-JIT calls into the invalidated ISEQ + for incoming in unsafe { version.as_ref() }.incoming.iter() { + if let Err(err) = gen_iseq_call($cb, incoming) { + debug!("{err:?}: gen_iseq_call failed on PatchPoint: {}", iseq_get_location(incoming.iseq.get(), 0)); + } + } + } + } } }); }; @@ -32,17 +52,17 @@ struct PatchPoint { patch_point_ptr: CodePtr, /// Code pointer to a side exit side_exit_ptr: CodePtr, - /// Raw pointer to the ISEQ payload - payload_ptr: *mut IseqPayload, + /// ISEQ version to be invalidated + version: IseqVersionRef, } impl PatchPoint { /// PatchPointer constructor - fn new(patch_point_ptr: CodePtr, side_exit_ptr: CodePtr, payload_ptr: *mut IseqPayload) -> PatchPoint { + fn new(patch_point_ptr: CodePtr, side_exit_ptr: CodePtr, version: IseqVersionRef) -> PatchPoint { Self { patch_point_ptr, side_exit_ptr, - payload_ptr, + version, } } } @@ -206,13 +226,13 @@ pub fn track_no_ep_escape_assumption( iseq: IseqPtr, patch_point_ptr: CodePtr, side_exit_ptr: CodePtr, - payload_ptr: *mut IseqPayload, + version: IseqVersionRef, ) { let invariants = ZJITState::get_invariants(); invariants.no_ep_escape_iseq_patch_points.entry(iseq).or_default().insert(PatchPoint::new( patch_point_ptr, side_exit_ptr, - payload_ptr, + version, )); } @@ -227,13 +247,13 @@ pub fn track_bop_assumption( bop: ruby_basic_operators, patch_point_ptr: CodePtr, side_exit_ptr: CodePtr, - payload_ptr: *mut IseqPayload, + version: IseqVersionRef, ) { let invariants = ZJITState::get_invariants(); invariants.bop_patch_points.entry((klass, bop)).or_default().insert(PatchPoint::new( patch_point_ptr, side_exit_ptr, - payload_ptr, + version, )); } @@ -242,13 +262,13 @@ pub fn track_cme_assumption( cme: *const rb_callable_method_entry_t, patch_point_ptr: CodePtr, side_exit_ptr: CodePtr, - payload_ptr: *mut IseqPayload, + version: IseqVersionRef, ) { let invariants = ZJITState::get_invariants(); invariants.cme_patch_points.entry(cme).or_default().insert(PatchPoint::new( patch_point_ptr, side_exit_ptr, - payload_ptr, + version, )); } @@ -257,7 +277,7 @@ pub fn track_stable_constant_names_assumption( idlist: *const ID, patch_point_ptr: CodePtr, side_exit_ptr: CodePtr, - payload_ptr: *mut IseqPayload, + version: IseqVersionRef, ) { let invariants = ZJITState::get_invariants(); @@ -271,7 +291,7 @@ pub fn track_stable_constant_names_assumption( invariants.constant_state_patch_points.entry(id).or_default().insert(PatchPoint::new( patch_point_ptr, side_exit_ptr, - payload_ptr, + version, )); idx += 1; @@ -283,13 +303,13 @@ pub fn track_no_singleton_class_assumption( klass: VALUE, patch_point_ptr: CodePtr, side_exit_ptr: CodePtr, - payload_ptr: *mut IseqPayload, + version: IseqVersionRef, ) { let invariants = ZJITState::get_invariants(); invariants.no_singleton_class_patch_points.entry(klass).or_default().insert(PatchPoint::new( patch_point_ptr, side_exit_ptr, - payload_ptr, + version, )); } @@ -339,12 +359,16 @@ pub extern "C" fn rb_zjit_constant_state_changed(id: ID) { } /// Track the JIT code that assumes that the interpreter is running with only one ractor -pub fn track_single_ractor_assumption(patch_point_ptr: CodePtr, side_exit_ptr: CodePtr, payload_ptr: *mut IseqPayload) { +pub fn track_single_ractor_assumption( + patch_point_ptr: CodePtr, + side_exit_ptr: CodePtr, + version: IseqVersionRef, +) { let invariants = ZJITState::get_invariants(); invariants.single_ractor_patch_points.insert(PatchPoint::new( patch_point_ptr, side_exit_ptr, - payload_ptr, + version, )); } @@ -368,19 +392,23 @@ pub extern "C" fn rb_zjit_before_ractor_spawn() { }); } -pub fn track_no_trace_point_assumption(patch_point_ptr: CodePtr, side_exit_ptr: CodePtr, payload_ptr: *mut IseqPayload) { +pub fn track_no_trace_point_assumption( + patch_point_ptr: CodePtr, + side_exit_ptr: CodePtr, + version: IseqVersionRef, +) { let invariants = ZJITState::get_invariants(); invariants.no_trace_point_patch_points.insert(PatchPoint::new( patch_point_ptr, side_exit_ptr, - payload_ptr, + version, )); } #[unsafe(no_mangle)] pub extern "C" fn rb_zjit_tracing_invalidate_all() { use crate::payload::{get_or_create_iseq_payload, IseqStatus}; - use crate::cruby::{for_each_iseq, rb_iseq_reset_jit_func}; + use crate::cruby::for_each_iseq; if !zjit_enabled_p() { return; @@ -393,7 +421,9 @@ pub extern "C" fn rb_zjit_tracing_invalidate_all() { for_each_iseq(|iseq| { let payload = get_or_create_iseq_payload(iseq); - payload.status = IseqStatus::NotCompiled; + if let Some(version) = payload.versions.last_mut() { + unsafe { version.as_mut() }.status = IseqStatus::Invalidated; + } unsafe { rb_iseq_reset_jit_func(iseq) }; }); diff --git a/zjit/src/json.rs b/zjit/src/json.rs index 1e63ba7b425902..fa4b2168217868 100644 --- a/zjit/src/json.rs +++ b/zjit/src/json.rs @@ -43,9 +43,9 @@ impl Json { match self { Json::Null => writer.write_all(b"null"), Json::Bool(b) => writer.write_all(if *b { b"true" } else { b"false" }), - Json::Integer(i) => write!(writer, "{}", i), - Json::UnsignedInteger(u) => write!(writer, "{}", u), - Json::Floating(f) => write!(writer, "{}", f), + Json::Integer(i) => write!(writer, "{i}"), + Json::UnsignedInteger(u) => write!(writer, "{u}"), + Json::Floating(f) => write!(writer, "{f}"), Json::String(s) => return Self::write_str(writer, s), Json::Array(jsons) => return Self::write_array(writer, jsons), Json::Object(map) => return Self::write_object(writer, map), @@ -69,9 +69,9 @@ impl Json { '\x0C' => write!(writer, "\\f")?, ch if ch.is_control() => { let code_point = ch as u32; - write!(writer, "\\u{:04X}", code_point)? + write!(writer, "\\u{code_point:04X}")? } - _ => write!(writer, "{}", ch)?, + _ => write!(writer, "{ch}")?, }; } @@ -83,7 +83,7 @@ impl Json { writer.write_all(b"[")?; let mut prefix = ""; for item in jsons { - write!(writer, "{}", prefix)?; + write!(writer, "{prefix}")?; item.marshal(writer)?; prefix = ", "; } @@ -96,7 +96,7 @@ impl Json { let mut prefix = ""; for (k, v) in pairs { // Escape the keys, despite not being `Json::String` objects. - write!(writer, "{}", prefix)?; + write!(writer, "{prefix}")?; Self::write_str(writer, k)?; writer.write_all(b":")?; v.marshal(writer)?; @@ -112,7 +112,7 @@ impl std::fmt::Display for Json { let mut buf = Vec::new(); self.marshal(&mut buf).map_err(|_| std::fmt::Error)?; let s = String::from_utf8(buf).map_err(|_| std::fmt::Error)?; - write!(f, "{}", s) + write!(f, "{s}") } } @@ -225,8 +225,8 @@ impl From for JsonError { impl fmt::Display for JsonError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { - JsonError::FloatError(v) => write!(f, "Cannot serialize float {}", v), - JsonError::IoError(e) => write!(f, "{}", e), + JsonError::FloatError(v) => write!(f, "Cannot serialize float {v}"), + JsonError::IoError(e) => write!(f, "{e}"), } } } diff --git a/zjit/src/options.rs b/zjit/src/options.rs index ea9dc3249b82ff..40b49146b726a3 100644 --- a/zjit/src/options.rs +++ b/zjit/src/options.rs @@ -247,11 +247,11 @@ fn parse_jit_list(path_like: &str) -> HashSet { } } } else { - eprintln!("Failed to read JIT list from '{}'", path_like); + eprintln!("Failed to read JIT list from '{path_like}'"); } eprintln!("JIT list:"); for item in &result { - eprintln!(" {}", item); + eprintln!(" {item}"); } result } @@ -373,7 +373,7 @@ fn parse_option(str_ptr: *const std::os::raw::c_char) -> Option<()> { .write(true) .truncate(true) .open(opt_val) - .map_err(|e| eprintln!("Failed to open file '{}': {}", opt_val, e)) + .map_err(|e| eprintln!("Failed to open file '{opt_val}': {e}")) .ok(); let opt_val = std::fs::canonicalize(opt_val).unwrap_or_else(|_| opt_val.into()); options.dump_hir_graphviz = Some(opt_val); @@ -400,7 +400,7 @@ fn parse_option(str_ptr: *const std::os::raw::c_char) -> Option<()> { _ => { let valid_options = DUMP_LIR_ALL.iter().map(|opt| format!("{opt:?}")).collect::>().join(", "); eprintln!("invalid --zjit-dump-lir option: '{filter}'"); - eprintln!("valid --zjit-dump-lir options: all, {}", valid_options); + eprintln!("valid --zjit-dump-lir options: all, {valid_options}"); return None; } }; @@ -421,7 +421,7 @@ fn parse_option(str_ptr: *const std::os::raw::c_char) -> Option<()> { .write(true) .truncate(true) .open(opt_val) - .map_err(|e| eprintln!("Failed to open file '{}': {}", opt_val, e)) + .map_err(|e| eprintln!("Failed to open file '{opt_val}': {e}")) .ok(); let opt_val = std::fs::canonicalize(opt_val).unwrap_or_else(|_| opt_val.into()); options.log_compiled_iseqs = Some(opt_val); diff --git a/zjit/src/payload.rs b/zjit/src/payload.rs index 1fb3f919946dbb..8540d5e35c498e 100644 --- a/zjit/src/payload.rs +++ b/zjit/src/payload.rs @@ -1,4 +1,5 @@ use std::ffi::c_void; +use std::ptr::NonNull; use crate::codegen::IseqCallRef; use crate::stats::CompileError; use crate::{cruby::*, profile::IseqProfile, virtualmem::CodePtr}; @@ -6,27 +7,55 @@ use crate::{cruby::*, profile::IseqProfile, virtualmem::CodePtr}; /// This is all the data ZJIT stores on an ISEQ. We mark objects in this struct on GC. #[derive(Debug)] pub struct IseqPayload { - /// Compilation status of the ISEQ. It has the JIT code address of the first block if Compiled. - pub status: IseqStatus, - /// Type information of YARV instruction operands pub profile: IseqProfile, + /// JIT code versions. Different versions should have different assumptions. + pub versions: Vec, +} + +impl IseqPayload { + fn new(iseq_size: u32) -> Self { + Self { + profile: IseqProfile::new(iseq_size), + versions: vec![], + } + } +} + +/// JIT code version. When the same ISEQ is compiled with a different assumption, a new version is created. +#[derive(Debug)] +pub struct IseqVersion { + /// ISEQ pointer. Stored here to minimize the size of PatchPoint. + pub iseq: IseqPtr, + + /// Compilation status of the ISEQ. It has the JIT code address of the first block if Compiled. + pub status: IseqStatus, /// GC offsets of the JIT code. These are the addresses of objects that need to be marked. pub gc_offsets: Vec, - /// JIT-to-JIT calls in the ISEQ. The IseqPayload's ISEQ is the caller of it. - pub iseq_calls: Vec, + /// JIT-to-JIT calls from the ISEQ. The IseqPayload's ISEQ is the caller of it. + pub outgoing: Vec, + + /// JIT-to-JIT calls to the ISEQ. The IseqPayload's ISEQ is the callee of it. + pub incoming: Vec, } -impl IseqPayload { - fn new(iseq_size: u32) -> Self { - Self { +/// We use a raw pointer instead of Rc to save space for refcount +pub type IseqVersionRef = NonNull; + +impl IseqVersion { + /// Allocate a new IseqVersion to be compiled + pub fn new(iseq: IseqPtr) -> IseqVersionRef { + let version = Self { + iseq, status: IseqStatus::NotCompiled, - profile: IseqProfile::new(iseq_size), gc_offsets: vec![], - iseq_calls: vec![], - } + outgoing: vec![], + incoming: vec![], + }; + let version_ptr = Box::into_raw(Box::new(version)); + NonNull::new(version_ptr).expect("no null from Box") } } @@ -44,6 +73,7 @@ pub enum IseqStatus { Compiled(IseqCodePtrs), CantCompile(CompileError), NotCompiled, + Invalidated, } /// Get a pointer to the payload object associated with an ISEQ. Create one if none exists. diff --git a/zjit/src/state.rs b/zjit/src/state.rs index fd59161812a7ee..a807be3f12d4c9 100644 --- a/zjit/src/state.rs +++ b/zjit/src/state.rs @@ -241,7 +241,7 @@ impl ZJITState { return; } }; - if let Err(e) = writeln!(file, "{}", iseq_name) { + if let Err(e) = writeln!(file, "{iseq_name}") { eprintln!("ZJIT: Failed to write to file '{}': {}", filename.display(), e); } } diff --git a/zjit/src/stats.rs b/zjit/src/stats.rs index 7d54f40c8d5dbd..25f0c638be2998 100644 --- a/zjit/src/stats.rs +++ b/zjit/src/stats.rs @@ -285,6 +285,7 @@ make_counters! { } // compile_error_: Compile error reasons + compile_error_iseq_version_limit_reached, compile_error_iseq_stack_too_large, compile_error_exception_handler, compile_error_out_of_memory, @@ -428,6 +429,7 @@ pub fn send_fallback_counter_ptr_for_opcode(opcode: u32) -> *mut u64 { /// Reason why ZJIT failed to produce any JIT code #[derive(Clone, Debug, PartialEq)] pub enum CompileError { + IseqVersionLimitReached, IseqStackTooLarge, ExceptionHandler, OutOfMemory, @@ -441,9 +443,10 @@ pub fn exit_counter_for_compile_error(compile_error: &CompileError) -> Counter { use crate::stats::CompileError::*; use crate::stats::Counter::*; match compile_error { - IseqStackTooLarge => compile_error_iseq_stack_too_large, - ExceptionHandler => compile_error_exception_handler, - OutOfMemory => compile_error_out_of_memory, + IseqVersionLimitReached => compile_error_iseq_version_limit_reached, + IseqStackTooLarge => compile_error_iseq_stack_too_large, + ExceptionHandler => compile_error_exception_handler, + OutOfMemory => compile_error_out_of_memory, ParseError(parse_error) => match parse_error { StackUnderflow(_) => compile_error_parse_stack_underflow, MalformedIseq(_) => compile_error_parse_malformed_iseq, @@ -786,21 +789,21 @@ pub extern "C" fn rb_zjit_stats(_ec: EcPtr, _self: VALUE, target_key: VALUE) -> // Set not inlined cfunc counters let not_inlined_cfuncs = ZJITState::get_not_inlined_cfunc_counter_pointers(); for (signature, counter) in not_inlined_cfuncs.iter() { - let key_string = format!("not_inlined_cfuncs_{}", signature); + let key_string = format!("not_inlined_cfuncs_{signature}"); set_stat_usize!(hash, &key_string, **counter); } // Set not annotated cfunc counters let not_annotated_cfuncs = ZJITState::get_not_annotated_cfunc_counter_pointers(); for (signature, counter) in not_annotated_cfuncs.iter() { - let key_string = format!("not_annotated_cfuncs_{}", signature); + let key_string = format!("not_annotated_cfuncs_{signature}"); set_stat_usize!(hash, &key_string, **counter); } // Set ccall counters let ccall = ZJITState::get_ccall_counter_pointers(); for (signature, counter) in ccall.iter() { - let key_string = format!("ccall_{}", signature); + let key_string = format!("ccall_{signature}"); set_stat_usize!(hash, &key_string, **counter); } diff --git a/zjit/zjit.mk b/zjit/zjit.mk index 01274dc3073642..2116775a91a182 100644 --- a/zjit/zjit.mk +++ b/zjit/zjit.mk @@ -14,12 +14,6 @@ ZJIT_SRC_FILES = $(wildcard \ $(RUST_LIB): $(ZJIT_SRC_FILES) -# Because of Cargo cache, if the actual binary is not changed from the -# previous build, the mtime is preserved as the cached file. -# This means the target is not updated actually, and it will need to -# rebuild at the next build. -ZJIT_LIB_TOUCH = touch $@ - # Absolute path to match RUST_LIB rules to avoid picking # the "target" dir in the source directory through VPATH. BUILD_ZJIT_LIBS = $(TOP_BUILD_DIR)/$(ZJIT_LIBS) @@ -28,9 +22,17 @@ BUILD_ZJIT_LIBS = $(TOP_BUILD_DIR)/$(ZJIT_LIBS) ifneq ($(strip $(ZJIT_LIBS)),) $(BUILD_ZJIT_LIBS): $(ZJIT_SRC_FILES) $(ECHO) 'building Rust ZJIT (release mode)' - +$(Q) $(RUSTC) $(ZJIT_RUSTC_ARGS) - $(ZJIT_LIB_TOUCH) -endif + $(Q) $(RUSTC) $(ZJIT_RUSTC_ARGS) +else ifneq ($(strip $(RLIB_DIR)),) # combo build +# Absolute path to avoid VPATH ambiguity +ZJIT_RLIB = $(TOP_BUILD_DIR)/$(RLIB_DIR)/libzjit.rlib + +$(ZJIT_RLIB): $(ZJIT_SRC_FILES) + $(ECHO) 'building $(@F)' + $(Q) $(RUSTC) '-L$(@D)' --extern=jit $(ZJIT_RUSTC_ARGS) + +$(RUST_LIB): $(ZJIT_RLIB) +endif # ifneq ($(strip $(ZJIT_LIBS)),) # By using ZJIT_BENCH_OPTS instead of RUN_OPTS, you can skip passing the options to `make install` ZJIT_BENCH_OPTS = $(RUN_OPTS) --enable-gems