Skip to content

Commit dd7802a

Browse files
committed
Implement calls to methods with simple optional params
* Implement calls to methods with simple optional params * Remove unnecessary MJIT_STATIC See comment for MJIT_STATIC. I added it not knowing whether it's required because the function next to it has it. Don't use it and wait for problems to come up instead. * Better naming, some comments * Count bailing on kw only iseqs On railsbench: ``` opt_send_without_block exit reasons: bmethod 59729 (27.7%) optimized_method 59137 (27.5%) iseq_complex_callee 41362 (19.2%) alias_method 33346 (15.5%) callsite_not_simple 19170 ( 8.9%) iseq_only_keywords 1300 ( 0.6%) kw_splat 1299 ( 0.6%) cfunc_ruby_array_varg 18 ( 0.0%) ```
1 parent b49d92f commit dd7802a

File tree

4 files changed

+96
-24
lines changed

4 files changed

+96
-24
lines changed

bootstraptest/test_yjit.rb

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -736,3 +736,39 @@ class Symbol
736736
737737
[dyn_sym.get_foo, sym.get_foo]
738738
}
739+
740+
# passing too few arguments to method with optional parameters
741+
assert_equal 'raised', %q{
742+
def opt(a, b = 0)
743+
end
744+
745+
def use
746+
opt
747+
end
748+
749+
use rescue nil
750+
begin
751+
use
752+
:ng
753+
rescue ArgumentError
754+
:raised
755+
end
756+
}
757+
758+
# passing too many arguments to method with optional parameters
759+
assert_equal 'raised', %q{
760+
def opt(a, b = 0)
761+
end
762+
763+
def use
764+
opt(1, 2, 3, 4)
765+
end
766+
767+
use rescue nil
768+
begin
769+
use
770+
:ng
771+
rescue ArgumentError
772+
:raised
773+
end
774+
}

vm_insnhelper.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2228,7 +2228,7 @@ rb_simple_iseq_p(const rb_iseq_t *iseq)
22282228
iseq->body->param.flags.has_block == FALSE;
22292229
}
22302230

2231-
static bool
2231+
bool
22322232
rb_iseq_only_optparam_p(const rb_iseq_t *iseq)
22332233
{
22342234
return iseq->body->param.flags.has_opt == TRUE &&
@@ -2240,7 +2240,7 @@ rb_iseq_only_optparam_p(const rb_iseq_t *iseq)
22402240
iseq->body->param.flags.has_block == FALSE;
22412241
}
22422242

2243-
static bool
2243+
bool
22442244
rb_iseq_only_kwparam_p(const rb_iseq_t *iseq)
22452245
{
22462246
return iseq->body->param.flags.has_opt == FALSE &&

yjit_codegen.c

Lines changed: 55 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -567,7 +567,8 @@ gen_putself(jitstate_t* jit, ctx_t* ctx)
567567
}
568568

