From 259a5f97cd39e5052e9fbe0cdf70afbd9c0012e9 Mon Sep 17 00:00:00 2001 From: Earlopain <14981592+Earlopain@users.noreply.github.com> Date: Tue, 25 Aug 2026 09:07:46 +0200 Subject: [PATCH 01/16] [ruby/prism] Fix extra doc docmment https://github.com/ruby/prism/commit/9df2a21995764880c6d27213e2c7960f2232c879 changed the comment style Now rdoc picks this up. The comment doesn't say much, seems fine to just remove https://github.com/ruby/prism/commit/f703c615ae --- prism/extension.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/prism/extension.c b/prism/extension.c index 2e3992474adf8a..7a5250f0d53d9b 100644 --- a/prism/extension.c +++ b/prism/extension.c @@ -1606,8 +1606,6 @@ Init_prism(void) { rb_ext_ractor_safe(true); #endif - /* Grab up references to all of the constants that we are going to need to - * reference throughout this extension. */ rb_cPrism = rb_define_module("Prism"); rb_cPrismNode = rb_define_class_under(rb_cPrism, "Node", rb_cObject); rb_cPrismSource = rb_define_class_under(rb_cPrism, "Source", rb_cObject); From f21a04fe83145ffdad21f7f954e816c18cda0130 Mon Sep 17 00:00:00 2001 From: Nobuyoshi Nakada Date: Tue, 25 Aug 2026 17:57:26 +0900 Subject: [PATCH 02/16] Fix maybe-uninitialized warning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With gcc 15.2.0 on aarch64 Ubuntu 26.4: ```console hash.c: In function ‘rb_hash_s_create’: hash.c:1982:20: warning: ‘hash’ may be used uninitialized [-Wmaybe-uninitialized] 1982 | return hash; | ^~~~ hash.c:1920:11: note: ‘hash’ was declared here 1920 | VALUE hash, tmp; | ^~~~ ``` --- hash.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/hash.c b/hash.c index 471d46049bc1eb..e263f65e27edc9 100644 --- a/hash.c +++ b/hash.c @@ -1946,6 +1946,7 @@ rb_hash_s_create(int argc, VALUE *argv, VALUE klass) return hash_new_capa(klass, 0); } + hash = 0; long i; for (i = 0; i < RARRAY_LEN(tmp); ++i) { VALUE e = RARRAY_AREF(tmp, i); @@ -1976,6 +1977,7 @@ rb_hash_s_create(int argc, VALUE *argv, VALUE klass) val = RARRAY_AREF(v, 1); case 1: key = RARRAY_AREF(v, 0); + ASSUME(hash); rb_hash_aset(hash, key, val); } } From 6f57a72f53e0ea043b17f68829231874a26a9ccc Mon Sep 17 00:00:00 2001 From: Nobuyoshi Nakada Date: Tue, 25 Aug 2026 17:57:36 +0900 Subject: [PATCH 03/16] Fix enum-compare warning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With gcc 15.2.0 on aarch64 Ubuntu 26.4: ```console io_buffer.c: In function ‘io_buffer_flags_for_new’: io_buffer.c:978:20: warning: comparison between ‘enum rb_io_buffer_flags’ and ‘enum ’ [-Wenum-compare] 978 | if (allocation == RB_IO_BUFFER_ALLOCATION_FLAGS) { | ^~ ``` --- io_buffer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/io_buffer.c b/io_buffer.c index f7283e9593ff13..e05d4119251103 100644 --- a/io_buffer.c +++ b/io_buffer.c @@ -1004,7 +1004,7 @@ io_buffer_flags_for_new(enum rb_io_buffer_flags flags, size_t size) enum rb_io_buffer_flags allocation = flags & RB_IO_BUFFER_ALLOCATION_FLAGS; RUBY_ASSERT(allocation != 0); - if (allocation == RB_IO_BUFFER_ALLOCATION_FLAGS) { + if ((unsigned int)allocation == RB_IO_BUFFER_ALLOCATION_FLAGS) { rb_raise(rb_eArgError, "Flags can't include both IO::Buffer::INTERNAL and IO::Buffer::MAPPED!"); } From a5338c320a4f4106144847f9a2af767a09f79c91 Mon Sep 17 00:00:00 2001 From: Nobuyoshi Nakada Date: Tue, 25 Aug 2026 18:51:44 +0900 Subject: [PATCH 04/16] Fix clobbered warning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With gcc 15.2.0 on aarch64 Ubuntu 26.4: ```console ractor_sync.c: In function ‘ractor_basket_new.constprop’: ractor_sync.c:1093:82: warning: argument ‘type’ might be clobbered by ‘longjmp’ or ‘vfork’ [-Wclobbered] 1093 | ractor_basket_new(rb_execution_context_t *ec, VALUE obj, enum ractor_basket_type type, bool exc) | ~~~~~~~~~~~~~~~~~~~~~~~~^~~~ ``` --- ractor_sync.c | 70 +++++++++++++++++++++++++++++---------------------- 1 file changed, 40 insertions(+), 30 deletions(-) diff --git a/ractor_sync.c b/ractor_sync.c index d8205775ba99ec..03e0adc0a4915d 100644 --- a/ractor_sync.c +++ b/ractor_sync.c @@ -1089,6 +1089,45 @@ ractor_prepare_payload(rb_execution_context_t *ec, VALUE obj, enum ractor_basket } } +#if RBIMPL_COMPILER_SINCE(GCC, 15, 0, 0) +/* GCC 15 produces false-positive -Wclobbered warnings after inlining + * this function into ractor_basket_new(). */ +NOINLINE(static void ractor_basket_build_payload(rb_execution_context_t *ec, struct ractor_basket *b, VALUE obj, enum ractor_basket_type type, bool exc)); +#endif +static void +ractor_basket_build_payload(rb_execution_context_t *ec, struct ractor_basket *b, VALUE obj, + enum ractor_basket_type type, bool exc) +{ + b->p.exception = exc; + if (type == basket_type_move) { + /* Serialize the graph into an off-heap courier; the sources become + * RactorMovedObject. While in flight there is no GC object left for the + * sender's GC to mark, sweep or move. The build publishes the courier into + * the basket as soon as it exists. */ + rb_ractor_courier_build_move(obj, &b->p.courier); + b->type = type; + b->p.v = Qfalse; + } + else { + bool marshaled = false; + VALUE v = ractor_prepare_payload(ec, obj, &type, &marshaled, &b->p.courier); + + if (type == basket_type_copy && marshaled) { + /* Take the dump off-heap: the sender's copy of it is ordinary garbage + * from here, so nothing of its heap is held while the message waits. */ + size_t mlen = (size_t)RSTRING_LEN(v); + char *mbuf = ALLOC_N(char, mlen > 0 ? mlen : 1); + b->p.marshaled = marshaled; + b->p.mbuf = mbuf; + b->p.mlen = mlen; + memcpy(mbuf, RSTRING_PTR(v), mlen); + v = Qundef; + } + b->type = type; + b->p.v = v; + } +} + static struct ractor_basket * ractor_basket_new(rb_execution_context_t *ec, VALUE obj, enum ractor_basket_type type, bool exc) { @@ -1099,46 +1138,17 @@ ractor_basket_new(rb_execution_context_t *ec, VALUE obj, enum ractor_basket_type struct ractor_basket *b = ractor_basket_alloc(); ractor_off_queue_add(cr, b); - volatile VALUE v = Qfalse; - bool marshaled = false; - char *mbuf = NULL; - size_t mlen = 0; - enum ruby_tag_type state; EC_PUSH_TAG(ec); if ((state = EC_EXEC_TAG()) == TAG_NONE) { - if (type == basket_type_move) { - /* Serialize the graph into an off-heap courier; the sources become - * RactorMovedObject. While in flight there is no GC object left for the - * sender's GC to mark, sweep or move. The build publishes the courier into - * the basket as soon as it exists. */ - rb_ractor_courier_build_move(obj, &b->p.courier); - } - else { - v = ractor_prepare_payload(ec, obj, &type, &marshaled, &b->p.courier); - if (type == basket_type_copy && marshaled) { - /* Take the dump off-heap: the sender's copy of it is ordinary garbage - * from here, so nothing of its heap is held while the message waits. */ - mlen = (size_t)RSTRING_LEN(v); - mbuf = ALLOC_N(char, mlen > 0 ? mlen : 1); - memcpy(mbuf, RSTRING_PTR(v), mlen); - v = Qundef; - } - } + ractor_basket_build_payload(ec, b, obj, type, exc); } EC_POP_TAG(); if (state != TAG_NONE) { - ruby_xfree(mbuf); ractor_basket_free(b); /* leaves the list and frees a courier already built */ EC_JUMP_TAG(ec, state); } - b->type = type; - b->p.exception = exc; - b->p.v = v; - b->p.marshaled = marshaled; - b->p.mbuf = mbuf; - b->p.mlen = mlen; return b; } From e77a77111224426ca18eaeff55d7ad1618cbacc9 Mon Sep 17 00:00:00 2001 From: Nobuyoshi Nakada Date: Tue, 25 Aug 2026 21:04:59 +0900 Subject: [PATCH 05/16] [Bug #22261] Keep GC root-marking frame alive Prevent a tail call from discarding saved machine-register roots before `rb_gc_mark_roots` scans the machine stack. --- gc/default/default.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/gc/default/default.c b/gc/default/default.c index 368b1a00c8982b..4e36f6d399b433 100644 --- a/gc/default/default.c +++ b/gc/default/default.c @@ -5925,6 +5925,7 @@ static void pinned_roots_mark(rb_objspace_t *objspace, rb_heap_t *heap); static void mark_roots(rb_objspace_t *objspace, const char **categoryp) { + VALUE objspace_guard = (VALUE)objspace; #define MARK_CHECKPOINT(category) do { \ if (categoryp) *categoryp = category; \ } while (0) @@ -5955,6 +5956,9 @@ mark_roots(rb_objspace_t *objspace, const char **categoryp) rb_gc_save_machine_context(); rb_gc_mark_roots(objspace, categoryp); + /* Keep this frame, including its saved registers, until root marking has + * scanned the machine stack. */ + RB_GC_GUARD(objspace_guard); gc_mark_set_parent_invalid(objspace); } From 6d4a6a939fb1721766dd93acdd0438c65482247c Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Fri, 14 Aug 2026 19:35:24 +0200 Subject: [PATCH 06/16] Parametize `RHASH_AR_TABLE_MAX_BOUND` For now it still returns a constant, but clarify when we're looking for the max bound, versus a miss, etc. --- hash.c | 62 ++++++++++++++++++++++++++++++++-------------------------- 1 file changed, 34 insertions(+), 28 deletions(-) diff --git a/hash.c b/hash.c index e263f65e27edc9..fe1a8bbb7fbae1 100644 --- a/hash.c +++ b/hash.c @@ -409,8 +409,14 @@ typedef st_index_t st_hash_t; * RHASH_ST_TABLE points st_table. */ -#define RHASH_AR_TABLE_MAX_BOUND RHASH_AR_TABLE_MAX_SIZE -#define RHASH_AR_TABLE_CONVERTED_TO_ST_TABLE (RHASH_AR_TABLE_MAX_BOUND + 1) +static inline unsigned int +RHASH_AR_TABLE_MAX_BOUND(VALUE h) +{ + return RHASH_AR_TABLE_MAX_SIZE; +} + +#define RHASH_AR_TABLE_CONVERTED_TO_ST_TABLE (RHASH_AR_TABLE_MAX_SIZE + 1) +#define RHASH_AR_TABLE_MISS RHASH_AR_TABLE_MAX_SIZE #define RHASH_AR_TABLE_REF(hash, n) (&RHASH_AR_TABLE(hash)->pairs[n]) #define RHASH_AR_CLEARED_HINT 0xff @@ -550,7 +556,7 @@ static inline void RHASH_AR_TABLE_BOUND_SET(VALUE h, st_index_t n) { HASH_ASSERT(RHASH_AR_TABLE_P(h)); - HASH_ASSERT(n <= RHASH_AR_TABLE_MAX_BOUND); + HASH_ASSERT(n <= RHASH_AR_TABLE_MAX_BOUND(h)); RBASIC(h)->flags &= ~RHASH_AR_TABLE_BOUND_MASK; RBASIC(h)->flags |= n << RHASH_AR_TABLE_BOUND_SHIFT; @@ -560,7 +566,7 @@ static inline void RHASH_AR_TABLE_SIZE_SET(VALUE h, st_index_t n) { HASH_ASSERT(RHASH_AR_TABLE_P(h)); - HASH_ASSERT(n <= RHASH_AR_TABLE_MAX_SIZE); + HASH_ASSERT(n <= RHASH_AR_TABLE_MAX_BOUND(h)); RBASIC(h)->flags &= ~RHASH_AR_TABLE_SIZE_MASK; RBASIC(h)->flags |= n << RHASH_AR_TABLE_SIZE_SHIFT; @@ -646,7 +652,7 @@ ar_hint_first_match(ar_hint_t needle, VALUE haystack) return index; } -// Returns the bin index if found, RHASH_AR_TABLE_MAX_BOUND if not found, +// Returns the bin index if found, RHASH_AR_TABLE_MISS if not found, // or RHASH_AR_TABLE_CONVERTED_TO_ST_TABLE if #eql? or a Thread converted the hash to st_table. static unsigned ar_find_entry_hint(VALUE hash, ar_hint_t hint, st_data_t key) @@ -655,7 +661,7 @@ ar_find_entry_hint(VALUE hash, ar_hint_t hint, st_data_t key) if (LIKELY(first_match >= RHASH_AR_TABLE_BOUND(hash))) { RB_DEBUG_COUNTER_INC(artable_hint_notfound); - return RHASH_AR_TABLE_MAX_BOUND; + return RHASH_AR_TABLE_MISS; } RUBY_ASSERT(RHASH_AR_TABLE(hash)->ar_hint.ary[first_match] == hint); @@ -690,7 +696,7 @@ ar_find_entry_hint(VALUE hash, ar_hint_t hint, st_data_t key) } RB_DEBUG_COUNTER_INC(artable_hint_notfound); - return RHASH_AR_TABLE_MAX_BOUND; + return RHASH_AR_TABLE_MISS; } static unsigned @@ -828,14 +834,14 @@ ar_add_direct_with_hash(VALUE hash, st_data_t key, st_data_t val, st_hash_t hash { unsigned bin = RHASH_AR_TABLE_BOUND(hash); - if (RHASH_AR_TABLE_SIZE(hash) >= RHASH_AR_TABLE_MAX_SIZE) { + if (RHASH_AR_TABLE_SIZE(hash) >= RHASH_AR_TABLE_MAX_BOUND(hash)) { return 1; } else { - if (UNLIKELY(bin >= RHASH_AR_TABLE_MAX_BOUND)) { + if (UNLIKELY(bin >= RHASH_AR_TABLE_MAX_BOUND(hash))) { bin = ar_compact_table(hash); } - HASH_ASSERT(bin < RHASH_AR_TABLE_MAX_BOUND); + HASH_ASSERT(bin < RHASH_AR_TABLE_MAX_BOUND(hash)); ar_set_entry(hash, bin, key, val, hash_value); RHASH_AR_TABLE_BOUND_SET(hash, bin+1); @@ -950,7 +956,7 @@ ar_foreach_check(VALUE hash, st_foreach_check_callback_func *func, st_data_t arg if (UNLIKELY(ret == RHASH_AR_TABLE_CONVERTED_TO_ST_TABLE)) { ensure_ar_table(hash); } - if (ret == RHASH_AR_TABLE_MAX_BOUND) { + if (ret == RHASH_AR_TABLE_MISS) { (*func)(0, 0, arg, 1); return 2; } @@ -978,7 +984,7 @@ ar_update(VALUE hash, st_data_t key, st_update_callback_func *func, st_data_t arg) { int retval, existing; - unsigned bin = RHASH_AR_TABLE_MAX_BOUND; + unsigned bin = RHASH_AR_TABLE_MISS; st_data_t value = 0, old_key; st_hash_t hash_value = ar_do_hash(key); @@ -992,7 +998,7 @@ ar_update(VALUE hash, st_data_t key, if (UNLIKELY(bin == RHASH_AR_TABLE_CONVERTED_TO_ST_TABLE)) { return -1; } - existing = (bin != RHASH_AR_TABLE_MAX_BOUND) ? TRUE : FALSE; + existing = (bin != RHASH_AR_TABLE_MISS); } else { existing = FALSE; @@ -1048,14 +1054,14 @@ ar_insert(VALUE hash, st_data_t key, st_data_t value) if (UNLIKELY(bin == RHASH_AR_TABLE_CONVERTED_TO_ST_TABLE)) { return -1; } - if (bin == RHASH_AR_TABLE_MAX_BOUND) { - if (RHASH_AR_TABLE_SIZE(hash) >= RHASH_AR_TABLE_MAX_SIZE) { - return -1; - } - else if (bin >= RHASH_AR_TABLE_MAX_BOUND) { - bin = ar_compact_table(hash); + + if (bin == RHASH_AR_TABLE_MISS) { + if (RHASH_AR_TABLE_SIZE(hash) == RHASH_AR_TABLE_MAX_BOUND(hash)) { + return -1; } - HASH_ASSERT(bin < RHASH_AR_TABLE_MAX_BOUND); + + bin = ar_compact_table(hash); + HASH_ASSERT(bin < RHASH_AR_TABLE_MAX_BOUND(hash)); ar_set_entry(hash, bin, key, value, hash_value); RHASH_AR_TABLE_BOUND_SET(hash, bin+1); @@ -1081,20 +1087,20 @@ ar_lookup(VALUE hash, st_data_t key, st_data_t *value) return st_lookup(RHASH_ST_TABLE(hash), key, value); } unsigned bin = ar_find_entry(hash, hash_value, key); + if (UNLIKELY(bin == RHASH_AR_TABLE_CONVERTED_TO_ST_TABLE)) { return st_lookup(RHASH_ST_TABLE(hash), key, value); } - if (bin == RHASH_AR_TABLE_MAX_BOUND) { + if (bin == RHASH_AR_TABLE_MISS) { return 0; } - else { - HASH_ASSERT(bin < RHASH_AR_TABLE_MAX_BOUND); - if (value != NULL) { - *value = RHASH_AR_TABLE_REF(hash, bin)->val; - } - return 1; + + HASH_ASSERT(bin < RHASH_AR_TABLE_MAX_BOUND(hash)); + if (value != NULL) { + *value = RHASH_AR_TABLE_REF(hash, bin)->val; } + return 1; } } @@ -1114,7 +1120,7 @@ ar_delete(VALUE hash, st_data_t *key, st_data_t *value) return st_delete(RHASH_ST_TABLE(hash), key, value); } - if (bin == RHASH_AR_TABLE_MAX_BOUND) { + if (bin == RHASH_AR_TABLE_MISS) { if (value != 0) *value = 0; return 0; } From 60a4894bdb3832f044bee27de95d6b15992e8d07 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Sat, 15 Aug 2026 16:25:06 +0200 Subject: [PATCH 07/16] Dynamic ar_table size Instead of always allocating `ar_table` backed hashes in 160B slots for them to have capacity of `RHASH_AR_TABLE_MAX_SIZE` (`8`), they can now be allocated in any slot size between `64B` and `160B`. `64B` is the minimum slot size that still allows transitioning to an `st_table` if necessary. The assumption is that there is a large number of literal hashes and keyword argument hashes being built, and the overwhelming majority of them aren't appended to. When this assumption hold, this patch saves memory and reduce GC pressure by allocating in a smaller slot than before. However when the assumption doesn't hold, it has the oppositve effect as the Hash will transition to an `st_table` when before it may have stayed an `ar_table`. Currently the main blocked for this change is that the compiler has a tendency to generate code that make the final Hash size unknown. For instance: `{a: 4, b: 4 + 1}` is compiled as: ``` 0000 duphash {a: 4} ( 1)[Li] 0002 putspecialobject 1 0004 swap 0005 putobject :b 0007 putobject 4 0009 putobject_INT2FIX_1_ 0010 opt_plus [CcCr] 0012 opt_send_without_block ``` Here `duphash` has no information about how large of a hash it needs to allocate. For this patch to be more efficient the compiler should generate something like: ``` 0000 putobject {a: 4} ( 1)[Li] 0002 putspecialobject 1 0004 swap 0005 putobject :b 0007 putobject 4 0009 putobject_INT2FIX_1_ 0010 opt_plus [CcCr] 0012 opt_send_without_block ``` So that it's `hash_merge_ptr` that is responsible for allocating the final hash, given it does have a much better clue of how large it would need to be. Demo: ```ruby require 'objspace' def size(hash) puts "size: #{hash.size}\tmemsize: #{ObjectSpace.memsize_of(hash)}" end size({}) size(a:1) size(a:1, b:2) size(a:1, b:2, c:3) size(a:1, b:2, c:3, d:4) size(a:1, b:2, c:3, d:4, e:5) size(a:1, b:2, c:3, d:4, e:5, f:6) size(a:1, b:2, c:3, d:4, e:5, f:6, g:7) size(a:1, b:2, c:3, d:4, e:5, f:6, g:7, h:8) size(a:1, b:2, c:3, d:4, e:5, f:6, g:7, h:8, i:9) ``` Before: ``` size: 0 memsize: 160 size: 1 memsize: 160 size: 2 memsize: 160 size: 3 memsize: 160 size: 4 memsize: 160 size: 5 memsize: 160 size: 6 memsize: 160 size: 7 memsize: 160 size: 8 memsize: 160 size: 9 memsize: 464 ``` After: ``` size: 0 memsize: 64 size: 1 memsize: 64 size: 2 memsize: 64 size: 3 memsize: 80 size: 4 memsize: 96 size: 5 memsize: 128 size: 6 memsize: 128 size: 7 memsize: 160 size: 8 memsize: 160 size: 9 memsize: 448 ``` --- gc.c | 13 ++- hash.c | 145 +++++++++++++++++++++++++-------- internal/hash.h | 1 + internal/st.h | 3 + object.c | 12 ++- st.c | 51 ++++++++++++ test/ruby/test_hash.rb | 17 +++- zjit.h | 2 +- zjit/src/codegen.rs | 4 +- zjit/src/cruby_bindings.inc.rs | 2 +- 10 files changed, 206 insertions(+), 44 deletions(-) diff --git a/gc.c b/gc.c index d21019dbab24c7..5101ba902f3e41 100644 --- a/gc.c +++ b/gc.c @@ -3699,10 +3699,17 @@ rb_gc_obj_optimal_size(VALUE obj) case T_HASH: { - if (RB_OBJ_FROZEN(obj) && RHASH_AR_TABLE_P(obj)) { - return sizeof(struct RHash) + offsetof(ar_table, pairs) + RHASH_AR_TABLE_BOUND(obj) * sizeof(ar_table_pair); + const size_t st_size = sizeof(struct RHash) + sizeof(st_table); + if (RHASH_ST_TABLE_P(obj)) { + return st_size; } - return sizeof(struct RHash) + (RHASH_ST_TABLE_P(obj) ? sizeof(st_table) : sizeof(ar_table)); + + const size_t ar_size = sizeof(struct RHash) + offsetof(ar_table, pairs) + RHASH_AR_TABLE_BOUND(obj) * sizeof(ar_table_pair); + if (OBJ_FROZEN(obj) || ar_size > st_size) { + return ar_size; + } + + return st_size; } default: diff --git a/hash.c b/hash.c index fe1a8bbb7fbae1..37fad66abcaaf6 100644 --- a/hash.c +++ b/hash.c @@ -412,7 +412,26 @@ typedef st_index_t st_hash_t; static inline unsigned int RHASH_AR_TABLE_MAX_BOUND(VALUE h) { - return RHASH_AR_TABLE_MAX_SIZE; + size_t usable_space = rb_obj_shape_slot_size(h) - sizeof(struct RHash) - offsetof(ar_table, pairs); + usable_space /= sizeof(ar_table_pair); +#if SIZEOF_VALUE == 8 + RBIMPL_ASSERT_OR_ASSUME(usable_space <= RHASH_AR_TABLE_MAX_SIZE); + return (unsigned)usable_space; +#else + return usable_space <= RHASH_AR_TABLE_MAX_SIZE ? (unsigned)usable_space : RHASH_AR_TABLE_MAX_SIZE; +#endif +} + +static inline size_t +ar_table_memsize(size_t capa) +{ + return offsetof(ar_table, pairs) + capa * sizeof(ar_table_pair); +} + +static inline size_t +ar_memsize(size_t capa) +{ + return sizeof(struct RHash) + ar_table_memsize(capa); } #define RHASH_AR_TABLE_CONVERTED_TO_ST_TABLE (RHASH_AR_TABLE_MAX_SIZE + 1) @@ -539,6 +558,7 @@ RHASH_TABLE_EMPTY_P(VALUE hash) static void hash_st_table_init(VALUE hash, const struct st_hash_type *type, st_index_t size) { + RUBY_ASSERT(rb_gc_obj_slot_size(hash) >= sizeof(struct RHash) + sizeof(st_table)); st_init_existing_table_with_size(RHASH_ST_TABLE(hash), type, size); RHASH_SET_ST_FLAG(hash); } @@ -603,11 +623,10 @@ RHASH_AR_TABLE_SIZE_DEC(VALUE h) static inline void RHASH_AR_TABLE_CLEAR(VALUE h) { - RUBY_ASSERT(rb_gc_obj_slot_size(h) >= sizeof(struct RHash) + sizeof(ar_table)); RBASIC(h)->flags &= ~RHASH_AR_TABLE_SIZE_MASK; RBASIC(h)->flags &= ~RHASH_AR_TABLE_BOUND_MASK; - memset(RHASH_AR_TABLE(h), 0, sizeof(ar_table)); + memset(RHASH_AR_TABLE(h), 0, rb_obj_shape_slot_size(h) - sizeof(struct RHash)); } NOINLINE(static int ar_equal(VALUE x, VALUE y)); @@ -757,7 +776,7 @@ ar_force_convert_table(VALUE hash, const char *file, int line) st_hash_t hashes[RHASH_AR_TABLE_MAX_SIZE]; unsigned int bound, size; - RUBY_ASSERT(rb_gc_obj_slot_size(hash) >= sizeof(struct RHash) + sizeof(ar_table)); + RUBY_ASSERT(rb_gc_obj_slot_size(hash) >= sizeof(struct RHash) + sizeof(st_table)); // prepare hash values while (1) { @@ -790,6 +809,28 @@ ar_force_convert_table(VALUE hash, const char *file, int line) } } +static void +ar_compact_into(VALUE dst, VALUE src) +{ + ar_table_pair *dst_pairs = RHASH_AR_TABLE(dst)->pairs; + ar_table_pair *src_pairs = RHASH_AR_TABLE(src)->pairs; + + const unsigned src_bound = RHASH_AR_TABLE_BOUND(src); + const unsigned src_size = RHASH_AR_TABLE_SIZE(src); + + unsigned j=0; + for (unsigned i = 0; i < src_bound; i++) { + if (!ar_cleared_entry(src, i)) { + dst_pairs[j] = src_pairs[i]; + ar_hint_set_hint(dst, j, (st_hash_t)ar_hint(src, i)); + j++; + } + } + RHASH_AR_TABLE_BOUND_SET(dst, src_size); + RHASH_AR_TABLE_SIZE_SET(dst, src_size); + hash_verify(dst); +} + static int ar_compact_table(VALUE hash) { @@ -1040,11 +1081,9 @@ ar_update(VALUE hash, st_data_t key, } static int -ar_insert(VALUE hash, st_data_t key, st_data_t value) +ar_insert_direct(VALUE hash, st_data_t key, st_data_t value, st_hash_t hash_value) { unsigned bin = RHASH_AR_TABLE_BOUND(hash); - st_hash_t hash_value = ar_do_hash(key); - if (UNLIKELY(!RHASH_AR_TABLE_P(hash))) { // `#hash` changes ar_table -> st_table return -1; @@ -1074,6 +1113,13 @@ ar_insert(VALUE hash, st_data_t key, st_data_t value) } } +static int +ar_insert(VALUE hash, st_data_t key, st_data_t value) +{ + st_hash_t hash_value = ar_do_hash(key); + return ar_insert_direct(hash, key, value, hash_value); +} + static int ar_lookup(VALUE hash, st_data_t key, st_data_t *value) { @@ -1199,16 +1245,21 @@ ar_values(VALUE hash, st_data_t *values, st_index_t size) static ar_table* ar_copy(VALUE hash1, VALUE hash2) { - RUBY_ASSERT(rb_gc_obj_slot_size(hash1) >= sizeof(struct RHash) + sizeof(ar_table)); - ar_table *old_tab = RHASH_AR_TABLE(hash2); + RUBY_ASSERT(rb_gc_obj_slot_size(hash1) >= ar_memsize(RHASH_SIZE(hash2))); ar_table *new_tab = RHASH_AR_TABLE(hash1); unsigned int bound = RHASH_AR_TABLE_BOUND(hash2); + unsigned int size = RHASH_AR_TABLE_SIZE(hash2); + if (UNLIKELY(bound != size)) { + ar_compact_into(hash1, hash2); + return new_tab; + } + + ar_table *old_tab = RHASH_AR_TABLE(hash2); new_tab->ar_hint.word = old_tab->ar_hint.word; MEMCPY(&new_tab->pairs, &old_tab->pairs, ar_table_pair, bound); RHASH_AR_TABLE_BOUND_SET(hash1, bound); RHASH_AR_TABLE_SIZE_SET(hash1, RHASH_AR_TABLE_SIZE(hash2)); - rb_gc_writebarrier_remember(hash1); return new_tab; @@ -1490,16 +1541,19 @@ compact_after_delete(VALUE hash) static inline size_t hash_slot_size(size_t capa, bool frozen) { + const size_t st_size = sizeof(struct RHash) + sizeof(st_table); if (capa > RHASH_AR_TABLE_MAX_SIZE) { - return sizeof(struct RHash) + sizeof(st_table); + return st_size; } + const size_t ar_size = ar_memsize(capa); // If the hash is immutable, we can allocate a slot with exactly as much space as needed. - if (frozen) { - return sizeof(struct RHash) + offsetof(ar_table, pairs) + capa * sizeof(ar_table_pair); + // But if mutable, we must ensure we have enough space to transition to an st_table. + if (frozen || ar_size >= st_size) { + return ar_size; } - return sizeof(struct RHash) + sizeof(ar_table); + return st_size; } static VALUE @@ -1524,6 +1578,9 @@ hash_init_capa(VALUE hash, size_t size) if (size > RHASH_AR_TABLE_MAX_SIZE) { hash_st_table_init(hash, &objhash, size); } + else { + RUBY_ASSERT(RHASH_AR_TABLE_MAX_BOUND(hash) >= size); + } return hash; } @@ -1545,15 +1602,6 @@ rb_hash_alloc_copy(VALUE klass, VALUE src) return hash_alloc_capa(klass, RHASH_SIZE(src)); } -#if USE_ZJIT -size_t -rb_zjit_hash_new_size(VALUE *flags_out) -{ - *flags_out = T_HASH; - return hash_slot_size(0, false); -} -#endif - static VALUE empty_hash_alloc(VALUE klass) { @@ -1598,14 +1646,32 @@ rb_hash_alloc_fixed_size(VALUE klass, st_index_t size) return hash_init_capa(hash_alloc(klass, 0, Qnil, size, true), size); } +static int +ar_add_direct_i(st_data_t key, st_data_t value, st_data_t hash_value, st_data_t arg) +{ + VALUE ret = (VALUE)arg; + ar_insert_direct(ret, key, value, hash_value); + return ST_CONTINUE; +} + static VALUE hash_copy(VALUE ret, VALUE hash) { RUBY_ASSERT(RHASH_SIZE(ret) == 0); + if (RHASH_ST_TABLE_P(ret)) { + RUBY_ASSERT(RHASH_ST_TABLE(ret)->entries == NULL); + RHASH_UNSET_ST_FLAG(ret); + } if (rb_hash_compare_by_id_p(hash)) { + // If `hash` is an ar_table it can't be `compare_by_identity?`. + RUBY_ASSERT(RHASH_ST_TABLE_P(hash)); + RHASH_SET_ST_FLAG(ret); rb_gc_register_pinning_obj(ret); } + else if (RHASH_AR_TABLE_MAX_BOUND(ret) < RHASH_SIZE(hash)) { + RHASH_SET_ST_FLAG(ret); + } if (RHASH_AR_TABLE_P(hash)) { if (RHASH_AR_TABLE_P(ret)) { @@ -1613,11 +1679,7 @@ hash_copy(VALUE ret, VALUE hash) } else { st_table *tab = RHASH_ST_TABLE(ret); - - // If `hash` is an ar_table it can't be `compare_by_identity?`. - RUBY_ASSERT(!rb_hash_compare_by_id_p(hash)); - RUBY_ASSERT(RHASH_ST_TABLE(ret)->entries == NULL); - st_init_existing_table_with_size(RHASH_ST_TABLE(ret), &objhash, RHASH_SIZE(hash)); + st_init_existing_table_with_size(tab, &objhash, RHASH_SIZE(hash)); int bound = RHASH_AR_TABLE_BOUND(hash); for (int i = 0; i < bound; i++) { @@ -1631,9 +1693,13 @@ hash_copy(VALUE ret, VALUE hash) } } else { - RHASH_SET_ST_FLAG(ret); - st_replace(RHASH_ST_TABLE(ret), RHASH_ST_TABLE(hash)); - rb_gc_writebarrier_remember(ret); + if (RHASH_AR_TABLE_P(ret)) { + rb_st_foreach_with_hash(RHASH_ST_TABLE(hash), ar_add_direct_i, (st_data_t)ret); + } + else { + st_replace(RHASH_ST_TABLE(ret), RHASH_ST_TABLE(hash)); + rb_gc_writebarrier_remember(ret); + } } return ret; } @@ -1663,6 +1729,9 @@ hash_dup_capa(VALUE hash, size_t capa) if (capa > RHASH_AR_TABLE_MAX_SIZE) { RHASH_SET_ST_FLAG(ret); } + else { + RUBY_ASSERT(RHASH_AR_TABLE_MAX_BOUND(ret) >= capa); + } hash_copy(ret, hash); return ret; } @@ -1685,6 +1754,14 @@ rb_hash_resurrect(VALUE hash) } #if USE_ZJIT +size_t +rb_zjit_hash_new_size(VALUE *flags_out, size_t size) +{ + RUBY_ASSERT(size <= RHASH_AR_TABLE_MAX_SIZE); + *flags_out = T_HASH; + return hash_slot_size(size, false); +} + bool rb_zjit_hash_dup_can_fastpath(VALUE hash, size_t *alloc_size_out, VALUE *flags_out, VALUE *ifnone_out, long *bound_out) { @@ -1693,7 +1770,7 @@ rb_zjit_hash_dup_can_fastpath(VALUE hash, size_t *alloc_size_out, VALUE *flags_o const unsigned int bound = RHASH_AR_TABLE_BOUND(hash); - *alloc_size_out = hash_slot_size(0, false); + *alloc_size_out = hash_slot_size(bound, false); *flags_out = T_HASH | ((VALUE)RHASH_AR_TABLE_SIZE(hash) << RHASH_AR_TABLE_SIZE_SHIFT) | ((VALUE)bound << RHASH_AR_TABLE_BOUND_SHIFT); @@ -1859,7 +1936,7 @@ rb_hash_init(rb_execution_context_t *ec, VALUE hash, VALUE capa_value, VALUE ifn if (capa_value != INT2FIX(0)) { long capa = NUM2LONG(capa_value); - if (capa > RHASH_AR_TABLE_MAX_SIZE && RHASH_SIZE(hash) == 0 && RHASH_AR_TABLE_P(hash)) { + if (capa > 0 && RHASH_AR_TABLE_P(hash) && RHASH_SIZE(hash) == 0 && capa > RHASH_AR_TABLE_MAX_BOUND(hash)) { hash_st_table_init(hash, &objhash, capa); } } @@ -5248,7 +5325,7 @@ rb_hash_bulk_insert(long argc, const VALUE *argv, VALUE hash) st_index_t size = argc / 2; if (RHASH_AR_TABLE_P(hash) && - (RHASH_AR_TABLE_SIZE(hash) + size <= RHASH_AR_TABLE_MAX_SIZE)) { + (RHASH_AR_TABLE_SIZE(hash) + size <= RHASH_AR_TABLE_MAX_BOUND(hash))) { ar_bulk_insert(hash, argc, argv); } else { diff --git a/internal/hash.h b/internal/hash.h index 3f7331ab265ef9..f698f9f5707393 100644 --- a/internal/hash.h +++ b/internal/hash.h @@ -89,6 +89,7 @@ bool rb_hash_default_unredefined(VALUE hash); VALUE rb_hash_alloc_fixed_size(VALUE klass, st_index_t size); VALUE rb_ident_hash_new_capa(long size); void rb_hash_free(VALUE hash); +VALUE rb_hash_alloc_copy(VALUE klass, VALUE src); RUBY_EXTERN VALUE rb_cHash_empty_frozen; static inline unsigned RHASH_AR_TABLE_SIZE_RAW(VALUE h); diff --git a/internal/st.h b/internal/st.h index 44885c09b92f53..bd666bb39b6c67 100644 --- a/internal/st.h +++ b/internal/st.h @@ -20,4 +20,7 @@ void rb_st_free_embedded_table(st_table *tab); int rb_st_insert_no_rebuild(st_table *tab, st_data_t key, st_data_t value); #define st_insert_no_rebuild rb_st_insert_no_rebuild +typedef int st_foreach_with_hash_callback_func(st_data_t, st_data_t, st_data_t, st_data_t); +int rb_st_foreach_with_hash(st_table *, st_foreach_with_hash_callback_func *, st_data_t); +#define st_foreach_with_hash rb_st_foreach_with_hash #endif diff --git a/object.c b/object.c index 27b8654a7383ad..6327f4aadc420e 100644 --- a/object.c +++ b/object.c @@ -28,6 +28,7 @@ #include "internal/eval.h" #include "internal/hash.h" #include "internal/inits.h" +#include "internal/hash.h" #include "internal/numeric.h" #include "internal/object.h" #include "internal/struct.h" @@ -612,7 +613,16 @@ rb_obj_dup(VALUE obj) if (special_object_p(obj)) { return obj; } - dup = rb_obj_alloc(rb_obj_class(obj)); + + switch (OBJ_BUILTIN_TYPE(obj)) { + case T_HASH: + dup = rb_hash_alloc_copy(rb_obj_class(obj), obj); + break; + default: + dup = rb_obj_alloc(rb_obj_class(obj)); + break; + } + return rb_obj_dup_setup(obj, dup); } diff --git a/st.c b/st.c index f7b870fece49d0..3fa125bbf72658 100644 --- a/st.c +++ b/st.c @@ -1719,6 +1719,57 @@ st_general_foreach(st_table *tab, st_foreach_check_callback_func *func, st_updat return 0; } +#ifdef INTERNAL_ST_H +int +st_foreach_with_hash(st_table *tab, st_foreach_with_hash_callback_func *func, st_data_t arg) +{ + st_table_entry *entries, *curr_entry_ptr; + enum st_retval retval; + st_index_t i, rebuilds_num; + st_hash_t hash; + st_data_t key; + int packed_p = !st_has_bins(tab); + + entries = tab->entries; + /* The bound can change inside the loop even without rebuilding + the table, e.g. by an entry insertion. */ + for (i = tab->entries_start; i < tab->entries_bound; i++) { + curr_entry_ptr = &entries[i]; + if (EXPECT(DELETED_ENTRY_P(curr_entry_ptr), 0)) + continue; + key = curr_entry_ptr->key; + rebuilds_num = tab->rebuilds_num; + hash = curr_entry_ptr->hash; + retval = (*func)(key, curr_entry_ptr->record, hash, arg); + + if (rebuilds_num != tab->rebuilds_num) { + retry: + entries = tab->entries; + packed_p = !st_has_bins(tab); + if (packed_p) { + i = find_entry(tab, hash, key); + if (EXPECT(i == REBUILT_TABLE_ENTRY_IND, 0)) + goto retry; + } + else { + i = find_table_entry_ind(tab, hash, key); + if (EXPECT(i == REBUILT_TABLE_ENTRY_IND, 0)) + goto retry; + i -= ENTRY_BASE; + } + curr_entry_ptr = &entries[i]; + } + switch (retval) { + case ST_STOP: + return 0; + default: + break; + } + } + return 0; +} +#endif + int st_foreach_with_replace(st_table *tab, st_foreach_check_callback_func *func, st_update_callback_func *replace, st_data_t arg) { diff --git a/test/ruby/test_hash.rb b/test/ruby/test_hash.rb index 6b4c72db7fb771..109b6334bee8c7 100644 --- a/test/ruby/test_hash.rb +++ b/test/ruby/test_hash.rb @@ -1958,10 +1958,23 @@ def hash end end obj.hash_calls = 0 - hash = {obj => 42} + + ar_hash = {obj => 42} assert_equal(1, obj.hash_calls) - yield hash + yield ar_hash assert_equal(1, obj.hash_calls) + + st_hash = {a:1, b:2, c:3, d:4, e:5, f:6, g:7, h:8, obj => 42} + assert_equal(2, obj.hash_calls) + yield st_hash + assert_equal(2, obj.hash_calls) + + st_hash.keys.first(8).each do |key| + st_hash.delete(key) + end + assert_equal(2, obj.hash_calls) + yield st_hash + assert_equal(2, obj.hash_calls) end def test_select_reject_will_not_rehash diff --git a/zjit.h b/zjit.h index 8757c4d386f01a..7bda68202d4e96 100644 --- a/zjit.h +++ b/zjit.h @@ -132,7 +132,7 @@ void rb_zjit_invalidate_root_box(void); void rb_zjit_jit_frame_update_references(zjit_jit_frame_t *jit_frame); void rb_zjit_materialize_frames(const rb_execution_context_t *ec, rb_control_frame_t *cfp); void rb_zjit_materialize_frames_for_longjmp(const rb_execution_context_t *ec, rb_control_frame_t *cfp); -size_t rb_zjit_hash_new_size(VALUE *flags_out); +size_t rb_zjit_hash_new_size(VALUE *flags_out, size_t size); VALUE rb_zjit_new_obj_shape(VALUE flags, size_t alloc_size); bool rb_zjit_class_allocate_instance_fastpath(VALUE klass, size_t *size_out, VALUE *flags_out); bool rb_zjit_str_resurrect_fastpath(VALUE str, bool chilled, size_t *size_out, VALUE *flags_out, long *len_out, size_t *byte_size_out); diff --git a/zjit/src/codegen.rs b/zjit/src/codegen.rs index 170f597735fbe8..01d355eb349a9c 100644 --- a/zjit/src/codegen.rs +++ b/zjit/src/codegen.rs @@ -2546,7 +2546,7 @@ fn gen_new_hash( gen_prepare_leaf_call_with_gc(asm, state); let mut flags = VALUE(0); - let alloc_size = unsafe { rb_zjit_hash_new_size(&mut flags) }; + let alloc_size = unsafe { rb_zjit_hash_new_size(&mut flags, 0) }; let klass = unsafe { rb_cHash }; gc_fastpath::gc_fastpath_new_obj(jit, asm, function, state, alloc_size, flags.into(), klass, @@ -2565,7 +2565,7 @@ fn gen_new_hash( let num_pairs = elements.len() / 2; let hash = if num_pairs <= RUBY_RHASH_AR_TABLE_MAX_SIZE as usize { let mut flags = VALUE(0); - let alloc_size = unsafe { rb_zjit_hash_new_size(&mut flags) }; + let alloc_size = unsafe { rb_zjit_hash_new_size(&mut flags, num_pairs) }; let klass = unsafe { rb_cHash }; gc_fastpath::gc_fastpath_new_obj(jit, asm, function, state, alloc_size, flags.into(), klass, diff --git a/zjit/src/cruby_bindings.inc.rs b/zjit/src/cruby_bindings.inc.rs index 68ff278b6748db..f85c74860356f0 100644 --- a/zjit/src/cruby_bindings.inc.rs +++ b/zjit/src/cruby_bindings.inc.rs @@ -2392,7 +2392,7 @@ unsafe extern "C" { pub fn rb_iseq_label(iseq: *const rb_iseq_t) -> VALUE; pub fn rb_iseq_defined_string(type_: defined_type) -> VALUE; pub fn rb_zjit_profile_enable(iseq: *const rb_iseq_t); - pub fn rb_zjit_hash_new_size(flags_out: *mut VALUE) -> usize; + pub fn rb_zjit_hash_new_size(flags_out: *mut VALUE, size: usize) -> usize; pub fn rb_zjit_new_obj_shape(flags: VALUE, alloc_size: usize) -> VALUE; pub fn rb_zjit_class_allocate_instance_fastpath( klass: VALUE, From 759cfbb903bf496d4a6a7abc85f4f357f172d088 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Sun, 23 Aug 2026 11:53:24 +0200 Subject: [PATCH 08/16] Hash#merge guess size --- hash.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 52 insertions(+), 8 deletions(-) diff --git a/hash.c b/hash.c index 37fad66abcaaf6..bf1306f3a7836e 100644 --- a/hash.c +++ b/hash.c @@ -1716,9 +1716,9 @@ hash_dup_with_compare_by_id(VALUE hash) } static VALUE -hash_dup(VALUE hash, VALUE klass, VALUE flags) +hash_dup(VALUE hash, VALUE klass, VALUE flags, size_t capa) { - VALUE dup = hash_alloc(klass, flags, RHASH_IFNONE(hash), RHASH_SIZE(hash), false); + VALUE dup = hash_alloc(klass, flags, RHASH_IFNONE(hash), capa, false); return hash_copy(dup, hash); } @@ -1736,21 +1736,27 @@ hash_dup_capa(VALUE hash, size_t capa) return ret; } -VALUE -rb_hash_dup(VALUE hash) +static VALUE +rb_hash_dup_capa(VALUE hash, size_t capa) { const VALUE flags = RBASIC(hash)->flags; - VALUE ret = hash_dup(hash, rb_obj_class(hash), flags & RHASH_PROC_DEFAULT); + VALUE ret = hash_dup(hash, rb_obj_class(hash), flags & RHASH_PROC_DEFAULT, capa); rb_copy_generic_ivar(ret, hash); return ret; } +VALUE +rb_hash_dup(VALUE hash) +{ + return rb_hash_dup_capa(hash, RHASH_SIZE(hash)); +} + VALUE rb_hash_resurrect(VALUE hash) { - return hash_dup(hash, rb_cHash, 0); + return hash_dup(hash, rb_cHash, 0, RHASH_SIZE(hash)); } #if USE_ZJIT @@ -3980,7 +3986,7 @@ rb_hash_to_h(VALUE hash) } if (rb_obj_class(hash) != rb_cHash) { const VALUE flags = RBASIC(hash)->flags; - hash = hash_dup(hash, rb_cHash, flags & RHASH_PROC_DEFAULT); + hash = hash_dup(hash, rb_cHash, flags & RHASH_PROC_DEFAULT, RHASH_SIZE(hash)); } return hash; } @@ -4528,6 +4534,42 @@ rb_hash_update_by(VALUE hash1, VALUE hash2, rb_hash_update_func *func) return hash1; } +static size_t +hash_merge_guess_size(int argc, VALUE *argv, VALUE self) +{ + // Merging small symbol keyed hashes together is common enough that + // it's worth specializing for it. + // Since symbols never call back into Ruby, we can safely look them + // up without fear for side effects. + if (argc != 1) { + return 0; + } + + VALUE other = argv[0]; + if (!RB_TYPE_P(other, T_HASH) || !RHASH_AR_TABLE_P(other)) { + return 0; + } + + size_t size = RHASH_SIZE(self); + unsigned bound = RHASH_AR_TABLE_BOUND(other); + for (unsigned i = 0; i < bound; i++) { + VALUE key = RHASH_AR_TABLE_REF(other, i)->key; + if (UNDEF_P(key)) { + continue; + } + + if (!SYMBOL_P(key)) { + return 0; + } + + if (!hash_stlike_lookup(self, key, NULL)) { + size++; + } + } + + return size; +} + /* * call-seq: * merge(*other_hashes) -> new_hash @@ -4577,7 +4619,9 @@ rb_hash_update_by(VALUE hash1, VALUE hash2, rb_hash_update_func *func) static VALUE rb_hash_merge(int argc, VALUE *argv, VALUE self) { - return rb_hash_update(argc, argv, copy_compare_by_id(rb_hash_dup(self), self)); + size_t guessed_size = hash_merge_guess_size(argc, argv, self); + VALUE ret = guessed_size ? rb_hash_dup_capa(self, guessed_size) : rb_hash_dup(self); + return rb_hash_update(argc, argv, copy_compare_by_id(ret, self)); } static int From 7a0361b3fbf4748df75fca8dbd7c4f768c2122f1 Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Tue, 25 Aug 2026 14:37:50 +0200 Subject: [PATCH 09/16] Skip Fiddle::TestFunction#test_no_memory_leak on parse.y --- tool/test-bundled-gems.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tool/test-bundled-gems.rb b/tool/test-bundled-gems.rb index 87e065719be81b..98350e88b2de5a 100644 --- a/tool/test-bundled-gems.rb +++ b/tool/test-bundled-gems.rb @@ -134,7 +134,10 @@ when "fiddle" # When ZJIT is compile-happy, skip Fiddle::TestFunction#test_no_memory_leak # since compiling uses more memory which the test does not expect. - if run_opts&.include?("--zjit-call-threshold=1") + # Similarly, it's extremely flaky when running with `parse.y` for unclear reasons, + # but closer inspection didn't reveal any leak. + # It's a bit of a badly crafted test, and is likely missing some warmup to stabilize the memory usage. + if run_opts&.include?("--zjit-call-threshold=1") || run_opts&.include?('--parser=parse.y') test_command[-2..-1] = %w[test/run.rb --ignore-name=/\Atest_no_memory_leak\z/] end From 85220035ca470284e11cd4627375c5a1ba5e13ed Mon Sep 17 00:00:00 2001 From: Jeremy Evans Date: Mon, 24 Aug 2026 17:51:17 -0700 Subject: [PATCH 10/16] Add rb_ident_set_new internal function This is designed to replace rb_ident_hash_new calls when such hashes are used purely with with true values. To implement this, add a set_alloc_with_size_and_type static function, and have set_alloc_with_size and rb_ident_set_new call it. --- set.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/set.c b/set.c index 55122e96a398a4..6954342ad8af6f 100644 --- a/set.c +++ b/set.c @@ -395,17 +395,22 @@ set_insert_wb(VALUE set, VALUE key) } static VALUE -set_alloc_with_size(VALUE klass, st_index_t size) +set_alloc_with_size_and_type(VALUE klass, st_index_t size, const struct st_hash_type *type) { VALUE set; struct set_object *sobj; set = TypedData_Make_Struct(klass, struct set_object, &set_data_type, sobj); - set_init_table_with_size(&sobj->table, &objhash, size); + set_init_table_with_size(&sobj->table, type, size); return set; } +static VALUE +set_alloc_with_size(VALUE klass, st_index_t size) +{ + return set_alloc_with_size_and_type(klass, size, &objhash); +} static VALUE set_s_alloc(VALUE klass) @@ -2264,6 +2269,14 @@ compat_loader(VALUE self, VALUE a) return set_i_from_hash(self, rb_ivar_get(a, id_i_hash)); } +/* Internal C-API functions */ + +VALUE +rb_ident_set_new(void) +{ + return set_alloc_with_size_and_type(rb_cSet, 0, &identhash); +} + /* C-API functions */ void From 17026737cc2a7d679f83d9b9d33d875cc373abcc Mon Sep 17 00:00:00 2001 From: Jeremy Evans Date: Mon, 24 Aug 2026 17:53:18 -0700 Subject: [PATCH 11/16] Replace Array#flatten{,!} internal ident hash with set This used a hash with all true values, with is simple to convert to a set to save on memory when flattening array with a large number of nested elements. --- array.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/array.c b/array.c index 2907dca2b0d5d8..f92b2925f886f3 100644 --- a/array.c +++ b/array.c @@ -6750,6 +6750,8 @@ rb_ary_count(int argc, VALUE *argv, VALUE ary) return LONG2NUM(n); } +VALUE rb_ident_set_new(void); + static VALUE flatten(VALUE ary, int level) { @@ -6777,9 +6779,9 @@ flatten(VALUE ary, int level) rb_ary_push(stack, LONG2NUM(i + 1)); if (level < 0) { - memo = rb_obj_hide(rb_ident_hash_new()); - rb_hash_aset(memo, ary, Qtrue); - rb_hash_aset(memo, tmp, Qtrue); + memo = rb_obj_hide(rb_ident_set_new()); + rb_set_add(memo, ary); + rb_set_add(memo, tmp); } ary = tmp; @@ -6795,7 +6797,7 @@ flatten(VALUE ary, int level) tmp = rb_check_array_type(elt); if (RBASIC(result)->klass) { if (RTEST(memo)) { - rb_hash_clear(memo); + rb_set_clear(memo); } rb_raise(rb_eRuntimeError, "flatten reentered"); } @@ -6804,11 +6806,11 @@ flatten(VALUE ary, int level) } else { if (memo) { - if (rb_hash_aref(memo, tmp) == Qtrue) { - rb_hash_clear(memo); + if (rb_set_lookup(memo, tmp)) { + rb_set_clear(memo); rb_raise(rb_eArgError, "tried to flatten recursive array"); } - rb_hash_aset(memo, tmp, Qtrue); + rb_set_add(memo, tmp); } rb_ary_push(stack, ary); rb_ary_push(stack, LONG2NUM(i)); @@ -6820,7 +6822,7 @@ flatten(VALUE ary, int level) break; } if (memo) { - rb_hash_delete(memo, ary); + rb_set_delete(memo, ary); } tmp = rb_ary_pop(stack); i = NUM2LONG(tmp); @@ -6828,7 +6830,7 @@ flatten(VALUE ary, int level) } if (memo) { - rb_hash_clear(memo); + rb_set_clear(memo); } RBASIC_SET_CLASS(result, rb_cArray); From cd929ef3c7769867116afefae2e24c460a603208 Mon Sep 17 00:00:00 2001 From: Nobuyoshi Nakada Date: Tue, 25 Aug 2026 22:51:46 +0900 Subject: [PATCH 12/16] Enable ubuntu-26.04-arm job --- .github/workflows/ubuntu.yml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ubuntu.yml b/.github/workflows/ubuntu.yml index c7b9aef0396bd6..8fcf4e3d8814a4 100644 --- a/.github/workflows/ubuntu.yml +++ b/.github/workflows/ubuntu.yml @@ -49,9 +49,8 @@ jobs: os: ubuntu-24.04 configure: 'CC=clang-18 cflags=-fsanitize=address cppflags=-DUSE_MN_THREADS=0' timeout: 60 - # ubuntu-24.04-arm jobs don't start on ruby/ruby as of 2025-10-29 - #- test_task: check - # os: ubuntu-24.04-arm + - test_task: check + os: ubuntu-26.04-arm fail-fast: false env: &make-env From 4d16abab7dd3cc65c3e7c58e1f4bb12aaaa5e14a Mon Sep 17 00:00:00 2001 From: Nobuyoshi Nakada Date: Tue, 25 Aug 2026 23:13:09 +0900 Subject: [PATCH 13/16] Avoid GCC clobbered warnings in Ractor payload build Keep the payload builder uninlined in optimized GCC builds because inlining causes false-positive `-Wclobbered` warnings. --- ractor_sync.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ractor_sync.c b/ractor_sync.c index 03e0adc0a4915d..9a63a5f84757c1 100644 --- a/ractor_sync.c +++ b/ractor_sync.c @@ -1089,8 +1089,8 @@ ractor_prepare_payload(rb_execution_context_t *ec, VALUE obj, enum ractor_basket } } -#if RBIMPL_COMPILER_SINCE(GCC, 15, 0, 0) -/* GCC 15 produces false-positive -Wclobbered warnings after inlining +#if RBIMPL_COMPILER_IS(GCC) && defined(__OPTIMIZE__) +/* GCC produces false-positive -Wclobbered warnings after inlining * this function into ractor_basket_new(). */ NOINLINE(static void ractor_basket_build_payload(rb_execution_context_t *ec, struct ractor_basket *b, VALUE obj, enum ractor_basket_type type, bool exc)); #endif From 89699e814e4d9efb3f75aacc35de74f62a1ca1be Mon Sep 17 00:00:00 2001 From: Jean Boussier Date: Tue, 25 Aug 2026 17:20:35 +0200 Subject: [PATCH 14/16] hash.c: memzero hashes before allocating st_table `hash_copy` will call `st_init_existing_table_with_size` which allocates and may trigger GC causing the current hash to be marked. So we need to zero-out the `st_table` to avoid a segfault. --- hash.c | 1 + 1 file changed, 1 insertion(+) diff --git a/hash.c b/hash.c index bf1306f3a7836e..ecc8099e9ee14f 100644 --- a/hash.c +++ b/hash.c @@ -1728,6 +1728,7 @@ hash_dup_capa(VALUE hash, size_t capa) VALUE ret = hash_alloc_capa(rb_cHash, capa); if (capa > RHASH_AR_TABLE_MAX_SIZE) { RHASH_SET_ST_FLAG(ret); + RHASH_ST_CLEAR(ret); // Ensure the hash can be marked. } else { RUBY_ASSERT(RHASH_AR_TABLE_MAX_BOUND(ret) >= capa); From 383397e1b8751d10a23512d9f6cdc101a4b9b946 Mon Sep 17 00:00:00 2001 From: Kevin Menard Date: Mon, 24 Aug 2026 19:35:31 -0400 Subject: [PATCH 15/16] ZJIT: Index into instruction vectors with InsnId directly --- zjit/src/backend/lir.rs | 2 +- zjit/src/codegen.rs | 8 +-- zjit/src/hir.rs | 136 ++++++++++++++++++++++------------------ 3 files changed, 81 insertions(+), 65 deletions(-) diff --git a/zjit/src/backend/lir.rs b/zjit/src/backend/lir.rs index 2846576322c325..3ab73cfcd9307b 100644 --- a/zjit/src/backend/lir.rs +++ b/zjit/src/backend/lir.rs @@ -2941,7 +2941,7 @@ impl Assembler StackMapEntry::Opnd(Opnd::UImm(value)) => !VALUE(*value as usize).special_const_p(), StackMapEntry::Opnd(Opnd::VReg { idx, .. }) => { matches!( - intervals[idx.to_usize()].assigned.get().expect("StackMap VReg should have an allocation"), + intervals[*idx].assigned.get().expect("StackMap VReg should have an allocation"), Allocation::Reg(_) ) } diff --git a/zjit/src/codegen.rs b/zjit/src/codegen.rs index 01d355eb349a9c..2f07ac249d9d08 100644 --- a/zjit/src/codegen.rs +++ b/zjit/src/codegen.rs @@ -83,7 +83,7 @@ impl JITState { /// Retrieve the output of a given instruction that has been compiled fn get_opnd(&self, insn_id: InsnId) -> lir::Opnd { - self.opnds[insn_id.to_usize()].unwrap_or_else(|| panic!("Failed to get_opnd({insn_id})")) + self.opnds[insn_id].unwrap_or_else(|| panic!("Failed to get_opnd({insn_id})")) } /// Get the ISEQ for the version currently being compiled. @@ -467,7 +467,7 @@ fn gen_function(cb: &mut CodeBlock, iseq: IseqPtr, version: IseqVersionRef, func // Param does not have operands, so fake a ResolvedInsnId. match crate::hir::ResolvedInsnId(insn_id).insn(function) { Insn::Param => { - jit.opnds[insn_id.to_usize()] = Some(gen_param(&mut asm, idx)); + jit.opnds[insn_id] = Some(gen_param(&mut asm, idx)); }, insn => unreachable!("Non-param insn found in block.params: {insn:?}"), } @@ -480,7 +480,7 @@ fn gen_function(cb: &mut CodeBlock, iseq: IseqPtr, version: IseqVersionRef, func let insn_id = function.find_id(insn_id); // Param does not have operands, so fake a ResolvedInsnId. if let &Insn::LoadArg { idx, .. } = crate::hir::ResolvedInsnId(insn_id).insn(function) { - jit.opnds[insn_id.to_usize()] = Some(gen_param(&mut asm, idx as usize)); + jit.opnds[insn_id] = Some(gen_param(&mut asm, idx as usize)); } } } @@ -811,7 +811,7 @@ fn gen_insn(cb: &mut CodeBlock, jit: &mut JITState, asm: &mut Assembler, functio assert!(insn.has_output(), "Cannot write LIR output of HIR instruction with no output: {insn}"); // If the instruction has an output, remember it in jit.opnds - jit.opnds[insn_id.to_usize()] = Some(out_opnd); + jit.opnds[insn_id] = Some(out_opnd); Ok(()) } diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index abe17e006691c5..bc0cc24a296418 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -49,15 +49,9 @@ use crate::options::INLINE_BUDGET_UNLIMITED; #[derive(Copy, Clone, Ord, PartialOrd, Eq, PartialEq, Hash, Debug)] pub struct InsnId(pub u32); -impl IntoUsize for InsnId { - fn to_usize(self) -> usize { - self.0.to_usize() - } -} - impl From for usize { fn from(val: InsnId) -> Self { - val.to_usize() + val.0.to_usize() } } @@ -67,6 +61,28 @@ impl From for InsnId { } } +impl std::ops::Index for [T] { + type Output = T; + #[inline] + fn index(&self, i: InsnId) -> &T { &self[usize::from(i)] } +} + +impl std::ops::IndexMut for [T] { + #[inline] + fn index_mut(&mut self, i: InsnId) -> &mut T { &mut self[usize::from(i)] } +} + +impl std::ops::Index for Vec { + type Output = T; + #[inline] + fn index(&self, i: InsnId) -> &T { &self[usize::from(i)] } +} + +impl std::ops::IndexMut for Vec { + #[inline] + fn index_mut(&mut self, i: InsnId) -> &mut T { &mut self[usize::from(i)] } +} + impl std::fmt::Display for InsnId { fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { write!(f, "v{}", self.0) @@ -2755,14 +2771,14 @@ impl ResolvedInsnId { /// union-find). Assumes the operands are resolved through union-find already. Use /// [`Function::resolve`] to resolve operands before calling this. pub fn insn_mut(self, fun: &mut Function) -> &mut Insn { - &mut fun.insns[self.0.to_usize()] + &mut fun.insns[self.0] } /// Return a reference to the instruction at `insn_id` (after resolving via union-find). /// Assumes the operands are resolved through union-find already. Use [`Function::resolve`] to /// resolve operands before calling this. pub fn insn(self, fun: &Function) -> &Insn { - &fun.insns[self.0.to_usize()] + &fun.insns[self.0] } } @@ -3228,7 +3244,7 @@ impl Function { /// and locals, the way [`Function::frame_state`] does. fn frame_depth(&self, insn_id: InsnId) -> InlineDepth { let insn_id = self.union_find.borrow().find_const(insn_id); - match &self.insns[insn_id.to_usize()] { + match &self.insns[insn_id] { Insn::Snapshot { state } => state.depth, insn => panic!("Unexpected non-Snapshot {insn} when looking up frame depth"), } @@ -3265,7 +3281,7 @@ impl Function { // produces no value, and `make_equal_to` asserts `has_output()`. So the instruction stored // at this id is always the terminator itself, and reading it by reference matches what // `find` would return. - let terminator = &self.insns[self.blocks[block.to_usize()].insns.last().unwrap().to_usize()]; + let terminator = &self.insns[*self.blocks[block.to_usize()].insns.last().unwrap()]; let (first, second, rest): (Option, Option, &[BlockId]) = match terminator { Insn::CondBranch { if_true, if_false, .. } => (Some(if_true.target), Some(if_false.target), &[]), @@ -3399,7 +3415,7 @@ impl Function { }; } let insn_id = find!(insn_id); - let mut result = self.insns[insn_id.to_usize()].clone(); + let mut result = self.insns[insn_id].clone(); result.for_each_operand_mut(&mut |operand: &mut InsnId| { *operand = find!(*operand); }); @@ -3411,7 +3427,7 @@ impl Function { /// to have been resolved first, so the returned instruction's operands may be stale. Use it /// when the caller only inspects the opcode, or resolves the operands itself. pub fn find_ref(&self, insn_id: InsnId) -> &Insn { - &self.insns[self.find_id(insn_id).to_usize()] + &self.insns[self.find_id(insn_id)] } /// Make the operands of the instruction at `find(insn_id)` point to the current representative @@ -3427,7 +3443,7 @@ impl Function { pub fn resolve(&mut self, insn_id: InsnId) -> ResolvedInsnId { let union_find = self.union_find.borrow(); let insn_id = union_find.find_const(insn_id); - self.insns[insn_id.to_usize()].for_each_operand_mut(&mut |operand: &mut InsnId| { + self.insns[insn_id].for_each_operand_mut(&mut |operand: &mut InsnId| { *operand = union_find.find_const(*operand); }); ResolvedInsnId(insn_id) @@ -3438,7 +3454,7 @@ impl Function { use Insn::*; // Always set the reason: convert_no_profile_sends depends on it to identify // sends that should be converted to side exits for exit-based recompilation. - match self.insns.get_mut(insn_id.to_usize()).unwrap() { + match &mut self.insns[insn_id] { Send { reason, .. } | SendForward { reason, .. } | InvokeSuper { reason, .. } @@ -3451,17 +3467,17 @@ impl Function { /// Replace `insn` with the new instruction `replacement`, which will get appended to `insns`. fn make_equal_to(&mut self, insn: InsnId, replacement: InsnId) { - assert!(self.insns[insn.to_usize()].has_output(), + assert!(self.insns[insn].has_output(), "Don't use make_equal_to for instruction with no output"); - assert!(self.insns[replacement.to_usize()].has_output(), + assert!(self.insns[replacement].has_output(), "Can't replace instruction that has output with instruction that has no output"); // Don't push it to the block self.union_find.borrow_mut().make_equal_to(insn, replacement); } pub fn type_of(&self, insn: InsnId) -> Type { - assert!(self.insns[insn.to_usize()].has_output()); - self.insn_types[self.union_find.borrow_mut().find(insn).to_usize()] + assert!(self.insns[insn].has_output()); + self.insn_types[self.union_find.borrow_mut().find(insn)] } /// Check if the type of `insn` is a subtype of `ty`. @@ -3470,8 +3486,8 @@ impl Function { } fn infer_type(&self, insn: InsnId) -> Type { - assert!(self.insns[insn.to_usize()].has_output()); - match &self.insns[insn.to_usize()] { + assert!(self.insns[insn].has_output()); + match &self.insns[insn] { Insn::Param => unimplemented!("params should not be present in block.insns"), Insn::LoadArg { val_type, .. } => *val_type, Insn::SetGlobal { .. } | Insn::Jump(_) | Insn::Entries { .. } | Insn::EntryPoint { .. } @@ -3483,7 +3499,7 @@ impl Function { | Insn::CheckInterrupts { .. } | Insn::BreakPoint | Insn::Unreachable | Insn::StoreField { .. } | Insn::WriteBarrier { .. } | Insn::HashAset { .. } | Insn::ArrayAset { .. } | Insn::PushInlineFrame { .. } | Insn::PopInlineFrame { .. } => - panic!("Cannot infer type of instruction with no output: {}. See Insn::has_output().", self.insns[insn.to_usize()]), + panic!("Cannot infer type of instruction with no output: {}. See Insn::has_output().", self.insns[insn]), Insn::Const { val: Const::Value(val) } => Type::from_value(*val), Insn::Const { val: Const::CBool(val) } => Type::from_cbool(*val), Insn::Const { val: Const::CInt8(val) } => Type::from_cint(types::CInt8, *val as i64), @@ -3647,7 +3663,7 @@ impl Function { ); for (param, param_type) in std::iter::zip(entry_params, param_types) { // We know that function parameters are BasicObject or some subclass - self.insn_types[param.to_usize()] = *param_type; + self.insn_types[*param] = *param_type; } } } @@ -3668,11 +3684,11 @@ impl Function { ($insn:expr, $new_type:expr) => {{ let insn = $insn; let new_type = $new_type; - let old_type = self.insn_types[self.union_find.borrow_mut().find(insn).to_usize()]; + let old_type = self.insn_types[self.union_find.borrow_mut().find(insn)]; if old_type.bit_equal(new_type) { false } else { - self.insn_types[insn.to_usize()] = new_type; + self.insn_types[insn] = new_type; true } }}; @@ -3711,12 +3727,12 @@ impl Function { if !reachable.get(block) { continue; } for i in 0..self.blocks[block.to_usize()].insns.len() { let insn_id = self.blocks[block.to_usize()].insns[i]; - if self.insns[insn_id.to_usize()].counts_against_inlining_budget() { + if self.insns[insn_id].counts_against_inlining_budget() { num_instructions += 1; } // Instructions without output, including branch instructions, can't be targets // of make_equal_to, so we don't need find() here. - let insn_type = match &self.insns[insn_id.to_usize()] { + let insn_type = match &self.insns[insn_id] { Insn::CondBranch { val, if_true, if_false } => { assert!(!self.type_of(*val).bit_equal(types::Empty)); if self.type_of(*val).could_be(Type::from_cbool(true)) { @@ -3773,7 +3789,7 @@ impl Function { fn chase_insn(&self, insn: InsnId) -> InsnId { let id = self.union_find.borrow().find_const(insn); - match self.insns[id.to_usize()] { + match self.insns[id] { Insn::GuardType { val, .. } | Insn::GuardBitEquals { val, .. } | Insn::GuardAnyBitSet { val, .. } @@ -4207,7 +4223,7 @@ impl Function { pub fn guard_type_recompile(&mut self, block: BlockId, val: InsnId, guard_type: Type, state: InsnId, recompile: Recompile) -> InsnId { let result = self.push_insn(block, Insn::GuardType { val, guard_type, state, recompile: Some(recompile) }); - self.insn_types[result.to_usize()] = self.infer_type(result); + self.insn_types[result] = self.infer_type(result); result } @@ -4380,7 +4396,7 @@ impl Function { self.count(block, Counter::inline_cfunc_optimized_send_count); if self.type_of(replacement).bit_equal(types::Any) { // Not set yet; infer type - self.insn_types[replacement.to_usize()] = self.infer_type(replacement); + self.insn_types[replacement] = self.infer_type(replacement); } self.remove_block(tmp_block); return replacement; @@ -4593,7 +4609,7 @@ impl Function { // Add GuardType for profiled receiver if let Some(profiled_type) = profiled_type { recv = self.push_insn(block, Insn::GuardType { val: recv, guard_type: Type::from_profiled_type(profiled_type), state, recompile: Some(Recompile) }); - self.insn_types[recv.to_usize()] = self.infer_type(recv); + self.insn_types[recv] = self.infer_type(recv); } let SendDirectArgs { state: send_state, args: send_args, kw_bits, jit_entry_idx } = @@ -4883,7 +4899,7 @@ impl Function { fun.make_equal_to(send_insn_id, replacement); if fun.type_of(replacement).bit_equal(types::Any) { // Not set yet; infer type - fun.insn_types[replacement.to_usize()] = fun.infer_type(replacement); + fun.insn_types[replacement] = fun.infer_type(replacement); } fun.remove_block(tmp_block); return Ok(()); @@ -4894,7 +4910,7 @@ impl Function { fun.count(block, Counter::inline_cfunc_optimized_send_count); let owner = unsafe { (*cme).owner }; let ccall = fun.push_insn(block, Insn::CCall { cfunc: cfunc_ptr, recv, args, name, owner, return_type, elidable }); - fun.insn_types[ccall.to_usize()] = fun.infer_type(ccall); + fun.insn_types[ccall] = fun.infer_type(ccall); fun.make_equal_to(send_insn_id, ccall); return Ok(()); } @@ -4916,7 +4932,7 @@ impl Function { elidable, block: blockiseq.map(BlockHandler::BlockIseq), }))); - fun.insn_types[ccall.to_usize()] = fun.infer_type(ccall); + fun.insn_types[ccall] = fun.infer_type(ccall); fun.make_equal_to(send_insn_id, ccall); Ok(()) } @@ -4950,7 +4966,7 @@ impl Function { fun.make_equal_to(send_insn_id, replacement); if fun.type_of(replacement).bit_equal(types::Any) { // Not set yet; infer type - fun.insn_types[replacement.to_usize()] = fun.infer_type(replacement); + fun.insn_types[replacement] = fun.infer_type(replacement); } fun.remove_block(tmp_block); return Ok(()); @@ -4961,7 +4977,7 @@ impl Function { fun.count(block, Counter::inline_cfunc_optimized_send_count); let owner = unsafe { (*cme).owner }; let ccall = fun.push_insn(block, Insn::CCall { cfunc: cfunc_ptr, recv, args, name, owner, return_type, elidable }); - fun.insn_types[ccall.to_usize()] = fun.infer_type(ccall); + fun.insn_types[ccall] = fun.infer_type(ccall); fun.make_equal_to(send_insn_id, ccall); return Ok(()); } @@ -4983,7 +4999,7 @@ impl Function { elidable, block: blockiseq.map(BlockHandler::BlockIseq), }))); - fun.insn_types[ccall.to_usize()] = fun.infer_type(ccall); + fun.insn_types[ccall] = fun.infer_type(ccall); fun.make_equal_to(send_insn_id, ccall); Ok(()) } @@ -5014,12 +5030,12 @@ impl Function { let method = unsafe { rb_vm_ci_mid((*cd).ci) }; self.push_insn(block, Insn::PatchPoint { invariant: Invariant::MethodRedefined { klass: class, method, cme }, state }); let replacement = self.push_insn(block, Insn::Const { val: Const::CBool(is_expected_cfunc) }); - self.insn_types[replacement.to_usize()] = self.infer_type(replacement); + self.insn_types[replacement] = self.infer_type(replacement); self.make_equal_to(insn_id, replacement); } &Insn::ObjectAlloc { val, state } => { if let Some(replacement) = self.try_inline_object_alloc(block, val, state) { - self.insn_types[replacement.to_usize()] = self.infer_type(replacement); + self.insn_types[replacement] = self.infer_type(replacement); self.make_equal_to(insn_id, replacement); } else { self.push_insn_id(block, insn_id); @@ -5034,7 +5050,7 @@ impl Function { let high_fix = self.coerce_to(block, high, types::Fixnum, state); let replacement = self.push_insn(block, Insn::NewRangeFixnum { low: low_fix, high: high_fix, flag, state }); self.make_equal_to(insn_id, replacement); - self.insn_types[replacement.to_usize()] = self.infer_type(replacement); + self.insn_types[replacement] = self.infer_type(replacement); } else { self.push_insn_id(block, insn_id); }; @@ -5232,7 +5248,7 @@ impl Function { self.make_equal_to(insn_id, replacement); if self.type_of(replacement).bit_equal(types::Any) { // Not set yet; infer type - self.insn_types[replacement.to_usize()] = self.infer_type(replacement); + self.insn_types[replacement] = self.infer_type(replacement); } self.remove_block(tmp_block); continue; @@ -5282,7 +5298,7 @@ impl Function { self.make_equal_to(insn_id, replacement); if self.type_of(replacement).bit_equal(types::Any) { // Not set yet; infer type - self.insn_types[replacement.to_usize()] = self.infer_type(replacement); + self.insn_types[replacement] = self.infer_type(replacement); } self.remove_block(tmp_block); continue; @@ -6122,12 +6138,12 @@ impl Function { fn outgoing_edges(fun: &Function, block_id: BlockId) -> impl Iterator { let insn_id = block_terminator(fun, block_id); - edges_of!(&fun.insns[insn_id.to_usize()]) + edges_of!(&fun.insns[insn_id]) } fn outgoing_edges_mut(fun: &mut Function, block_id: BlockId) -> impl Iterator { let insn_id = block_terminator(fun, block_id); - edges_of!(&mut fun.insns[insn_id.to_usize()]) + edges_of!(&mut fun.insns[insn_id]) } // Instantiate the domain for abstract interpretation. @@ -6332,14 +6348,14 @@ impl Function { let canonical_id = self.union_find.borrow().find_const(insn_id); let union_find = &self.union_find; - self.insns[canonical_id.to_usize()].for_each_operand_mut(|operand| { + self.insns[canonical_id].for_each_operand_mut(|operand| { let canon = union_find.borrow().find_const(*operand); *operand = rewrite_map.get(&canon).copied().unwrap_or(canon); }); // For the binary guards only `left` is registered because their infer_type is // type_of(left). - match &self.insns[canonical_id.to_usize()] { + match &self.insns[canonical_id] { Insn::GuardType { val: src, .. } | Insn::GuardBitEquals { val: src, .. } | Insn::GuardAnyBitSet { val: src, .. } @@ -6541,11 +6557,11 @@ impl Function { // quotient towards negative infinity, so this holds for all fixnums. (None, Some(d)) if is_power_of_two(d) => { let shift = self.new_insn(Insn::Const { val: Const::Value(VALUE::fixnum_from_isize(d.trailing_zeros() as isize)) }); - self.insn_types[shift.to_usize()] = self.infer_type(shift); + self.insn_types[shift] = self.infer_type(shift); new_insns.push(shift); let replacement = self.new_insn(Insn::FixnumRShift { left, right: shift }); self.make_equal_to(insn_id, replacement); - self.insn_types[replacement.to_usize()] = self.infer_type(replacement); + self.insn_types[replacement] = self.infer_type(replacement); new_insns.push(replacement); continue; } @@ -6569,11 +6585,11 @@ impl Function { // in [0, d), which matches two's complement AND for all fixnums. (None, Some(d)) if is_power_of_two(d) => { let mask = self.new_insn(Insn::Const { val: Const::Value(VALUE::fixnum_from_isize((d - 1) as isize)) }); - self.insn_types[mask.to_usize()] = self.infer_type(mask); + self.insn_types[mask] = self.infer_type(mask); new_insns.push(mask); let replacement = self.new_insn(Insn::FixnumAnd { left, right: mask }); self.make_equal_to(insn_id, replacement); - self.insn_types[replacement.to_usize()] = self.infer_type(replacement); + self.insn_types[replacement] = self.infer_type(replacement); new_insns.push(replacement); continue; } @@ -6688,14 +6704,14 @@ impl Function { }; // If we're adding a new instruction, mark the two equivalent in the union-find and // do an incremental flow typing of the new instruction. - if insn_id != replacement_id && self.insns[replacement_id.to_usize()].has_output() { + if insn_id != replacement_id && self.insns[replacement_id].has_output() { self.make_equal_to(insn_id, replacement_id); - self.insn_types[replacement_id.to_usize()] = self.infer_type(replacement_id); + self.insn_types[replacement_id] = self.infer_type(replacement_id); } new_insns.push(replacement_id); // If we've just folded an IfTrue into a Jump, for example, don't bother copying // over unreachable instructions afterward. - if self.insns[replacement_id.to_usize()].is_terminator() { + if self.insns[replacement_id].is_terminator() { break; } } @@ -6712,7 +6728,7 @@ impl Function { // otherwise necessary to keep around for block_id in &rpo { for insn_id in &self.blocks[block_id.to_usize()].insns { - if !&self.insns[insn_id.to_usize()].is_elidable() { + if !&self.insns[*insn_id].is_elidable() { worklist.push_back(*insn_id); } } @@ -6723,7 +6739,7 @@ impl Function { if necessary.get(insn_id) { continue; } necessary.insert(insn_id); let insn_id = self.union_find.borrow().find_const(insn_id); - self.insns[insn_id.to_usize()].for_each_operand(|operand| { + self.insns[insn_id].for_each_operand(|operand| { worklist.push_back(self.union_find.borrow().find_const(operand)); }); } @@ -6831,7 +6847,7 @@ impl Function { let insns = std::mem::take(&mut self.blocks[block_id.to_usize()].insns); let mut new_insns = Vec::with_capacity(insns.len()); for insn_id in insns { - let insn = &self.insns[insn_id.to_usize()]; + let insn = &self.insns[insn_id]; if matches!(insn, Insn::CheckInterrupts { .. }) { if seen { continue; } seen = true; @@ -6872,7 +6888,7 @@ impl Function { let mut references_snapshot = false; insn.for_each_operand(|opnd| { let opnd = self.union_find.borrow().find_const(opnd); - if matches!(&self.insns[opnd.to_usize()], Insn::Snapshot { .. }) { + if matches!(&self.insns[opnd], Insn::Snapshot { .. }) { references_snapshot = true; } }); @@ -7397,14 +7413,14 @@ impl Function { } for &insn_id in &self.blocks[block.to_usize()].insns { let insn_id = self.union_find.borrow().find_const(insn_id); - self.insns[insn_id.to_usize()].try_for_each_operand(|operand| { + self.insns[insn_id].try_for_each_operand(|operand| { let operand = self.union_find.borrow().find_const(operand); if !assigned.get(operand) { return Err(ValidationError::OperandNotDefined(block, insn_id, operand)); } Ok(()) })?; - if self.insns[insn_id.to_usize()].has_output() { + if self.insns[insn_id].has_output() { assigned.insert(insn_id); } } @@ -9142,7 +9158,7 @@ fn add_iseq_to_hir( let zjit_module = VALUE(state::ZJIT_MODULE.load(Ordering::Relaxed)); let lookedup_module = rb_const_lookup(rb_cRubyVM, ID!(ZJIT)); if !lookedup_module.is_null() && (*lookedup_module).value == zjit_module { - fun.insn_types[result.to_usize()] = Type::from_value(zjit_module); + fun.insn_types[result] = Type::from_value(zjit_module); } } } From abc5a924d955b6babe353b55936d10c233ceed6e Mon Sep 17 00:00:00 2001 From: Kevin Menard Date: Mon, 24 Aug 2026 19:37:17 -0400 Subject: [PATCH 16/16] ZJIT: Index into block vectors with BlockId directly --- zjit/src/codegen.rs | 10 +- zjit/src/hir.rs | 224 ++++++++++++++++++++------------------ zjit/src/hir/opt_tests.rs | 2 +- 3 files changed, 126 insertions(+), 110 deletions(-) diff --git a/zjit/src/codegen.rs b/zjit/src/codegen.rs index 2f07ac249d9d08..0166ea35950a4f 100644 --- a/zjit/src/codegen.rs +++ b/zjit/src/codegen.rs @@ -438,7 +438,7 @@ fn gen_function(cb: &mut CodeBlock, iseq: IseqPtr, version: IseqVersionRef, func // Skip the entries superblock -- it's an internal CFG artifact if block_id == function.entries_block { continue; } let lir_block_id = asm.new_block(block_id, function.is_entry_block(block_id), rpo_idx); - hir_to_lir[block_id.to_usize()] = Some(lir_block_id); + hir_to_lir[block_id] = Some(lir_block_id); } // Compile each basic block @@ -447,7 +447,7 @@ fn gen_function(cb: &mut CodeBlock, iseq: IseqPtr, version: IseqVersionRef, func if block_id == function.entries_block { continue; } // Set the current block to the LIR block that corresponds to this // HIR block. - let lir_block_id = hir_to_lir[block_id.to_usize()].unwrap(); + let lir_block_id = hir_to_lir[block_id].unwrap(); asm.set_current_block(lir_block_id); // Write a label to jump to the basic block @@ -493,8 +493,8 @@ fn gen_function(cb: &mut CodeBlock, iseq: IseqPtr, version: IseqVersionRef, func let result = match &insn { Insn::CondBranch { val, if_true, if_false } => { let val_opnd = jit.get_opnd(*val); - let true_target = hir_to_lir[if_true.target.to_usize()].unwrap(); - let false_target = hir_to_lir[if_false.target.to_usize()].unwrap(); + let true_target = hir_to_lir[if_true.target].unwrap(); + let false_target = hir_to_lir[if_false.target].unwrap(); let true_branch = lir::BranchEdge { target: true_target, @@ -514,7 +514,7 @@ fn gen_function(cb: &mut CodeBlock, iseq: IseqPtr, version: IseqVersionRef, func Ok(()) } Insn::Jump(target) => { - let lir_target = hir_to_lir[target.target.to_usize()].unwrap(); + let lir_target = hir_to_lir[target.target].unwrap(); let branch_edge = lir::BranchEdge { target: lir_target, args: target.args.iter().map(|insn_id| jit.get_opnd(*insn_id)).collect() diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index bc0cc24a296418..9fc2915df28339 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -93,15 +93,9 @@ impl std::fmt::Display for InsnId { #[derive(Copy, Clone, Eq, PartialEq, Hash, Debug, PartialOrd, Ord)] pub struct BlockId(pub u32); -impl IntoUsize for BlockId { - fn to_usize(self) -> usize { - self.0.to_usize() - } -} - impl From for usize { fn from(val: BlockId) -> Self { - val.to_usize() + val.0.to_usize() } } @@ -111,6 +105,28 @@ impl From for BlockId { } } +impl std::ops::Index for [T] { + type Output = T; + #[inline] + fn index(&self, i: BlockId) -> &T { &self[usize::from(i)] } +} + +impl std::ops::IndexMut for [T] { + #[inline] + fn index_mut(&mut self, i: BlockId) -> &mut T { &mut self[usize::from(i)] } +} + +impl std::ops::Index for Vec { + type Output = T; + #[inline] + fn index(&self, i: BlockId) -> &T { &self[usize::from(i)] } +} + +impl std::ops::IndexMut for Vec { + #[inline] + fn index_mut(&mut self, i: BlockId) -> &mut T { &mut self[usize::from(i)] } +} + impl std::fmt::Display for BlockId { fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { write!(f, "bb{}", self.0) @@ -3048,9 +3064,9 @@ impl Function { let is_param = matches!(insn, Insn::Param); let id = self.new_insn(insn); if is_param { - self.blocks[block.to_usize()].params.push(id); + self.blocks[block].params.push(id); } else { - self.blocks[block.to_usize()].insns.push(id); + self.blocks[block].insns.push(id); } id } @@ -3190,7 +3206,7 @@ impl Function { // Add an instruction to an SSA block fn push_insn_id(&mut self, block: BlockId, insn_id: InsnId) -> InsnId { - self.blocks[block.to_usize()].insns.push(insn_id); + self.blocks[block].insns.push(insn_id); insn_id } @@ -3281,7 +3297,7 @@ impl Function { // produces no value, and `make_equal_to` asserts `has_output()`. So the instruction stored // at this id is always the terminator itself, and reading it by reference matches what // `find` would return. - let terminator = &self.insns[*self.blocks[block.to_usize()].insns.last().unwrap()]; + let terminator = &self.insns[*self.blocks[block].insns.last().unwrap()]; let (first, second, rest): (Option, Option, &[BlockId]) = match terminator { Insn::CondBranch { if_true, if_false, .. } => (Some(if_true.target), Some(if_false.target), &[]), @@ -3302,12 +3318,12 @@ impl Function { /// Return a reference to the Block at the given index. pub fn block(&self, block_id: BlockId) -> &Block { - &self.blocks[block_id.to_usize()] + &self.blocks[block_id] } /// Return a reference to the entry block. pub fn entry_block(&self) -> &Block { - &self.blocks[self.entry_block.to_usize()] + &self.blocks[self.entry_block] } /// Return the number of blocks @@ -3655,7 +3671,7 @@ impl Function { /// Copy self.param_types to the param types of jit_entry_blocks. fn copy_param_types(&mut self) { for jit_entry_block in self.jit_entry_blocks.iter() { - let entry_params = self.blocks[jit_entry_block.to_usize()].params.iter(); + let entry_params = self.blocks[*jit_entry_block].params.iter(); let param_types = self.param_types.iter(); assert!( param_types.len() >= entry_params.len(), @@ -3717,7 +3733,7 @@ impl Function { // includes reachable blocks. Any blocks not present in `rpo` default to `usize::MAX`. let mut rpo_order = vec![usize::MAX; self.blocks.len()]; for (idx, &block_id) in rpo.iter().enumerate() { - rpo_order[block_id.to_usize()] = idx; + rpo_order[block_id] = idx; } loop { let mut changed = false; @@ -3725,8 +3741,8 @@ impl Function { let mut num_instructions = 0; for (rpo_index, &block) in rpo.iter().enumerate() { if !reachable.get(block) { continue; } - for i in 0..self.blocks[block.to_usize()].insns.len() { - let insn_id = self.blocks[block.to_usize()].insns[i]; + for i in 0..self.blocks[block].insns.len() { + let insn_id = self.blocks[block].insns[i]; if self.insns[insn_id].counts_against_inlining_budget() { num_instructions += 1; } @@ -3742,19 +3758,19 @@ impl Function { // params of `target` itself). let arg_types: Vec = if_true.args.iter().map(|a| self.type_of(*a)).collect(); for (idx, arg_type) in arg_types.into_iter().enumerate() { - let param = self.blocks[if_true.target.to_usize()].params[idx]; + let param = self.blocks[if_true.target].params[idx]; changed |= set_type!(param, self.type_of(param).union(arg_type)); } - traversed_back_edge |= rpo_order[if_true.target.to_usize()] <= rpo_index; + traversed_back_edge |= rpo_order[if_true.target] <= rpo_index; } if self.type_of(*val).could_be(Type::from_cbool(false)) { reachable.insert(if_false.target); let arg_types: Vec = if_false.args.iter().map(|a| self.type_of(*a)).collect(); for (idx, arg_type) in arg_types.into_iter().enumerate() { - let param = self.blocks[if_false.target.to_usize()].params[idx]; + let param = self.blocks[if_false.target].params[idx]; changed |= set_type!(param, self.type_of(param).union(arg_type)); } - traversed_back_edge |= rpo_order[if_false.target.to_usize()] <= rpo_index; + traversed_back_edge |= rpo_order[if_false.target] <= rpo_index; } continue; } @@ -3762,10 +3778,10 @@ impl Function { reachable.insert(target); let arg_types: Vec = args.iter().map(|a| self.type_of(*a)).collect(); for (idx, arg_type) in arg_types.into_iter().enumerate() { - let param = self.blocks[target.to_usize()].params[idx]; + let param = self.blocks[target].params[idx]; changed |= set_type!(param, self.type_of(param).union(arg_type)); } - traversed_back_edge |= rpo_order[target.to_usize()] <= rpo_index; + traversed_back_edge |= rpo_order[target] <= rpo_index; continue; } Insn::Entries { targets } => { @@ -4391,8 +4407,8 @@ 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); - let insns = std::mem::take(&mut self.blocks[tmp_block.to_usize()].insns); - self.blocks[block.to_usize()].insns.extend(insns); + let insns = std::mem::take(&mut self.blocks[tmp_block].insns); + self.blocks[block].insns.extend(insns); self.count(block, Counter::inline_cfunc_optimized_send_count); if self.type_of(replacement).bit_equal(types::Any) { // Not set yet; infer type @@ -4460,8 +4476,8 @@ impl Function { /// Also try and inline constant caches, specialize object allocations, and more. fn type_specialize(&mut self) { for block in self.reverse_post_order() { - let old_insns = std::mem::take(&mut self.blocks[block.to_usize()].insns); - assert!(self.blocks[block.to_usize()].insns.is_empty()); + let old_insns = std::mem::take(&mut self.blocks[block].insns); + assert!(self.blocks[block].insns.is_empty()); for insn_id in old_insns { let resolved = self.resolve(insn_id); match resolved.insn(self) { @@ -4893,8 +4909,8 @@ impl Function { if let Some(replacement) = (props.inline)(fun, tmp_block, recv, &args, state) { // Copy contents of tmp_block to block assert_ne!(block, tmp_block); - let insns = std::mem::take(&mut fun.blocks[tmp_block.to_usize()].insns); - fun.blocks[block.to_usize()].insns.extend(insns); + let insns = std::mem::take(&mut fun.blocks[tmp_block].insns); + fun.blocks[block].insns.extend(insns); fun.count(block, Counter::inline_cfunc_optimized_send_count); fun.make_equal_to(send_insn_id, replacement); if fun.type_of(replacement).bit_equal(types::Any) { @@ -4960,8 +4976,8 @@ impl Function { if let Some(replacement) = (props.inline)(fun, tmp_block, recv, &args, state) { // Copy contents of tmp_block to block assert_ne!(block, tmp_block); - let insns = std::mem::take(&mut fun.blocks[tmp_block.to_usize()].insns); - fun.blocks[block.to_usize()].insns.extend(insns); + let insns = std::mem::take(&mut fun.blocks[tmp_block].insns); + fun.blocks[block].insns.extend(insns); fun.count(block, Counter::inline_cfunc_optimized_send_count); fun.make_equal_to(send_insn_id, replacement); if fun.type_of(replacement).bit_equal(types::Any) { @@ -5242,8 +5258,8 @@ 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); - let insns = std::mem::take(&mut self.blocks[tmp_block.to_usize()].insns); - self.blocks[block.to_usize()].insns.extend(insns); + let insns = std::mem::take(&mut self.blocks[tmp_block].insns); + self.blocks[block].insns.extend(insns); self.count(block, Counter::inline_cfunc_optimized_send_count); self.make_equal_to(insn_id, replacement); if self.type_of(replacement).bit_equal(types::Any) { @@ -5292,8 +5308,8 @@ 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); - let insns = std::mem::take(&mut self.blocks[tmp_block.to_usize()].insns); - self.blocks[block.to_usize()].insns.extend(insns); + let insns = std::mem::take(&mut self.blocks[tmp_block].insns); + self.blocks[block].insns.extend(insns); self.count(block, Counter::inline_cfunc_optimized_send_count); self.make_equal_to(insn_id, replacement); if self.type_of(replacement).bit_equal(types::Any) { @@ -5470,14 +5486,14 @@ impl Function { // inlinable SendDirect in the same block still gets a chance. let mut search_start = 0; loop { - let Some(offset) = self.blocks[block.to_usize()].insns[search_start..].iter() + let Some(offset) = self.blocks[block].insns[search_start..].iter() .position(|&id| self.is_send_direct(id)) else { break; }; let send_pos = search_start + offset; - let send_insn_id = self.blocks[block.to_usize()].insns[send_pos]; + let send_insn_id = self.blocks[block].insns[send_pos]; let send = self.resolve(send_insn_id); let Insn::SendDirect(data) = send.insn(self) else { @@ -5591,7 +5607,7 @@ impl Function { // constants land in `block` at the correct position (after the // pre-Send body, before the PushLightweightFrame and Jump we add // last). - let tail = self.blocks[block.to_usize()].insns.split_off(send_pos); + let tail = self.blocks[block].insns.split_off(send_pos); debug_assert!(self.is_send_direct(tail[0])); let omitted_opt_num = opt_num - passed_opt_num; @@ -5626,7 +5642,7 @@ impl Function { // same value back to the runtime frame so a resuming interpreter sees // the correct bitmask. // * any remaining non-parameter locals are nil-initialized. - let callee_body_params: Vec = self.blocks[callee_entry_body_block.to_usize()].params.clone(); + let callee_body_params: Vec = self.blocks[callee_entry_body_block].params.clone(); // First param is self. if !callee_body_params.is_empty() { @@ -5667,7 +5683,7 @@ impl Function { // Clear the callee body entry block's params since we've aliased // them via make_equal_to rather than passing them as branch // arguments. This keeps validation happy (the Jump passes 0 args). - self.blocks[callee_entry_body_block.to_usize()].params.clear(); + self.blocks[callee_entry_body_block].params.clear(); // Set up the continuation block: a single Param merges all return // values jumped in from the callee's leaves, then PopLightweightFrame @@ -6066,8 +6082,8 @@ impl Function { return; } for block in self.reverse_post_order() { - let old_insns = std::mem::take(&mut self.blocks[block.to_usize()].insns); - assert!(self.blocks[block.to_usize()].insns.is_empty()); + let old_insns = std::mem::take(&mut self.blocks[block].insns); + assert!(self.blocks[block].insns.is_empty()); for insn_id in old_insns { match self.resolve(insn_id).insn(self) { &Insn::Send { state, reason: SendFallbackReason::SendNoProfiles, .. } => { @@ -6123,7 +6139,7 @@ impl Function { } fn block_terminator(fun: &Function, block_id: BlockId) -> InsnId { - *fun.blocks[block_id.to_usize() as usize].insns().last().unwrap() + *fun.blocks[block_id].insns().last().unwrap() } macro_rules! edges_of { @@ -6161,11 +6177,11 @@ impl Function { // We only need to update blocks that have params. (Blocks without params cannot be improved) let blocks_receiving_params: Vec = blocks.iter().copied() .filter(|&block_id| - self.blocks[block_id.to_usize()].params().len() != 0) + self.blocks[block_id].params().len() != 0) .collect(); // Create a vec to represent trivial indices - let max_params = blocks.iter().copied().map(|id| self.blocks[id.to_usize()].params.len()).max().unwrap_or(0); + let max_params = blocks.iter().copied().map(|id| self.blocks[id].params.len()).max().unwrap_or(0); let mut trivial_indices: Vec = Vec::with_capacity(max_params); let mut changed = true; @@ -6185,10 +6201,10 @@ impl Function { for (i, param) in params.iter().enumerate() { let param = self.find_id(*param); // If the param is the same as passed into the block, it is a self loop and provides no new predecessor information. - if param == self.find_id(self.blocks[block_id.to_usize()].params[i]) { + if param == self.find_id(self.blocks[*block_id].params[i]) { continue } - param_values[block_id.to_usize()][i].update(param); + param_values[*block_id][i].update(param); } } } @@ -6199,7 +6215,7 @@ impl Function { // 2. Remove trivial params from the basic block definition // 3. Remove trivial params from each CondBranch and Jump that targets the basic block that was just updated for block_id in &blocks_receiving_params { - let block_preds = ¶m_values[block_id.to_usize()]; + let block_preds = ¶m_values[*block_id]; trivial_indices.clear(); for (idx, state) in block_preds.iter().enumerate() { if let ParamValue::One(_) = state { @@ -6210,13 +6226,13 @@ impl Function { // Replace uses of the trivial params with the concretized value for param_index in &trivial_indices { if let ParamValue::One(insn_id) = block_preds[*param_index] { - self.make_equal_to(self.blocks[block_id.to_usize()].params[*param_index], insn_id); + self.make_equal_to(self.blocks[*block_id].params[*param_index], insn_id); changed = true; } } // Update the block - prune_vec_by_indices(&mut self.blocks[block_id.to_usize()].params, &trivial_indices); + prune_vec_by_indices(&mut self.blocks[*block_id].params, &trivial_indices); // Update the terminators (basic blocks can only branch at the terminator. This is where block params are passed) for jump_block_id in &blocks_sending_params { @@ -6234,7 +6250,7 @@ impl Function { fn optimize_load_store(&mut self) { for block in self.reverse_post_order() { let mut compile_time_heap: HashMap<(InsnId, i32), InsnId> = HashMap::new(); - let old_insns = std::mem::take(&mut self.blocks[block.to_usize()].insns); + let old_insns = std::mem::take(&mut self.blocks[block].insns); let mut new_insns = Vec::with_capacity(old_insns.len()); for insn_id in old_insns { let replacement_insn: InsnId = match self.resolve(insn_id).insn(self) { @@ -6301,7 +6317,7 @@ impl Function { }; new_insns.push(replacement_insn); } - self.blocks[block.to_usize()].insns = new_insns; + self.blocks[block].insns = new_insns; } } @@ -6342,9 +6358,9 @@ impl Function { let mut rewrite_maps: Vec>> = vec![None; self.blocks.len()]; let dominators = Dominators::new(self); for &block in dominators.cfi.reverse_post_order() { - let mut rewrite_map = rewrite_maps[dominators.idom(block).to_usize()].clone().unwrap_or_else(|| HashMap::new()); - for i in 0..self.blocks[block.to_usize()].insns.len() { - let insn_id = self.blocks[block.to_usize()].insns[i]; + let mut rewrite_map = rewrite_maps[dominators.idom(block)].clone().unwrap_or_else(|| HashMap::new()); + for i in 0..self.blocks[block].insns.len() { + let insn_id = self.blocks[block].insns[i]; let canonical_id = self.union_find.borrow().find_const(insn_id); let union_find = &self.union_find; @@ -6367,7 +6383,7 @@ impl Function { _ => {} } } - rewrite_maps[block.to_usize()] = Some(rewrite_map); + rewrite_maps[block] = Some(rewrite_map); } crate::stats::trace_compile_phase("infer_types", || self.infer_types()); @@ -6387,7 +6403,7 @@ impl Function { // This would require 1) fixpointing, 2) worklist, or 3) (slightly less powerful) calling a // function-level infer_types after each pruned branch. for block in self.reverse_post_order() { - let old_insns = std::mem::take(&mut self.blocks[block.to_usize()].insns); + let old_insns = std::mem::take(&mut self.blocks[block].insns); let mut new_insns = Vec::with_capacity(old_insns.len()); for insn_id in old_insns { let replacement_id = match self.resolve(insn_id).insn(self) { @@ -6715,7 +6731,7 @@ impl Function { break; } } - self.blocks[block.to_usize()].insns = new_insns; + self.blocks[block].insns = new_insns; } } @@ -6727,7 +6743,7 @@ impl Function { // Find all of the instructions that have side effects, are control instructions, or are // otherwise necessary to keep around for block_id in &rpo { - for insn_id in &self.blocks[block_id.to_usize()].insns { + for insn_id in &self.blocks[*block_id].insns { if !&self.insns[*insn_id].is_elidable() { worklist.push_back(*insn_id); } @@ -6745,12 +6761,12 @@ impl Function { } // Now remove all unnecessary instructions for block_id in &rpo { - self.blocks[block_id.to_usize()].insns.retain(|&insn_id| necessary.get(insn_id)); + self.blocks[*block_id].insns.retain(|&insn_id| necessary.get(insn_id)); } } fn absorb_dst_block(&mut self, num_in_edges: &[u32], block: BlockId) -> bool { - let Some(&terminator_id) = self.blocks[block.to_usize()].insns.last() + let Some(&terminator_id) = self.blocks[block].insns.last() else { return false }; let &mut Insn::Jump(ref mut edge) = self.resolve(terminator_id).insn_mut(self) else { return false }; @@ -6758,7 +6774,7 @@ impl Function { // Can't absorb self return false; } - if num_in_edges[edge.target.to_usize()] != 1 { + if num_in_edges[edge.target] != 1 { // Can't absorb block if it's the target of more than one branch return false; } @@ -6768,16 +6784,16 @@ impl Function { // Drop the borrow of edge, which drops the borrow of self, which allows us to mutate self // again. let _ = edge; - let params = std::mem::take(&mut self.blocks[target.to_usize()].params); + let params = std::mem::take(&mut self.blocks[target].params); assert_eq!(args.len(), params.len()); for (arg, param) in args.iter().zip(params) { self.make_equal_to(param, *arg); } // Remove branch instruction - self.blocks[block.to_usize()].insns.pop(); + self.blocks[block].insns.pop(); // Move target instructions into block - let target_insns = std::mem::take(&mut self.blocks[target.to_usize()].insns); - self.blocks[block.to_usize()].insns.extend(target_insns); + let target_insns = std::mem::take(&mut self.blocks[target].insns); + self.blocks[block].insns.extend(target_insns); true } @@ -6790,7 +6806,7 @@ impl Function { let mut num_in_edges = vec![0; self.blocks.len()]; for block in self.reverse_post_order() { for target in self.successors(block) { - num_in_edges[target.to_usize()] += 1; + num_in_edges[target] += 1; } } let mut changed = false; @@ -6798,7 +6814,7 @@ impl Function { let mut iter_changed = false; for block in self.reverse_post_order() { // Ignore transient empty blocks - if self.blocks[block.to_usize()].insns.is_empty() { continue; } + if self.blocks[block].insns.is_empty() { continue; } loop { let absorbed = self.absorb_dst_block(&num_in_edges, block); if !absorbed { break; } @@ -6819,7 +6835,7 @@ impl Function { fn remove_redundant_patch_points(&mut self) { for block_id in self.reverse_post_order() { let mut seen = HashSet::new(); - let insns = std::mem::take(&mut self.blocks[block_id.to_usize()].insns); + let insns = std::mem::take(&mut self.blocks[block_id].insns); let mut new_insns = Vec::with_capacity(insns.len()); for insn_id in insns { // PatchPoint is never in union-find and it does not have operands, so fake a @@ -6834,7 +6850,7 @@ impl Function { } new_insns.push(insn_id); } - self.blocks[block_id.to_usize()].insns = new_insns; + self.blocks[block_id].insns = new_insns; } } @@ -6844,7 +6860,7 @@ impl Function { fn remove_duplicate_check_interrupts(&mut self) { for block_id in self.reverse_post_order() { let mut seen = false; - let insns = std::mem::take(&mut self.blocks[block_id.to_usize()].insns); + let insns = std::mem::take(&mut self.blocks[block_id].insns); let mut new_insns = Vec::with_capacity(insns.len()); for insn_id in insns { let insn = &self.insns[insn_id]; @@ -6856,7 +6872,7 @@ impl Function { } new_insns.push(insn_id); } - self.blocks[block_id.to_usize()].insns = new_insns; + self.blocks[block_id].insns = new_insns; } } @@ -6927,7 +6943,7 @@ impl Function { // First, find the (PushInlineFrame, PopInlineFrame) pairs to elide. let mut elided_pairs: Vec<(InsnId, InsnId)> = Vec::new(); let mut pending_pushes: Vec = Vec::new(); - for &insn_id in &self.blocks[block_id.to_usize()].insns { + for &insn_id in &self.blocks[block_id].insns { match self.find_ref(insn_id) { Insn::PushInlineFrame { .. } => { pending_pushes.push(PendingPush { push_id: insn_id, frame_observed: false }); @@ -6976,7 +6992,7 @@ impl Function { rewrites.insert(push_id, replacement); rewrites.insert(pop_id, None); } - self.blocks[block_id.to_usize()].insns.retain_mut(|insn_id| { + self.blocks[block_id].insns.retain_mut(|insn_id| { match rewrites.get(insn_id) { Some(Some(replacement)) => { *insn_id = *replacement; @@ -7071,8 +7087,8 @@ impl Function { .insert("id", id.0) .insert("loopDepth", loop_depth) .insert("attributes", Json::array(attributes)) - .insert("predecessors", Json::array(predecessors.iter().map(|x| x.to_usize()).collect::>())) - .insert("successors", Json::array(successors.iter().map(|x| x.to_usize()).collect::>())) + .insert("predecessors", Json::array(predecessors.iter().map(|x| usize::from(*x)).collect::>())) + .insert("successors", Json::array(successors.iter().map(|x| usize::from(*x)).collect::>())) .insert("instructions", Json::array(instructions)) .build() } @@ -7108,7 +7124,7 @@ impl Function { // Push each block from the iteration in reverse post order to `hir_blocks`. for block_id in self.reverse_post_order() { // Create the block with instructions. - let block = &self.blocks[block_id.to_usize()]; + let block = &self.blocks[block_id]; let predecessors = cfi.predecessors(block_id); let successors = cfi.successors(block_id); let mut instructions = Vec::new(); @@ -7304,7 +7320,7 @@ impl Function { /// 3. Every block must have a terminator. fn validate_block_terminators_and_jumps(&self) -> Result<(), ValidationError> { let check_edge = |block_id: BlockId, edge: &BranchEdge| -> Result<(), ValidationError> { - let target_len = self.blocks[edge.target.to_usize()].params.len(); + let target_len = self.blocks[edge.target].params.len(); let args_len = edge.args.len(); if target_len != args_len { return Err(ValidationError::MismatchedBlockArity(block_id, target_len, args_len)); @@ -7313,7 +7329,7 @@ impl Function { }; for block_id in self.reverse_post_order() { - let insns = &self.blocks[block_id.to_usize()].insns; + let insns = &self.blocks[block_id].insns; for (idx, insn_id) in insns.iter().enumerate() { // No need for resolve(): we only look at edge targets/arity and terminators. let insn = self.find_ref(*insn_id); @@ -7359,27 +7375,27 @@ impl Function { // starts with nothing defined. for &block in &rpo { if block == self.entries_block { - assigned_in[block.to_usize()] = Some(InsnSet::with_capacity(self.insns.len())); + assigned_in[block] = Some(InsnSet::with_capacity(self.insns.len())); } else { let mut all_ones = InsnSet::with_capacity(self.insns.len()); all_ones.insert_all(); - assigned_in[block.to_usize()] = Some(all_ones); + assigned_in[block] = Some(all_ones); } } let mut worklist = VecDeque::with_capacity(self.num_blocks()); worklist.push_back(self.entries_block); while let Some(block) = worklist.pop_front() { - let mut assigned = assigned_in[block.to_usize()].clone().unwrap(); - for ¶m in &self.blocks[block.to_usize()].params { + let mut assigned = assigned_in[block].clone().unwrap(); + for ¶m in &self.blocks[block].params { assigned.insert(param); } - for &insn_id in &self.blocks[block.to_usize()].insns { + for &insn_id in &self.blocks[block].insns { let insn_id = self.union_find.borrow().find_const(insn_id); // No need for resolve(): we only look at jump targets here, and the // operand check below resolves each operand itself. let insn = self.find_ref(insn_id); let mut propagate = |target: BlockId| -> Result<(), ValidationError> { - let Some(block_in) = assigned_in[target.to_usize()].as_mut() else { + let Some(block_in) = assigned_in[target].as_mut() else { return Err(ValidationError::JumpTargetNotInRPO(target)); }; if block_in.intersect_with(&assigned) { @@ -7407,11 +7423,11 @@ impl Function { } // Check that each instruction's operands are assigned for &block in &rpo { - let mut assigned = assigned_in[block.to_usize()].clone().unwrap(); - for ¶m in &self.blocks[block.to_usize()].params { + let mut assigned = assigned_in[block].clone().unwrap(); + for ¶m in &self.blocks[block].params { assigned.insert(param); } - for &insn_id in &self.blocks[block.to_usize()].insns { + for &insn_id in &self.blocks[block].insns { let insn_id = self.union_find.borrow().find_const(insn_id); self.insns[insn_id].try_for_each_operand(|operand| { let operand = self.union_find.borrow().find_const(operand); @@ -7432,7 +7448,7 @@ impl Function { fn validate_insn_uniqueness(&self) -> Result<(), ValidationError> { let mut seen = InsnSet::with_capacity(self.insns.len()); for block_id in self.reverse_post_order() { - for &insn_id in &self.blocks[block_id.to_usize()].insns { + for &insn_id in &self.blocks[block_id].insns { let insn_id = self.union_find.borrow().find_const(insn_id); if !seen.insert(insn_id) { return Err(ValidationError::DuplicateInstruction(block_id, insn_id)); @@ -7794,7 +7810,7 @@ impl Function { /// Check that insn types match the expected types for each instruction. fn validate_types(&self) -> Result<(), ValidationError> { for block_id in self.reverse_post_order() { - for &insn_id in &self.blocks[block_id.to_usize()].insns { + for &insn_id in &self.blocks[block_id].insns { self.validate_insn_type(insn_id)?; } } @@ -7987,9 +8003,9 @@ impl<'a> std::fmt::Display for FunctionPrinter<'a> { continue; } write!(f, "{block_id}(")?; - if !fun.blocks[block_id.to_usize()].params.is_empty() { + if !fun.blocks[block_id].params.is_empty() { let mut sep = ""; - for param in &fun.blocks[block_id.to_usize()].params { + for param in &fun.blocks[block_id].params { write!(f, "{sep}{param}")?; let insn_type = fun.type_of(*param); if !insn_type.is_subtype(types::Empty) { @@ -7999,7 +8015,7 @@ impl<'a> std::fmt::Display for FunctionPrinter<'a> { } } writeln!(f, "):")?; - for insn_id in &fun.blocks[block_id.to_usize()].insns { + for insn_id in &fun.blocks[block_id].insns { let insn = fun.find(*insn_id); if !self.display_snapshot_and_tp_patchpoints && matches!(insn, Insn::Snapshot {..} | Insn::PatchPoint { invariant: Invariant::NoTracePoint, .. }) { @@ -10741,13 +10757,13 @@ impl Dominators { // Map BlockId -> RPO index for O(1) lookup in intersect. let mut rpo_order = vec![usize::MAX; num_blocks]; for (idx, &block) in rpo.iter().enumerate() { - rpo_order[block.to_usize()] = idx; + rpo_order[block] = idx; } // Initialize idom: root's idom is itself, everything else is undefined. let mut idoms = vec![IDOM_NONE; num_blocks]; let root = f.entries_block; - idoms[root.to_usize()] = root; + idoms[root] = root; let mut changed = true; while changed { @@ -10759,7 +10775,7 @@ impl Dominators { let preds = cfi.predecessors(block); let mut new_idom = IDOM_NONE; for &p in preds { - if idoms[p.to_usize()] != IDOM_NONE { + if idoms[p] != IDOM_NONE { new_idom = p; break; } @@ -10769,13 +10785,13 @@ impl Dominators { // Intersect with remaining processed predecessors. for &p in preds { if p == new_idom { continue; } - if idoms[p.to_usize()] != IDOM_NONE { + if idoms[p] != IDOM_NONE { new_idom = Self::intersect(&idoms, &rpo_order, p, new_idom); } } - if idoms[block.to_usize()] != new_idom { - idoms[block.to_usize()] = new_idom; + if idoms[block] != new_idom { + idoms[block] = new_idom; changed = true; } } @@ -10788,11 +10804,11 @@ impl Dominators { /// Uses RPO indices: a node with a *lower* RPO index is *higher* in the tree. fn intersect(idoms: &[BlockId], rpo_order: &[usize], mut b1: BlockId, mut b2: BlockId) -> BlockId { while b1 != b2 { - while rpo_order[b1.to_usize()] > rpo_order[b2.to_usize()] { - b1 = idoms[b1.to_usize()]; + while rpo_order[b1] > rpo_order[b2] { + b1 = idoms[b1]; } - while rpo_order[b2.to_usize()] > rpo_order[b1.to_usize()] { - b2 = idoms[b2.to_usize()]; + while rpo_order[b2] > rpo_order[b1] { + b2 = idoms[b2]; } } b1 @@ -10800,7 +10816,7 @@ impl Dominators { /// Return the immediate dominator of `block`. pub fn idom(&self, block: BlockId) -> BlockId { - self.idoms[block.to_usize()] + self.idoms[block] } /// Return true if `left` is dominated by `right`. diff --git a/zjit/src/hir/opt_tests.rs b/zjit/src/hir/opt_tests.rs index f05eb4f592bd04..134f57a5d6d44b 100644 --- a/zjit/src/hir/opt_tests.rs +++ b/zjit/src/hir/opt_tests.rs @@ -2761,7 +2761,7 @@ mod hir_opt_tests { function.eliminate_dead_code(); - let insns = &function.blocks[block.to_usize()].insns; + let insns = &function.blocks[block].insns; assert!(insns.contains(&comment)); assert!(!insns.contains(&dead_const)); }