Skip to content

Commit d524e79

Browse files
k0kubunXrXr
andauthored
ZJIT: Implement getblockparamproxy (ruby#14483)
Co-authored-by: Alan Wu <[email protected]>
1 parent f7fe436 commit d524e79

File tree

4 files changed

+92
-19
lines changed

4 files changed

+92
-19
lines changed

test/ruby/test_zjit.rb

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -308,6 +308,15 @@ def test_setlocal_on_eval
308308
}
309309
end
310310

311+
def test_getblockparamproxy
312+
assert_compiles '1', %q{
313+
def test(&block)
314+
0.then(&block)
315+
end
316+
test { 1 }
317+
}, insns: [:getblockparamproxy]
318+
end
319+
311320
def test_call_a_forwardable_method
312321
assert_runs '[]', %q{
313322
def test_root = forwardable

zjit/src/codegen.rs

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -398,6 +398,7 @@ fn gen_insn(cb: &mut CodeBlock, jit: &mut JITState, asm: &mut Assembler, functio
398398
Insn::GetGlobal { id, state } => gen_getglobal(jit, asm, *id, &function.frame_state(*state)),
399399
&Insn::GetLocal { ep_offset, level } => gen_getlocal_with_ep(asm, ep_offset, level),
400400
&Insn::SetLocal { val, ep_offset, level } => no_output!(gen_setlocal_with_ep(asm, opnd!(val), function.type_of(val), ep_offset, level)),
401+
&Insn::GetBlockParamProxy { level, state } => gen_get_block_param_proxy(jit, asm, level, &function.frame_state(state)),
401402
Insn::GetConstantPath { ic, state } => gen_get_constant_path(jit, asm, *ic, &function.frame_state(*state)),
402403
Insn::SetIvar { self_val, id, val, state: _ } => no_output!(gen_setivar(asm, opnd!(self_val), *id, opnd!(val))),
403404
Insn::SideExit { state, reason } => no_output!(gen_side_exit(jit, asm, reason, &function.frame_state(*state))),
@@ -548,6 +549,29 @@ fn gen_setlocal_with_ep(asm: &mut Assembler, val: Opnd, val_type: Type, local_ep
548549
}
549550
}
550551

552+
fn gen_get_block_param_proxy(jit: &JITState, asm: &mut Assembler, level: u32, state: &FrameState) -> lir::Opnd {
553+
// Bail out if the `&block` local variable has been modified
554+
let ep = gen_get_ep(asm, level);
555+
let flags = Opnd::mem(64, ep, SIZEOF_VALUE_I32 * (VM_ENV_DATA_INDEX_FLAGS as i32));
556+
asm.test(flags, VM_FRAME_FLAG_MODIFIED_BLOCK_PARAM.into());
557+
asm.jnz(side_exit(jit, state, SideExitReason::BlockParamProxyModified));
558+
559+
// This handles two cases which are nearly identical
560+
// Block handler is a tagged pointer. Look at the tag.
561+
// VM_BH_ISEQ_BLOCK_P(): block_handler & 0x03 == 0x01
562+
// VM_BH_IFUNC_P(): block_handler & 0x03 == 0x03
563+
// So to check for either of those cases we can use: val & 0x1 == 0x1
564+
const _: () = assert!(RUBY_SYMBOL_FLAG & 1 == 0, "guard below rejects symbol block handlers");
565+
566+
// Bail ouf if the block handler is neither ISEQ nor ifunc
567+
let block_handler = asm.load(Opnd::mem(64, ep, SIZEOF_VALUE_I32 * VM_ENV_DATA_INDEX_SPECVAL));
568+
asm.test(block_handler, 0x1.into());
569+
asm.jz(side_exit(jit, state, SideExitReason::BlockParamProxyNotIseqOrIfunc));
570+
571+
// Return the rb_block_param_proxy instance (GC root, so put as a number to avoid unnecessary GC tracing)
572+
unsafe { rb_block_param_proxy }.as_u64().into()
573+
}
574+
551575
fn gen_get_constant_path(jit: &JITState, asm: &mut Assembler, ic: *const iseq_inline_constant_cache, state: &FrameState) -> Opnd {
552576
unsafe extern "C" {
553577
fn rb_vm_opt_getconstant_path(ec: EcPtr, cfp: CfpPtr, ic: *const iseq_inline_constant_cache) -> VALUE;
@@ -1623,12 +1647,12 @@ fn compile_iseq(iseq: IseqPtr) -> Result<Function, CompileError> {
16231647
}
16241648

16251649
/// Build a Target::SideExit for non-PatchPoint instructions
1626-
fn side_exit(jit: &mut JITState, state: &FrameState, reason: SideExitReason) -> Target {
1650+
fn side_exit(jit: &JITState, state: &FrameState, reason: SideExitReason) -> Target {
16271651
build_side_exit(jit, state, reason, None)
16281652
}
16291653

16301654
/// Build a Target::SideExit out of a FrameState
1631-
fn build_side_exit(jit: &mut JITState, state: &FrameState, reason: SideExitReason, label: Option<Label>) -> Target {
1655+
fn build_side_exit(jit: &JITState, state: &FrameState, reason: SideExitReason, label: Option<Label>) -> Target {
16321656
let mut stack = Vec::new();
16331657
for &insn_id in state.stack() {
16341658
stack.push(jit.get_opnd(insn_id));

zjit/src/hir.rs

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -462,6 +462,8 @@ pub enum SideExitReason {
462462
CalleeSideExit,
463463
ObjToStringFallback,
464464
Interrupt,
465+
BlockParamProxyModified,
466+
BlockParamProxyNotIseqOrIfunc,
465467
}
466468

467469
impl std::fmt::Display for SideExitReason {
@@ -556,6 +558,9 @@ pub enum Insn {
556558
GetLocal { level: u32, ep_offset: u32 },
557559
/// Set a local variable in a higher scope or the heap
558560
SetLocal { level: u32, ep_offset: u32, val: InsnId },
561+
/// Get a special singleton instance `rb_block_param_proxy` if the block
562+
/// handler for the EP specified by `level` is an ISEQ or an ifunc.
563+
GetBlockParamProxy { level: u32, state: InsnId },
559564
GetSpecialSymbol { symbol_type: SpecialBackrefSymbol, state: InsnId },
560565
GetSpecialNumber { nth: u64, state: InsnId },
561566

@@ -900,6 +905,7 @@ impl<'a> std::fmt::Display for InsnPrinter<'a> {
900905
Insn::SetGlobal { id, val, .. } => write!(f, "SetGlobal :{}, {val}", id.contents_lossy()),
901906
Insn::GetLocal { level, ep_offset } => write!(f, "GetLocal l{level}, EP@{ep_offset}"),
902907
Insn::SetLocal { val, level, ep_offset } => write!(f, "SetLocal l{level}, EP@{ep_offset}, {val}"),
908+
Insn::GetBlockParamProxy { level, .. } => write!(f, "GetBlockParamProxy l{level}"),
903909
Insn::GetSpecialSymbol { symbol_type, .. } => write!(f, "GetSpecialSymbol {symbol_type:?}"),
904910
Insn::GetSpecialNumber { nth, .. } => write!(f, "GetSpecialNumber {nth}"),
905911
Insn::ToArray { val, .. } => write!(f, "ToArray {val}"),
@@ -1361,6 +1367,7 @@ impl Function {
13611367
&NewRange { low, high, flag, state } => NewRange { low: find!(low), high: find!(high), flag, state: find!(state) },
13621368
&NewRangeFixnum { low, high, flag, state } => NewRangeFixnum { low: find!(low), high: find!(high), flag, state: find!(state) },
13631369
&ArrayMax { ref elements, state } => ArrayMax { elements: find_vec!(elements), state: find!(state) },
1370+
&GetBlockParamProxy { level, state } => GetBlockParamProxy { level, state: find!(state) },
13641371
&SetGlobal { id, val, state } => SetGlobal { id, val: find!(val), state },
13651372
&GetIvar { self_val, id, state } => GetIvar { self_val: find!(self_val), id, state },
13661373
&LoadIvarEmbedded { self_val, id, index } => LoadIvarEmbedded { self_val: find!(self_val), id, index },
@@ -1472,6 +1479,7 @@ impl Function {
14721479
Insn::ObjToString { .. } => types::BasicObject,
14731480
Insn::AnyToString { .. } => types::String,
14741481
Insn::GetLocal { .. } => types::BasicObject,
1482+
Insn::GetBlockParamProxy { .. } => types::BasicObject,
14751483
// The type of Snapshot doesn't really matter; it's never materialized. It's used only
14761484
// as a reference for FrameState, which we use to generate side-exit code.
14771485
Insn::Snapshot { .. } => types::Any,
@@ -2300,6 +2308,7 @@ impl Function {
23002308
| &Insn::LoadIvarExtended { self_val, .. } => {
23012309
worklist.push_back(self_val);
23022310
}
2311+
&Insn::GetBlockParamProxy { state, .. } |
23032312
&Insn::GetGlobal { state, .. } |
23042313
&Insn::GetSpecialSymbol { state, .. } |
23052314
&Insn::GetSpecialNumber { state, .. } |
@@ -3466,6 +3475,11 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result<Function, ParseError> {
34663475
let level = get_arg(pc, 1).as_u32();
34673476
fun.push_insn(block, Insn::SetLocal { val: state.stack_pop()?, ep_offset, level });
34683477
}
3478+
YARVINSN_getblockparamproxy => {
3479+
let level = get_arg(pc, 1).as_u32();
3480+
let exit_id = fun.push_insn(block, Insn::Snapshot { state: exit_state });
3481+
state.stack_push(fun.push_insn(block, Insn::GetBlockParamProxy { level, state: exit_id }));
3482+
}
34693483
YARVINSN_pop => { state.stack_pop()?; }
34703484
YARVINSN_dup => { state.stack_push(state.stack_top()?); }
34713485
YARVINSN_dupn => {
@@ -5816,7 +5830,16 @@ mod tests {
58165830
v5:NilClass = Const Value(nil)
58175831
v10:BasicObject = InvokeBuiltin dir_s_open, v0, v1, v2
58185832
PatchPoint NoEPEscape(open)
5819-
SideExit UnhandledYARVInsn(getblockparamproxy)
5833+
v16:BasicObject = GetBlockParamProxy l0
5834+
CheckInterrupts
5835+
v19:CBool = Test v16
5836+
IfFalse v19, bb1(v0, v1, v2, v3, v4, v10)
5837+
PatchPoint NoEPEscape(open)
5838+
SideExit UnhandledYARVInsn(invokeblock)
5839+
bb1(v27:BasicObject, v28:BasicObject, v29:BasicObject, v30:BasicObject, v31:BasicObject, v32:BasicObject):
5840+
PatchPoint NoEPEscape(open)
5841+
CheckInterrupts
5842+
Return v32
58205843
");
58215844
}
58225845

@@ -7980,6 +8003,19 @@ mod opt_tests {
79808003
");
79818004
}
79828005

8006+
#[test]
8007+
fn test_getblockparamproxy() {
8008+
eval("
8009+
def test(&block) = tap(&block)
8010+
");
8011+
assert_snapshot!(hir_string("test"), @r"
8012+
fn test@<compiled>:2:
8013+
bb0(v0:BasicObject, v1:BasicObject):
8014+
v6:BasicObject = GetBlockParamProxy l0
8015+
SideExit UnhandledCallType(BlockArg)
8016+
");
8017+
}
8018+
79838019
#[test]
79848020
fn test_getinstancevariable() {
79858021
eval("

zjit/src/stats.rs

Lines changed: 20 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,8 @@ make_counters! {
103103
exit_obj_to_string_fallback,
104104
exit_interrupt,
105105
exit_optional_arguments,
106+
exit_block_param_proxy_modified,
107+
exit_block_param_proxy_not_iseq_or_ifunc,
106108
}
107109

108110
// unhanded_call_: Unhandled call types
@@ -219,22 +221,24 @@ pub fn exit_counter_ptr(reason: crate::hir::SideExitReason) -> *mut u64 {
219221
use crate::hir::SideExitReason::*;
220222
use crate::stats::Counter::*;
221223
let counter = match reason {
222-
UnknownNewarraySend(_) => exit_unknown_newarray_send,
223-
UnhandledCallType(_) => exit_unhandled_call_type,
224-
UnknownSpecialVariable(_) => exit_unknown_special_variable,
225-
UnhandledHIRInsn(_) => exit_unhandled_hir_insn,
226-
UnhandledYARVInsn(_) => exit_unhandled_yarv_insn,
227-
FixnumAddOverflow => exit_fixnum_add_overflow,
228-
FixnumSubOverflow => exit_fixnum_sub_overflow,
229-
FixnumMultOverflow => exit_fixnum_mult_overflow,
230-
GuardType(_) => exit_guard_type_failure,
231-
GuardTypeNot(_) => exit_guard_type_not_failure,
232-
GuardBitEquals(_) => exit_guard_bit_equals_failure,
233-
GuardShape(_) => exit_guard_shape_failure,
234-
PatchPoint(_) => exit_patchpoint,
235-
CalleeSideExit => exit_callee_side_exit,
236-
ObjToStringFallback => exit_obj_to_string_fallback,
237-
Interrupt => exit_interrupt,
224+
UnknownNewarraySend(_) => exit_unknown_newarray_send,
225+
UnhandledCallType(_) => exit_unhandled_call_type,
226+
UnknownSpecialVariable(_) => exit_unknown_special_variable,
227+
UnhandledHIRInsn(_) => exit_unhandled_hir_insn,
228+
UnhandledYARVInsn(_) => exit_unhandled_yarv_insn,
229+
FixnumAddOverflow => exit_fixnum_add_overflow,
230+
FixnumSubOverflow => exit_fixnum_sub_overflow,
231+
FixnumMultOverflow => exit_fixnum_mult_overflow,
232+
GuardType(_) => exit_guard_type_failure,
233+
GuardTypeNot(_) => exit_guard_type_not_failure,
234+
GuardBitEquals(_) => exit_guard_bit_equals_failure,
235+
GuardShape(_) => exit_guard_shape_failure,
236+
PatchPoint(_) => exit_patchpoint,
237+
CalleeSideExit => exit_callee_side_exit,
238+
ObjToStringFallback => exit_obj_to_string_fallback,
239+
Interrupt => exit_interrupt,
240+
BlockParamProxyModified => exit_block_param_proxy_modified,
241+
BlockParamProxyNotIseqOrIfunc => exit_block_param_proxy_not_iseq_or_ifunc,
238242
};
239243
counter_ptr(counter)
240244
}

0 commit comments

Comments
 (0)