569569
// Compute the index of a local variable from its slot index
570-
uint32_t slot_to_local_idx(const rb_iseq_t *iseq, int32_t slot_idx)
570+
static uint32_t
571+
slot_to_local_idx(const rb_iseq_t *iseq, int32_t slot_idx)
571572
{
572573
// Convoluted rules from local_var_name() in iseq.c
573574
int32_t local_table_size = iseq->body->local_table_size;
@@ -633,10 +634,10 @@ gen_setlocal_wc0(jitstate_t* jit, ctx_t* ctx)
633634
{
634635
VALUE flags = ep[VM_ENV_DATA_INDEX_FLAGS];
635636
if (LIKELY((flags & VM_ENV_FLAG_WB_REQUIRED) == 0)) {
636-
VM_STACK_ENV_WRITE(ep, index, v);
637+
VM_STACK_ENV_WRITE(ep, index, v);
637638
}
638639
else {
639-
vm_env_write_slowpath(ep, index, v);
640+
vm_env_write_slowpath(ep, index, v);
640641
}
641642
}
642643
*/
@@ -1772,8 +1773,6 @@ gen_oswb_cfunc(jitstate_t *jit, ctx_t *ctx, const struct rb_callinfo *ci, const
17721773
return YJIT_END_BLOCK;
17731774
}
17741775

1775-
bool rb_simple_iseq_p(const rb_iseq_t *iseq);
1776-
17771776
static void
17781777
gen_return_branch(codeblock_t* cb, uint8_t* target0, uint8_t* target1, uint8_t shape)
17791778
{
@@ -1791,31 +1790,67 @@ gen_return_branch(codeblock_t* cb, uint8_t* target0, uint8_t* target1, uint8_t s
17911790
}
17921791
}
17931792

1793+
bool rb_simple_iseq_p(const rb_iseq_t *iseq);
1794+
bool rb_iseq_only_optparam_p(const rb_iseq_t *iseq);
1795+
bool rb_iseq_only_kwparam_p(const rb_iseq_t *iseq);
1796+
17941797
static codegen_status_t
1795-
gen_oswb_iseq(jitstate_t *jit, ctx_t *ctx, const struct rb_callinfo *ci, const rb_callable_method_entry_t *cme, int32_t argc)
1798+
gen_oswb_iseq(jitstate_t *jit, ctx_t *ctx, const struct rb_callinfo *ci, const rb_callable_method_entry_t *cme, const int32_t argc)
17961799
{
17971800
const rb_iseq_t *iseq = def_iseq_ptr(cme->def);
1798-
const VALUE* start_pc = iseq->body->iseq_encoded;
1799-
int num_params = iseq->body->param.size;
1800-
int num_locals = iseq->body->local_table_size - num_params;
18011801

1802-
if (num_params != argc) {
1803-
GEN_COUNTER_INC(cb, oswb_iseq_argc_mismatch);
1802+
if (vm_ci_flag(ci) & VM_CALL_TAILCALL) {
1803+
// We can't handle tailcalls
1804+
GEN_COUNTER_INC(cb, oswb_iseq_tailcall);
18041805
return YJIT_CANT_COMPILE;
18051806
}
18061807

1807-
if (!rb_simple_iseq_p(iseq)) {
1808-
// Only handle iseqs that have simple parameters.
1809-
// See vm_callee_setup_arg().
1810-
GEN_COUNTER_INC(cb, oswb_iseq_not_simple);
1811-
return YJIT_CANT_COMPILE;
1808+
// Arity handling and optional parameter setup
1809+
int num_params = iseq->body->param.size;
1810+
uint32_t start_pc_offset = 0;
1811+
if (rb_simple_iseq_p(iseq)) {
1812+
if (num_params != argc) {
1813+
GEN_COUNTER_INC(cb, oswb_iseq_arity_error);
1814+
return YJIT_CANT_COMPILE;
1815+
}
18121816
}
1817+
else if (rb_iseq_only_optparam_p(iseq)) {
1818+
// These are iseqs with 0 or more required parameters followed by 1
1819+
// or more optional parameters.
1820+
// We follow the logic of vm_call_iseq_setup_normal_opt_start()
1821+
// and these are the preconditions required for using that fast path.
1822+
RUBY_ASSERT(vm_ci_markable(ci) && ((vm_ci_flag(ci) &
1823+
(VM_CALL_KW_SPLAT | VM_CALL_KWARG | VM_CALL_ARGS_SPLAT)) == 0));
1824+
1825+
const int required_num = iseq->body->param.lead_num;
1826+
const int opts_filled = argc - required_num;
1827+
const int opt_num = iseq->body->param.opt_num;
1828+
1829+
if (opts_filled < 0 || opts_filled > opt_num) {
1830+
GEN_COUNTER_INC(cb, oswb_iseq_arity_error);
1831+
return YJIT_CANT_COMPILE;
1832+
}
18131833

1814-
if (vm_ci_flag(ci) & VM_CALL_TAILCALL) {
1815-
// We can't handle tailcalls
1816-
GEN_COUNTER_INC(cb, oswb_iseq_tailcall);
1834+
num_params -= opt_num - opts_filled;
1835+
start_pc_offset = (uint32_t)iseq->body->param.opt_table[opts_filled];
1836+
}
1837+
else if (rb_iseq_only_kwparam_p(iseq)) {
1838+
// vm_callee_setup_arg() has a fast path for this.
1839+
GEN_COUNTER_INC(cb, oswb_iseq_only_keywords);
18171840
return YJIT_CANT_COMPILE;
18181841
}
1842+
else {
1843+
// Only handle iseqs that have simple parameter setup.
1844+
// See vm_callee_setup_arg().
1845+
GEN_COUNTER_INC(cb, oswb_iseq_complex_callee);
1846+
return YJIT_CANT_COMPILE;
1847+
}
1848+
1849+
// The starting pc of the callee frame
1850+
const VALUE *start_pc = &iseq->body->iseq_encoded[start_pc_offset];
1851+
1852+
// Number of locals that are not parameters
1853+
const int num_locals = iseq->body->local_table_size - num_params;
18191854

18201855
// Create a size-exit to fall back to the interpreter
18211856
uint8_t* side_exit = yjit_side_exit(jit, ctx);
@@ -1934,7 +1969,7 @@ gen_oswb_iseq(jitstate_t *jit, ctx_t *ctx, const struct rb_callinfo *ci, const r
19341969
gen_direct_jump(
19351970
jit->block,
19361971
&callee_ctx,
1937-
(blockid_t){ iseq, 0 }
1972+
(blockid_t){ iseq, start_pc_offset }
19381973
);
19391974

19401975
return true;

yjit_iface.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,9 @@ YJIT_DECLARE_COUNTERS(
3838
oswb_cfunc_argc_mismatch,
3939
oswb_cfunc_toomany_args,
4040
oswb_iseq_tailcall,
41-
oswb_iseq_argc_mismatch,
42-
oswb_iseq_not_simple,
41+
oswb_iseq_arity_error,
42+
oswb_iseq_only_keywords,
43+
oswb_iseq_complex_callee,
4344
oswb_not_implemented_method,
4445
oswb_getter_arity,
4546
oswb_se_receiver_not_heap,

0 commit comments

Comments
 (0)