Skip to content

Commit 8117600

Browse files
authored
ZJIT: Implement include_p for opt_(new|dup)array_send YARV insns (#14885)
These just call to the C functions that do the optimized test but this avoids the side exit. See #12123 for the original CRuby/YJIT implementation.
1 parent 16af727 commit 8117600

File tree

5 files changed

+188
-1
lines changed

5 files changed

+188
-1
lines changed

test/ruby/test_zjit.rb

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -957,6 +957,24 @@ def test
957957
}, insns: [:opt_new], call_threshold: 2
958958
end
959959

960+
def test_opt_newarray_send_include_p
961+
assert_compiles '[true, false]', %q{
962+
def test(x)
963+
[:y, 1, Object.new].include?(x)
964+
end
965+
[test(1), test("n")]
966+
}, insns: [:opt_newarray_send], call_threshold: 1
967+
end
968+
969+
def test_opt_duparray_send_include_p
970+
assert_compiles '[true, false]', %q{
971+
def test(x)
972+
[:y, 1].include?(x)
973+
end
974+
[test(1), test("n")]
975+
}, insns: [:opt_duparray_send], call_threshold: 1
976+
end
977+
960978
def test_new_hash_empty
961979
assert_compiles '{}', %q{
962980
def test = {}

zjit/src/codegen.rs

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -450,6 +450,8 @@ fn gen_insn(cb: &mut CodeBlock, jit: &mut JITState, asm: &mut Assembler, functio
450450
Insn::LoadSelf => gen_load_self(),
451451
&Insn::LoadField { recv, id, offset, return_type: _ } => gen_load_field(asm, opnd!(recv), id, offset),
452452
&Insn::IsBlockGiven => gen_is_block_given(jit, asm),
453+
Insn::ArrayInclude { elements, target, state } => gen_array_include(jit, asm, opnds!(elements), opnd!(target), &function.frame_state(*state)),
454+
&Insn::DupArrayInclude { ary, target, state } => gen_dup_array_include(jit, asm, ary, opnd!(target), &function.frame_state(state)),
453455
&Insn::ArrayMax { state, .. }
454456
| &Insn::FixnumDiv { state, .. }
455457
| &Insn::Throw { state, .. }
@@ -1328,6 +1330,50 @@ fn gen_array_length(asm: &mut Assembler, array: Opnd) -> lir::Opnd {
13281330
asm_ccall!(asm, rb_jit_array_len, array)
13291331
}
13301332

1333+
fn gen_array_include(
1334+
jit: &JITState,
1335+
asm: &mut Assembler,
1336+
elements: Vec<Opnd>,
1337+
target: Opnd,
1338+
state: &FrameState,
1339+
) -> lir::Opnd {
1340+
gen_prepare_non_leaf_call(jit, asm, state);
1341+
1342+
let num: c_long = elements.len().try_into().expect("Unable to fit length of elements into c_long");
1343+
1344+
// After gen_prepare_non_leaf_call, the elements are spilled to the Ruby stack.
1345+
// The elements are at the bottom of the virtual stack, followed by the target.
1346+
// Get a pointer to the first element on the Ruby stack.
1347+
let stack_bottom = state.stack().len() - elements.len() - 1;
1348+
let elements_ptr = asm.lea(Opnd::mem(64, SP, stack_bottom as i32 * SIZEOF_VALUE_I32));
1349+
1350+
unsafe extern "C" {
1351+
fn rb_vm_opt_newarray_include_p(ec: EcPtr, num: c_long, elts: *const VALUE, target: VALUE) -> VALUE;
1352+
}
1353+
asm.ccall(
1354+
rb_vm_opt_newarray_include_p as *const u8,
1355+
vec![EC, num.into(), elements_ptr, target],
1356+
)
1357+
}
1358+
1359+
fn gen_dup_array_include(
1360+
jit: &JITState,
1361+
asm: &mut Assembler,
1362+
ary: VALUE,
1363+
target: Opnd,
1364+
state: &FrameState,
1365+
) -> lir::Opnd {
1366+
gen_prepare_non_leaf_call(jit, asm, state);
1367+
1368+
unsafe extern "C" {
1369+
fn rb_vm_opt_duparray_include_p(ec: EcPtr, ary: VALUE, target: VALUE) -> VALUE;
1370+
}
1371+
asm.ccall(
1372+
rb_vm_opt_duparray_include_p as *const u8,
1373+
vec![EC, ary.into(), target],
1374+
)
1375+
}
1376+
13311377
/// Compile a new hash instruction
13321378
fn gen_new_hash(
13331379
jit: &mut JITState,

zjit/src/hir.rs

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -457,6 +457,7 @@ impl PtrPrintMap {
457457
#[derive(Debug, Clone, Copy)]
458458
pub enum SideExitReason {
459459
UnknownNewarraySend(vm_opt_newarray_send_type),
460+
UnknownDuparraySend(u64),
460461
UnknownSpecialVariable(u64),
461462
UnhandledHIRInsn(InsnId),
462463
UnhandledYARVInsn(u32),
@@ -548,6 +549,7 @@ impl std::fmt::Display for SideExitReason {
548549
SideExitReason::UnknownNewarraySend(VM_OPT_NEWARRAY_SEND_PACK) => write!(f, "UnknownNewarraySend(PACK)"),
549550
SideExitReason::UnknownNewarraySend(VM_OPT_NEWARRAY_SEND_PACK_BUFFER) => write!(f, "UnknownNewarraySend(PACK_BUFFER)"),
550551
SideExitReason::UnknownNewarraySend(VM_OPT_NEWARRAY_SEND_INCLUDE_P) => write!(f, "UnknownNewarraySend(INCLUDE_P)"),
552+
SideExitReason::UnknownDuparraySend(method_id) => write!(f, "UnknownDuparraySend({})", method_id),
551553
SideExitReason::GuardType(guard_type) => write!(f, "GuardType({guard_type})"),
552554
SideExitReason::GuardTypeNot(guard_type) => write!(f, "GuardTypeNot({guard_type})"),
553555
SideExitReason::GuardBitEquals(value) => write!(f, "GuardBitEquals({})", value.print(&PtrPrintMap::identity())),
@@ -616,6 +618,8 @@ pub enum Insn {
616618
NewRangeFixnum { low: InsnId, high: InsnId, flag: RangeType, state: InsnId },
617619
ArrayDup { val: InsnId, state: InsnId },
618620
ArrayMax { elements: Vec<InsnId>, state: InsnId },
621+
ArrayInclude { elements: Vec<InsnId>, target: InsnId, state: InsnId },
622+
DupArrayInclude { ary: VALUE, target: InsnId, state: InsnId },
619623
/// Extend `left` with the elements from `right`. `left` and `right` must both be `Array`.
620624
ArrayExtend { left: InsnId, right: InsnId, state: InsnId },
621625
/// Push `val` onto `array`, where `array` is already `Array`.
@@ -988,6 +992,18 @@ impl<'a> std::fmt::Display for InsnPrinter<'a> {
988992
}
989993
Ok(())
990994
}
995+
Insn::ArrayInclude { elements, target, .. } => {
996+
write!(f, "ArrayInclude")?;
997+
let mut prefix = " ";
998+
for element in elements {
999+
write!(f, "{prefix}{element}")?;
1000+
prefix = ", ";
1001+
}
1002+
write!(f, " | {target}")
1003+
}
1004+
Insn::DupArrayInclude { ary, target, .. } => {
1005+
write!(f, "DupArrayInclude {} | {}", ary.print(self.ptr_map), target)
1006+
}
9911007
Insn::ArrayDup { val, .. } => { write!(f, "ArrayDup {val}") }
9921008
Insn::HashDup { val, .. } => { write!(f, "HashDup {val}") }
9931009
Insn::HashAref { hash, key, .. } => { write!(f, "HashAref {hash}, {key}")}
@@ -1789,6 +1805,8 @@ impl Function {
17891805
&ArrayPop { array, state } => ArrayPop { array: find!(array), state: find!(state) },
17901806
&ArrayLength { array } => ArrayLength { array: find!(array) },
17911807
&ArrayMax { ref elements, state } => ArrayMax { elements: find_vec!(elements), state: find!(state) },
1808+
&ArrayInclude { ref elements, target, state } => ArrayInclude { elements: find_vec!(elements), target: find!(target), state: find!(state) },
1809+
&DupArrayInclude { ary, target, state } => DupArrayInclude { ary, target: find!(target), state: find!(state) },
17921810
&SetGlobal { id, val, state } => SetGlobal { id, val: find!(val), state },
17931811
&GetIvar { self_val, id, state } => GetIvar { self_val: find!(self_val), id, state },
17941812
&LoadField { recv, id, offset, return_type } => LoadField { recv: find!(recv), id, offset, return_type },
@@ -1923,6 +1941,8 @@ impl Function {
19231941
Insn::GetConstantPath { .. } => types::BasicObject,
19241942
Insn::IsBlockGiven => types::BoolExact,
19251943
Insn::ArrayMax { .. } => types::BasicObject,
1944+
Insn::ArrayInclude { .. } => types::BoolExact,
1945+
Insn::DupArrayInclude { .. } => types::BoolExact,
19261946
Insn::GetGlobal { .. } => types::BasicObject,
19271947
Insn::GetIvar { .. } => types::BasicObject,
19281948
Insn::LoadPC => types::CPtr,
@@ -3211,6 +3231,15 @@ impl Function {
32113231
worklist.extend(elements);
32123232
worklist.push_back(state);
32133233
}
3234+
&Insn::ArrayInclude { ref elements, target, state } => {
3235+
worklist.extend(elements);
3236+
worklist.push_back(target);
3237+
worklist.push_back(state);
3238+
}
3239+
&Insn::DupArrayInclude { target, state, .. } => {
3240+
worklist.push_back(target);
3241+
worklist.push_back(state);
3242+
}
32143243
&Insn::NewRange { low, high, state, .. }
32153244
| &Insn::NewRangeFixnum { low, high, state, .. } => {
32163245
worklist.push_back(low);
@@ -4448,6 +4477,11 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result<Function, ParseError> {
44484477
let exit_id = fun.push_insn(block, Insn::Snapshot { state: exit_state });
44494478
let (bop, insn) = match method {
44504479
VM_OPT_NEWARRAY_SEND_MAX => (BOP_MAX, Insn::ArrayMax { elements, state: exit_id }),
4480+
VM_OPT_NEWARRAY_SEND_INCLUDE_P => {
4481+
let target = elements[elements.len() - 1];
4482+
let array_elements = elements[..elements.len() - 1].to_vec();
4483+
(BOP_INCLUDE_P, Insn::ArrayInclude { elements: array_elements, target, state: exit_id })
4484+
},
44514485
_ => {
44524486
// Unknown opcode; side-exit into the interpreter
44534487
fun.push_insn(block, Insn::SideExit { state: exit_id, reason: SideExitReason::UnknownNewarraySend(method) });
@@ -4468,6 +4502,30 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result<Function, ParseError> {
44684502
let insn_id = fun.push_insn(block, Insn::ArrayDup { val, state: exit_id });
44694503
state.stack_push(insn_id);
44704504
}
4505+
YARVINSN_opt_duparray_send => {
4506+
let ary = get_arg(pc, 0);
4507+
let method_id = get_arg(pc, 1).as_u64();
4508+
let argc = get_arg(pc, 2).as_usize();
4509+
if argc != 1 {
4510+
break;
4511+
}
4512+
let target = state.stack_pop()?;
4513+
let exit_id = fun.push_insn(block, Insn::Snapshot { state: exit_state });
4514+
let bop = match method_id {
4515+
x if x == ID!(include_p).0 => BOP_INCLUDE_P,
4516+
_ => {
4517+
fun.push_insn(block, Insn::SideExit { state: exit_id, reason: SideExitReason::UnknownDuparraySend(method_id) });
4518+
break;
4519+
},
4520+
};
4521+
if !unsafe { rb_BASIC_OP_UNREDEFINED_P(bop, ARRAY_REDEFINED_OP_FLAG) } {
4522+
fun.push_insn(block, Insn::SideExit { state: exit_id, reason: SideExitReason::PatchPoint(Invariant::BOPRedefined { klass: ARRAY_REDEFINED_OP_FLAG, bop }) });
4523+
break;
4524+
}
4525+
fun.push_insn(block, Insn::PatchPoint { invariant: Invariant::BOPRedefined { klass: ARRAY_REDEFINED_OP_FLAG, bop }, state: exit_id });
4526+
let insn_id = fun.push_insn(block, Insn::DupArrayInclude { ary, target, state: exit_id });
4527+
state.stack_push(insn_id);
4528+
}
44714529
YARVINSN_newhash => {
44724530
let count = get_arg(pc, 0).as_usize();
44734531
assert!(count % 2 == 0, "newhash count should be even");

zjit/src/hir/tests.rs

Lines changed: 64 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2023,7 +2023,70 @@ pub mod hir_build_tests {
20232023
Jump bb2(v8, v9, v10, v11, v12)
20242024
bb2(v14:BasicObject, v15:BasicObject, v16:BasicObject, v17:NilClass, v18:NilClass):
20252025
v25:BasicObject = SendWithoutBlock v15, :+, v16
2026-
SideExit UnknownNewarraySend(INCLUDE_P)
2026+
PatchPoint BOPRedefined(ARRAY_REDEFINED_OP_FLAG, 33)
2027+
v30:BoolExact = ArrayInclude v15, v16 | v16
2028+
PatchPoint NoEPEscape(test)
2029+
v35:ArrayExact[VALUE(0x1000)] = Const Value(VALUE(0x1000))
2030+
v37:ArrayExact = ArrayDup v35
2031+
v39:BasicObject = SendWithoutBlock v14, :puts, v37
2032+
PatchPoint NoEPEscape(test)
2033+
CheckInterrupts
2034+
Return v30
2035+
");
2036+
}
2037+
2038+
#[test]
2039+
fn test_opt_duparray_send_include_p() {
2040+
eval("
2041+
def test(x)
2042+
[:a, :b].include?(x)
2043+
end
2044+
");
2045+
assert_contains_opcode("test", YARVINSN_opt_duparray_send);
2046+
assert_snapshot!(hir_string("test"), @r"
2047+
fn test@<compiled>:3:
2048+
bb0():
2049+
EntryPoint interpreter
2050+
v1:BasicObject = LoadSelf
2051+
v2:BasicObject = GetLocal l0, SP@4
2052+
Jump bb2(v1, v2)
2053+
bb1(v5:BasicObject, v6:BasicObject):
2054+
EntryPoint JIT(0)
2055+
Jump bb2(v5, v6)
2056+
bb2(v8:BasicObject, v9:BasicObject):
2057+
PatchPoint BOPRedefined(ARRAY_REDEFINED_OP_FLAG, 33)
2058+
v15:BoolExact = DupArrayInclude VALUE(0x1000) | v9
2059+
CheckInterrupts
2060+
Return v15
2061+
");
2062+
}
2063+
2064+
#[test]
2065+
fn test_opt_duparray_send_include_p_redefined() {
2066+
eval("
2067+
class Array
2068+
alias_method :old_include?, :include?
2069+
def include?(x)
2070+
old_include?(x)
2071+
end
2072+
end
2073+
def test(x)
2074+
[:a, :b].include?(x)
2075+
end
2076+
");
2077+
assert_contains_opcode("test", YARVINSN_opt_duparray_send);
2078+
assert_snapshot!(hir_string("test"), @r"
2079+
fn test@<compiled>:9:
2080+
bb0():
2081+
EntryPoint interpreter
2082+
v1:BasicObject = LoadSelf
2083+
v2:BasicObject = GetLocal l0, SP@4
2084+
Jump bb2(v1, v2)
2085+
bb1(v5:BasicObject, v6:BasicObject):
2086+
EntryPoint JIT(0)
2087+
Jump bb2(v5, v6)
2088+
bb2(v8:BasicObject, v9:BasicObject):
2089+
SideExit PatchPoint(BOPRedefined(ARRAY_REDEFINED_OP_FLAG, 33))
20272090
");
20282091
}
20292092

zjit/src/stats.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,7 @@ make_counters! {
128128
// exit_: Side exits reasons
129129
exit_compile_error,
130130
exit_unknown_newarray_send,
131+
exit_unknown_duparray_send,
131132
exit_unhandled_tailcall,
132133
exit_unhandled_splat,
133134
exit_unhandled_kwarg,
@@ -366,6 +367,7 @@ pub fn side_exit_counter(reason: crate::hir::SideExitReason) -> Counter {
366367
use crate::stats::Counter::*;
367368
match reason {
368369
UnknownNewarraySend(_) => exit_unknown_newarray_send,
370+
UnknownDuparraySend(_) => exit_unknown_duparray_send,
369371
UnhandledCallType(Tailcall) => exit_unhandled_tailcall,
370372
UnhandledCallType(Splat) => exit_unhandled_splat,
371373
UnhandledCallType(Kwarg) => exit_unhandled_kwarg,

0 commit comments

Comments
 (0)