mirror/ruby
1
0
Fork 0
mirror of https://github.com/ruby/ruby.git synced 2026-01-26 20:19:19 +00:00
Code Issues Packages Projects Releases Wiki Activity

ZJIT: Avoid optimizing locals on eval (#13840)

Browse Source 
* ZJIT: Avoid optimizing locals on eval

* Maintain the local state for eval
This commit is contained in:
Takashi Kokubun 2025-07-10 12:08:09 -07:00 committed by GitHub
parent 9d41541b0c
commit 9ab80a7455
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 53 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
.github/workflows/zjit-macos.yml vendored
View File

@ -112,6 +112,7 @@ jobs:
../src/bootstraptest/test_class.rb \
../src/bootstraptest/test_constant_cache.rb \
../src/bootstraptest/test_env.rb \
../src/bootstraptest/test_eval.rb \
../src/bootstraptest/test_exception.rb \
../src/bootstraptest/test_fiber.rb \
../src/bootstraptest/test_finalizer.rb \
@ -138,7 +139,6 @@ jobs:
../src/bootstraptest/test_yjit_30k_ifelse.rb \
../src/bootstraptest/test_yjit_30k_methods.rb \
../src/bootstraptest/test_yjit_rust_port.rb
# ../src/bootstraptest/test_eval.rb \
# ../src/bootstraptest/test_yjit.rb \
if: ${{ matrix.test_task == 'btest' }}

2
.github/workflows/zjit-ubuntu.yml vendored
View File

@ -134,6 +134,7 @@ jobs:
../src/bootstraptest/test_class.rb \
../src/bootstraptest/test_constant_cache.rb \
../src/bootstraptest/test_env.rb \
../src/bootstraptest/test_env.rb \
../src/bootstraptest/test_exception.rb \
../src/bootstraptest/test_fiber.rb \
../src/bootstraptest/test_finalizer.rb \
@ -160,7 +161,6 @@ jobs:
../src/bootstraptest/test_yjit_30k_ifelse.rb \
../src/bootstraptest/test_yjit_30k_methods.rb \
../src/bootstraptest/test_yjit_rust_port.rb
# ../src/bootstraptest/test_eval.rb \
# ../src/bootstraptest/test_yjit.rb \
if: ${{ matrix.test_task == 'btest' }}

16
test/ruby/test_zjit.rb
View File

@ -62,6 +62,22 @@ class TestZJIT < Test::Unit::TestCase
}
end
def test_setlocal_on_eval
assert_compiles '1', %q{
@b = binding
eval('a = 1', @b)
eval('a', @b)
}
end
def test_setlocal_on_eval_with_spill
assert_compiles '1', %q{
@b = binding
eval('a = 1; itself', @b)
eval('a', @b)
}
end
def test_nested_local_access
assert_compiles '[1, 2, 3]', %q{
1.times do |l2|

21
zjit/src/codegen.rs
View File

@ -1,6 +1,5 @@
use std::cell::Cell;
use std::rc::Rc;
use std::num::NonZeroU32;
use crate::backend::current::{Reg, ALLOC_REGS};
use crate::invariants::track_bop_assumption;
@ -302,8 +301,8 @@ fn gen_insn(cb: &mut CodeBlock, jit: &mut JITState, asm: &mut Assembler, functio
Insn::GetIvar { self_val, id, state: _ } => gen_getivar(asm, opnd!(self_val), *id),
Insn::SetGlobal { id, val, state: _ } => return Some(gen_setglobal(asm, *id, opnd!(val))),
Insn::GetGlobal { id, state: _ } => gen_getglobal(asm, *id),
&Insn::GetLocal { ep_offset, level } => gen_nested_getlocal(asm, ep_offset, level)?,
Insn::SetLocal { val, ep_offset, level } => return gen_nested_setlocal(asm, opnd!(val), *ep_offset, *level),
&Insn::GetLocal { ep_offset, level } => gen_getlocal_with_ep(asm, ep_offset, level)?,
Insn::SetLocal { val, ep_offset, level } => return gen_setlocal_with_ep(asm, opnd!(val), *ep_offset, *level),
Insn::GetConstantPath { ic, state } => gen_get_constant_path(asm, *ic, &function.frame_state(*state)),
Insn::SetIvar { self_val, id, val, state: _ } => return gen_setivar(asm, opnd!(self_val), *id, opnd!(val)),
Insn::SideExit { state, reason: _ } => return gen_side_exit(jit, asm, &function.frame_state(*state)),
@ -383,16 +382,20 @@ fn gen_defined(jit: &JITState, asm: &mut Assembler, op_type: usize, _obj: VALUE,
}
}
/// Get a local variable from a higher scope. `local_ep_offset` is in number of VALUEs.
fn gen_nested_getlocal(asm: &mut Assembler, local_ep_offset: u32, level: NonZeroU32) -> Option<lir::Opnd> {
let ep = gen_get_ep(asm, level.get());
/// Get a local variable from a higher scope or the heap. `local_ep_offset` is in number of VALUEs.
/// We generate this instruction with level=0 only when the local variable is on the heap, so we
/// can't optimize the level=0 case using the SP register.
fn gen_getlocal_with_ep(asm: &mut Assembler, local_ep_offset: u32, level: u32) -> Option<lir::Opnd> {
let ep = gen_get_ep(asm, level);
let offset = -(SIZEOF_VALUE_I32 * i32::try_from(local_ep_offset).ok()?);
Some(asm.load(Opnd::mem(64, ep, offset)))
}
/// Set a local variable from a higher scope. `local_ep_offset` is in number of VALUEs.
fn gen_nested_setlocal(asm: &mut Assembler, val: Opnd, local_ep_offset: u32, level: NonZeroU32) -> Option<()> {
let ep = gen_get_ep(asm, level.get());
/// Set a local variable from a higher scope or the heap. `local_ep_offset` is in number of VALUEs.
/// We generate this instruction with level=0 only when the local variable is on the heap, so we
/// can't optimize the level=0 case using the SP register.
fn gen_setlocal_with_ep(asm: &mut Assembler, val: Opnd, local_ep_offset: u32, level: u32) -> Option<()> {
let ep = gen_get_ep(asm, level);
let offset = -(SIZEOF_VALUE_I32 * i32::try_from(local_ep_offset).ok()?);
asm.mov(Opnd::mem(64, ep, offset), val);
Some(())

37
zjit/src/hir.rs
View File

@ -11,7 +11,6 @@ use std::{
collections::{HashMap, HashSet, VecDeque},
ffi::{c_int, c_void, CStr},
mem::{align_of, size_of},
num::NonZeroU32,
ptr,
slice::Iter
};
@ -480,10 +479,10 @@ pub enum Insn {
/// Check whether an instance variable exists on `self_val`
DefinedIvar { self_val: InsnId, id: ID, pushval: VALUE, state: InsnId },
/// Get a local variable from a higher scope
GetLocal { level: NonZeroU32, ep_offset: u32 },
/// Set a local variable in a higher scope
SetLocal { level: NonZeroU32, ep_offset: u32, val: InsnId },
/// Get a local variable from a higher scope or the heap
GetLocal { level: u32, ep_offset: u32 },
/// Set a local variable in a higher scope or the heap
SetLocal { level: u32, ep_offset: u32, val: InsnId },
/// Own a FrameState so that instructions can look up their dominating FrameState when
/// generating deopt side-exits and frame reconstruction metadata. Does not directly generate
@ -2439,6 +2438,7 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result<Function, ParseError> {
let mut visited = HashSet::new();
let iseq_size = unsafe { get_iseq_encoded_size(iseq) };
let iseq_type = unsafe { get_iseq_body_type(iseq) };
while let Some((incoming_state, block, mut insn_idx)) = queue.pop_front() {
if visited.contains(&block) { continue; }
visited.insert(block);
@ -2682,12 +2682,17 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result<Function, ParseError> {
break; // Don't enqueue the next block as a successor
}
YARVINSN_getlocal_WC_0 => {
// TODO(alan): This implementation doesn't read from EP, so will miss writes
// from nested ISeqs. This will need to be amended when we add codegen for
// Send.
let ep_offset = get_arg(pc, 0).as_u32();
let val = state.getlocal(ep_offset);
state.stack_push(val);
if iseq_type == ISEQ_TYPE_EVAL {
// On eval, the locals are always on the heap, so read the local using EP.
state.stack_push(fun.push_insn(block, Insn::GetLocal { ep_offset, level: 0 }));
} else {
// TODO(alan): This implementation doesn't read from EP, so will miss writes
// from nested ISeqs. This will need to be amended when we add codegen for
// Send.
let val = state.getlocal(ep_offset);
state.stack_push(val);
}
}
YARVINSN_setlocal_WC_0 => {
// TODO(alan): This implementation doesn't write to EP, where nested scopes
@ -2696,23 +2701,27 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result<Function, ParseError> {
let ep_offset = get_arg(pc, 0).as_u32();
let val = state.stack_pop()?;
state.setlocal(ep_offset, val);
if iseq_type == ISEQ_TYPE_EVAL {
// On eval, the locals are always on the heap, so write the local using EP.
fun.push_insn(block, Insn::SetLocal { val, ep_offset, level: 0 });
}
}
YARVINSN_getlocal_WC_1 => {
let ep_offset = get_arg(pc, 0).as_u32();
state.stack_push(fun.push_insn(block, Insn::GetLocal { ep_offset, level: NonZeroU32::new(1).unwrap() }));
state.stack_push(fun.push_insn(block, Insn::GetLocal { ep_offset, level: 1 }));
}
YARVINSN_setlocal_WC_1 => {
let ep_offset = get_arg(pc, 0).as_u32();
fun.push_insn(block, Insn::SetLocal { val: state.stack_pop()?, ep_offset, level: NonZeroU32::new(1).unwrap() });
fun.push_insn(block, Insn::SetLocal { val: state.stack_pop()?, ep_offset, level: 1 });
}
YARVINSN_getlocal => {
let ep_offset = get_arg(pc, 0).as_u32();
let level = NonZeroU32::try_from(get_arg(pc, 1).as_u32()).map_err(|_| ParseError::MalformedIseq(insn_idx))?;
let level = get_arg(pc, 1).as_u32();
state.stack_push(fun.push_insn(block, Insn::GetLocal { ep_offset, level }));
}
YARVINSN_setlocal => {
let ep_offset = get_arg(pc, 0).as_u32();
let level = NonZeroU32::try_from(get_arg(pc, 1).as_u32()).map_err(|_| ParseError::MalformedIseq(insn_idx))?;
let level = get_arg(pc, 1).as_u32();
fun.push_insn(block, Insn::SetLocal { val: state.stack_pop()?, ep_offset, level });
}
YARVINSN_pop => { state.stack_pop()?; }