8000 Implement calls to methods with simple optional params · github/ruby@56a7e7c · GitHub
[go: up one dir, main page]

Skip to content

Commit 56a7e7c

Browse files
authored
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 71878e9 commit 56a7e7c

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
@@ -2187,7 +2187,7 @@ rb_simple_iseq_p(const rb_iseq_t *iseq)
21872187
iseq->body->param.flags.has_block == FALSE;
21882188
}
21892189

2190-
static bool
2190+
bool
21912191
rb_iseq_only_optparam_p(const rb_iseq_t *iseq)
21922192
{
21932193
return iseq->body->param.flags.has_opt == TRUE &&
@@ -2199,7 +2199,7 @@ rb_iseq_only_optparam_p(const rb_iseq_t *iseq)
21992199
iseq->body->param.flags.has_block == FALSE;
22002200
}
22012201

2202-
static bool
2202+
bool
22032203
rb_iseq_only_kwparam_p(const rb_iseq_t *iseq)
22042204
{
22052205
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
@@ -565,7 +565,8 @@ gen_putself(jitstate_t* jit, ctx_t* ctx)
565565
}
566566

567567
// Compute the index of a local variable from its slot index
568-
uint32_t slot_to_local_idx(const rb_iseq_t *iseq, int32_t slot_idx)
568+
static uint32_t
569+
slot_to_local_idx(const rb_iseq_t *iseq, int32_t slot_idx)
569570
{
570571
// Convoluted rules from local_var_name() in iseq.c
571572
int32_t local_table_size = iseq->body->local_table_size;
@@ -631,10 +632,10 @@ gen_setlocal_wc0(jitstate_t* jit, ctx_t* ctx)
631632
{
632633
VALUE flags = ep[VM_ENV_DATA_INDEX_FLAGS];
633634
if (LIKELY((flags & VM_ENV_FLAG_WB_REQUIRED) == 0)) {
634-
VM_STACK_ENV_WRITE(ep, index, v);
635+
VM_STACK_ENV_WRITE(ep, index, v);
635636
}
636637
else {
637-
vm_env_write_slowpath(ep, index, v);
638+
vm_env_write_slowpath(ep, index, v);
638639
}
639640
}
640641
*/
@@ -1771,8 +1772,6 @@ gen_oswb_cfunc(jitstate_t *jit, ctx_t *ctx, const struct rb_callinfo *ci, const
17711772
return YJIT_END_BLOCK;
17721773
}
17731774

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

1792+
bool rb_simple_iseq_p(const rb_iseq_t *iseq);
1793+
bool rb_iseq_only_optparam_p(const rb_iseq_t *iseq);
1794+
bool rb_iseq_only_kwparam_p(const rb_iseq_t *iseq);
1795+
17931796
static codegen_status_t
1794-
gen_oswb_iseq(jitstate_t *jit, ctx_t *ctx, const struct rb_callinfo *ci, const rb_callable_method_entry_t *cme, int32_t argc)
1797+
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)
17951798
{
17961799
const rb_iseq_t *iseq = def_iseq_ptr(cme->def);
1797-
const VALUE* start_pc = iseq->body->iseq_encoded;
1798-
int num_params = iseq->body->param.size;
1799-
int num_locals = iseq->body->local_table_size - num_params;
18001800

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

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

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

18191854
// Create a size-exit to fall back to the interpreter
18201855
uint8_t* side_exit = yjit_side_exit(jit, ctx);
@@ -1933,7 +1968,7 @@ gen_oswb_iseq(jitstate_t *jit, ctx_t *ctx, const struct rb_callinfo *ci, const r
19331968
gen_direct_jump(
19341969
jit->block,
19351970
&callee_ctx,
1936-
(blockid_t){ iseq, 0 }
1971+
(blockid_t){ iseq, start_pc_offset }
19371972
);
19381973

19391974
return YJIT_END_BLOCK;

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)
0