diff options
| author | Takashi Kokubun <takashikkbn@gmail.com> | 2024-05-28 17:10:33 -0700 |
|---|---|---|
| committer | Takashi Kokubun <takashikkbn@gmail.com> | 2024-05-28 17:10:33 -0700 |
| commit | 6383d0afac6aa02b3e72d08128cc1d8327f149fa (patch) | |
| tree | 283ba110d9602218608d23b5bf5806c828482c35 /yjit/src/codegen.rs | |
| parent | d7ad60373988487ae746129dfd7f65df1dd13b86 (diff) | |
merge revision(s) 015b0e2e1d312e2be60551587389c8da5c585e6f,ac1e9e443a0d6a4d4c0801c26d1d8bd33d9eb431: [Backport #20195]
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]
Diffstat (limited to 'yjit/src/codegen.rs')
| -rw-r--r-- | yjit/src/codegen.rs | 38 |
1 files changed, 10 insertions, 28 deletions
diff --git a/yjit/src/codegen.rs b/yjit/src/codegen.rs index e5ca2f0bf0..ea00889c0c 100644 --- a/yjit/src/codegen.rs +++ b/yjit/src/codegen.rs @@ -5413,18 +5413,6 @@ fn gen_send_cfunc( return None; } - // In order to handle backwards compatibility between ruby 3 and 2 - // ruby2_keywords was introduced. It is called only on methods - // with splat and changes they way they handle them. - // We are just going to not compile these. - // https://docs.ruby-lang.org/en/3.2/Module.html#method-i-ruby2_keywords - if unsafe { - get_iseq_flags_ruby2_keywords(jit.iseq) && flags & VM_CALL_ARGS_SPLAT != 0 - } { - gen_counter_incr(asm, Counter::send_args_splat_cfunc_ruby2_keywords); - return None; - } - let kw_arg = unsafe { vm_ci_kwarg(ci) }; let kw_arg_num = if kw_arg.is_null() { 0 @@ -6004,7 +5992,6 @@ fn gen_send_iseq( exit_if_has_post(asm, iseq)?; exit_if_has_kwrest(asm, iseq)?; exit_if_kw_splat(asm, flags)?; - exit_if_splat_and_ruby2_keywords(asm, jit, flags)?; exit_if_has_rest_and_captured(asm, iseq_has_rest, captured_opnd)?; exit_if_has_rest_and_supplying_kws(asm, iseq_has_rest, iseq, supplying_kws)?; exit_if_supplying_kw_and_has_no_kw(asm, supplying_kws, iseq)?; @@ -6282,6 +6269,8 @@ fn gen_send_iseq( asm.jbe(Target::side_exit(Counter::guard_send_se_cf_overflow)); if iseq_has_rest && flags & VM_CALL_ARGS_SPLAT != 0 { + let splat_pos = i32::from(block_arg) + kw_arg_num; + // Insert length guard for a call to copy_splat_args_for_rest_callee() // that will come later. We will have made changes to // the stack by spilling or handling __send__ shifting @@ -6295,12 +6284,19 @@ fn gen_send_iseq( if take_count > 0 { asm_comment!(asm, "guard splat_array_length >= {take_count}"); - let splat_array = asm.stack_opnd(i32::from(block_arg) + kw_arg_num); + let splat_array = asm.stack_opnd(splat_pos); let array_len_opnd = get_array_len(asm, splat_array); asm.cmp(array_len_opnd, take_count.into()); asm.jl(Target::side_exit(Counter::guard_send_iseq_has_rest_and_splat_too_few)); } } + + // All splats need to guard for ruby2_keywords hash. Check with a function call when + // splatting into a rest param since the index for the last item in the array is dynamic. + asm_comment!(asm, "guard no ruby2_keywords hash in splat"); + let bad_splat = asm.ccall(rb_yjit_ruby2_keywords_splat_p as _, vec![asm.stack_opnd(splat_pos)]); + asm.cmp(bad_splat, 0.into()); + asm.jnz(Target::side_exit(Counter::guard_send_splatarray_last_ruby_2_keywords)); } match block_arg_type { @@ -6846,20 +6842,6 @@ fn exit_if_kw_splat(asm: &mut Assembler, flags: u32) -> Option<()> { } #[must_use] -fn exit_if_splat_and_ruby2_keywords(asm: &mut Assembler, jit: &mut JITState, flags: u32) -> Option<()> { - // In order to handle backwards compatibility between ruby 3 and 2 - // ruby2_keywords was introduced. It is called only on methods - // with splat and changes they way they handle them. - // We are just going to not compile these. - // https://www.rubydoc.info/stdlib/core/Proc:ruby2_keywords - exit_if( - asm, - unsafe { get_iseq_flags_ruby2_keywords(jit.iseq) } && flags & VM_CALL_ARGS_SPLAT != 0, - Counter::send_iseq_ruby2_keywords, - ) -} - -#[must_use] fn exit_if_has_rest_and_captured(asm: &mut Assembler, iseq_has_rest: bool, captured_opnd: Option<Opnd>) -> Option<()> { exit_if(asm, iseq_has_rest && captured_opnd.is_some(), Counter::send_iseq_has_rest_and_captured) } |
