Skip to content

Commit 0d61066

Browse files
tekknolagitekknolagi
committed
ZJIT: Pull load out of IsBlockParamModified
This makes IsBlockParamModified pure and the loads more understandable to load-store forwarding.
1 parent 2fda295 commit 0d61066

5 files changed

Lines changed: 278 additions & 246 deletions

File tree

‎zjit/src/codegen.rs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -731,7 +731,7 @@ fn gen_insn(cb: &mut CodeBlock, jit: &mut JITState, asm: &mut Assembler, functio
731731
Insn::GetIvar { self_val, id, ic, state: _ } => gen_getivar(jit, asm, opnd!(self_val), *id, *ic),
732732
Insn::SetGlobal { id, val, state } => no_output!(gen_setglobal(jit, asm, *id, opnd!(val), &function.frame_state(*state))),
733733
Insn::GetGlobal { id, state } => gen_getglobal(jit, asm, *id, &function.frame_state(*state)),
734-
&Insn::IsBlockParamModified { ep } => gen_is_block_param_modified(asm, opnd!(ep)),
734+
&Insn::IsBlockParamModified { flags } => gen_is_block_param_modified(asm, opnd!(flags)),
735735
&Insn::GetBlockParam { ep_offset, level, state } => gen_getblockparam(jit, asm, ep_offset, level, &function.frame_state(state)),
736736
&Insn::SetLocal { val, ep_offset, level } => no_output!(gen_setlocal(asm, opnd!(val), function.type_of(val), ep_offset, level)),
737737
Insn::GetConstant { klass, id, allow_nil, state } => gen_getconstant(jit, asm, opnd!(klass), *id, opnd!(allow_nil), &function.frame_state(*state)),
@@ -896,8 +896,7 @@ fn gen_setlocal(asm: &mut Assembler, val: Opnd, val_type: Type, local_ep_offset:
896896
}
897897

898898
/// Returns 1 (as CBool) when VM_FRAME_FLAG_MODIFIED_BLOCK_PARAM is set; returns 0 otherwise.
899-
fn gen_is_block_param_modified(asm: &mut Assembler, ep: Opnd) -> Opnd {
900-
let flags = asm.load(Opnd::mem(VALUE_BITS, ep, SIZEOF_VALUE_I32 * (VM_ENV_DATA_INDEX_FLAGS as i32)));
899+
fn gen_is_block_param_modified(asm: &mut Assembler, flags: Opnd) -> Opnd {
901900
asm.test(flags, VM_FRAME_FLAG_MODIFIED_BLOCK_PARAM.into());
902901
asm.csel_nz(Opnd::Imm(1), Opnd::Imm(0))
903902
}

‎zjit/src/cruby.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1564,6 +1564,7 @@ pub(crate) mod ids {
15641564
name: _env_data_index_specval
15651565
name: _ep_method_entry
15661566
name: _ep_specval
1567+
name: _ep_flags
15671568
name: _rbasic_flags
15681569
name: RUBY_FL_FREEZE
15691570
name: RUBY_ELTS_SHARED

‎zjit/src/hir.rs‎

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -923,9 +923,9 @@ pub enum Insn {
923923
StoreField { recv: InsnId, id: ID, offset: i32, val: InsnId },
924924
WriteBarrier { recv: InsnId, val: InsnId },
925925

926-
/// Check whether VM_FRAME_FLAG_MODIFIED_BLOCK_PARAM is set in the environment flags.
926+
/// Check whether VM_FRAME_FLAG_MODIFIED_BLOCK_PARAM is set in the (already loaded) environment flags.
927927
/// Returns CBool (0/1).
928-
IsBlockParamModified { ep: InsnId },
928+
IsBlockParamModified { flags: InsnId },
929929
/// Get the block parameter as a Proc.
930930
GetBlockParam { level: u32, ep_offset: u32, state: InsnId },
931931
/// Set a local variable in a higher scope or the heap
@@ -1160,8 +1160,8 @@ macro_rules! for_each_operand_impl {
11601160
Insn::IsBlockGiven { lep } => {
11611161
$visit_one!(lep);
11621162
}
1163-
Insn::IsBlockParamModified { ep } => {
1164-
$visit_one!(ep);
1163+
Insn::IsBlockParamModified { flags } => {
1164+
$visit_one!(flags);
11651165
}
11661166
Insn::CheckMatch { target, pattern, state, .. } => {
11671167
$visit_one!(target);
@@ -1586,7 +1586,7 @@ impl Insn {
15861586
Insn::GetSpecialNumber { .. } => effects::Any,
15871587
Insn::GetClassVar { .. } => effects::Any,
15881588
Insn::SetClassVar { .. } => effects::Any,
1589-
Insn::IsBlockParamModified { .. } => effects::Any,
1589+
Insn::IsBlockParamModified { .. } => effects::Empty,
15901590
Insn::GetBlockParam { .. } => effects::Any,
15911591
Insn::Snapshot { .. } => effects::Empty,
15921592
Insn::Jump(_) => effects::Any,
@@ -2139,8 +2139,8 @@ impl<'a> std::fmt::Display for InsnPrinter<'a> {
21392139
Insn::SetIvar { self_val, id, val, .. } => write!(f, "SetIvar {self_val}, :{}, {val}", id.contents_lossy()),
21402140
Insn::GetGlobal { id, .. } => write!(f, "GetGlobal :{}", id.contents_lossy()),
21412141
Insn::SetGlobal { id, val, .. } => write!(f, "SetGlobal :{}, {val}", id.contents_lossy()),
2142-
&Insn::IsBlockParamModified { ep } => {
2143-
write!(f, "IsBlockParamModified {ep}")
2142+
&Insn::IsBlockParamModified { flags } => {
2143+
write!(f, "IsBlockParamModified {flags}")
21442144
},
21452145
&Insn::SetLocal { val, level, ep_offset } => {
21462146
let name = get_local_var_name_for_printer(self.iseq, level, ep_offset).map_or(String::new(), |x| format!("{x}, "));
@@ -2831,7 +2831,7 @@ impl Function {
28312831
&GuardGreaterEq { left, right, reason, state } => GuardGreaterEq { left: find!(left), right: find!(right), reason, state },
28322832
&GuardLess { left, right, state } => GuardLess { left: find!(left), right: find!(right), state },
28332833
&IsBlockGiven { lep } => IsBlockGiven { lep: find!(lep) },
2834-
&IsBlockParamModified { ep } => IsBlockParamModified { ep: find!(ep) },
2834+
&IsBlockParamModified { flags } => IsBlockParamModified { flags: find!(flags) },
28352835
&GetBlockParam { level, ep_offset, state } => GetBlockParam { level, ep_offset, state: find!(state) },
28362836
&FixnumAdd { left, right, state } => FixnumAdd { left: find!(left), right: find!(right), state },
28372837
&FixnumSub { left, right, state } => FixnumSub { left: find!(left), right: find!(right), state },
@@ -3618,6 +3618,10 @@ impl Function {
36183618
self.push_insn(block, Insn::LoadField { recv, id: ID!(_rbasic_flags), offset: RUBY_OFFSET_RBASIC_FLAGS, return_type: types::CUInt64 })
36193619
}
36203620

3621+
fn load_ep_flags(&mut self, block: BlockId, ep: InsnId) -> InsnId {
3622+
self.push_insn(block, Insn::LoadField { recv: ep, id: ID!(_ep_flags), offset: SIZEOF_VALUE_I32 * (VM_ENV_DATA_INDEX_FLAGS as i32), return_type: types::CUInt64 })
3623+
}
3624+
36213625
pub fn guard_not_frozen(&mut self, block: BlockId, recv: InsnId, state: InsnId) {
36223626
let flags = self.load_rbasic_flags(block, recv);
36233627
self.push_insn(block, Insn::GuardNoBitsSet { val: flags, mask: Const::CUInt64(RUBY_FL_FREEZE as u64), mask_name: Some(ID!(RUBY_FL_FREEZE)), reason: SideExitReason::GuardNotFrozen, state });
@@ -6165,7 +6169,6 @@ impl Function {
61656169
| Insn::LoadField { .. }
61666170
| Insn::GetConstantPath { .. }
61676171
| Insn::IsBlockGiven { .. }
6168-
| Insn::IsBlockParamModified { .. }
61696172
| Insn::GetGlobal { .. }
61706173
| Insn::LoadPC
61716174
| Insn::LoadSP
@@ -6454,6 +6457,7 @@ impl Function {
64546457
}
64556458
Insn::RefineType { .. } => Ok(()),
64566459
Insn::HasType { val, .. } => self.assert_subtype(insn_id, val, types::BasicObject),
6460+
Insn::IsBlockParamModified { flags } => self.assert_subtype(insn_id, flags, types::CUInt64),
64576461
}
64586462
}
64596463

@@ -7596,7 +7600,8 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result<Function, ParseError> {
75967600
let join_local = if level == 0 { Some(fun.push_insn(join_block, Insn::Param)) } else { None };
75977601

75987602
let ep = fun.push_insn(block, Insn::GetEP { level });
7599-
let is_modified = fun.push_insn(block, Insn::IsBlockParamModified { ep });
7603+
let flags = fun.load_ep_flags(block, ep);
7604+
let is_modified = fun.push_insn(block, Insn::IsBlockParamModified { flags });
76007605

76017606
fun.push_insn(block, Insn::IfTrue { val: is_modified, target: BranchEdge { target: modified_block, args: vec![] }});
76027607
fun.push_insn(block, Insn::Jump(BranchEdge { target: unmodified_block, args: vec![] }));
@@ -7659,7 +7664,8 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result<Function, ParseError> {
76597664
let join_param = fun.push_insn(join_block, Insn::Param);
76607665

76617666
let ep = fun.push_insn(block, Insn::GetEP { level });
7662-
let is_modified = fun.push_insn(block, Insn::IsBlockParamModified { ep });
7667+
let flags = fun.load_ep_flags(block, ep);
7668+
let is_modified = fun.push_insn(block, Insn::IsBlockParamModified { flags });
76637669

76647670
fun.push_insn(block, Insn::IfTrue {
76657671
val: is_modified,

0 commit comments

Comments
 (0)