diff --git a/.buildbot.sh b/.buildbot.sh index 2c847567a..fdfd27b2a 100644 --- a/.buildbot.sh +++ b/.buildbot.sh @@ -6,7 +6,7 @@ ROOT_DIR=$(realpath $(pwd)) # What git commit hash of yklua & ykcbf will we test in buildbot? YKLUA_REPO="https://github.com/ykjit/yklua.git" -YKLUA_COMMIT="484deee46c9a26ca32434f7fc0b992b97cbcd855" +YKLUA_COMMIT="c94e6feb51a362148951722fcc9ce231b58bb923" YKCBF_REPO="https://github.com/ykjit/ykcbf.git" YKCBF_COMMIT="431b92593180e1e376d08ecf383c4a1ab8473b3d" diff --git a/tests/c/swt_module_clone.c b/tests/c/swt_module_clone.c index 2206da48c..b289d974b 100644 --- a/tests/c/swt_module_clone.c +++ b/tests/c/swt_module_clone.c @@ -20,17 +20,17 @@ // ... // %{{dec_ptr}}: ptr = load %{{dec_addr}} // %{{i}}: i32 = load %{{_}} -// %{{_}}: i32 = icall %{{dec_ptr}}(%{{i}}) +// %{{_}}: i32 = icall %{{dec_ptr}}(%{{i}})... // ... // --- End aot --- // --- Begin jit-pre-opt --- // ... -// %{{_}}: i32 = icall %{{_}}(%{{_}}) +// %{{_}}: i32 = icall %{{_}}(%{{_}})... // ... // --- End jit-pre-opt --- // --- Begin jit-post-opt --- // ... -// %{{_}}: i32 = icall %{{_}}(%{{_}}) +// %{{_}}: i32 = icall %{{_}}(%{{_}})... // ... // --- End jit-post-opt --- // 3 diff --git a/tests/ir_lowering/unsupported_variants.ll b/tests/ir_lowering/unsupported_variants.ll index 3945eda77..e97036086 100644 --- a/tests/ir_lowering/unsupported_variants.ll +++ b/tests/ir_lowering/unsupported_variants.ll @@ -17,13 +17,13 @@ ; %{{_}}: ?ty<<4 x i32>> = unimplemented << %{{15}} = add <4 x i32> %{{44}}, %{{44}}>> ; br bb3 ; bb3: -; %{{_}}: i32 = unimplemented << %{{17}} = call i32 @f(i32 swiftself 5)>> -; %{{_}}: i32 = unimplemented << %{{18}} = call inreg i32 @f(i32 5)>> -; %{{_}}: i32 = unimplemented << %{{19}} = call i32 @f(i32 5) #{{0}}>> -; %{{_}}: float = unimplemented << %{{20}} = call nnan float @g()>> -; %{{_}}: i32 = unimplemented << %{{21}} = call ghccc i32 @f(i32 5)>> -; %{{_}}: i32 = unimplemented << %{{22}} = call i32 @f(i32 5) [ "kcfi"(i32 1234) ]>> -; %{{_}}: ptr = unimplemented << %{{23}} = call addrspace(6) ptr @p()>> +; %{{_}}: i32 = unimplemented << %{{17}} = call i32 @f(i32 swiftself 5) >> +; %{{_}}: i32 = unimplemented << %{{18}} = call inreg i32 @f(i32 5) >> +; %{{_}}: i32 = unimplemented << %{{19}} = call i32 @f(i32 5) #{{0}} >> +; %{{_}}: float = unimplemented << %{{20}} = call nnan float @g() >> +; %{{_}}: i32 = unimplemented << %{{21}} = call ghccc i32 @f(i32 5) >> +; %{{_}}: i32 = unimplemented << %{{22}} = call i32 @f(i32 5) [ "kcfi"(i32 1234) ] >> +; %{{_}}: ptr = unimplemented << %{{23}} = call addrspace(6) ptr @p() >> ; br bb4 ; bb4: ; %{{_}}: ?ty<<8 x i8>> = unimplemented << %{{26}} = ptrtoint <8 x ptr> %{{ptrs}} to <8 x i8>>> @@ -36,7 +36,7 @@ ; br bb6 ; bb6: ; %{{_}}: i32 = unimplemented << %{{_}} = load atomic i32, ptr %{{_}} acquire, align 4>> -; %{{_}}: i32 = unimplemented << %{{_}} = load i32, ptr addrspace(10) %{{_}}, align 4>> +; %{{_}}: i32 = unimplemented << %{{_}} = load i32, ptr addrspace(10) %{{_}}, align 4 >> ; %{{_}}: i32 = unimplemented << %{{_}} = load i32, ptr %{{_}}, align 2>> ; br ... ; ... @@ -45,7 +45,7 @@ ; br bb11 ; bb11: ; unimplemented << store atomic i32 0, ptr %0 release, align 4>> -; unimplemented << store i32 0, ptr addrspace(10) %5, align 4>> +; unimplemented << store i32 0, ptr addrspace(10) %5, align 4 >> ; unimplemented << store i32 0, ptr %0, align 2>> ; ret ; } diff --git a/tests/lua/inline_indirect_call.lua b/tests/lua/inline_indirect_call.lua new file mode 100644 index 000000000..9340a3e24 --- /dev/null +++ b/tests/lua/inline_indirect_call.lua @@ -0,0 +1,16 @@ +-- ignore-if: test "$YKB_TRACER" = "swt" +-- Run-time: +-- env-var: YK_HOT_THRESHOLD=3 +-- env-var: YKD_LOG=4 +-- env-var: YKD_LOG_IR=jit-pre-opt +-- env-var: YKD_SERIALISE_COMPILATION=1 + +-- FIXME: I don't think there's any output we could easily check to ensure +-- luaB_assert has been inlined? + +for i = 0, 100 do + A = {} + assert(A) -- causes an icall in the trace. +end + +io.stderr:write("exit\n") diff --git a/tests/yklua b/tests/yklua index c504ee857..c94e6feb5 160000 --- a/tests/yklua +++ b/tests/yklua @@ -1 +1 @@ -Subproject commit c504ee857e41ba2b428fb72ff586ecdde0464445 +Subproject commit c94e6feb51a362148951722fcc9ce231b58bb923 diff --git a/ykllvm b/ykllvm index a436320ca..0316ef37b 160000 --- a/ykllvm +++ b/ykllvm @@ -1 +1 @@ -Subproject commit a436320ca79e57bb9ca691f5939d8c9ff55bde14 +Subproject commit 0316ef37b86266d5bbd5a34d48e63484c83fe660 diff --git a/ykrt/src/compile/jitc_yk/aot_ir.rs b/ykrt/src/compile/jitc_yk/aot_ir.rs index 81506e3cf..58ffd0772 100644 --- a/ykrt/src/compile/jitc_yk/aot_ir.rs +++ b/ykrt/src/compile/jitc_yk/aot_ir.rs @@ -961,6 +961,7 @@ pub(crate) enum Inst { num_args: u32, #[deku(count = "num_args")] args: Vec, + safepoint: DeoptSafepoint, }, #[deku(id = "16")] Select { @@ -1087,13 +1088,6 @@ impl Inst { } } - pub(crate) fn is_mappable_call(&self, m: &Module) -> bool { - match self { - Self::Call { callee, .. } => !m.func(*callee).is_declaration(), - _ => false, - } - } - /// Returns whether `self` is a call to the control point. pub(crate) fn is_control_point(&self, m: &Module) -> bool { // FIXME: We don't really expect any other patchpoints here, but it would be nice to check @@ -1332,13 +1326,20 @@ impl fmt::Display for DisplayableInst<'_> { ftyidx: _, callop, args, + safepoint, } => { let args_s = args .iter() .map(|a| a.display(self.m).to_string()) .collect::>() .join(", "); - write!(f, "icall {}({})", callop.display(self.m), args_s) + write!( + f, + "icall {}({}) {}", + callop.display(self.m), + args_s, + safepoint.display(self.m) + ) } Inst::Select { cond, diff --git a/ykrt/src/compile/jitc_yk/jit_ir/mod.rs b/ykrt/src/compile/jitc_yk/jit_ir/mod.rs index 88055e55a..4b0c78a36 100644 --- a/ykrt/src/compile/jitc_yk/jit_ir/mod.rs +++ b/ykrt/src/compile/jitc_yk/jit_ir/mod.rs @@ -377,7 +377,9 @@ impl Module { /// If `iidx` points to a `Const`, `Copy`, or `Tombstone` instruction. pub(crate) fn inst(&self, iidx: InstIdx) -> Inst { match self.insts[usize::from(iidx)] { - Inst::Const(_) | Inst::Copy(_) | Inst::Tombstone => todo!(), + Inst::Const(_) | Inst::Copy(_) | Inst::Tombstone => { + todo!("{:?}", self.insts[usize::from(iidx)]) + } x => x, } } @@ -1296,6 +1298,24 @@ impl Operand { f(*iidx) } } + + /// If self is [Operand::Var] and references a [Inst::Copy], recursively search until a + /// non-`Copy` instruction is found. + pub(crate) fn decopy(&self, m: &Module) -> Self { + let mut ret = self.clone(); + loop { + match ret { + Self::Var(iidx) => { + if let Inst::Copy(next_iidx) = m.inst_raw(iidx) { + ret = Operand::Var(next_iidx); + } else { + return ret; + } + } + Self::Const(_) => return ret, + } + } + } } pub(crate) struct DisplayableOperand<'a> { @@ -2030,7 +2050,13 @@ impl Inst { /// /// `%1`, `%2`, and `%3` are all "decopy equal" to each other, but `%4` is not "decopy equal" /// to any other instruction. + /// + /// # Panics + /// + /// If either `self` or `other` are `Inst::Copy`. pub(crate) fn decopy_eq(&self, m: &Module, other: Inst) -> bool { + assert!(!matches!(self, Inst::Copy(..))); + assert!(!matches!(other, Inst::Copy(..))); if std::mem::discriminant(self) != std::mem::discriminant(&other) { return false; } diff --git a/ykrt/src/compile/jitc_yk/trace_builder.rs b/ykrt/src/compile/jitc_yk/trace_builder.rs index d3c1d1275..879d025b8 100644 --- a/ykrt/src/compile/jitc_yk/trace_builder.rs +++ b/ykrt/src/compile/jitc_yk/trace_builder.rs @@ -17,6 +17,7 @@ use crate::{ trace::{AOTTraceIterator, TraceAction}, }; use std::{collections::HashMap, ffi::CString, marker::PhantomData, sync::Arc}; +use ykaddr::addr::symbol_to_ptr; /// Given an execution trace and AOT IR, creates a JIT IR trace. pub(crate) struct TraceBuilder { @@ -51,6 +52,8 @@ pub(crate) struct TraceBuilder { debug_strs: Vec, /// The trace's current position in the [Self::debug_strs] vector. debug_str_idx: usize, + /// Local variables that we have inferred to be constant. + inferred_consts: HashMap, } impl TraceBuilder { @@ -89,6 +92,7 @@ impl TraceBuilder { phantom: PhantomData, debug_strs, debug_str_idx: 0, + inferred_consts: HashMap::new(), }) } @@ -214,19 +218,26 @@ impl TraceBuilder { volatile, } => self.handle_load(bid, iidx, ptr, tyidx, *volatile), // FIXME: ignore remaining instructions after a call. - aot_ir::Inst::Call { callee, args, .. } => { + aot_ir::Inst::Call { + callee, + args, + safepoint, + } => { // Get the branch instruction of this block. let nextinst = blk.insts.last().unwrap(); - self.handle_call(inst, bid, iidx, callee, args, nextinst) + self.handle_call(inst, bid, iidx, callee, args, safepoint.as_ref(), nextinst) } aot_ir::Inst::IndirectCall { ftyidx, callop, args, + safepoint, } => { // Get the branch instruction of this block. let nextinst = blk.insts.last().unwrap(); - self.handle_indirectcall(inst, bid, iidx, ftyidx, callop, args, nextinst) + self.handle_indirectcall( + inst, bid, iidx, ftyidx, callop, args, nextinst, safepoint, + ) } aot_ir::Inst::Store { tgt, val, volatile } => { self.handle_store(bid, iidx, tgt, val, *volatile) @@ -446,7 +457,9 @@ impl TraceBuilder { op: &aot_ir::Operand, ) -> Result { match op { - aot_ir::Operand::LocalVariable(iid) => Ok(self.local_map[iid].clone()), + aot_ir::Operand::LocalVariable(iid) => { + Ok(self.local_map[iid].decopy(&self.jit_mod).clone()) + } aot_ir::Operand::Const(cidx) => { let jit_const = self.handle_const(self.aot_mod.const_(*cidx))?; Ok(jit_ir::Operand::Const( @@ -457,7 +470,17 @@ impl TraceBuilder { let load = jit_ir::LookupGlobalInst::new(self.handle_global(*gd_idx)?)?; self.jit_mod.push_and_make_operand(load.into()) } - _ => todo!("{}", op.display(self.aot_mod)), + aot_ir::Operand::Func(fidx) => { + // We reduce a function operand to a constant pointer. + let aot_func = self.aot_mod.func(*fidx); + let fname = aot_func.name(); + let vaddr = symbol_to_ptr(fname).unwrap(); + let cidx = self + .jit_mod + .insert_const(jit_ir::Const::Ptr(vaddr as usize)) + .unwrap(); + Ok(jit_ir::Operand::Const(cidx)) + } } } @@ -610,6 +633,28 @@ impl TraceBuilder { let gi = jit_ir::GuardInfo::new(bid.clone(), live_vars, callframes, safepoint.id); let gi_idx = self.jit_mod.push_guardinfo(gi).unwrap(); + // Can we infer a constant from this? + // + // If this is a `guard true` and the condition is a `eq` with a constant, then we have + // inferred a constant *after* the guard. + if expect { + match cond { + jit_ir::Operand::Var(iidx) => { + // Using `inst_nocopy()` here because `Inst::Const` can arise. + if let jit_ir::Inst::ICmp(icmp) = self.jit_mod.inst_nocopy(*iidx).unwrap() + && let jit_ir::Operand::Const(const_rhs) = icmp.rhs(&self.jit_mod) + && let jit_ir::Operand::Var(var_lhs) = icmp.lhs(&self.jit_mod) + && icmp.predicate() == jit_ir::Predicate::Equal + { + // Check we store a canonical (de-copied) instruction index. + assert!(self.jit_mod.inst_nocopy(var_lhs).is_some()); + self.inferred_consts.insert(var_lhs, const_rhs); + } + } + jit_ir::Operand::Const(_) => todo!(), + }; + } + Ok(jit_ir::GuardInst::new(cond.clone(), expect, gi_idx)) } @@ -708,9 +753,36 @@ impl TraceBuilder { callop: &aot_ir::Operand, args: &[aot_ir::Operand], nextinst: &'static aot_ir::Inst, + safepoint: &'static aot_ir::DeoptSafepoint, ) -> Result<(), CompilationError> { debug_assert!(!inst.is_debug_call(self.aot_mod)); + let jit_callop = self.handle_operand(callop)?; + if let jit_ir::Operand::Var(callee_iidx) = jit_callop { + // let callee = self.jit_mod.inst(callee_iidx); + if let Some(cidx) = self.inferred_consts.get(&callee_iidx) { + // The callee is constant. We can treat this *indirect* call as if it were a + // *direct* call, and maybe even inline it. + let Const::Ptr(vaddr) = self.jit_mod.const_(*cidx) else { + panic!(); + }; + // Find the function the constant pointer is referring to. + let dli = ykaddr::addr::dladdr(*vaddr).unwrap(); + assert_eq!(dli.dli_saddr(), *vaddr); + let callee = self + .aot_mod + .funcidx(dli.dli_sname().unwrap().to_str().unwrap()); + return self.direct_call_impl( + bid, + aot_inst_idx, + &callee, + args, + Some(safepoint), + nextinst, + ); + } + } + // Convert AOT args to JIT args. let mut jit_args = Vec::new(); for arg in args { @@ -724,7 +796,6 @@ impl TraceBuilder { // "yk_outline". Any mappable, indirect call is then guaranteed to be inline safe. self.outline_until_after_call(bid, nextinst); - let jit_callop = self.handle_operand(callop)?; let jit_tyidx = self.handle_type(self.aot_mod.type_(*ftyidx))?; let inst = jit_ir::IndirectCallInst::new(&mut self.jit_mod, jit_tyidx, jit_callop, jit_args)?; @@ -732,6 +803,7 @@ impl TraceBuilder { self.copy_inst(jit_ir::Inst::IndirectCall(idx), bid, aot_inst_idx) } + /// Handle a *direct* call instruction. fn handle_call( &mut self, inst: &'static aot_ir::Inst, @@ -739,6 +811,7 @@ impl TraceBuilder { aot_inst_idx: usize, callee: &aot_ir::FuncIdx, args: &[aot_ir::Operand], + safepoint: Option<&'static aot_ir::DeoptSafepoint>, nextinst: &'static aot_ir::Inst, ) -> Result<(), CompilationError> { // Ignore special functions that we neither want to inline nor copy. @@ -757,6 +830,18 @@ impl TraceBuilder { return Ok(()); } + self.direct_call_impl(bid, aot_inst_idx, callee, args, safepoint, nextinst) + } + + fn direct_call_impl( + &mut self, + bid: &aot_ir::BBlockId, + aot_inst_idx: usize, + callee: &aot_ir::FuncIdx, + args: &[aot_ir::Operand], + safepoint: Option<&'static aot_ir::DeoptSafepoint>, + nextinst: &'static aot_ir::Inst, + ) -> Result<(), CompilationError> { // Convert AOT args to JIT args. let mut jit_args = Vec::new(); for arg in args { @@ -778,17 +863,17 @@ impl TraceBuilder { let is_recursive = self.frames.iter().any(|f| f.funcidx == Some(*callee)); let func = self.aot_mod.func(*callee); - if inst.is_mappable_call(self.aot_mod) + if !func.is_declaration() && !func.is_outline() && !func.is_idempotent() && !func.contains_call_to(self.aot_mod, "llvm.va_start") && !is_recursive { // This is a mappable call that we want to inline. - debug_assert!(inst.safepoint().is_some()); + debug_assert!(safepoint.is_some()); // Assign safepoint to the current frame. // Unwrap is safe as there's always at least one frame. - self.frames.last_mut().unwrap().safepoint = inst.safepoint(); + self.frames.last_mut().unwrap().safepoint = safepoint; // Create a new frame for the inlined call and pass in the arguments of the caller. let aot_iid = aot_ir::InstID::new( bid.funcidx(),