From 872ee2045ec477fad164c00044dde39c69451337 Mon Sep 17 00:00:00 2001 From: Takashi Kokubun Date: Thu, 18 Jan 2024 14:49:27 -0800 Subject: [PATCH 1/3] YJIT: Optimize defined?(yield) --- bootstraptest/test_yjit.rb | 6 +++ yjit/bindgen/src/main.rs | 1 + yjit/src/codegen.rs | 74 +++++++++++++++++++++------------- yjit/src/cruby_bindings.inc.rs | 19 +++++++++ 4 files changed, 73 insertions(+), 27 deletions(-) diff --git a/bootstraptest/test_yjit.rb b/bootstraptest/test_yjit.rb index 5b53e8089bfbf4..6833924a7ad7a2 100644 --- a/bootstraptest/test_yjit.rb +++ b/bootstraptest/test_yjit.rb @@ -4357,3 +4357,9 @@ def entry assert_equal '[0, 1, -4]', %q{ [0 >> 1, 2 >> 1, -7 >> 1] } + +# Integer right shift +assert_equal '[nil, "yield"]', %q{ + def defined_yield = defined?(yield) + [defined_yield, defined_yield {}] +} diff --git a/yjit/bindgen/src/main.rs b/yjit/bindgen/src/main.rs index 848e9fadc484ba..e249181d515275 100644 --- a/yjit/bindgen/src/main.rs +++ b/yjit/bindgen/src/main.rs @@ -348,6 +348,7 @@ fn main() { .allowlist_function("rb_iseqw_to_iseq") .allowlist_function("rb_iseq_label") .allowlist_function("rb_iseq_line_no") + .allowlist_type("defined_type") // From builtin.h .allowlist_type("rb_builtin_function.*") diff --git a/yjit/src/codegen.rs b/yjit/src/codegen.rs index 0471f7a20743dc..122e9ad5923f99 100644 --- a/yjit/src/codegen.rs +++ b/yjit/src/codegen.rs @@ -2655,31 +2655,41 @@ fn gen_defined( let obj = jit.get_arg(1); let pushval = jit.get_arg(2); - // Save the PC and SP because the callee may allocate - // Note that this modifies REG_SP, which is why we do it first - jit_prepare_routine_call(jit, asm); + match op_type as u32 { + DEFINED_YIELD => { + asm.stack_pop(1); // v operand is not used + let out_opnd = asm.stack_push(Type::Unknown); // nil or "yield" - // Get the operands from the stack - let v_opnd = asm.stack_opnd(0); + get_block_given(jit, asm, out_opnd, pushval.into(), Qnil.into()); + } + _ => { + // Save the PC and SP because the callee may allocate + // Note that this modifies REG_SP, which is why we do it first + jit_prepare_routine_call(jit, asm); - // Call vm_defined(ec, reg_cfp, op_type, obj, v) - let def_result = asm.ccall(rb_vm_defined as *const u8, vec![EC, CFP, op_type.into(), obj.into(), v_opnd]); - asm.stack_pop(1); // Keep it on stack during ccall for GC + // Get the operands from the stack + let v_opnd = asm.stack_opnd(0); - // if (vm_defined(ec, GET_CFP(), op_type, obj, v)) { - // val = pushval; - // } - asm.test(def_result, Opnd::UImm(255)); - let out_value = asm.csel_nz(pushval.into(), Qnil.into()); + // Call vm_defined(ec, reg_cfp, op_type, obj, v) + let def_result = asm.ccall(rb_vm_defined as *const u8, vec![EC, CFP, op_type.into(), obj.into(), v_opnd]); + asm.stack_pop(1); // Keep it on stack during ccall for GC - // Push the return value onto the stack - let out_type = if pushval.special_const_p() { - Type::UnknownImm - } else { - Type::Unknown - }; - let stack_ret = asm.stack_push(out_type); - asm.mov(stack_ret, out_value); + // if (vm_defined(ec, GET_CFP(), op_type, obj, v)) { + // val = pushval; + // } + asm.test(def_result, Opnd::UImm(255)); + let out_value = asm.csel_nz(pushval.into(), Qnil.into()); + + // Push the return value onto the stack + let out_type = if pushval.special_const_p() { + Type::UnknownImm + } else { + Type::Unknown + }; + let stack_ret = asm.stack_push(out_type); + asm.mov(stack_ret, out_value); + } + } Some(KeepCompiling) } @@ -5265,6 +5275,21 @@ fn jit_rb_f_block_given_p( _argc: i32, _known_recv_class: *const VALUE, ) -> bool { + asm.stack_pop(1); + let out_opnd = asm.stack_push(Type::UnknownImm); + + get_block_given(jit, asm, out_opnd, Qtrue.into(), Qfalse.into()); + + true +} + +fn get_block_given( + jit: &mut JITState, + asm: &mut Assembler, + out_opnd: Opnd, + true_opnd: Opnd, + false_opnd: Opnd, +) { asm_comment!(asm, "block_given?"); // Same as rb_vm_frame_block_handler @@ -5273,15 +5298,10 @@ fn jit_rb_f_block_given_p( Opnd::mem(64, ep_opnd, SIZEOF_VALUE_I32 * VM_ENV_DATA_INDEX_SPECVAL) ); - asm.stack_pop(1); - let out_opnd = asm.stack_push(Type::UnknownImm); - // Return `block_handler != VM_BLOCK_HANDLER_NONE` asm.cmp(block_handler, VM_BLOCK_HANDLER_NONE.into()); - let block_given = asm.csel_ne(Qtrue.into(), Qfalse.into()); + let block_given = asm.csel_ne(true_opnd, false_opnd); asm.mov(out_opnd, block_given); - - true } fn jit_thread_s_current( diff --git a/yjit/src/cruby_bindings.inc.rs b/yjit/src/cruby_bindings.inc.rs index d71441c0bed462..d67653890c9ea5 100644 --- a/yjit/src/cruby_bindings.inc.rs +++ b/yjit/src/cruby_bindings.inc.rs @@ -874,6 +874,25 @@ pub type ruby_vminsn_type = u32; pub type rb_iseq_callback = ::std::option::Option< unsafe extern "C" fn(arg1: *const rb_iseq_t, arg2: *mut ::std::os::raw::c_void), >; +pub const DEFINED_NOT_DEFINED: defined_type = 0; +pub const DEFINED_NIL: defined_type = 1; +pub const DEFINED_IVAR: defined_type = 2; +pub const DEFINED_LVAR: defined_type = 3; +pub const DEFINED_GVAR: defined_type = 4; +pub const DEFINED_CVAR: defined_type = 5; +pub const DEFINED_CONST: defined_type = 6; +pub const DEFINED_METHOD: defined_type = 7; +pub const DEFINED_YIELD: defined_type = 8; +pub const DEFINED_ZSUPER: defined_type = 9; +pub const DEFINED_SELF: defined_type = 10; +pub const DEFINED_TRUE: defined_type = 11; +pub const DEFINED_FALSE: defined_type = 12; +pub const DEFINED_ASGN: defined_type = 13; +pub const DEFINED_EXPR: defined_type = 14; +pub const DEFINED_REF: defined_type = 15; +pub const DEFINED_FUNC: defined_type = 16; +pub const DEFINED_CONST_FROM: defined_type = 17; +pub type defined_type = u32; pub const ROBJECT_OFFSET_AS_HEAP_IVPTR: robject_offsets = 16; pub const ROBJECT_OFFSET_AS_HEAP_IV_INDEX_TBL: robject_offsets = 24; pub const ROBJECT_OFFSET_AS_ARY: robject_offsets = 16; From a6d7115c91a6235c70f86b8e1df9ad26487ddcb4 Mon Sep 17 00:00:00 2001 From: Takashi Kokubun Date: Thu, 18 Jan 2024 16:20:04 -0800 Subject: [PATCH 2/3] Remove an irrelevant comment --- bootstraptest/test_yjit.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/bootstraptest/test_yjit.rb b/bootstraptest/test_yjit.rb index 6833924a7ad7a2..75c6c6182d187b 100644 --- a/bootstraptest/test_yjit.rb +++ b/bootstraptest/test_yjit.rb @@ -4358,7 +4358,6 @@ def entry [0 >> 1, 2 >> 1, -7 >> 1] } -# Integer right shift assert_equal '[nil, "yield"]', %q{ def defined_yield = defined?(yield) [defined_yield, defined_yield {}] From d3b3da1c2f3ecad7fea7fd0850fb81ba9066f967 Mon Sep 17 00:00:00 2001 From: Takashi Kokubun Date: Thu, 18 Jan 2024 16:21:16 -0800 Subject: [PATCH 3/3] s/get/gen/ --- yjit/src/codegen.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/yjit/src/codegen.rs b/yjit/src/codegen.rs index 122e9ad5923f99..04c48c25d9a83e 100644 --- a/yjit/src/codegen.rs +++ b/yjit/src/codegen.rs @@ -2660,7 +2660,7 @@ fn gen_defined( asm.stack_pop(1); // v operand is not used let out_opnd = asm.stack_push(Type::Unknown); // nil or "yield" - get_block_given(jit, asm, out_opnd, pushval.into(), Qnil.into()); + gen_block_given(jit, asm, out_opnd, pushval.into(), Qnil.into()); } _ => { // Save the PC and SP because the callee may allocate @@ -5278,12 +5278,12 @@ fn jit_rb_f_block_given_p( asm.stack_pop(1); let out_opnd = asm.stack_push(Type::UnknownImm); - get_block_given(jit, asm, out_opnd, Qtrue.into(), Qfalse.into()); + gen_block_given(jit, asm, out_opnd, Qtrue.into(), Qfalse.into()); true } -fn get_block_given( +fn gen_block_given( jit: &mut JITState, asm: &mut Assembler, out_opnd: Opnd,