YJIT: Fix unused warnings ``` warning: unused import: `condition::Condition` --> src/asm/arm64/arg/mod.rs:13:9 | 13 | pub use condition::Condition; | ^^^^^^^^^^^^^^^^^^^^ | = note: `#[warn(unused_imports)]` on by default warning: unused import: `rb_yjit_fix_mul_fix as rb_fix_mul_fix` --> src/cruby.rs:188:9 | 188 | pub use rb_yjit_fix_mul_fix as rb_fix_mul_fix; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ warning: unused import: `rb_insn_len as raw_insn_len` --> src/cruby.rs:142:9 | 142 | pub use rb_insn_len as raw_insn_len; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = note: `#[warn(unused_imports)]` on by default ``` Make asm public so it stops warning about unused public stuff in there. YJIT: Fix ruby2_keywords splat+rest and drop bogus checks YJIT didn't guard for ruby2_keywords hash in case of splat calls that land in methods with a rest parameter, creating incorrect results. The compile-time checks didn't correspond to any actual effects of ruby2_keywords, so it was masking this bug and YJIT was needlessly refusing to compile some code. About 16% of fallback reasons in `lobsters` was due to the ISeq check. We already handle the tagging part with exit_if_supplying_kw_and_has_no_kw() and should now have a dynamic guard for all splat cases. Note for backporting: You also need 7f51959ff1. [Bug #20195]
We've seen quite a few compaction bugs lately, and these assertions should give clearer symptoms. We only call class_of() on objects that the Ruby code can see.
* YJIT: Inline basic Ruby methods * YJIT: Fix "InsnOut operand made it past register allocation" checktype should not generate a useless instruction.
Co-authored-by: Alan Wu <alansi.xingwu@shopify.com>
Previously, the version-controlled `cruby_bindings.inc.rs` file contained the build-time artifact `id.h`, which nobu mentioned hinders the goal of having fewer magic numbers in the repository. Lookup the IDs YJIT needs on boot. It costs cycles, but it's fine since YJIT only uses a handful of IDs at the moment. No perceptible degradation to boot time found in my testing.
* Remove function call for String#bytesize String size is stored in a consistent location, so we can eliminate the function call. * Update yjit/src/codegen.rs Co-authored-by: Takashi Kokubun <takashikkbn@gmail.com> --------- Co-authored-by: Maxime Chevalier-Boisvert <maximechevalierb@gmail.com> Co-authored-by: Takashi Kokubun <takashikkbn@gmail.com>
New Clippy lint in 1.72.0 is breaking our build as GitHub has updated their image. No point hearing about lints from generated code we don't manually write.
Co-authored-by: Maxime Chevalier-Boisvert <maximechevalierb@gmail.com>
Remove rb_control_frame_t::__bp__ and optimize bmethod calls
This commit removes the __bp__ field from rb_control_frame_t. It was
introduced to help MJIT, but since MJIT was replaced by RJIT, we can use
vm_base_ptr() to compute it from the SP of the previous control frame
instead. Removing the field avoids needing to set it up when pushing new
frames.
Simply removing __bp__ would cause crashes since RJIT and YJIT used a
slightly different stack layout for bmethod calls than the interpreter.
At the moment of the call, the two layouts looked as follows:
┌────────────┐ ┌────────────┐
│ frame_base │ │ frame_base │
├────────────┤ ├────────────┤
│ ... │ │ ... │
├────────────┤ ├────────────┤
│ args │ │ args │
├────────────┤ └────────────┘<─prev_frame_sp
│ receiver │
prev_frame_sp─>└────────────┘
RJIT & YJIT interpreter
Essentially, vm_base_ptr() needs to compute the address to frame_base
given prev_frame_sp in the diagrams. The presence of the receiver
created an off-by-one situation.
Make the interpreter use the layout the JITs use for iseq-to-iseq
bmethod calls. Doing so removes unnecessary argument shifting and
vm_exec_core() re-entry from the interpreter, yielding a speed
improvement visible through `benchmark/vm_defined_method.yml`:
patched: 7578743.1 i/s
master: 4796596.3 i/s - 1.58x slower
C-to-iseq bmethod calls now store one more VALUE than before, but that
should have negligible impact on overall performance.
Note that re-entering vm_exec_core() used to be necessary for firing
TracePoint events, but that's no longer the case since
9121e57a5f50bc91bae48b3b91edb283bf96cb6b.
Closes ruby/ruby#6428
The benchmark results show that this feature has either a positive or no impact on performance. The memory usage is also mostly unchanged, except in hexapdf, where there is a decrease in RSS. -------------- ----------- ---------- --------- ----------- ---------- --------- -------------- ------------- bench master (ms) stddev (%) RSS (MiB) branch (ms) stddev (%) RSS (MiB) branch 1st itr master/branch activerecord 70.8 2.2 56.0 71.7 2.2 56.0 0.99 0.99 erubi_rails 20.5 13.6 94.7 20.5 14.3 94.2 0.93 1.00 hexapdf 2541.0 0.7 212.8 2544.4 0.7 203.4 1.00 1.00 liquid-c 65.6 0.3 38.9 65.3 0.3 38.9 1.01 1.01 liquid-compile 63.7 0.3 34.6 61.1 0.2 34.6 1.04 1.04 liquid-render 163.1 0.1 37.1 163.3 0.1 37.1 1.00 1.00 mail 139.3 0.1 50.5 137.0 0.1 50.1 0.99 1.02 psych-load 2065.7 0.1 36.9 2068.2 0.1 37.3 1.00 1.00 railsbench 2034.6 0.5 103.9 2031.9 0.5 103.8 1.02 1.00 ruby-lsp 65.3 3.1 89.8 66.2 3.0 89.7 1.01 0.99 sequel 73.2 1.0 40.3 73.4 1.0 40.3 1.00 1.00 -------------- ----------- ---------- --------- ----------- ---------- --------- -------------- -------------