From b08e20d34afb1cb99eeb97b4b7330290556511d3 Mon Sep 17 00:00:00 2001 From: Max Bernstein Date: Tue, 20 May 2025 18:41:37 -0400 Subject: [PATCH] ZJIT: Allow DCE to remove some CCalls (#13363) Allow DCE to remove some CCalls Add `elidable` field that signals that there would be no discernible effect if the call to the method were removed. The default is false. --- zjit/src/codegen.rs | 2 +- zjit/src/cruby_methods.rs | 8 ++++--- zjit/src/hir.rs | 49 +++++++++++++++++++++++++++++++++++---- 3 files changed, 50 insertions(+), 9 deletions(-) diff --git a/zjit/src/codegen.rs b/zjit/src/codegen.rs index 24dbd30e70..844ac5df42 100644 --- a/zjit/src/codegen.rs +++ b/zjit/src/codegen.rs @@ -273,7 +273,7 @@ fn gen_insn(cb: &mut CodeBlock, jit: &mut JITState, asm: &mut Assembler, functio Insn::GuardType { val, guard_type, state } => gen_guard_type(asm, opnd!(val), *guard_type, &function.frame_state(*state))?, Insn::GuardBitEquals { val, expected, state } => gen_guard_bit_equals(asm, opnd!(val), *expected, &function.frame_state(*state))?, Insn::PatchPoint(_) => return Some(()), // For now, rb_zjit_bop_redefined() panics. TODO: leave a patch point and fix rb_zjit_bop_redefined() - Insn::CCall { cfun, args, name: _, return_type: _ } => gen_ccall(jit, asm, *cfun, args)?, + Insn::CCall { cfun, args, name: _, return_type: _, elidable: _ } => gen_ccall(jit, asm, *cfun, args)?, _ => { debug!("ZJIT: gen_function: unexpected insn {:?}", insn); return None; diff --git a/zjit/src/cruby_methods.rs b/zjit/src/cruby_methods.rs index dadd2fc643..26ad349e29 100644 --- a/zjit/src/cruby_methods.rs +++ b/zjit/src/cruby_methods.rs @@ -26,6 +26,8 @@ pub struct FnProperties { pub leaf: bool, /// What Type the C function returns pub return_type: Type, + /// Whether it's legal to remove the call if the result is unused + pub elidable: bool, } impl Annotations { @@ -64,7 +66,7 @@ pub fn init() -> Annotations { macro_rules! annotate { ($module:ident, $method_name:literal, $return_type:expr, $($properties:ident),+) => { - let mut props = FnProperties { no_gc: false, leaf: false, return_type: $return_type }; + let mut props = FnProperties { no_gc: false, leaf: false, elidable: false, return_type: $return_type }; $( props.$properties = true; )+ @@ -72,9 +74,9 @@ pub fn init() -> Annotations { } } - annotate!(rb_mKernel, "itself", types::BasicObject, no_gc, leaf); + annotate!(rb_mKernel, "itself", types::BasicObject, no_gc, leaf, elidable); annotate!(rb_cString, "bytesize", types::Fixnum, no_gc, leaf); - annotate!(rb_cModule, "name", types::StringExact.union(types::NilClassExact), no_gc, leaf); + annotate!(rb_cModule, "name", types::StringExact.union(types::NilClassExact), no_gc, leaf, elidable); annotate!(rb_cModule, "===", types::BoolExact, no_gc, leaf); Annotations { diff --git a/zjit/src/hir.rs b/zjit/src/hir.rs index a005df202f..b4b0ffe33a 100644 --- a/zjit/src/hir.rs +++ b/zjit/src/hir.rs @@ -348,7 +348,7 @@ pub enum Insn { // Call a C function // `name` is for printing purposes only - CCall { cfun: *const u8, args: Vec, name: ID, return_type: Type }, + CCall { cfun: *const u8, args: Vec, name: ID, return_type: Type, elidable: bool }, // Send without block with dynamic dispatch // Ignoring keyword arguments etc for now @@ -429,6 +429,7 @@ impl Insn { Insn::FixnumLe { .. } => false, Insn::FixnumGt { .. } => false, Insn::FixnumGe { .. } => false, + Insn::CCall { elidable, .. } => !elidable, _ => true, } } @@ -510,7 +511,7 @@ impl<'a> std::fmt::Display for InsnPrinter<'a> { Insn::GuardBitEquals { val, expected, .. } => { write!(f, "GuardBitEquals {val}, {}", expected.print(self.ptr_map)) }, Insn::PatchPoint(invariant) => { write!(f, "PatchPoint {}", invariant.print(self.ptr_map)) }, Insn::GetConstantPath { ic } => { write!(f, "GetConstantPath {:p}", self.ptr_map.map_ptr(ic)) }, - Insn::CCall { cfun, args, name, return_type: _ } => { + Insn::CCall { cfun, args, name, return_type: _, elidable: _ } => { write!(f, "CCall {}@{:p}", name.contents_lossy(), self.ptr_map.map_ptr(cfun))?; for arg in args { write!(f, ", {arg}")?; @@ -850,7 +851,7 @@ impl Function { }, ArraySet { array, idx, val } => ArraySet { array: find!(*array), idx: *idx, val: find!(*val) }, ArrayDup { val , state } => ArrayDup { val: find!(*val), state: *state }, - CCall { cfun, args, name, return_type } => CCall { cfun: *cfun, args: args.iter().map(|arg| find!(*arg)).collect(), name: *name, return_type: *return_type }, + &CCall { cfun, ref args, name, return_type, elidable } => CCall { cfun: cfun, args: args.iter().map(|arg| find!(*arg)).collect(), name: name, return_type: return_type, elidable }, Defined { .. } => todo!("find(Defined)"), NewArray { elements, state } => NewArray { elements: find_vec!(*elements), state: find!(*state) }, ArrayMax { elements, state } => ArrayMax { elements: find_vec!(*elements), state: find!(*state) }, @@ -1194,7 +1195,7 @@ impl Function { // Filter for a leaf and GC free function use crate::cruby_methods::FnProperties; - let Some(FnProperties { leaf: true, no_gc: true, return_type }) = + let Some(FnProperties { leaf: true, no_gc: true, return_type, elidable }) = ZJITState::get_method_annotations().get_cfunc_properties(method) else { return Err(()); @@ -1212,7 +1213,7 @@ impl Function { let cfun = unsafe { get_mct_func(cfunc) }.cast(); let mut cfunc_args = vec![self_val]; cfunc_args.append(&mut args); - let ccall = fun.push_insn(block, Insn::CCall { cfun, args: cfunc_args, name: method_id, return_type }); + let ccall = fun.push_insn(block, Insn::CCall { cfun, args: cfunc_args, name: method_id, return_type, elidable }); fun.make_equal_to(send_insn_id, ccall); return Ok(()); } @@ -3832,6 +3833,44 @@ mod opt_tests { "#]]); } + #[test] + fn eliminate_kernel_itself() { + eval(" + def test + x = [].itself + 1 + end + "); + assert_optimized_method_hir("test", expect![[r#" + fn test: + bb0(): + PatchPoint MethodRedefined(Array@0x1000, itself@0x1008) + v6:Fixnum[1] = Const Value(1) + Return v6 + "#]]); + } + + #[test] + fn eliminate_module_name() { + eval(" + module M; end + def test + x = M.name + 1 + end + test + "); + assert_optimized_method_hir("test", expect![[r#" + fn test: + bb0(): + PatchPoint SingleRactorMode + PatchPoint StableConstantNames(0x1000, M) + PatchPoint MethodRedefined(Module@0x1008, name@0x1010) + v5:Fixnum[1] = Const Value(1) + Return v5 + "#]]); + } + #[test] fn kernel_itself_argc_mismatch() { eval("