* YJIT: handle expandarray_rhs_too_small case
YJIT: fix csel bug in x86 backend, add test
Remove commented out lines
Refactor expandarray to use chain guards
Propagate Type::Nil when known
Update yjit/src/codegen.rs
Co-authored-by: Takashi Kokubun <takashikkbn@gmail.com>
* Add missing counter, use get_array_ptr() in expandarray
* Make change suggested by Kokubun to reuse loop
---------
Co-authored-by: Takashi Kokubun <takashikkbn@gmail.com>
Followup: https://github.com/ruby/ruby/pull/8152
If the receiver is a T_MODULE or T_CLASS and has a lot of
ivars, `get_next_shape_internal` will return `NULL`.
Co-authored-by: Jean Boussier <byroot@ruby-lang.org>
YJIT: implement missing jg instruction in backend
While trying to implement a specialize integer left shift, I ran
into a problem where we have no way to do a greater-than comparison
at the moment. Surprising we went this far without ever needing it.
The rest of the counters are prefixed with `gbpp_` and that's what
`yjit.rb` uses when printing the summary. This counter wasn't included
in the summary.
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
9121e57a5f.
Closesruby/ruby#6428
Closes [Feature #19729]
Previously 2 bits of the flags on each RVALUE are reserved to store the
number of GC cycles that each object has survived. This commit
introduces a new bit array on the heap page, called age_bits, to store
that information instead.
This patch still reserves one of the age bits in the flags (the old
FL_PROMOTED0 bit, now renamed FL_PROMOTED).
This is set to 0 for young objects and 1 for old objects, and is used as
a performance optimisation for the write barrier. Fetching the age_bits
from the heap page and doing the required math to calculate if the
object was old or not would slow down the write barrier. So we keep this
bit synced in the flags for fast access.
* Revert "Revert "YJIT: Break register cycles for C arguments (#7918)""
This reverts commit 78ca085785.
* Use shfited_live_ranges for the last-insn check
* YJIT: Fix autosplat miscomp for blocks with optionals
When passing an array as the sole argument to `yield`, and the yieldee
takes more than 1 optional parameter, the array is expanded similar
to `*array` splat calls. This is called "autosplat" in
`setup_parameters_complex()`.
Previously, YJIT did not detect this autosplat condition. It passed the
array without expanding it, deviating from interpreter behavior.
Detect this conditon and refuse to compile it.
Fixes: Shopify/yjit#313
* RJIT: Fix autosplat miscomp for blocks with optionals
This is mirrors the same issue as YJIT. See previous commit.
* Make TAINT and UNTRUSTED flags zero
These flags do nothing already, and should break nothing.
* Remove TAINT and UNTRUSTED macros same as functions
These macros had been defined to use with `#ifdef`, but should not be
used anymore.
`IO#reopen` is very special in that it is able to change the class and
singleton class of IO instances. In its presence, it is not correct to
assume that IO instances has a stable class/singleton class and guard
by comparing identity.
* Unify length field for embedded and heap strings
The length field is of the same type and position in RString for both
embedded and heap allocated strings, so we can unify it.
* Remove RSTRING_EMBED_LEN
This reverts commit 10621f7cb9.
This was reverted because the gc integrity build started failing. We
have figured out a fix so I'm reopening the PR.
Original commit message:
Fix cvar caching when class is cloned
The class variable cache that was added in
ruby#4544 changed the behavior of class
variables on cloned classes. As reported when a class is cloned AND a
class variable was set, and the class variable was read from the
original class, reading a class variable from the cloned class would
return the value from the original class.
This was happening because the IC (inline cache) is stored on the ISEQ
which is shared between the original and cloned class, therefore they
share the cache too.
To fix this we are now storing the `cref` in the cache so that we can
check if it's equal to the current `cref`. If it's different we don't
want to read from the cache. If it's the same we do. Cloned classes
don't share the same cref with their original class.
This will need to be backported to 3.1 in addition to 3.2 since the bug
exists in both versions.
We also added a marking function which was missing.
Fixes [Bug #19379]
Co-authored-by: Aaron Patterson <tenderlove@ruby-lang.org>
The class variable cache that was added in
https://github.com/ruby/ruby/pull/4544 changed the behavior of class
variables on cloned classes. As reported when a class is cloned AND a
class variable was set, and the class variable was read from the
original class, reading a class variable from the cloned class would
return the value from the original class.
This was happening because the IC (inline cache) is stored on the ISEQ
which is shared between the original and cloned class, therefore they
share the cache too.
To fix this we are now storing the `cref` in the cache so that we can
check if it's equal to the current `cref`. If it's different we don't
want to read from the cache. If it's the same we do. Cloned classes
don't share the same cref with their original class.
This will need to be backported to 3.1 in addition to 3.2 since the bug
exists in both versions.
We also added a marking function which was missing.
Fixes [Bug #19379]
Co-authored-by: Aaron Patterson <tenderlove@ruby-lang.org>
Originally, when 2e7bceb34e fixed cfuncs to no
longer use the VM stack for large array splats, it was thought to have fully
fixed Bug #4040, since the issue was fixed for methods defined in Ruby (iseqs)
back in Ruby 2.2.
After additional research, I determined that same issue affects almost all
types of method calls, not just iseq and cfunc calls. There were two main
types of remaining issues, important cases (where large array splat should
work) and pedantic cases (where large array splat raised SystemStackError
instead of ArgumentError).
Important cases:
```ruby
define_method(:a){|*a|}
a(*1380888.times)
def b(*a); end
send(:b, *1380888.times)
:b.to_proc.call(self, *1380888.times)
def d; yield(*1380888.times) end
d(&method(:b))
def self.method_missing(*a); end
not_a_method(*1380888.times)
```
Pedantic cases:
```ruby
def a; end
a(*1380888.times)
def b(_); end
b(*1380888.times)
def c(_=nil); end
c(*1380888.times)
c = Class.new do
attr_accessor :a
alias b a=
end.new
c.a(*1380888.times)
c.b(*1380888.times)
c = Struct.new(:a) do
alias b a=
end.new
c.a(*1380888.times)
c.b(*1380888.times)
```
This patch fixes all usage of CALLER_SETUP_ARG with splatting a large
number of arguments, and required similar fixes to use a temporary
hidden array in three other cases where the VM would use the VM stack
for handling a large number of arguments. However, it is possible
there may be additional cases where splatting a large number
of arguments still causes a SystemStackError.
This has a measurable performance impact, as it requires additional
checks for a large number of arguments in many additional cases.
This change is fairly invasive, as there were many different VM
functions that needed to be modified to support this. To avoid
too much API change, I modified struct rb_calling_info to add a
heap_argv member for storing the array, so I would not have to
thread it through many functions. This struct is always stack
allocated, which helps ensure sure GC doesn't collect it early.
Because of how invasive the changes are, and how rarely large
arrays are actually splatted in Ruby code, the existing test/spec
suites are not great at testing for correct behavior. To try to
find and fix all issues, I tested this in CI with
VM_ARGC_STACK_MAX to -1, ensuring that a temporary array is used
for all array splat method calls. This was very helpful in
finding breaking cases, especially ones involving flagged keyword
hashes.
Fixes [Bug #4040]
Co-authored-by: Jimmy Miller <jimmy.miller@shopify.com>
This check was introduced to match an assertion in the C YJIT when this
was originally introduced. I don't believe it's necessary for
correctness of the generated code.
Co-authored-by: Adam Hess <HParker@github.com>
Co-authored-by: Daniel Colson <danieljamescolson@gmail.com>
Co-authored-by: Luan Vieira <luanzeba@github.com>
* YJIT: Avoid splitting mov for small values on arm64
* Fix a comment
Co-authored-by: Alan Wu <XrXr@users.noreply.github.com>
* YJIT: Test the 0xffff boundary
---------
Co-authored-by: Alan Wu <XrXr@users.noreply.github.com>
yjit-trace-exits appends a synthetic sample for the instruction being
exited, but we didn't increment the size of the stack. Fixing this count
correctly lets us successfully generate a flamegraph from the exits.
I also replaced the line number for instructions with 0, as I don't
think the previous value had meaning.
Co-authored-by: Adam Hess <HParker@github.com>
Previously, setinstancevariable could generate code that calls
`rb_ensure_iv_list_size()` without first updating `cfp->sp`. This means
in the event that a GC start from within said routine the top few
objects would not be marked, causing them to be falsly collected.
Call `jit_prepare_routine_call()` first.
[Bug #19601]
Add a sampling option to trace exits
Running YJIT with trace exits enabled can make very large metrics files.
This allows us to configure a sample rate to make tracing exits possible
on larger tests. This also updates the documented YJIT options.
Co-authored-by: Alan Wu <XrXr@users.noreply.github.com>
Co-authored-by: John Hawthorn <john@hawthorn.email>
Co-authored-by: Maxime Chevalier-Boisvert <maximechevalierb@gmail.com>
Previously we were missing a compile-time check that the known cfuncs
receive the correct number of arguments.
We noticied this because in particular when using ARGS_SPLAT, which also
wasn't checked, YJIT would crash on code which was otherwise correct
(didn't raise exceptions in the VM).
This still supports vararg (argc == -1) cfuncs. I added an additional
assertion that when we use the specialized codegen for one of these
known functions that the argc are popped off the stack correctly, which
should help ensure they're implemented correctly (previously the crash
was usually observed on a future `leave` insn).
[Bug #19595]
* YJIT: Let Assembler own Context
* Update a comment
Co-authored-by: Maxime Chevalier-Boisvert <maximechevalierb@gmail.com>
---------
Co-authored-by: Maxime Chevalier-Boisvert <maximechevalierb@gmail.com>
* YJIT: Stack temp register allocation for arm64
* Update a comment
Co-authored-by: Maxime Chevalier-Boisvert <maximechevalierb@gmail.com>
* Update comments about assertion
* Update a comment
Co-authored-by: Maxime Chevalier-Boisvert <maximechevalierb@gmail.com>
---------
Co-authored-by: Maxime Chevalier-Boisvert <maximechevalierb@gmail.com>
The socket extensions rubysocket.h pulls in the "private" include/gc.h,
which now depends on vm_core.h. vm_core.h pulls in id.h
when tool/update-deps generates the dependencies for the makefiles, it
generates the line for id.h to be based on VPATH, which is configured in
the extconf.rb for each of the extensions. By default VPATH does not
include the actual source directory of the current Ruby so the
dependency fails to resolve and linking fails.
We need to append the topdir and top_srcdir to VPATH to have the
dependancy picked up correctly (and I believe we need both of these to
cope with in-tree and out-of-tree builds).
I copied this from the approach taken in
https://github.com/ruby/ruby/blob/master/ext/objspace/extconf.rb#L3
Remove !USE_RVARGC code
[Feature #19579]
The Variable Width Allocation feature was turned on by default in Ruby
3.2. Since then, we haven't received bug reports or backports to the
non-Variable Width Allocation code paths, so we assume that nobody is
using it. We also don't plan on maintaining the non-Variable Width
Allocation code, so we are going to remove it.
C function frames don't need to use the VM-specific pc field to run
properly. When pushing a control frame from output code, save one
instruction by leaving the field uninitialized.
Fix-up rb_vm_svar_lep(), which is used while setting local variables via
Regexp#=~. Use cfp->iseq as a secondary signal so it can stop assuming
that all CFUNC frames always have zero pc's.
Making overlapping `&mut`s triggers Undefined Bahavior. This function
previously had them through `cb` and `ocb` aliasing with `self` or live
references in the caller.
To fix the overlap, take `ocb` as a parameter and don't use `get_inline_cb()`
in the body of the function.
* YJIT: Add --yjit-pause and RubyVM::YJIT.resume
This allows booting YJIT in a suspended state. We chose to add a new
command line option as opposed to simply allowing YJIT.resume to work
without any command line option because it allows for combining with
YJIT tuning command line options. It also simpifies implementation.
Paired with Kokubun and Maxime.
* Update yjit.rb
Co-authored-by: Takashi Kokubun <takashikkbn@gmail.com>
---------
Co-authored-by: Alan Wu <XrXr@users.noreply.github.com>
Co-authored-by: Takashi Kokubun <takashikkbn@gmail.com>
If the iseq only contains `opt_invokebuiltin_delegate_leave` insn and
the builtin-function (bf) is inline-able, the caller doesn't need to
build a method frame.
`vm_call_single_noarg_inline_builtin` is fast path for such cases.
We crashed in some edge cases due to the recent change to not compile
encoded iseqs that are larger than `u16::MAX`.
- Match the C signature of rb_yjit_constant_ic_update() and clamp down
to `IseqIdx` size
- Return failure instead of panicking with `unwrap()` in codegen when
the iseq is too large
Co-authored-by: Maxime Chevalier-Boisvert <maxime.chevalierboisvert@shopify.com>
Co-authored-by: Noah Gibbs <noah.gibbs@shopify.com>
So by itself, this shouldn't have been a correctness issue, but we
also pop the stack for block_args. Doing stack manipulation like that
and then side-exiting causes issues. So, while this fixes the
immediate failure, we have a bigger issue with block_args popping and
then exiting that we need to deal with.
This reverts commit 5d0a1ffafa.
This commit is causing sequel in yjit-bench to raise with this stack trace:
```
sequel-5.64.0/lib/sequel/dataset/sql.rb:266:in `literal': wrong argument type Array (expected Proc) (TypeError)
from sequel-5.64.0/lib/sequel/database/misc.rb:269:in `literal'
from sequel-5.64.0/lib/sequel/adapters/shared/sqlite.rb:314:in `column_definition_default_sql'
from sequel-5.64.0/lib/sequel/database/schema_methods.rb:564:in `block in column_definition_sql'
from sequel-5.64.0/lib/sequel/database/schema_methods.rb:564:in `each'
from sequel-5.64.0/lib/sequel/database/schema_methods.rb:564:in `column_definition_sql'
from sequel-5.64.0/lib/sequel/database/schema_methods.rb:634:in `block in column_list_sql'
from sequel-5.64.0/lib/sequel/database/schema_methods.rb:634:in `map'
from sequel-5.64.0/lib/sequel/database/schema_methods.rb:634:in `column_list_sql'
from sequel-5.64.0/lib/sequel/database/schema_methods.rb:753:in `create_table_sql'
from sequel-5.64.0/lib/sequel/adapters/shared/sqlite.rb:348:in `create_table_sql'
from sequel-5.64.0/lib/sequel/database/schema_methods.rb:702:in `create_table_from_generator'
from sequel-5.64.0/lib/sequel/database/schema_methods.rb:203:in `create_table'
from benchmarks/sequel/benchmark.rb:19:in `<main>'
```
This assert would've caught a bug I wrote while developing
ruby/ruby#7443 so I figured it would be good to commit it
as it could be helpful in the future.
* YJIT: Rest and block_arg support
* Update bootstraptest/test_yjit.rb
---------
Co-authored-by: Maxime Chevalier-Boisvert <maximechevalierb@gmail.com>
`Rc` and `RefCell` both incur runtime space costs.
In addition, `RefCell` has given us some headaches with the
non obvious borrow panics it likes to throw out. The latest
one started with 7fd53eeb46
and is yet to be resolved.
Since we already rely on the GC to properly reclaim memory for `Block`
and `Branch`, we might as well stop paying the overhead of `Rc` and
`RefCell`. The `RefCell` panics go away with this change, too.
On 25 iterations of `railsbench` with a stats build I got
`yjit_alloc_size: 8,386,129 => 7,348,637`, with the new memory size 87.6%
of the status quo. This makes the metadata and machine code size roughly
line up one-to-one.
The general idea here is to use `&` shared references with
[interior mutability][1] with `Cell`, which doesn't take any extra
space. The `noalias` requirement that `&mut` imposes is way too hard to
meet and verify. Imagine replacing places where we would've gotten
`BorrowError` from `RefCell` with Rust/LLVM miscompiling us due to aliasing
violations. With shared references, we don't have to think about subtle
cases like the GC _sometimes_ calling the mark callback while codegen
has an aliasing reference in a stack frame below. We mostly only need to
worry about liveness, with which the GC already helps.
There is now a clean split between blocks and branches that are not yet
fully constructed and ones that are "in-service", so to speak. Working
with `PendingBranch` and `JITState` don't really involve `unsafe` stuff.
This change allows `Branch` and `Block` to not have as many optional
fields as many of them are only optional during compilation. Fields that
change post-compilation are wrapped in `Cell` to facilitate mutation
through shared references.
I do some `unsafe` dances here. I've included just a couple tests to run
with Miri (`cargo +nightly miri test miri`). We can add more Miri tests
if desired.
[1]: https://doc.rust-lang.org/std/cell/struct.UnsafeCell.html
* Make EC required on JIT state
Lets make EC required on the JITState object so we don't need to
`unwrap` it.
* Minor nitpicks
---------
Co-authored-by: Alan Wu <XrXr@users.noreply.github.com>
* YJIT: upgrade type in guard_object_is_string
Also make logic more in line with other guard_xxx methods
* Update yjit/src/core.rs
* Revert changes to Type::upgrade()
This works much like the existing `defined` implementation,
but calls out to rb_ivar_defined instead of the more general
rb_vm_defined.
Other difference to the existing `defined` implementation is
that this new instruction has to take the same operands as
the CRuby `defined_ivar` instruction.
Fix https://github.com/Shopify/yjit/issues/310
[Bug #19483]
Co-authored-by: Maxime Chevalier-Boisvert <maxime.chevalierboisvert@shopify.com>
Co-authored-by: Jimmy Miller <jimmy.miller@shopify.com>
If you have a method that takes rest arguments and a splat call that
happens to line up perfectly with that rest, you can just dupe the
array rather than move anything around. We still have to dupe, because
people could have a custom to_a method or something like that which
means it is hard to guarantee we have exclusive access to that array.
Example:
```ruby
def foo(a, b, *rest)
end
foo(1, 2, *[3, 4])
```
The code and comments in there have been disabled by comments for a long
time. The issues that the counter used to solve are now solved more
comprehensively by "runningness" [tracking][1] introduced by Code GC
and [delayed deallocation][2].
Having a single counter doesn't fit our current model where code pages
that could be touched or not are interleaved, anyway.
Just delete the code.
[1]: e7c71c6c92
[2]: a0b0365e90
Related to:
https://github.com/ruby/ruby/pull/7377
Previously it was believed that there was a problem with a combination
of cfuncs + splat + send, but it turns out the same issue happened
without send. For example `Integer.sqrt(1, *[])`. The issue was
happened not because of send, but because of setting the wrong argc
when we don't need to splat any args.
* YJIT: Use a boxed slice for gc_obj_offsets
* YJIT: Stop using Option
* YJIT: s/add_counter/incr_counter_by/
Co-authored-by: Maxime Chevalier-Boisvert <maxime.chevalierboisvert@shopify.com>
Previously, YJIT failed to put the stack into the correct shape when
`BasicObject#send` calls an alias method for the send method itself.
This can manifest as strange `NoMethodError`s in the final non-send
receiver, as [seen][1] with the kt-paperclip gem. I also found a case
where it makes YJIT fail the stack size assertion while compiling
`leave`.
YJIT's `BasicObject#__send__` implementation already rejects sends to
`send`, but didn't detect sends to aliases of `send`. Adjust the
detection and reject these cases.
Fixes [Bug #19464]
[1]: https://github.com/Shopify/yjit/issues/306
`make test-spec` revealed this issue after applying an unrelated bug
fix. A crashing case is included, though I suspect there are other
scenarios where it misbehaves. Don't compile for now.
Note that this is *not* an issue on the 3.2.x series; it has
`send_args_splat_non_iseq` which already rejects all splats to cfuncs,
including sends with splats.
Previously, when Block::entry_exit is requested from any instruction
that is not the first one in the block, we generated the exit with an
incorrect PC. We should always be using the PC for the entry of the
block for Block::entry_exit.
It was a simple typo. The bug was [introduced][1] while we were
refactoring to use the current backend. Later, we had a chance to spot
this issue while [preparing][2] to enable unused variable warnings, but
didn't spot the issue.
Fixes [Bug #19463]
[1]: 27fcab995e
[2]: 31461c7e0e
If the previous instruction is not a leaf instruction, then the PC was
incremented before the instruction was ran (meaning the currently
executing instruction is actually the previous instruction), so we
should not increment the PC otherwise we will calculate the source
line for the next instruction.
This bug can be reproduced in the following script:
```
require "objspace"
ObjectSpace.trace_object_allocations_start
a =
1.0 / 0.0
p [ObjectSpace.allocation_sourceline(a), ObjectSpace.allocation_sourcefile(a)]
```
Which outputs: [4, "test.rb"]
This is incorrect because the object was allocated on line 10 and not
line 4. The behaviour is correct when we use a leaf instruction (e.g.
if we replaced `1.0 / 0.0` with `"hello"`), then the output is:
[10, "test.rb"].
[Bug #19456]
I have this as a shell command and Maxime told me that she finds it
useful, too. I tested this on a release build and a dev build.
Note I intentional didn't put `$(Q)` in front of everything so `make`
echos the command it runs.
These jit_* methods don't jit code, but instead check things on the
JITState. We had other methods that did the same thing that were just
added on the impl JITState. For consistency I added these methods there.
We saw SEGVs due to this when running with StackProf, which needs a
correct PC for RUBY_INTERNAL_EVENT_NEWOBJ, the same event used for
ObjectSpace allocation tracing.
[Bug #19444]
Given that signleton classes don't have an allocator,
we can re-use these bytes to store the attached object
in `rb_classext_struct` without making it larger.
Right now the attached object is stored as an instance variable
and all the call sites that either get or set it have to know how it's
stored.
It's preferable to hide this implementation detail behind accessors
so that it is easier to change how it's stored.
This patch is follo-up of 0a82bfe.
Without this patch, if env is escaped (Proc'ed), strange svar
can be touched.
This patch tracks escaped env and use it.
Support invokesuper in a block on YJIT
invokesuper previously side exited when it is in a block. To make sure we're compiling the correct method in super, we now use the local environment pointer (LEP) to get the method, which will work in a block.
Co-authored-by: John Hawthorn <john@hawthorn.email>
Previously on ARM64 Linux systems that use 64 KiB pages
(`CONFIG_ARM64_64K_PAGES=y`), YJIT was panicking on boot due to a failed
assertion.
The assertion was making sure that code GC can free the last code page
that YJIT manages without freeing unrelated memory. YJIT prefers picking
16 KiB as the granularity at which to free code memory, but when the
system can only free at 64 KiB granularity, that is not possible.
The fix is to use the system page size as the code page size when the
system page size is 64 KiB. Continue to use 16 KiB as the code page size
on common systems that use 16/4 KiB pages.
Add asserts to code_gc() and free_page() about code GC's assumptions.
Fixes [Bug #19400]
We set the block address as soon as we make the block, so there is no
point in making it `Option<CodePtr>`. No memory saving, unfortunately,
as `mem::size_of::<Block>() = 176` before and after this change. Still
a simplification for the logic, though.
Previously, with Code GC, YJIT panicked while trying to emit a B.cond
instruction with an offset that is not encodable in 19 bits. This only
happens when the code in an assembler instance straddles two pages.
To fix this, when we detect that a jump to a label can land on a
different page, we switch to a fresh new page and regenerate all the
code in the assembler there. We still assume that no one assembler has
so much code that it wouldn't fit inside a fresh new page.
[Bug #19385]
Co-authored-by: Takashi Kokubun <takashikkbn@gmail.com>
Co-authored-by: Maxime Chevalier-Boisvert <maxime.chevalierboisvert@shopify.com>
* use correct svar
Without this patch, svar location is used "nearest Ruby frame".
It is almost correct but it doesn't correct when the `each` method
is written in Ruby.
```ruby
class C
include Enumerable
def each
%w(bar baz).each{|e| yield e}
end
end
C.new.grep(/(b.)/){|e| p [$1, e]}
```
This patch fix this issue by traversing ifunc's cfp.
Note that if cfp doesn't specify this Thread's cfp stack, reserved
svar location (`ec->root_svar`) is used.
* make yjit-bindgen
---------
Co-authored-by: Takashi Kokubun <takashikkbn@gmail.com>
Grouping these together helps with finding all of the unimplemented
method types. It was interleaved with some other match arm long and
short previously.
Rust 1.58.0 unfortunately doesn't provide facilities to control symbol
visibility/presence, but we care about controlling the list of
symbols exported from libruby-static.a and libruby.so.
This commit uses `ld -r` to make a single object out of rustc's
staticlib output, libyjit.a. This moves libyjit.a out of MAINLIBS and adds
libyjit.o into COMMONOBJS, which obviates the code for merging libyjit.a
into libruby-static.a. The odd appearance of libyjit.a in SOLIBS is also
gone.
To filter out symbols we do not want to export on ELF platforms, we use
objcopy after the partial link. On darwin, we supply a symbol list to
the linker which takes care of hiding unprefixed symbols.
[Bug #19255]
Co-authored-by: Nobuyoshi Nakada <nobu@ruby-lang.org>
Note: On the new code of yjit/src/core.rs:2178, we no longer leave the state `.block=None` but `.address=Some...`, which might be important.
We assume it's actually not needed and take a risk here to minimize heap allocations, but in case it turns out to be necessary, we could signal/resurrect that state by introducing a new BranchTarget (or BranchShape) variant dedicated to it.
YJIT: Implement splat for cfuncs. Split exit cases
This also implements a new check for ruby2keywords as the last
argument of a splat. This does mean that we generate more code, but in
actual benchmarks where we gained speed from this (binarytrees) I
don't see any significant slow down. I did have to struggle here with
the register allocator to find code that didn't allocate too many
registers. It's a bit hard when everything is implicit. But I think I
got to the minimal amount of copying and stuff given our current
allocation strategy.
This reduces the code size of libyjit.a by a lot. On darwin it went from
23 MiB to 12 MiB for me. I chose ThinLTO over fat LTO for the relatively
fast build time; in case we need to debug release-build-only problems
it won't be painful.
* Add job to check clippy lints in CI
* Address all remaining clippy lints
* Check lints on arm64 as well
* Apply latest clippy lints
* Do not exit 0 on clippy warnings
* Differentiate T_ARRAY and array subclasses
This commit teaches the YJIT context the difference between Arrays
(objects with type T_ARRAY and class rb_cArray) vs Array subclasses
(objects with type T_ARRAY but _not_ class rb_cArray). It uses this
information to reduce the number of guards emitted when using
`jit_guard_known_klass` with rb_cArray, notably opt_aref
* Update yjit/src/core.rs
Co-authored-by: Maxime Chevalier-Boisvert <maximechevalierb@gmail.com>
Previously, we did not update `cfp->sp` before calling the C function of
ISEQs marked with `Primitive.attr! "inline"` (leaf builtins). This
caused the GC to miss temporary values on the stack in case the function
allocates and triggers a GC run. Right now, there is only a few leaf
builtins in numeric.rb on Integer methods such as `Integer#~`. Since
these methods only allocate when operating on big numbers, we missed
this issue.
Fix by saving PC and SP before calling the functions -- our usual
protocol for calling C functions that may allocate on the GC heap.
[Bug #19316]
* YJIT: Make iseq_get_location consistent with iseq.c
* YJIT: Call it "YJIT entry point"
Co-authored-by: Maxime Chevalier-Boisvert <maximechevalierb@gmail.com>
Co-authored-by: Maxime Chevalier-Boisvert <maximechevalierb@gmail.com>
We should differentiate between set and get for megamorphic exits. This
patch fixes the megamorphic exit name in gen_setinstancevariable so that
we can tell the difference between megamorphic get / set sites
On a hash miss we need to call default if it is redefined in order to
return the default value to be used. Previously we checked this with
rb_method_basic_definition_p, which avoids the method call but requires
a method lookup.
This commit replaces the previous check with BASIC_OP_UNREDEFINED_P and
a new BOP_DEFAULT. We still need to fall back to
rb_method_basic_definition_p when called on a subclasss of hash.
| |compare-ruby|built-ruby|
|:---------------|-----------:|---------:|
|hash_aref_miss | 2.692| 3.531|
| | -| 1.31x|
Co-authored-by: Daniel Colson <danieljamescolson@gmail.com>
Co-authored-by: "Ian C. Anderson" <ian@iancanderson.com>
Co-authored-by: Jack McCracken <me@jackmc.xyz>
All the method call types need to handle argument shifting in case they're
called by `.send`, and we weren't handling that in `OPTIMIZED_METHOD_TYPE_CALL`.
Lack of shifting caused the stack size assertion in gen_leave() to fail.
Discovered by Rails CI: https://buildkite.com/rails/rails/builds/91705#018516c4-f8f8-469e-bc2d-ddeb25ca8317/1920-2067
Diagnosed with help from `@eileencodes` and `@k0kubun`.
SIZE_POOL_COUNT is a GC macro, it should belong in gc.h and not shape.h.
SIZE_POOL_COUNT doesn't depend on shape.h so we can have shape.h depend
on gc.h.
Co-Authored-By: Matt Valentine-House <matt@eightbitraptor.com>
Stubs we generate for invalidation don't necessarily co-locate with the
code that jump to the stub. Since we rely on co-location to keep stubs
alive as they are in the outlined code block, it used to be possible for
code GC inside branch_stub_hit() to free the stub that's its direct
caller, leading us to return to freed code after.
Stubs used to look like:
```
mov arg0, branch_ptr
mov arg1, target_idx
mov arg2, ec
call branch_stub_hit
jmp return_reg
```
Since the call and the jump after the call is the same for all stubs, we
can extract them and use a static trampoline for them. That makes
branch_stub_hit() always return to static code. Stubs now look like:
```
mov arg0, branch_ptr
mov arg1, target_idx
jmp trampoline
```
Where the trampoline is:
```
mov arg2, ec
call branch_stub_hit
jmp return_reg
```
Code GC can now free stubs without problems since we'll always return
to the trampoline, which we generate once on boot and lives forever.
This might save a small bit of memory due to factoring out the static
part of stubs, but it's probably minor.
[Bug #19234]
Co-authored-by: Takashi Kokubun <takashikkbn@gmail.com>
When an object becomes "too complex" (in other words it has too many
variations in the shape tree), we transition it to use a "too complex"
shape and use a hash for storing instance variables.
Without this patch, there were rare cases where shape tree growth could
"explode" and cause performance degradation on what would otherwise have
been cached fast paths.
This patch puts a limit on shape tree growth, and gracefully degrades in
the rare case where there could be a factorial growth in the shape tree.
For example:
```ruby
class NG; end
HUGE_NUMBER.times do
NG.new.instance_variable_set(:"@unique_ivar_#{_1}", 1)
end
```
We consider objects to be "too complex" when the object's class has more
than SHAPE_MAX_VARIATIONS (currently 8) leaf nodes in the shape tree and
the object introduces a new variation (a new leaf node) associated with
that class.
For example, new variations on instances of the following class would be
considered "too complex" because those instances create more than 8
leaves in the shape tree:
```ruby
class Foo; end
9.times { Foo.new.instance_variable_set(":@uniq_#{_1}", 1) }
```
However, the following class is *not* too complex because it only has
one leaf in the shape tree:
```ruby
class Foo
def initialize
@a = @b = @c = @d = @e = @f = @g = @h = @i = nil
end
end
9.times { Foo.new }
``
This case is rare, so we don't expect this change to impact performance
of most applications, but it needs to be handled.
Co-Authored-By: Aaron Patterson <tenderlove@ruby-lang.org>
* YJIT: Generate debug info in release builds
They are helpful in case we need to do core dump debugging.
* Remove Cirrus DOC skip rule
The syntax for this is weird, and escaping [ and ] cause parse failures.
Cirrus' docs said to surround with .*, but then that seems to skip
everything. Revert e0a4205eb7 for now.
* YJIT: implement getconstant YARV instruction
* Constant id is not a pointer
* Stack operands must be read after jit_prepare_routine_call
Co-authored-by: Takashi Kokubun <takashikkbn@gmail.com>
The new version has an option to merge everything into a big
`extern "C"` block and it's nicer.
More importantly, this upgrade fixes an issue where Ubuntu with Clang 12
and macOS with Clang 14 gave a one line diff for `rb_shape_t`. It was
slightly annoying because we use macOS locally.
With this change, we're storing the iv name on an inline cache on
setinstancevariable instructions. This allows us to check the inline
cache to count instance variables set in initialize and give us an
estimate of iv capacity for an object.
For the purpose of estimating the number of instance variables required
for an object, we're assuming that all initialize methods will call
`super`.
This change allows us to estimate the number of instance variables
required without disassembling instruction sequences.
Co-Authored-By: Aaron Patterson <tenderlove@ruby-lang.org>
Prior to this commit the `OPTIMIZED_CMP` macro relied on a method lookup
to determine whether `<=>` was overridden. The result of the lookup was
cached, but only for the duration of the specific method that
initialized the cmp_opt_data cache structure.
With this method lookup, `[x,y].max` is slower than doing `x > y ?
x : y` even though there's an optimized instruction for "new array max".
(John noticed somebody a proposed micro-optimization based on this fact
in https://github.com/mastodon/mastodon/pull/19903.)
```rb
a, b = 1, 2
Benchmark.ips do |bm|
bm.report('conditional') { a > b ? a : b }
bm.report('method') { [a, b].max }
bm.compare!
end
```
Before:
```
Comparison:
conditional: 22603733.2 i/s
method: 19820412.7 i/s - 1.14x (± 0.00) slower
```
This commit replaces the method lookup with a new CMP basic op, which
gives the examples above equivalent performance.
After:
```
Comparison:
method: 24022466.5 i/s
conditional: 23851094.2 i/s - same-ish: difference falls within
error
```
Relevant benchmarks show an improvement to Array#max and Array#min when
not using the optimized newarray_max instruction as well. They are
noticeably faster for small arrays with the relevant types, and the same
or maybe a touch faster on larger arrays.
```
$ make benchmark COMPARE_RUBY=<master@5958c305> ITEM=array_min
$ make benchmark COMPARE_RUBY=<master@5958c305> ITEM=array_max
```
The benchmarks added in this commit also look generally improved.
Co-authored-by: John Hawthorn <jhawthorn@github.com>
This commit moves ruby_basic_operators and the unredefined macros out of
vm_core.h and into basic_operators.h so that we can use them more
broadly in places where we currently use a method look up via
`rb_method_basic_definition_p` (e.g. object.c, numeric.c, complex.c,
enum.c, but also in internal/compar.h after introducing BOP_CMP and
elsewhere if we introduce more BOPs)
The most controversial part of this change is probably moving
redefined_flag out of rb_vm_t. [vm_opt_method_def_table and
vm_opt_mid_table](9da2a5204f/vm.c)
are not part of rb_vm_t either, and I think this fits well with those.
But more significantly it seems to result in one fewer instruction. For
example:
Before:
```
(lldb) disassemble -n vm_opt_str_freeze
miniruby`vm_exec_core:
miniruby[0x10028233e] <+14558>: movq 0x11a86b(%rip), %rax ; ruby_current_vm_ptr
miniruby[0x100282345] <+14565>: testb $0x4, 0x242c(%rax)
```
After:
```
(lldb) disassemble -n vm_opt_str_freeze
ruby`vm_exec_core:
ruby[0x100280ebe] <+14510>: testb $0x4, 0x120147(%rip) ; ruby_vm_redefined_flag + 43
```
Co-authored-by: John Hawthorn <jhawthorn@github.com>
Certain code page sizes don't work and can cause crashes, so having this
value available as a command-line option is a bit dangerous. Remove it
and turn it into a constant instead.
* YJIT: Make case-when optimization respect === redefinition
Even when a fixnum key is in the dispatch hash, if there is a case such
that its basic operations for === is redefined, we need to fall back to
checking each case like the interpreter. Semantically we're always
checking each case by calling === in order, it's just that this is not
observable when basic operations are intact.
When all the keys are fixnums, though, we can do the optimization we're
doing right now. Check for this condition.
* Update yjit/src/cruby_bindings.inc.rs
Co-authored-by: Takashi Kokubun <takashikkbn@gmail.com>
Co-authored-by: Takashi Kokubun <takashikkbn@gmail.com>
* YJIT: Reorder branches for Fixnum opt_case_dispatch
Co-authored-by: Maxime Chevalier-Boisvert <maxime.chevalierboisvert@shopify.com>
Co-authored-by: Alan Wu <alansi.xingwu@shopify.com>
* YJIT: Don't support too large values
Co-authored-by: Maxime Chevalier-Boisvert <maxime.chevalierboisvert@shopify.com>
Co-authored-by: Alan Wu <alansi.xingwu@shopify.com>
* Fix 32 and 16 bit register store in YJIT
Co-Authored-By: Takashi Kokubun <takashikkbn@gmail.com>
* Remove an unnecessary diff
* Reuse an rm_num_bits result
* Use u16::MAX instead
* Update the link
Co-authored-by: Alan Wu <XrXr@users.noreply.github.com>
* Just use sturh for 16 bits
Co-authored-by: Takashi Kokubun <takashikkbn@gmail.com>
Co-authored-by: Alan Wu <XrXr@users.noreply.github.com>
Previously we essentially never freed block even after invalidation.
Their reference count never reached zero for a couple of reasons:
1. `Branch::block` formed a cycle with the block holding the branch
2. Strong count on a branch that has ever contained a stub never
reached 0 because we increment the `.clone()` call for
`BranchRef::into_raw()` didn't have a matching decrement.
It's not safe to immediately deallocate blocks during
invalidation since `branch_stub_hit()` can end up
running with a branch pointer from an invalidated branch.
To plug the leaks, we wait until code GC or global invalidation and
deallocate the blocks for iseqs that are definitely not running.
When we run global invalidation for TracePoints or code GC, we clear out
all blocks in our assumptions table but we don't deallocate the backing
buffers. Let's reclaim some memory during these rare events.
HashSet::clear() doesn't deallocate the backing buffer and shrink the
capacity. Replace with a 0-capcity set instead so we reclaim some memory
each code GC.
instead of FILE*.
Using C.fprintf is slower than String manipulation on memory. I'm going
to change the way MJIT writes files, and this is a prerequisite for it.
We frequently make branches that only have one target but we used to
always allocate space for two branch targets. This patch moves all the
information a branch target has into a struct and refer to them using
Option<Box<BranchTarget>>, this way when the second branch target is not
present it only takes 8 bytes.
Retained heap size on railsbench went from 16.17 MiB to 14.57 MiB, a
ratio of about 1.1.
YJIT: x86_64: Fix cmp with number where sign bit is set
Before this commit, we were unconditionally treating unsigned ints as
signed ints when counting the number of bits required for representing
the immediate in machine code. When the size of the immediate matches
the size of the other operand, no sign extension happens, so this was
incorrect. `asm.cmp(opnd64, 0x8000_0000)` panicked even though it's
encodable as `CMP r/m32, imm32`. Large shape ids were impacted by this
issue.
Co-Authored-By: Aaron Patterson <tenderlove@ruby-lang.org>
Co-Authored-By: Alan Wu <alanwu@ruby-lang.org>
Co-authored-by: Aaron Patterson <tenderlove@ruby-lang.org>
Co-authored-by: Alan Wu <alanwu@ruby-lang.org>
YJIT: Skip padding jumps to side exits
Co-authored-by: Maxime Chevalier-Boisvert <maxime.chevalierboisvert@shopify.com>
Co-authored-by: Alan Wu <alansi.xingwu@shopify.com>
Co-authored-by: Maxime Chevalier-Boisvert <maxime.chevalierboisvert@shopify.com>
Co-authored-by: Alan Wu <alansi.xingwu@shopify.com>
@casperisfine reporting a bug in this gist https://gist.github.com/casperisfine/d59e297fba38eb3905a3d7152b9e9350
After investigating I found it was caused by a combination of send and a c_func that we have overwritten in the JIT. For send calls, we need to do some stack manipulation before making the call. Because of the way exits works, we need to do that stack manipulation at the last possible moment. In this case, we weren't doing that stack manipulation at all. Unfortunately, with how the code is structured there isn't a great place to do that stack manipulation for our overridden C funcs.
Each overridden C func can return a boolean stating that it shouldn't be used. We would need to do the stack manipulation after all of those checks are done. We could pass a lambda(?) or separate out the logic for "can I run this override" from "now generate the code for it". Since we are coming up on a release, I went with the path of least resistence and just decided to not use these overrides if we are in a send call.
We definitely should revist this in the future.
* YJIT: Always encode Opnd::Value in 64 bits on x86_64
for GC offsets
Co-authored-by: Alan Wu <alansi.xingwu@shopify.com>
* Introduce heap_object_p
* Leave original mov intact
* Remove unneeded branches
* Add a test for movabs
Co-authored-by: Alan Wu <alansi.xingwu@shopify.com>
When RUBY_DEBUG is enabled, shape ids are 16 bits. I would like to do
16 bit comparisons, so I need to load halfwords sometimes. This commit
adds LDURH so that I can load halfwords.
https://developer.arm.com/documentation/ddi0596/2021-12/Base-Instructions/LDURH--Load-Register-Halfword--unscaled--?lang=en
I verified the bytes using clang:
```
$ cat asmthing.s
.global _start
.align 2
_start:
ldurh w10, [x1]
ldurh w10, [x1, #123]
$ as asmthing.s -o asmthing.o && objdump --disassemble asmthing.o
asmthing.o: file format mach-o arm64
Disassembly of section __TEXT,__text:
0000000000000000 <ltmp0>:
0: 2a 00 40 78 ldurh w10, [x1]
4: 2a b0 47 78 ldurh w10, [x1, #123]
```
Previously, there is no instruction boundary patch point after
the call to a non-leaf C function we generate for
OPTIMIZED_METHOD_TYPE_CALL. This meant that if code GC is triggered
while inside the C function, we would keep running invalidated code when
we return from the C function. This had the effect of running
stale branch stubs, jumping to bad code, etc.
Use jit_prepare_routine_call() to make sure we exit from the invalidated
region as soon as possible after the C call in case of invalidation.
* Enable --yjit-stats for release builds
In order for people in the real world to report information about how their application runs with YJIT, we want to expose stats without requiring rebuilding ruby. We can do this without overhead, with the exception of count ratio in yjit, since this relies on the interpreter also counting instructions.
This change exposes those stats, while not showing ratio in yjit if we are not in a stats build.
* Update yjit.rb
Co-authored-by: Takashi Kokubun <takashikkbn@gmail.com>
Co-authored-by: Maxime Chevalier-Boisvert <maximechevalierb@gmail.com>
Since object shapes store the capacity of an object, we no longer
need the numiv field on RObjects. This gives us one extra slot which
we can use to give embedded objects one more instance variable (for a
total of 3 ivs). This commit removes the concept of numiv from RObject.
This commit adds a `capacity` field to shapes, and adds shape
transitions whenever an object's capacity changes. Objects which are
allocated out of a bigger size pool will also make a transition from the
root shape to the shape with the correct capacity for their size pool
when they are allocated.
This commit will allow us to remove numiv from objects completely, and
will also mean we can guarantee that if two objects share shapes, their
IVs are in the same positions (an embedded and extended object cannot
share shapes). This will enable us to implement ivar sets in YJIT using
object shapes.
Co-Authored-By: Aaron Patterson <tenderlove@ruby-lang.org>
We switch to a new page when we detect dropped_bytes flipping from false
to true. Previously, when we patch code for invalidation during code gc,
we start with the flag being set to true, so we failed to apply patches
that straddle pages. We would write out jumps half way and then stop,
which left the code corrupted.
Reset the flag before patching so we patch across pages properly.
This dispatches to a c func for doing the dynamic lookup. I experimented with chain on the proc but wasn't able to detect which call sites would be monomorphic vs polymorphic. There is definitely room for optimization here, but it does reduce exits.
Co-Authored-By: Alan Wu <alansi.xingwu@shopify.com>
Co-Authored-By: Maxime Chevalier-Boisvert <maxime.chevalierboisvert@shopify.com>
Co-authored-by: Alan Wu <alansi.xingwu@shopify.com>
Co-authored-by: Maxime Chevalier-Boisvert <maxime.chevalierboisvert@shopify.com>
While experimenting I found that it's easy to change Context and forget
to also change the copying operation in limit_block_versions(). Add an
assert to make sure we substitute a compatible generic context when
limiting the number of versions.
This code used to roll its own heap object check before we made a better
version in guard_known_class(). The improved version uses one fewer
comparison, so let's use that.
Maxime Chevalier-Boisvert
Kevin Newton
Takashi Kokubun
Alan Wu
Jean byroot Boussier
Hiroshi SHIBATA
ywenc
Peter Zhu
Matt Valentine-House
Nobuyoshi Nakada
eileencodes
Aaron Patterson
Jimmy Miller
Jeremy Evans
John Hawthorn
Adam Hess
Alan Wu
Ole Friis Østergaard
Koichi Sasada
Jean Boussier
Maple Ong
Ian Ker-Seymer
Jemma Issroff
Samuel Williams
Daniel Colson
Maxime Chevalier-Boisvert