YJIT: Filter `&` calls from specialized C method codegen Evident with the crash reported in [Bug #20997], the C replacement codegen functions aren't authored to handle block arguments (nor should they because the extra code from the complexity defeats optimization). Filter sites with VM_CALL_ARGS_BLOCKARG.
Don't call `Warning.warn` unless the category is enabled (#10981)
Don't call `Warning.warn` unless the category is enabled
The warning category should be enabled if we want to call
`Warning.warn`.
This commit speeds up the following benchmark:
```ruby
eval "def test; " +
1000.times.map { "' '.chomp!" }.join(";") + "; end"
def run_benchmark count
i = 0
while i < count
start = Process.clock_gettime(Process::CLOCK_MONOTONIC)
yield
ms = Process.clock_gettime(Process::CLOCK_MONOTONIC) - start
puts "itr ##{i}: #{(ms * 1000).to_i}ms"
i += 1
end
end
run_benchmark(25) do
250.times do
test
end
end
```
On `master` this runs at about 92ms per iteration. With this patch, it
is 7ms per iteration.
[Bug #20573]
YJIT: Fix false object collection when setting ivar 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] --- bootstraptest/test_yjit.rb | 20 ++++++++++++++++++++ yjit/src/codegen.rs | 5 +++++ 2 files changed, 25 insertions(+) YJIT: Remove duplicate `asm.spill_temps()` `jit_prepare_routine_call()` calls it, and there is another call above on line 2302. Co-authored-by: Takashi Kokubun <takashikkbn@gmail.com> --- yjit/src/codegen.rs | 1 - 1 file changed, 1 deletion(-)
YJIT: Fix missing argc check in known cfuncs 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] --- bootstraptest/test_yjit.rb | 32 ++++++++++++++++++++++++++++++++ yjit/src/codegen.rs | 4 +++- 2 files changed, 35 insertions(+), 1 deletion(-)
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.
I noticed this while running test_yjit with --mjit-call-threshold=1, which redefines `Integer#<`. When Ruby is monkey-patched, MJIT itself could be broken. Similarly, Ruby scripts could break MJIT in many different ways. I prepared the same set of hooks as YJIT so that we could possibly override it and disable it on those moments. Every constant under RubyVM::MJIT is private and thus it's an unsupported behavior though.
* 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>
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.
@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.
Follow-up for 2b8191bdad7545b71f270d2b25a34cd2b3afa02f. Since that commit, we stopped doing code invalidation the second time the call and return events are enabled. We need to do it every time these events are enabled because we might have generated code while these events are disabled. Also rename locals and edit comments to make it more clear that the iseq rewrite code path only happens the first time a particular iseq trace event is enabled.