Skip to content

Commit

Permalink
YJIT: Speculate block arg for c_func_method(&nil) calls
Browse files Browse the repository at this point in the history
A good amount of call sites always pass nil as block argument, but the
nil doesn't show up in the context. Put a runtime guard for those
cases to handle it. Particular relevant for the `ruby-lsp` benchmark in
`yjit-bench`. Up to a 2% speedup across headline benchmarks.

Co-authored-by: Aaron Patterson <tenderlove@ruby-lang.org>
Co-authored-by: Maxime Chevalier-Boisvert <maxime.chevalierboisvert@shopify.com>
Co-authored-by: Kevin Menard <kevin@nirvdrum.com>
Co-authored-by: Takashi Kokubun <takashikkbn@gmail.com>
Co-authored-by: Randy Stauner <randy.stauner@shopify.com>
  • Loading branch information
6 people committed Dec 12, 2024
1 parent c0caf1c commit 8ae3c4c
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 18 deletions.
33 changes: 15 additions & 18 deletions yjit/src/codegen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6949,16 +6949,28 @@ fn gen_send_cfunc(
return None;
}

let block_arg_type = if block_arg {
let mut block_arg_type = if block_arg {
Some(asm.ctx.get_opnd_type(StackOpnd(0)))
} else {
None
};

match block_arg_type {
Some(Type::Nil | Type::BlockParamProxy) => {
// We'll handle this later
// We don't need the actual stack value for these
asm.stack_pop(1);
}
Some(Type::Unknown | Type::UnknownImm) if {
let sample_blockarg = jit.peek_at_stack(&asm.ctx, 0);
Type::from(sample_blockarg) == Type::Nil
} => {
// The sample indicates that we'll likely get a nil, speculate that the blockarg
// will be nil.
asm.cmp(asm.stack_opnd(0), Qnil.into());
asm.jne(Target::side_exit(Counter::guard_send_cfunc_block_not_nil));
block_arg_type = Some(Type::Nil);
asm.stack_pop(1);
}
None => {
// Nothing to do
}
Expand All @@ -6967,23 +6979,8 @@ fn gen_send_cfunc(
return None;
}
}
let block_arg_type = block_arg_type; // drop `mut`

match block_arg_type {
Some(Type::Nil) => {
// We have a nil block arg, so let's pop it off the args
asm.stack_pop(1);
}
Some(Type::BlockParamProxy) => {
// We don't need the actual stack value
asm.stack_pop(1);
}
None => {
// Nothing to do
}
_ => {
assert!(false);
}
}

// Pop the empty kw_splat hash
if kw_splat {
Expand Down
1 change: 1 addition & 0 deletions yjit/src/stats.rs
Original file line number Diff line number Diff line change
Expand Up @@ -470,6 +470,7 @@ make_counters! {
guard_send_str_aref_not_fixnum,

guard_send_cfunc_bad_splat_vargs,
guard_send_cfunc_block_not_nil,

guard_invokesuper_me_changed,

Expand Down

0 comments on commit 8ae3c4c

Please sign in to comment.