ruby-changes:72730
From: Matthew <ko1@a...>
Date: Fri, 29 Jul 2022 00:38:29 +0900 (JST)
Subject: [ruby-changes:72730] ab08a43ec5 (master): YJIT: Teach getblockparamproxy to handle the no-block case without exiting (#6191)
https://git.ruby-lang.org/ruby.git/commit/?id=ab08a43ec5 From ab08a43ec5ab7a8d82c980dc1eefc9e5d71e926d Mon Sep 17 00:00:00 2001 From: Matthew Draper <matthew@t...> Date: Fri, 29 Jul 2022 01:08:07 +0930 Subject: YJIT: Teach getblockparamproxy to handle the no-block case without exiting (#6191) Teach getblockparamproxy to handle the no-block case without exiting Co-authored-by: John Hawthorn <john@h...> Co-authored-by: John Hawthorn <john@h...> --- bootstraptest/test_yjit.rb | 13 +++++++ test/ruby/test_yjit.rb | 16 +++++++++ yjit.c | 11 ++++++ yjit/src/codegen.rs | 88 ++++++++++++++++++++++++++++++++++++---------- yjit/src/cruby.rs | 3 ++ 5 files changed, 113 insertions(+), 18 deletions(-) diff --git a/bootstraptest/test_yjit.rb b/bootstraptest/test_yjit.rb index 7158486d50..966a5f3002 100644 --- a/bootstraptest/test_yjit.rb +++ b/bootstraptest/test_yjit.rb @@ -1224,6 +1224,19 @@ assert_equal '[1, 2, 42]', %q{ https://github.com/ruby/ruby/blob/trunk/bootstraptest/test_yjit.rb#L1224 [foo {1}, foo {2}, foo {42}] } +# test calling without block param +assert_equal '[1, false, 2, false]', %q{ + def bar + block_given? && yield + end + + def foo(&block) + bar(&block) + end + + [foo { 1 }, foo, foo { 2 }, foo] +} + # test calling block param failing assert_equal '42', %q{ def foo(&block) diff --git a/test/ruby/test_yjit.rb b/test/ruby/test_yjit.rb index f8fc4f21ef..37e72dcafa 100644 --- a/test/ruby/test_yjit.rb +++ b/test/ruby/test_yjit.rb @@ -513,6 +513,22 @@ 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 bar + end + + def foo &blk + bar(&blk) + bar(&blk) + end + + foo + foo + RUBY + end + def test_send_splat assert_compiles(<<~'RUBY', result: "3#1,2,3/P", exits: {}) def internal_method(*args) diff --git a/yjit.c b/yjit.c index edf5a139cc..1a2f71a959 100644 --- a/yjit.c +++ b/yjit.c @@ -699,6 +699,17 @@ rb_get_cfp_ep(struct rb_control_frame_struct *cfp) https://github.com/ruby/ruby/blob/trunk/yjit.c#L699 return (VALUE*)cfp->ep; } +const VALUE * +rb_get_cfp_ep_level(struct rb_control_frame_struct *cfp, uint32_t lv) +{ + uint32_t i; + const VALUE *ep = (VALUE*)cfp->ep; + for (i = 0; i < lv; i++) { + ep = VM_ENV_PREV_EP(ep); + } + return ep; +} + VALUE rb_yarv_class_of(VALUE obj) { diff --git a/yjit/src/codegen.rs b/yjit/src/codegen.rs index af501158ff..0acd1972c3 100644 --- a/yjit/src/codegen.rs +++ b/yjit/src/codegen.rs @@ -214,6 +214,15 @@ fn jit_peek_at_local(jit: &JITState, n: i32) -> VALUE { https://github.com/ruby/ruby/blob/trunk/yjit/src/codegen.rs#L214 } } +fn jit_peek_at_block_handler(jit: &JITState, level: u32) -> VALUE { + assert!(jit_at_current_insn(jit)); + + unsafe { + let ep = get_cfp_ep_level(get_ec_cfp(jit.ec.unwrap()), level); + *ep.offset(VM_ENV_DATA_INDEX_SPECVAL as isize) + } +} + // Add a comment at the current position in the code block fn add_comment(cb: &mut CodeBlock, comment_str: &str) { if cfg!(feature = "asm_comments") { @@ -5634,6 +5643,13 @@ fn gen_getblockparamproxy( https://github.com/ruby/ruby/blob/trunk/yjit/src/codegen.rs#L5643 cb: &mut CodeBlock, ocb: &mut OutlinedCb, ) -> CodegenStatus { + if !jit_at_current_insn(jit) { + defer_compilation(jit, ctx, cb, ocb); + return EndBlock; + } + + let starting_context = *ctx; // make a copy for use with jit_chain_guard + // A mirror of the interpreter code. Checking for the case // where it's pushing rb_block_param_proxy. let side_exit = get_side_exit(jit, ocb, ctx); @@ -5641,6 +5657,15 @@ fn gen_getblockparamproxy( https://github.com/ruby/ruby/blob/trunk/yjit/src/codegen.rs#L5657 // EP level let level = jit_get_arg(jit, 1).as_u32(); + // Peek at the block handler so we can check whether it's nil + let comptime_handler = jit_peek_at_block_handler(jit, level); + + // When a block handler is present, it should always be a GC-guarded + // pointer (VM_BH_ISEQ_BLOCK_P) + if comptime_handler.as_u64() != 0 && comptime_handler.as_u64() & 0x3 != 0x1 { + return CantCompile; + } + // Load environment pointer EP from CFP gen_get_ep(cb, REG0, level); @@ -5669,27 +5694,54 @@ fn gen_getblockparamproxy( https://github.com/ruby/ruby/blob/trunk/yjit/src/codegen.rs#L5694 ), ); - // Block handler is a tagged pointer. Look at the tag. 0x03 is from VM_BH_ISEQ_BLOCK_P(). - and(cb, REG0_8, imm_opnd(0x3)); + // Specialize compilation for the case where no block handler is present + if comptime_handler.as_u64() == 0 { + // Bail if there is a block handler + cmp(cb, REG0, uimm_opnd(0)); - // Bail unless VM_BH_ISEQ_BLOCK_P(bh). This also checks for null. - cmp(cb, REG0_8, imm_opnd(0x1)); - jnz_ptr( - cb, - counted_exit!(ocb, side_exit, gbpp_block_handler_not_iseq), - ); + jit_chain_guard( + JCC_JNZ, + jit, + &starting_context, + cb, + ocb, + SEND_MAX_DEPTH, + side_exit, + ); - // Push rb_block_param_proxy. It's a root, so no need to use jit_mov_gc_ptr. - mov( - cb, - REG0, - const_ptr_opnd(unsafe { rb_block_param_proxy }.as_ptr()), - ); - assert!(!unsafe { rb_block_param_proxy }.special_const_p()); - let top = ctx.stack_push(Type::UnknownHeap); - mov(cb, top, REG0); + jit_putobject(jit, ctx, cb, Qnil); + } else { + // Block handler is a tagged pointer. Look at the tag. 0x03 is from VM_BH_ISEQ_BLOCK_P(). + and(cb, REG0_8, imm_opnd(0x3)); - KeepCompiling + // Bail unless VM_BH_ISEQ_BLOCK_P(bh). This also checks for null. + cmp(cb, REG0_8, imm_opnd(0x1)); + + jit_chain_guard( + JCC_JNZ, + jit, + &starting_context, + cb, + ocb, + SEND_MAX_DEPTH, + side_exit, + ); + + // Push rb_block_param_proxy. It's a root, so no need to use jit_mov_gc_ptr. + mov( + cb, + REG0, + const_ptr_opnd(unsafe { rb_block_param_proxy }.as_ptr()), + ); + assert!(!unsafe { rb_block_param_proxy }.special_const_p()); + + let top = ctx.stack_push(Type::Unknown); + mov(cb, top, REG0); + } + + jump_to_next_insn(jit, ctx, cb, ocb); + + EndBlock } fn gen_getblockparam( diff --git a/yjit/src/cruby.rs b/yjit/src/cruby.rs index 7c21bd962b..9195578172 100644 --- a/yjit/src/cruby.rs +++ b/yjit/src/cruby.rs @@ -135,6 +135,9 @@ extern "C" { https://github.com/ruby/ruby/blob/trunk/yjit/src/cruby.rs#L135 #[link_name = "rb_get_cfp_ep"] pub fn get_cfp_ep(cfp: CfpPtr) -> *mut VALUE; + #[link_name = "rb_get_cfp_ep_level"] + pub fn get_cfp_ep_level(cfp: CfpPtr, lv: u32) -> *const VALUE; + #[link_name = "rb_get_cme_def_type"] pub fn get_cme_def_type(cme: *const rb_callable_method_entry_t) -> rb_method_type_t; -- cgit v1.2.1 -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/