ruby-changes:74267
From: Matthew <ko1@a...>
Date: Thu, 27 Oct 2022 04:28:26 +0900 (JST)
Subject: [ruby-changes:74267] c746f380f2 (master): YJIT: Support nil and blockparamproxy as blockarg in send (#6492)
https://git.ruby-lang.org/ruby.git/commit/?id=c746f380f2 From c746f380f278683e98262883ed69319bd9fa680e Mon Sep 17 00:00:00 2001 From: Matthew Draper <matthew@t...> Date: Thu, 27 Oct 2022 05:57:59 +1030 Subject: YJIT: Support nil and blockparamproxy as blockarg in send (#6492) Co-authored-by: John Hawthorn <john@h...> Co-authored-by: John Hawthorn <john@h...> --- bootstraptest/test_yjit.rb | 43 +++++++++ test/ruby/test_yjit.rb | 8 +- yjit.c | 6 ++ yjit/bindgen/src/main.rs | 1 + yjit/src/codegen.rs | 195 ++++++++++++++++++++++++++++++++--------- yjit/src/core.rs | 12 ++- yjit/src/cruby_bindings.inc.rs | 3 + 7 files changed, 221 insertions(+), 47 deletions(-) diff --git a/bootstraptest/test_yjit.rb b/bootstraptest/test_yjit.rb index 2dc460c628..7361d2a725 100644 --- a/bootstraptest/test_yjit.rb +++ b/bootstraptest/test_yjit.rb @@ -3337,3 +3337,46 @@ assert_equal '[[1, nil, 2]]', %q{ https://github.com/ruby/ruby/blob/trunk/bootstraptest/test_yjit.rb#L3337 5.times.map { opt_and_kwargs(1, c: 2) }.uniq } + +# bmethod with forwarded block +assert_equal '2', %q{ + define_method(:foo) do |&block| + block.call + end + + def bar(&block) + foo(&block) + end + + bar { 1 } + bar { 2 } +} + +# bmethod with forwarded block and arguments +assert_equal '5', %q{ + define_method(:foo) do |n, &block| + n + block.call + end + + def bar(n, &block) + foo(n, &block) + end + + bar(0) { 1 } + bar(3) { 2 } +} + +# bmethod with forwarded unwanted block +assert_equal '1', %q{ + one = 1 + define_method(:foo) do + one + end + + def bar(&block) + foo(&block) + end + + bar { } + bar { } +} diff --git a/test/ruby/test_yjit.rb b/test/ruby/test_yjit.rb index 09b5989a06..d740870736 100644 --- a/test/ruby/test_yjit.rb +++ b/test/ruby/test_yjit.rb @@ -513,9 +513,8 @@ class TestYJIT < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_yjit.rb#L513 RUBY end - def test_getblockparamproxy_with_no_block - # Currently side exits on the send - assert_compiles(<<~'RUBY', insns: [:getblockparamproxy], exits: { send: 2 }) + def test_send_blockarg + assert_compiles(<<~'RUBY', insns: [:getblockparamproxy, :send], exits: {}) def bar end @@ -526,6 +525,9 @@ class TestYJIT < Test::Unit::TestCase https://github.com/ruby/ruby/blob/trunk/test/ruby/test_yjit.rb#L525 foo foo + + foo { } + foo { } RUBY end diff --git a/yjit.c b/yjit.c index 7e6fc9e3fb..c53444d5a3 100644 --- a/yjit.c +++ b/yjit.c @@ -592,6 +592,12 @@ rb_get_iseq_body_local_iseq(const rb_iseq_t *iseq) https://github.com/ruby/ruby/blob/trunk/yjit.c#L592 return iseq->body->local_iseq; } +const rb_iseq_t * +rb_get_iseq_body_parent_iseq(const rb_iseq_t *iseq) +{ + return iseq->body->parent_iseq; +} + unsigned int rb_get_iseq_body_local_table_size(const rb_iseq_t *iseq) { diff --git a/yjit/bindgen/src/main.rs b/yjit/bindgen/src/main.rs index 60a9d1b87d..21aaec84cb 100644 --- a/yjit/bindgen/src/main.rs +++ b/yjit/bindgen/src/main.rs @@ -350,6 +350,7 @@ fn main() { https://github.com/ruby/ruby/blob/trunk/yjit/bindgen/src/main.rs#L350 .allowlist_function("rb_get_def_bmethod_proc") .allowlist_function("rb_iseq_encoded_size") .allowlist_function("rb_get_iseq_body_local_iseq") + .allowlist_function("rb_get_iseq_body_parent_iseq") .allowlist_function("rb_get_iseq_body_iseq_encoded") .allowlist_function("rb_get_iseq_body_stack_max") .allowlist_function("rb_get_iseq_flags_has_opt") diff --git a/yjit/src/codegen.rs b/yjit/src/codegen.rs index d6ea8996e1..51645f0744 100644 --- a/yjit/src/codegen.rs +++ b/yjit/src/codegen.rs @@ -1542,6 +1542,22 @@ fn gen_get_ep(asm: &mut Assembler, level: u32) -> Opnd { https://github.com/ruby/ruby/blob/trunk/yjit/src/codegen.rs#L1542 ep_opnd } +// Gets the EP of the ISeq of the containing method, or "local level". +// Equivalent of GET_LEP() macro. +fn gen_get_lep(jit: &mut JITState, asm: &mut Assembler) -> Opnd { + // Equivalent of get_lvar_level() in compile.c + fn get_lvar_level(iseq: IseqPtr) -> u32 { + if iseq == unsafe { rb_get_iseq_body_local_iseq(iseq) } { + 0 + } else { + 1 + get_lvar_level(unsafe { rb_get_iseq_body_parent_iseq(iseq) }) + } + } + + let level = get_lvar_level(jit.get_iseq()); + gen_get_ep(asm, level) +} + fn gen_getlocal_generic( ctx: &mut Context, asm: &mut Assembler, @@ -3995,9 +4011,14 @@ unsafe extern "C" fn build_kwhash(ci: *const rb_callinfo, sp: *const VALUE) -> V https://github.com/ruby/ruby/blob/trunk/yjit/src/codegen.rs#L4011 hash } -enum BlockHandler { +// SpecVal is a single value in an iseq invocation's environment on the stack, +// at sp[-2]. Depending on the frame type, it can serve different purposes, +// which are covered here by enum variants. +enum SpecVal { None, - CurrentFrame, + BlockISeq(IseqPtr), + BlockParamProxy, + PrevEP(*const VALUE), } struct ControlFrame { @@ -4006,8 +4027,7 @@ struct ControlFrame { https://github.com/ruby/ruby/blob/trunk/yjit/src/codegen.rs#L4027 iseq: Option<IseqPtr>, pc: Option<u64>, frame_type: u32, - block_handler: BlockHandler, - prev_ep: Option<*const VALUE>, + specval: SpecVal, cme: *const rb_callable_method_entry_t, local_size: i32 } @@ -4028,7 +4048,7 @@ struct ControlFrame { https://github.com/ruby/ruby/blob/trunk/yjit/src/codegen.rs#L4048 // * Stack overflow is not checked (should be done by the caller) // * Interrupts are not checked (should be done by the caller) fn gen_push_frame( - _jit: &mut JITState, + jit: &mut JITState, _ctx: &mut Context, asm: &mut Assembler, set_sp_cfp: bool, // if true CFP and SP will be switched to the callee @@ -4060,19 +4080,33 @@ fn gen_push_frame( https://github.com/ruby/ruby/blob/trunk/yjit/src/codegen.rs#L4080 // Write special value at sp[-2]. It's either a block handler or a pointer to // the outer environment depending on the frame type. // sp[-2] = specval; - let specval: Opnd = match (frame.prev_ep, frame.block_handler) { - (None, BlockHandler::None) => { + let specval: Opnd = match frame.specval { + SpecVal::None => { VM_BLOCK_HANDLER_NONE.into() } - (None, BlockHandler::CurrentFrame) => { + SpecVal::BlockISeq(block_iseq) => { + // Change cfp->block_code in the current frame. See vm_caller_setup_arg_block(). + // VM_CFP_TO_CAPTURED_BLOCK does &cfp->self, rb_captured_block->code.iseq aliases + // with cfp->block_code. + asm.store(Opnd::mem(64, CFP, RUBY_OFFSET_CFP_BLOCK_CODE), VALUE::from(block_iseq).into()); + let cfp_self = asm.lea(Opnd::mem(64, CFP, RUBY_OFFSET_CFP_SELF)); asm.or(cfp_self, Opnd::Imm(1)) } - (Some(prev_ep), BlockHandler::None) => { + SpecVal::BlockParamProxy => { + let ep_opnd = gen_get_lep(jit, asm); + let block_handler = asm.load( + Opnd::mem(64, ep_opnd, (SIZEOF_VALUE as i32) * (VM_ENV_DATA_INDEX_SPECVAL as i32)) + ); + + asm.store(Opnd::mem(64, CFP, RUBY_OFFSET_CFP_BLOCK_CODE), block_handler); + + block_handler + } + SpecVal::PrevEP(prev_ep) => { let tagged_prev_ep = (prev_ep as usize) | 1; VALUE(tagged_prev_ep).into() } - (_, _) => panic!("specval can only be one of prev_ep or block_handler") }; asm.store(Opnd::mem(64, sp, SIZEOF_VALUE_I32 * -2), specval); @@ -4218,6 +4252,43 @@ fn gen_send_cfunc( https://github.com/ruby/ruby/blob/trunk/yjit/src/codegen.rs#L4252 return CantCompile; } + let block_arg = flags & VM_CALL_ARGS_BLOCKARG != 0; + let block_arg_type = if block_arg { + Some(ctx.get_opnd_type(StackOpnd(0))) + } else { + None + }; + + match block_arg_type { + Some(Type::Nil | Type::BlockParamProxy) => { + // We'll handle this later + } + None => { + // Nothing to do + } + _ => { + gen_counter_incr!(asm, send_block_arg); + return CantCompile; + } + } + + match block_arg_type { + Some(Type::Nil) => { + // We have a nil block arg, so let's pop it off the args + ctx.stack_pop(1); + } + Some(Type::BlockParamProxy) => { + // We don't need the actual stack value + ctx.stack_pop(1); + } + None => { + // Nothing to do + } + _ => { + assert!(false); + } + } + // This is a .send call and we need to adjust the stack if flags & VM_CALL_OPT_SEND != 0 { handle_opt_send_shift_stack(asm, argc as i32, ctx); @@ -4229,21 +4300,16 @@ fn gen_send_cfunc( https://github.com/ruby/ruby/blob/trunk/yjit/src/codegen.rs#L4300 // Store incremented PC into current control frame in case callee raises. jit_save_pc(jit, asm); - if let Some(block_iseq) = block { - // Change cfp->block_code in the current frame. See vm_caller_setup_arg_block(). - // VM_CFP_TO_CAPTURED_BLOCK does &cfp->self, rb_captured_block->code.iseq aliases - // with cfp->block_code. - asm.mov(Opnd::mem(64, CFP, RUBY_OFFSET_CFP_BLOCK_CODE), VALUE::from(block_iseq).into()); - } - // Increment the stack pointer by 3 (in the callee) // sp += 3 let sp = asm.lea(ctx.sp_opnd((SIZEOF_VALUE as isize) * 3)); - let frame_block_handler = if let Some(_block_iseq) = block { - BlockHandler::CurrentFrame + let specval = if block_arg_type == Some(Type::BlockParamProxy) { + SpecVal::BlockParamProxy + } else if let Some(block_iseq) = block { + SpecVal::BlockISeq(block_iseq) } else { - BlockHandler::None + Sp (... truncated) -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/