Skip to content

Commit 7e028b8

Browse files
tekknolagitekknolagi
authored
ZJIT: Pull out GetEP from IsBlockParamModified (ruby#16234)
We can hopefully de-duplicate this GetEP soon.
1 parent 56bdb93 commit 7e028b8

4 files changed

Lines changed: 56 additions & 50 deletions

File tree

‎zjit/src/codegen.rs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -571,7 +571,7 @@ fn gen_insn(cb: &mut CodeBlock, jit: &mut JITState, asm: &mut Assembler, functio
571571
Insn::SetGlobal { id, val, state } => no_output!(gen_setglobal(jit, asm, *id, opnd!(val), &function.frame_state(*state))),
572572
Insn::GetGlobal { id, state } => gen_getglobal(jit, asm, *id, &function.frame_state(*state)),
573573
&Insn::GetLocal { ep_offset, level, use_sp, .. } => gen_getlocal(asm, ep_offset, level, use_sp),
574-
&Insn::IsBlockParamModified { level } => gen_is_block_param_modified(asm, level),
574+
&Insn::IsBlockParamModified { ep } => gen_is_block_param_modified(asm, opnd!(ep)),
575575
&Insn::GetBlockParam { ep_offset, level, state } => gen_getblockparam(jit, asm, ep_offset, level, &function.frame_state(state)),
576576
&Insn::SetLocal { val, ep_offset, level } => no_output!(gen_setlocal(asm, opnd!(val), function.type_of(val), ep_offset, level)),
577577
Insn::GetConstant { klass, id, allow_nil, state } => gen_getconstant(jit, asm, opnd!(klass), *id, opnd!(allow_nil), &function.frame_state(*state)),
@@ -766,8 +766,7 @@ fn gen_setlocal(asm: &mut Assembler, val: Opnd, val_type: Type, local_ep_offset:
766766
}
767767

768768
/// Returns 1 (as CBool) when VM_FRAME_FLAG_MODIFIED_BLOCK_PARAM is set; returns 0 otherwise.
769-
fn gen_is_block_param_modified(asm: &mut Assembler, level: u32) -> Opnd {
770-
let ep = gen_get_ep(asm, level);
769+
fn gen_is_block_param_modified(asm: &mut Assembler, ep: Opnd) -> Opnd {
771770
let flags = asm.load(Opnd::mem(VALUE_BITS, ep, SIZEOF_VALUE_I32 * (VM_ENV_DATA_INDEX_FLAGS as i32)));
772771
asm.test(flags, VM_FRAME_FLAG_MODIFIED_BLOCK_PARAM.into());
773772
asm.csel_nz(Opnd::Imm(1), Opnd::Imm(0))

‎zjit/src/hir.rs‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -858,7 +858,7 @@ pub enum Insn {
858858
GetLocal { level: u32, ep_offset: u32, use_sp: bool, rest_param: bool },
859859
/// Check whether VM_FRAME_FLAG_MODIFIED_BLOCK_PARAM is set in the environment flags.
860860
/// Returns CBool (0/1).
861-
IsBlockParamModified { level: u32 },
861+
IsBlockParamModified { ep: InsnId },
862862
/// Get the block parameter as a Proc.
863863
GetBlockParam { level: u32, ep_offset: u32, state: InsnId },
864864
/// Set a local variable in a higher scope or the heap
@@ -1658,8 +1658,8 @@ impl<'a> std::fmt::Display for InsnPrinter<'a> {
16581658
let name = get_local_var_name_for_printer(self.iseq, level, ep_offset).map_or(String::new(), |x| format!("{x}, "));
16591659
write!(f, "GetLocal {name}l{level}, EP@{ep_offset}{}", if rest_param { ", *" } else { "" })
16601660
},
1661-
&Insn::IsBlockParamModified { level } => {
1662-
write!(f, "IsBlockParamModified l{level}")
1661+
&Insn::IsBlockParamModified { ep } => {
1662+
write!(f, "IsBlockParamModified {ep}")
16631663
},
16641664
&Insn::SetLocal { val, level, ep_offset } => {
16651665
let name = get_local_var_name_for_printer(self.iseq, level, ep_offset).map_or(String::new(), |x| format!("{x}, "));
@@ -2225,7 +2225,6 @@ impl Function {
22252225
| PutSpecialObject {..}
22262226
| GetGlobal {..}
22272227
| GetLocal {..}
2228-
| IsBlockParamModified {..}
22292228
| SideExit {..}
22302229
| EntryPoint {..}
22312230
| LoadPC
@@ -2278,6 +2277,7 @@ impl Function {
22782277
&GuardGreaterEq { left, right, reason, state } => GuardGreaterEq { left: find!(left), right: find!(right), reason, state },
22792278
&GuardLess { left, right, state } => GuardLess { left: find!(left), right: find!(right), state },
22802279
&IsBlockGiven { lep } => IsBlockGiven { lep: find!(lep) },
2280+
&IsBlockParamModified { ep } => IsBlockParamModified { ep: find!(ep) },
22812281
&GetBlockParam { level, ep_offset, state } => GetBlockParam { level, ep_offset, state: find!(state) },
22822282
&FixnumAdd { left, right, state } => FixnumAdd { left: find!(left), right: find!(right), state },
22832283
&FixnumSub { left, right, state } => FixnumSub { left: find!(left), right: find!(right), state },
@@ -4686,14 +4686,16 @@ impl Function {
46864686
| &Insn::GetLEP
46874687
| &Insn::LoadSelf
46884688
| &Insn::GetLocal { .. }
4689-
| &Insn::IsBlockParamModified { .. }
46904689
| &Insn::PutSpecialObject { .. }
46914690
| &Insn::IncrCounter(_)
46924691
| &Insn::IncrCounterPtr { .. } =>
46934692
{}
46944693
| &Insn::IsBlockGiven { lep } => {
46954694
worklist.push_back(lep);
46964695
}
4696+
&Insn::IsBlockParamModified { ep } => {
4697+
worklist.push_back(ep);
4698+
}
46974699
&Insn::PatchPoint { state, .. }
46984700
| &Insn::CheckInterrupts { state }
46994701
| &Insn::GetBlockParam { state, .. }
@@ -5544,6 +5546,7 @@ impl Function {
55445546
| Insn::LoadField { .. }
55455547
| Insn::GetConstantPath { .. }
55465548
| Insn::IsBlockGiven { .. }
5549+
| Insn::IsBlockParamModified { .. }
55475550
| Insn::GetGlobal { .. }
55485551
| Insn::LoadPC
55495552
| Insn::LoadEC
@@ -5564,7 +5567,6 @@ impl Function {
55645567
| Insn::GetSpecialSymbol { .. }
55655568
| Insn::GetLocal { .. }
55665569
| Insn::GetBlockParam { .. }
5567-
| Insn::IsBlockParamModified { .. }
55685570
| Insn::StoreField { .. } => {
55695571
Ok(())
55705572
}
@@ -6995,7 +6997,8 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result<Function, ParseError> {
69956997

69966998
// If the block param is already a Proc (modified), read it from EP.
69976999
// Otherwise, convert it to a Proc and store it to EP.
6998-
let is_modified = fun.push_insn(block, Insn::IsBlockParamModified { level });
7000+
let ep = fun.push_insn(block, Insn::GetEP { level });
7001+
let is_modified = fun.push_insn(block, Insn::IsBlockParamModified { ep });
69997002

70007003
let locals_count = state.locals.len();
70017004
let stack_count = state.stack.len();

‎zjit/src/hir/opt_tests.rs‎

Lines changed: 21 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -4476,16 +4476,17 @@ mod hir_opt_tests {
44764476
v6:BasicObject = LoadArg :block@1
44774477
Jump bb3(v5, v6)
44784478
bb3(v8:BasicObject, v9:BasicObject):
4479-
v13:CBool = IsBlockParamModified l0
4480-
IfTrue v13, bb4(v8, v9)
4481-
v24:BasicObject = GetBlockParam :block, l0, EP@3
4482-
Jump bb6(v8, v24, v24)
4483-
bb4(v14:BasicObject, v15:BasicObject):
4484-
v22:BasicObject = GetLocal :block, l0, EP@3
4485-
Jump bb6(v14, v22, v22)
4486-
bb6(v26:BasicObject, v27:BasicObject, v28:BasicObject):
4479+
v13:CPtr = GetEP 0
4480+
v14:CBool = IsBlockParamModified v13
4481+
IfTrue v14, bb4(v8, v9)
4482+
v25:BasicObject = GetBlockParam :block, l0, EP@3
4483+
Jump bb6(v8, v25, v25)
4484+
bb4(v15:BasicObject, v16:BasicObject):
4485+
v23:BasicObject = GetLocal :block, l0, EP@3
4486+
Jump bb6(v15, v23, v23)
4487+
bb6(v27:BasicObject, v28:BasicObject, v29:BasicObject):
44874488
CheckInterrupts
4488-
Return v28
4489+
Return v29
44894490
");
44904491
}
44914492

@@ -4509,17 +4510,18 @@ mod hir_opt_tests {
45094510
v4:BasicObject = LoadArg :self@0
45104511
Jump bb3(v4)
45114512
bb3(v6:BasicObject):
4512-
v10:CBool = IsBlockParamModified l1
4513-
IfTrue v10, bb4(v6)
4514-
v20:BasicObject = GetBlockParam :block, l1, EP@3
4515-
Jump bb6(v6, v20)
4516-
bb4(v11:BasicObject):
4517-
v17:CPtr = GetEP 1
4518-
v18:BasicObject = LoadField v17, :block@0x1000
4519-
Jump bb6(v11, v18)
4520-
bb6(v22:BasicObject, v23:BasicObject):
4513+
v10:CPtr = GetEP 1
4514+
v11:CBool = IsBlockParamModified v10
4515+
IfTrue v11, bb4(v6)
4516+
v21:BasicObject = GetBlockParam :block, l1, EP@3
4517+
Jump bb6(v6, v21)
4518+
bb4(v12:BasicObject):
4519+
v18:CPtr = GetEP 1
4520+
v19:BasicObject = LoadField v18, :block@0x1000
4521+
Jump bb6(v12, v19)
4522+
bb6(v23:BasicObject, v24:BasicObject):
45214523
CheckInterrupts
4522-
Return v23
4524+
Return v24
45234525
");
45244526
}
45254527

‎zjit/src/hir/tests.rs‎

Lines changed: 23 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -3089,18 +3089,19 @@ pub mod hir_build_tests {
30893089
v6:BasicObject = LoadArg :block@1
30903090
Jump bb3(v5, v6)
30913091
bb3(v8:BasicObject, v9:BasicObject):
3092-
v13:CBool = IsBlockParamModified l0
3093-
IfTrue v13, bb4(v8, v9)
3092+
v13:CPtr = GetEP 0
3093+
v14:CBool = IsBlockParamModified v13
3094+
IfTrue v14, bb4(v8, v9)
30943095
Jump bb5(v8, v9)
3095-
bb4(v14:BasicObject, v15:BasicObject):
3096-
v22:BasicObject = GetLocal :block, l0, EP@3
3097-
Jump bb6(v14, v22, v22)
3098-
bb5(v17:BasicObject, v18:BasicObject):
3099-
v24:BasicObject = GetBlockParam :block, l0, EP@3
3100-
Jump bb6(v17, v24, v24)
3101-
bb6(v26:BasicObject, v27:BasicObject, v28:BasicObject):
3096+
bb4(v15:BasicObject, v16:BasicObject):
3097+
v23:BasicObject = GetLocal :block, l0, EP@3
3098+
Jump bb6(v15, v23, v23)
3099+
bb5(v18:BasicObject, v19:BasicObject):
3100+
v25:BasicObject = GetBlockParam :block, l0, EP@3
3101+
Jump bb6(v18, v25, v25)
3102+
bb6(v27:BasicObject, v28:BasicObject, v29:BasicObject):
31023103
CheckInterrupts
3103-
Return v28
3104+
Return v29
31043105
");
31053106
}
31063107

@@ -3124,19 +3125,20 @@ pub mod hir_build_tests {
31243125
v4:BasicObject = LoadArg :self@0
31253126
Jump bb3(v4)
31263127
bb3(v6:BasicObject):
3127-
v10:CBool = IsBlockParamModified l1
3128-
IfTrue v10, bb4(v6)
3128+
v10:CPtr = GetEP 1
3129+
v11:CBool = IsBlockParamModified v10
3130+
IfTrue v11, bb4(v6)
31293131
Jump bb5(v6)
3130-
bb4(v11:BasicObject):
3131-
v17:CPtr = GetEP 1
3132-
v18:BasicObject = LoadField v17, :block@0x1000
3133-
Jump bb6(v11, v18)
3134-
bb5(v13:BasicObject):
3135-
v20:BasicObject = GetBlockParam :block, l1, EP@3
3136-
Jump bb6(v13, v20)
3137-
bb6(v22:BasicObject, v23:BasicObject):
3132+
bb4(v12:BasicObject):
3133+
v18:CPtr = GetEP 1
3134+
v19:BasicObject = LoadField v18, :block@0x1000
3135+
Jump bb6(v12, v19)
3136+
bb5(v14:BasicObject):
3137+
v21:BasicObject = GetBlockParam :block, l1, EP@3
3138+
Jump bb6(v14, v21)
3139+
bb6(v23:BasicObject, v24:BasicObject):
31383140
CheckInterrupts
3139-
Return v23
3141+
Return v24
31403142
");
31413143
}
31423144

0 commit comments

Comments
 (0)