Skip to content

Commit 4710284

Browse files
authored
ZJIT: Pessimize locals in the presence of send (with block) (ruby#14374)
We can refine this later by some kind of analysis of the block we're sending to: maybe it doesn't write to our locals, or at least doesn't write to all of them.
1 parent 984e05a commit 4710284

File tree

1 file changed

+45
-12
lines changed

1 file changed

+45
-12
lines changed

zjit/src/hir.rs

Lines changed: 45 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2661,10 +2661,16 @@ fn insn_idx_at_offset(idx: u32, offset: i64) -> u32 {
26612661
((idx as isize) + (offset as isize)) as u32
26622662
}
26632663

2664-
fn compute_jump_targets(iseq: *const rb_iseq_t) -> Vec<u32> {
2664+
struct BytecodeInfo {
2665+
jump_targets: Vec<u32>,
2666+
has_send: bool,
2667+
}
2668+
2669+
fn compute_bytecode_info(iseq: *const rb_iseq_t) -> BytecodeInfo {
26652670
let iseq_size = unsafe { get_iseq_encoded_size(iseq) };
26662671
let mut insn_idx = 0;
26672672
let mut jump_targets = HashSet::new();
2673+
let mut has_send = false;
26682674
while insn_idx < iseq_size {
26692675
// Get the current pc and opcode
26702676
let pc = unsafe { rb_iseq_pc_at_idx(iseq, insn_idx) };
@@ -2688,12 +2694,13 @@ fn compute_jump_targets(iseq: *const rb_iseq_t) -> Vec<u32> {
26882694
jump_targets.insert(insn_idx);
26892695
}
26902696
}
2697+
YARVINSN_send => has_send = true,
26912698
_ => {}
26922699
}
26932700
}
26942701
let mut result = jump_targets.into_iter().collect::<Vec<_>>();
26952702
result.sort();
2696-
result
2703+
BytecodeInfo { jump_targets: result, has_send }
26972704
}
26982705

26992706
#[derive(Debug, PartialEq)]
@@ -2800,7 +2807,7 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result<Function, ParseError> {
28002807
let mut profiles = ProfileOracle::new(payload);
28012808
let mut fun = Function::new(iseq);
28022809
// Compute a map of PC->Block by finding jump targets
2803-
let jump_targets = compute_jump_targets(iseq);
2810+
let BytecodeInfo { jump_targets, has_send } = compute_bytecode_info(iseq);
28042811
let mut insn_idx_to_block = HashMap::new();
28052812
for insn_idx in jump_targets {
28062813
if insn_idx == 0 {
@@ -3120,7 +3127,7 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result<Function, ParseError> {
31203127
}
31213128
YARVINSN_getlocal_WC_0 => {
31223129
let ep_offset = get_arg(pc, 0).as_u32();
3123-
if iseq_type == ISEQ_TYPE_EVAL {
3130+
if iseq_type == ISEQ_TYPE_EVAL || has_send {
31243131
// On eval, the locals are always on the heap, so read the local using EP.
31253132
state.stack_push(fun.push_insn(block, Insn::GetLocal { ep_offset, level: 0 }));
31263133
} else {
@@ -3138,7 +3145,7 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result<Function, ParseError> {
31383145
let ep_offset = get_arg(pc, 0).as_u32();
31393146
let val = state.stack_pop()?;
31403147
state.setlocal(ep_offset, val);
3141-
if iseq_type == ISEQ_TYPE_EVAL {
3148+
if iseq_type == ISEQ_TYPE_EVAL || has_send {
31423149
// On eval, the locals are always on the heap, so write the local using EP.
31433150
fun.push_insn(block, Insn::SetLocal { val, ep_offset, level: 0 });
31443151
}
@@ -4674,9 +4681,10 @@ mod tests {
46744681
assert_snapshot!(hir_string("test"), @r"
46754682
fn test@<compiled>:3:
46764683
bb0(v0:BasicObject, v1:BasicObject):
4677-
v4:BasicObject = Send v1, 0x1000, :each
4684+
v3:BasicObject = GetLocal l0, EP@3
4685+
v5:BasicObject = Send v3, 0x1000, :each
46784686
CheckInterrupts
4679-
Return v4
4687+
Return v5
46804688
");
46814689
}
46824690

@@ -4742,11 +4750,12 @@ mod tests {
47424750
eval("
47434751
def test(a) = foo(&a)
47444752
");
4745-
assert_snapshot!(hir_string("test"), @r#"
4746-
fn test@<compiled>:2:
4747-
bb0(v0:BasicObject, v1:BasicObject):
4748-
SideExit UnknownCallType
4749-
"#);
4753+
assert_snapshot!(hir_string("test"), @r"
4754+
fn test@<compiled>:2:
4755+
bb0(v0:BasicObject, v1:BasicObject):
4756+
v3:BasicObject = GetLocal l0, EP@3
4757+
SideExit UnknownCallType
4758+
");
47504759
}
47514760

47524761
#[test]
@@ -7195,6 +7204,30 @@ mod opt_tests {
71957204
");
71967205
}
71977206

7207+
#[test]
7208+
fn reload_local_across_send() {
7209+
eval("
7210+
def foo(&block) = 1
7211+
def test
7212+
a = 1
7213+
foo {|| }
7214+
a
7215+
end
7216+
test
7217+
test
7218+
");
7219+
assert_snapshot!(hir_string("test"), @r"
7220+
fn test@<compiled>:4:
7221+
bb0(v0:BasicObject):
7222+
v3:Fixnum[1] = Const Value(1)
7223+
SetLocal l0, EP@3, v3
7224+
v6:BasicObject = Send v0, 0x1000, :foo
7225+
v7:BasicObject = GetLocal l0, EP@3
7226+
CheckInterrupts
7227+
Return v7
7228+
");
7229+
}
7230+
71987231
#[test]
71997232
fn dont_specialize_call_to_iseq_with_rest() {
72007233
eval("

0 commit comments

Comments
 (0)