Skip to content

Commit d7c280d

Browse files
nirvdrumnirvdrum
committed
ZJIT: Simplify invokesuper specialization to most common case
Looking at ruby-bench, most `super` calls don't pass a block, which means we can use the already optimized `SendWithoutBlockDirect`.
1 parent e227bd5 commit d7c280d

3 files changed

Lines changed: 87 additions & 93 deletions

File tree

‎zjit/src/codegen.rs‎

Lines changed: 25 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -403,8 +403,6 @@ fn gen_insn(cb: &mut CodeBlock, jit: &mut JITState, asm: &mut Assembler, functio
403403
gen_send_without_block(jit, asm, *cd, &function.frame_state(*state), SendFallbackReason::TooManyArgsForLir),
404404
Insn::SendWithoutBlockDirect { cme, iseq, recv, args, state, .. } => gen_send_iseq_direct(cb, jit, asm, *cme, *iseq, opnd!(recv), opnds!(args), &function.frame_state(*state), None),
405405
&Insn::InvokeSuper { cd, blockiseq, state, reason, .. } => gen_invokesuper(jit, asm, cd, blockiseq, &function.frame_state(state), reason),
406-
Insn::InvokeSuperDirect { recv, current_cme, super_cme, args, state, .. } =>
407-
gen_invokesuper_direct(cb, jit, asm, *current_cme, *super_cme, opnd!(recv), opnds!(args), &function.frame_state(*state)),
408406
&Insn::InvokeBlock { cd, state, reason, .. } => gen_invokeblock(jit, asm, cd, &function.frame_state(state), reason),
409407
// Ensure we have enough room fit ec, self, and arguments
410408
// TODO remove this check when we have stack args (we can use Time.new to test it)
@@ -453,6 +451,8 @@ fn gen_insn(cb: &mut CodeBlock, jit: &mut JITState, asm: &mut Assembler, functio
453451
Insn::GuardNotFrozen { recv, state } => gen_guard_not_frozen(jit, asm, opnd!(recv), &function.frame_state(*state)),
454452
&Insn::GuardLess { left, right, state } => gen_guard_less(jit, asm, opnd!(left), opnd!(right), &function.frame_state(state)),
455453
&Insn::GuardGreaterEq { left, right, state } => gen_guard_greater_eq(jit, asm, opnd!(left), opnd!(right), &function.frame_state(state)),
454+
&Insn::GuardSuperMethodEntry { cme, state } => no_output!(gen_guard_super_method_entry(jit, asm, cme, &function.frame_state(state))),
455+
Insn::GetBlockHandler => gen_get_block_handler(jit, asm),
456456
Insn::PatchPoint { invariant, state } => no_output!(gen_patch_point(jit, asm, invariant, &function.frame_state(*state))),
457457
Insn::CCall { cfunc, recv, args, name, return_type: _, elidable: _ } => gen_ccall(asm, *cfunc, *name, opnd!(recv), opnds!(args)),
458458
// Give up CCallWithFrame for 7+ args since asm.ccall() supports at most 6 args (recv + args).
@@ -706,6 +706,29 @@ fn gen_guard_greater_eq(jit: &JITState, asm: &mut Assembler, left: Opnd, right:
706706
left
707707
}
708708

709+
/// Guard that the method entry at ep[VM_ENV_DATA_INDEX_ME_CREF] matches the expected CME.
710+
/// This ensures we're calling super from the expected method context.
711+
fn gen_guard_super_method_entry(
712+
jit: &JITState,
713+
asm: &mut Assembler,
714+
cme: *const rb_callable_method_entry_t,
715+
state: &FrameState,
716+
) {
717+
asm_comment!(asm, "guard super method entry");
718+
let lep = gen_get_lep(jit, asm);
719+
let ep_me_opnd = Opnd::mem(64, lep, SIZEOF_VALUE_I32 * VM_ENV_DATA_INDEX_ME_CREF);
720+
let ep_me = asm.load(ep_me_opnd);
721+
asm.cmp(ep_me, Opnd::UImm(cme as u64));
722+
asm.jne(side_exit(jit, state, SideExitReason::GuardSuperMethodEntry));
723+
}
724+
725+
/// Get the block handler from ep[VM_ENV_DATA_INDEX_SPECVAL] at the local EP (LEP).
726+
fn gen_get_block_handler(jit: &JITState, asm: &mut Assembler) -> Opnd {
727+
asm_comment!(asm, "get block handler from LEP");
728+
let lep = gen_get_lep(jit, asm);
729+
asm.load(Opnd::mem(64, lep, SIZEOF_VALUE_I32 * VM_ENV_DATA_INDEX_SPECVAL))
730+
}
731+
709732
fn gen_get_constant_path(jit: &JITState, asm: &mut Assembler, ic: *const iseq_inline_constant_cache, state: &FrameState) -> Opnd {
710733
unsafe extern "C" {
711734
fn rb_vm_opt_getconstant_path(ec: EcPtr, cfp: CfpPtr, ic: *const iseq_inline_constant_cache) -> VALUE;
@@ -1486,37 +1509,6 @@ fn gen_invokesuper(
14861509
)
14871510
}
14881511

1489-
/// Compile an optimized super call to an ISEQ method
1490-
fn gen_invokesuper_direct(
1491-
cb: &mut CodeBlock,
1492-
jit: &mut JITState,
1493-
asm: &mut Assembler,
1494-
current_cme: *const rb_callable_method_entry_t,
1495-
super_cme: *const rb_callable_method_entry_t,
1496-
recv: lir::Opnd,
1497-
args: Vec<lir::Opnd>,
1498-
state: &FrameState,
1499-
) -> lir::Opnd {
1500-
// Guard that ep[VM_ENV_DATA_INDEX_ME_CREF] matches current_cme, ensuring that we're calling
1501-
// `super` from the expected method context.
1502-
asm_comment!(asm, "guard super method entry");
1503-
let lep = gen_get_lep(jit, asm);
1504-
let ep_me_opnd = Opnd::mem(64, lep, SIZEOF_VALUE_I32 * VM_ENV_DATA_INDEX_ME_CREF);
1505-
let ep_me = asm.load(ep_me_opnd);
1506-
asm.cmp(ep_me, Opnd::UImm(current_cme as u64));
1507-
asm.jne(side_exit(jit, state, SideExitReason::GuardSuperMethodEntry));
1508-
1509-
// Get the super method's ISEQ since we'll be dispatching to that.
1510-
let super_iseq = unsafe { get_def_iseq_ptr((*super_cme).def) };
1511-
1512-
// Read the block handler from the LEP to forward the caller's block.
1513-
asm_comment!(asm, "load block handler from LEP");
1514-
let block_handler = asm.load(Opnd::mem(64, lep, SIZEOF_VALUE_I32 * VM_ENV_DATA_INDEX_SPECVAL));
1515-
1516-
// Generate the code to invoke the `super` target method.
1517-
gen_send_iseq_direct(cb, jit, asm, super_cme, super_iseq, recv, args, state, Some(block_handler))
1518-
}
1519-
15201512
/// Compile a string resurrection
15211513
fn gen_string_copy(asm: &mut Assembler, recv: Opnd, chilled: bool, state: &FrameState) -> Opnd {
15221514
// TODO: split rb_ec_str_resurrect into separate functions

‎zjit/src/hir.rs‎

Lines changed: 40 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -892,17 +892,6 @@ pub enum Insn {
892892
state: InsnId,
893893
reason: SendFallbackReason,
894894
},
895-
/// Optimized super call to an ISEQ method
896-
InvokeSuperDirect {
897-
recv: InsnId,
898-
cd: *const rb_call_data,
899-
/// The CME of the method containing the super call (for runtime guard)
900-
current_cme: *const rb_callable_method_entry_t,
901-
/// The resolved super method's CME (for PatchPoint and to get target ISEQ)
902-
super_cme: *const rb_callable_method_entry_t,
903-
args: Vec<InsnId>,
904-
state: InsnId,
905-
},
906895
InvokeBlock {
907896
cd: *const rb_call_data,
908897
args: Vec<InsnId>,
@@ -976,6 +965,11 @@ pub enum Insn {
976965
GuardGreaterEq { left: InsnId, right: InsnId, state: InsnId },
977966
/// Side-exit if left is not less than right (both operands are C long).
978967
GuardLess { left: InsnId, right: InsnId, state: InsnId },
968+
/// Side-exit if the method entry at ep[VM_ENV_DATA_INDEX_ME_CREF] doesn't match the expected CME.
969+
/// Used to ensure super calls are made from the expected method context.
970+
GuardSuperMethodEntry { cme: *const rb_callable_method_entry_t, state: InsnId },
971+
/// Get the block handler from ep[VM_ENV_DATA_INDEX_SPECVAL] at the local EP (LEP).
972+
GetBlockHandler,
979973

980974
/// Generate no code (or padding if necessary) and insert a patch point
981975
/// that can be rewritten to a side exit when the Invariant is broken.
@@ -1004,7 +998,8 @@ impl Insn {
1004998
| Insn::PatchPoint { .. } | Insn::SetIvar { .. } | Insn::SetClassVar { .. } | Insn::ArrayExtend { .. }
1005999
| Insn::ArrayPush { .. } | Insn::SideExit { .. } | Insn::SetGlobal { .. }
10061000
| Insn::SetLocal { .. } | Insn::Throw { .. } | Insn::IncrCounter(_) | Insn::IncrCounterPtr { .. }
1007-
| Insn::CheckInterrupts { .. } | Insn::GuardBlockParamProxy { .. } | Insn::StoreField { .. } | Insn::WriteBarrier { .. }
1001+
| Insn::CheckInterrupts { .. } | Insn::GuardBlockParamProxy { .. } | Insn::GuardSuperMethodEntry { .. }
1002+
| Insn::StoreField { .. } | Insn::WriteBarrier { .. }
10081003
| Insn::HashAset { .. } => false,
10091004
_ => true,
10101005
}
@@ -1300,13 +1295,6 @@ impl<'a> std::fmt::Display for InsnPrinter<'a> {
13001295
write!(f, " # SendFallbackReason: {reason}")?;
13011296
Ok(())
13021297
}
1303-
Insn::InvokeSuperDirect { recv, args, .. } => {
1304-
write!(f, "InvokeSuperDirect {recv}")?;
1305-
for arg in args {
1306-
write!(f, ", {arg}")?;
1307-
}
1308-
Ok(())
1309-
}
13101298
Insn::InvokeBlock { args, reason, .. } => {
13111299
write!(f, "InvokeBlock")?;
13121300
for arg in args {
@@ -1353,6 +1341,8 @@ impl<'a> std::fmt::Display for InsnPrinter<'a> {
13531341
Insn::GuardNotFrozen { recv, .. } => write!(f, "GuardNotFrozen {recv}"),
13541342
Insn::GuardLess { left, right, .. } => write!(f, "GuardLess {left}, {right}"),
13551343
Insn::GuardGreaterEq { left, right, .. } => write!(f, "GuardGreaterEq {left}, {right}"),
1344+
Insn::GuardSuperMethodEntry { cme, .. } => write!(f, "GuardSuperMethodEntry {:p}", self.ptr_map.map_ptr(cme)),
1345+
Insn::GetBlockHandler => write!(f, "GetBlockHandler"),
13561346
Insn::PatchPoint { invariant, .. } => { write!(f, "PatchPoint {}", invariant.print(self.ptr_map)) },
13571347
Insn::GetConstantPath { ic, .. } => { write!(f, "GetConstantPath {:p}", self.ptr_map.map_ptr(ic)) },
13581348
Insn::IsBlockGiven => { write!(f, "IsBlockGiven") },
@@ -1989,6 +1979,8 @@ impl Function {
19891979
&GuardNotFrozen { recv, state } => GuardNotFrozen { recv: find!(recv), state },
19901980
&GuardGreaterEq { left, right, state } => GuardGreaterEq { left: find!(left), right: find!(right), state },
19911981
&GuardLess { left, right, state } => GuardLess { left: find!(left), right: find!(right), state },
1982+
&GuardSuperMethodEntry { cme, state } => GuardSuperMethodEntry { cme, state },
1983+
&GetBlockHandler => GetBlockHandler,
19921984
&FixnumAdd { left, right, state } => FixnumAdd { left: find!(left), right: find!(right), state },
19931985
&FixnumSub { left, right, state } => FixnumSub { left: find!(left), right: find!(right), state },
19941986
&FixnumMult { left, right, state } => FixnumMult { left: find!(left), right: find!(right), state },
@@ -2054,14 +2046,6 @@ impl Function {
20542046
state,
20552047
reason,
20562048
},
2057-
&InvokeSuperDirect { recv, cd, current_cme, super_cme, ref args, state } => InvokeSuperDirect {
2058-
recv: find!(recv),
2059-
cd,
2060-
current_cme,
2061-
super_cme,
2062-
args: find_vec!(args),
2063-
state,
2064-
},
20652049
&InvokeBlock { cd, ref args, state, reason } => InvokeBlock {
20662050
cd,
20672051
args: find_vec!(args),
@@ -2169,8 +2153,8 @@ impl Function {
21692153
| Insn::IfTrue { .. } | Insn::IfFalse { .. } | Insn::Return { .. } | Insn::Throw { .. }
21702154
| Insn::PatchPoint { .. } | Insn::SetIvar { .. } | Insn::SetClassVar { .. } | Insn::ArrayExtend { .. }
21712155
| Insn::ArrayPush { .. } | Insn::SideExit { .. } | Insn::SetLocal { .. } | Insn::IncrCounter(_)
2172-
| Insn::CheckInterrupts { .. } | Insn::GuardBlockParamProxy { .. } | Insn::IncrCounterPtr { .. }
2173-
| Insn::StoreField { .. } | Insn::WriteBarrier { .. } | Insn::HashAset { .. } =>
2156+
| Insn::CheckInterrupts { .. } | Insn::GuardBlockParamProxy { .. } | Insn::GuardSuperMethodEntry { .. }
2157+
| Insn::IncrCounterPtr { .. } | Insn::StoreField { .. } | Insn::WriteBarrier { .. } | Insn::HashAset { .. } =>
21742158
panic!("Cannot infer type of instruction with no output: {}. See Insn::has_output().", self.insns[insn.0]),
21752159
Insn::Const { val: Const::Value(val) } => Type::from_value(*val),
21762160
Insn::Const { val: Const::CBool(val) } => Type::from_cbool(*val),
@@ -2249,7 +2233,6 @@ impl Function {
22492233
Insn::Send { .. } => types::BasicObject,
22502234
Insn::SendForward { .. } => types::BasicObject,
22512235
Insn::InvokeSuper { .. } => types::BasicObject,
2252-
Insn::InvokeSuperDirect { .. } => types::BasicObject,
22532236
Insn::InvokeBlock { .. } => types::BasicObject,
22542237
Insn::InvokeBuiltin { return_type, .. } => return_type.unwrap_or(types::BasicObject),
22552238
Insn::Defined { pushval, .. } => Type::from_value(*pushval).union(types::NilClass),
@@ -2277,6 +2260,7 @@ impl Function {
22772260
Insn::AnyToString { .. } => types::String,
22782261
Insn::GetLocal { rest_param: true, .. } => types::ArrayExact,
22792262
Insn::GetLocal { .. } => types::BasicObject,
2263+
Insn::GetBlockHandler => types::RubyValue,
22802264
// The type of Snapshot doesn't really matter; it's never materialized. It's used only
22812265
// as a reference for FrameState, which we use to generate side-exit code.
22822266
Insn::Snapshot { .. } => types::Any,
@@ -3031,7 +3015,7 @@ impl Function {
30313015
self.push_insn_id(block, insn_id); continue;
30323016
}
30333017

3034-
// Don't handle calls with complex arguments (kwarg, splat, kw_splat, blockarg, forwarding)
3018+
// Don't handle calls with complex arguments.
30353019
let ci = unsafe { get_call_data_ci(cd) };
30363020
let flags = unsafe { rb_vm_ci_flag(ci) };
30373021
if unspecializable_call_type(flags) {
@@ -3040,7 +3024,7 @@ impl Function {
30403024

30413025
let frame_state = self.frame_state(state);
30423026

3043-
// Get the profiled CME from the current method
3027+
// Get the profiled CME from the current method.
30443028
let Some(profiles) = self.profiles.as_ref() else {
30453029
self.push_insn_id(block, insn_id); continue;
30463030
};
@@ -3049,7 +3033,7 @@ impl Function {
30493033
self.push_insn_id(block, insn_id); continue;
30503034
};
30513035

3052-
// Get defined_class and method ID from the profiled CME
3036+
// Get defined_class and method ID from the profiled CME.
30533037
let current_defined_class = unsafe { (*current_cme).defined_class };
30543038
let mid = unsafe { get_def_original_id((*current_cme).def) };
30553039

@@ -3059,16 +3043,15 @@ impl Function {
30593043
self.push_insn_id(block, insn_id); continue;
30603044
}
30613045

3062-
// Look up the super method
3046+
// Look up the super method.
30633047
let super_cme = unsafe { rb_callable_method_entry(superclass, mid) };
30643048
if super_cme.is_null() {
30653049
self.push_insn_id(block, insn_id); continue;
30663050
}
30673051

3068-
// Check if it's an ISEQ method
3052+
// Check if it's an ISEQ method; bail if it isn't.
30693053
let def_type = unsafe { get_cme_def_type(super_cme) };
30703054
if def_type != VM_METHOD_TYPE_ISEQ {
3071-
// TODO: Handle CFUNCs
30723055
self.push_insn_id(block, insn_id); continue;
30733056
}
30743057

@@ -3079,8 +3062,7 @@ impl Function {
30793062
self.push_insn_id(block, insn_id); continue;
30803063
}
30813064

3082-
// Add PatchPoints for method redefinition
3083-
// TODO: Add guard that ep[-2] matches current_cme
3065+
// Add PatchPoint for method redefinition.
30843066
self.push_insn(block, Insn::PatchPoint {
30853067
invariant: Invariant::MethodRedefined {
30863068
klass: unsafe { (*super_cme).defined_class },
@@ -3090,11 +3072,23 @@ impl Function {
30903072
state
30913073
});
30923074

3093-
let send_direct = self.push_insn(block, Insn::InvokeSuperDirect {
3075+
// Guard that we're calling `super` from the expected method context.
3076+
self.push_insn(block, Insn::GuardSuperMethodEntry { cme: current_cme, state });
3077+
3078+
// Guard that no block is being passed (implicit or explicit).
3079+
let block_handler = self.push_insn(block, Insn::GetBlockHandler);
3080+
self.push_insn(block, Insn::GuardBitEquals {
3081+
val: block_handler,
3082+
expected: Const::Value(VALUE(VM_BLOCK_HANDLER_NONE as usize)),
3083+
state
3084+
});
3085+
3086+
// Use SendWithoutBlockDirect with the super method's CME and ISEQ.
3087+
let send_direct = self.push_insn(block, Insn::SendWithoutBlockDirect {
30943088
recv,
30953089
cd,
3096-
current_cme,
3097-
super_cme,
3090+
cme: super_cme,
3091+
iseq: super_iseq,
30983092
args,
30993093
state
31003094
});
@@ -3962,6 +3956,7 @@ impl Function {
39623956
| &Insn::LoadEC
39633957
| &Insn::LoadSelf
39643958
| &Insn::GetLocal { .. }
3959+
| &Insn::GetBlockHandler
39653960
| &Insn::PutSpecialObject { .. }
39663961
| &Insn::IsBlockGiven
39673962
| &Insn::IncrCounter(_)
@@ -4132,8 +4127,7 @@ impl Function {
41324127
| &Insn::CCallWithFrame { recv, ref args, state, .. }
41334128
| &Insn::SendWithoutBlockDirect { recv, ref args, state, .. }
41344129
| &Insn::InvokeBuiltin { recv, ref args, state, .. }
4135-
| &Insn::InvokeSuper { recv, ref args, state, .. }
4136-
| &Insn::InvokeSuperDirect { recv, ref args, state, .. } => {
4130+
| &Insn::InvokeSuper { recv, ref args, state, .. } => {
41374131
worklist.push_back(recv);
41384132
worklist.extend(args);
41394133
worklist.push_back(state);
@@ -4185,6 +4179,7 @@ impl Function {
41854179
worklist.push_back(val);
41864180
}
41874181
&Insn::GuardBlockParamProxy { state, .. } |
4182+
&Insn::GuardSuperMethodEntry { state, .. } |
41884183
&Insn::GetGlobal { state, .. } |
41894184
&Insn::GetSpecialSymbol { state, .. } |
41904185
&Insn::GetSpecialNumber { state, .. } |
@@ -4696,6 +4691,8 @@ impl Function {
46964691
| Insn::Jump { .. }
46974692
| Insn::EntryPoint { .. }
46984693
| Insn::GuardBlockParamProxy { .. }
4694+
| Insn::GuardSuperMethodEntry { .. }
4695+
| Insn::GetBlockHandler
46994696
| Insn::PatchPoint { .. }
47004697
| Insn::SideExit { .. }
47014698
| Insn::IncrCounter { .. }
@@ -4749,7 +4746,6 @@ impl Function {
47494746
| Insn::Send { recv, ref args, .. }
47504747
| Insn::SendForward { recv, ref args, .. }
47514748
| Insn::InvokeSuper { recv, ref args, .. }
4752-
| Insn::InvokeSuperDirect { recv, ref args, .. }
47534749
| Insn::CCallWithFrame { recv, ref args, .. }
47544750
| Insn::CCallVariadic { recv, ref args, .. }
47554751
| Insn::InvokeBuiltin { recv, ref args, .. }

0 commit comments

Comments
 (0)