From bd40930564f7504f1dbe250654f2a8e14f497bc7 Mon Sep 17 00:00:00 2001 From: Laurence Tratt Date: Fri, 29 Nov 2024 16:27:31 +0000 Subject: [PATCH] Add early return and recursive tests. --- tests/c/early_return.c | 63 ++++++++ tests/c/fcmp_double.c | 1 - tests/c/fcmp_float.c | 1 - tests/c/recursive.c | 63 ++++++++ ykcapi/src/lib.rs | 7 + ykcapi/yk.h | 16 ++ ykrt/src/compile/jitc_yk/codegen/x64/deopt.rs | 2 +- ykrt/src/mt.rs | 147 ++++++++++++------ 8 files changed, 252 insertions(+), 48 deletions(-) create mode 100644 tests/c/early_return.c create mode 100644 tests/c/recursive.c diff --git a/tests/c/early_return.c b/tests/c/early_return.c new file mode 100644 index 000000000..0b3e99020 --- /dev/null +++ b/tests/c/early_return.c @@ -0,0 +1,63 @@ +// Run-time: +// env-var: YKD_LOG_IR=-:jit-post-opt +// env-var: YKD_SERIALISE_COMPILATION=1 +// env-var: YK_LOG=4 +// stderr: +// 3 +// 2 +// yk-jit-event: start-tracing +// 1 +// yk-jit-event: stop-tracing-early-return +// return +// yk-jit-event: start-tracing +// 3 +// yk-jit-event: stop-tracing +// --- Begin jit-pre-opt --- +// ... +// --- End jit-pre-opt --- +// exit + +// Check that basic trace compilation works. + +#include +#include +#include +#include +#include +#include + +void loop(YkMT *, YkLocation *); + +void loop(YkMT *mt, YkLocation *loc) { + int res = 9998; + int i = 3; + NOOPT_VAL(res); + NOOPT_VAL(i); + while (i > 0) { + yk_mt_control_point(mt, loc); + fprintf(stderr, "%d\n", i); + i--; + } + yk_mt_early_return(mt); + fprintf(stderr, "return\n"); + NOOPT_VAL(res); +} + +int main(int argc, char **argv) { + YkMT *mt = yk_mt_new(NULL); + yk_mt_hot_threshold_set(mt, 2); + YkLocation loc = yk_location_new(); + + int res = 9998; + int i = 4; + NOOPT_VAL(loc); + NOOPT_VAL(res); + NOOPT_VAL(i); + loop(mt, &loc); + loop(mt, &loc); + fprintf(stderr, "exit\n"); + NOOPT_VAL(res); + yk_location_drop(loc); + yk_mt_shutdown(mt); + return (EXIT_SUCCESS); +} diff --git a/tests/c/fcmp_double.c b/tests/c/fcmp_double.c index 1fd425a85..30a6063f1 100644 --- a/tests/c/fcmp_double.c +++ b/tests/c/fcmp_double.c @@ -1,5 +1,4 @@ // Run-time: -// env-var: YKD_LOG_IR=aot,jit-pre-opt,jit-asm // env-var: YKD_SERIALISE_COMPILATION=1 // env-var: YK_LOG=4 // stderr: diff --git a/tests/c/fcmp_float.c b/tests/c/fcmp_float.c index 7089b0516..ef8b9e9e2 100644 --- a/tests/c/fcmp_float.c +++ b/tests/c/fcmp_float.c @@ -1,5 +1,4 @@ // Run-time: -// env-var: YKD_LOG_IR=aot,jit-pre-opt,jit-asm // env-var: YKD_SERIALISE_COMPILATION=1 // env-var: YK_LOG=4 // stderr: diff --git a/tests/c/recursive.c b/tests/c/recursive.c new file mode 100644 index 000000000..f9f369e74 --- /dev/null +++ b/tests/c/recursive.c @@ -0,0 +1,63 @@ +// Run-time: +// env-var: YKD_LOG_IR=-:jit-pre-opt,jit-post-opt +// env-var: YKD_SERIALISE_COMPILATION=1 +// env-var: YK_LOG=4 +// stderr: +// 3 +// 2 +// yk-jit-event: start-tracing +// 1 +// yk-jit-event: stop-tracing-early-return +// return +// yk-jit-event: start-tracing +// 3 +// yk-jit-event: stop-tracing +// --- Begin jit-pre-opt --- +// ... +// --- End jit-pre-opt --- +// exit + +// Check that basic trace compilation works. + +#include +#include +#include +#include +#include +#include + +int loop(YkMT *, YkLocation *, int); + +int loop(YkMT *mt, YkLocation *loc, int i) { + int res = 9998; + NOOPT_VAL(res); + NOOPT_VAL(i); + while (i > 0) { + yk_mt_control_point(mt, loc); + if (i > 2) { + loop(mt, loc, i - 1); + } + fprintf(stderr, "%d\n", i); + i--; + } + yk_mt_early_return(mt); + fprintf(stderr, "return\n"); + NOOPT_VAL(res); + return i; +} + +int main(int argc, char **argv) { + YkMT *mt = yk_mt_new(NULL); + yk_mt_hot_threshold_set(mt, 2); + YkLocation loc = yk_location_new(); + + int res = 9998; + NOOPT_VAL(loc); + NOOPT_VAL(res); + loop(mt, &loc, 3); + fprintf(stderr, "exit\n"); + NOOPT_VAL(res); + yk_location_drop(loc); + yk_mt_shutdown(mt); + return (EXIT_SUCCESS); +} diff --git a/ykcapi/src/lib.rs b/ykcapi/src/lib.rs index ba06dcc3f..22220ca58 100644 --- a/ykcapi/src/lib.rs +++ b/ykcapi/src/lib.rs @@ -50,6 +50,13 @@ pub extern "C" fn yk_mt_control_point(_mt: *mut MT, _loc: *mut Location) { // Intentionally empty. } +#[no_mangle] +pub extern "C" fn __yk_mt_early_return(mt: *mut MT, frameaddr: *mut c_void) { + let mt = unsafe { &*mt }; + let arc = unsafe { Arc::from_raw(mt) }; + arc.early_return(frameaddr); +} + // The new control point called after the interpreter has been patched by ykllvm. #[cfg(target_arch = "x86_64")] #[naked] diff --git a/ykcapi/yk.h b/ykcapi/yk.h index fad7223ca..244ccc4ed 100644 --- a/ykcapi/yk.h +++ b/ykcapi/yk.h @@ -49,6 +49,22 @@ void yk_mt_shutdown(YkMT *); // execute JITted code. void yk_mt_control_point(YkMT *, YkLocation *); +// At each point a function containing a control point can exit "early" this +// function must be called. "Early" includes, but is not +// limited to, the following: +// +// 1. Immediately after a non-infinite loop containing a call to +// `yk_mt_control_point`. +// 2. Immediately before `return` statements in code reachable from a +// `yk_mt_control_point`. +// +// Failure to call this function will lead to undefined behaviour. +# define yk_mt_early_return(mt) __yk_mt_early_return(mt, __builtin_frame_address(0)) + +// This is an internal function to yk: calling it directly leads to undefined +// behaviour. +void __yk_mt_early_return(YkMT *, void *); + // Set the threshold at which `YkLocation`'s are considered hot. void yk_mt_hot_threshold_set(YkMT *, YkHotThreshold); diff --git a/ykrt/src/compile/jitc_yk/codegen/x64/deopt.rs b/ykrt/src/compile/jitc_yk/codegen/x64/deopt.rs index f30bcbda9..a55ab0a0e 100644 --- a/ykrt/src/compile/jitc_yk/codegen/x64/deopt.rs +++ b/ykrt/src/compile/jitc_yk/codegen/x64/deopt.rs @@ -92,7 +92,7 @@ fn running_trace(gidxs: &[usize]) -> Arc { /// * glen - Length for list in `gptr`. #[no_mangle] pub(crate) extern "C" fn __yk_deopt( - frameaddr: *const c_void, + frameaddr: *mut c_void, gidx: u64, gp_regs: &[u64; 16], fp_regs: &[u64; 16], diff --git a/ykrt/src/mt.rs b/ykrt/src/mt.rs index 699532414..a4d14d3c0 100644 --- a/ykrt/src/mt.rs +++ b/ykrt/src/mt.rs @@ -401,7 +401,7 @@ impl MT { #[allow(clippy::not_unsafe_ptr_arg_deref)] pub fn control_point(self: &Arc, loc: &Location, frameaddr: *mut c_void, smid: u64) { - match self.transition_control_point(loc) { + match self.transition_control_point(loc, frameaddr) { TransitionControlPoint::NoAction => (), TransitionControlPoint::Execute(ctr) => { self.log.log(Verbosity::JITEvent, "enter-jit-code"); @@ -456,7 +456,7 @@ impl MT { promotions, frameaddr: tracing_frameaddr, } => { - assert_eq!(frameaddr as *const c_void, tracing_frameaddr); + assert_eq!(frameaddr, tracing_frameaddr); (hl, thread_tracer, promotions) } _ => unreachable!(), @@ -492,7 +492,7 @@ impl MT { promotions, frameaddr: tracing_frameaddr, } => { - assert_eq!(frameaddr as *const c_void, tracing_frameaddr); + assert_eq!(frameaddr, tracing_frameaddr); (hl, thread_tracer, promotions) } _ => unreachable!(), @@ -524,7 +524,11 @@ impl MT { /// Perform the next step to `loc` in the `Location` state-machine for a control point. If /// `loc` moves to the Compiled state, return a pointer to a [CompiledTrace] object. - fn transition_control_point(self: &Arc, loc: &Location) -> TransitionControlPoint { + fn transition_control_point( + self: &Arc, + loc: &Location, + frameaddr: *mut c_void, + ) -> TransitionControlPoint { MTThread::with(|mtt| { let is_tracing = mtt.is_tracing(); match loc.hot_location() { @@ -587,16 +591,31 @@ impl MT { HotLocationKind::Tracing => { let hl = loc.hot_location_arc_clone().unwrap(); match &*mtt.tstate.borrow() { - MTThreadState::Tracing { hl: thread_hl, .. } => { + MTThreadState::Tracing { + hl: thread_hl, + frameaddr: tracing_frameaddr, + .. + } => { // This thread is tracing something... if !Arc::ptr_eq(thread_hl, &hl) { // ...but not this Location. TransitionControlPoint::NoAction } else { - // ...and it's this location: we have therefore finished - // tracing the loop. - lk.kind = HotLocationKind::Compiling; - TransitionControlPoint::StopTracing + // ...and it's this location... + if frameaddr == *tracing_frameaddr { + lk.kind = HotLocationKind::Compiling; + TransitionControlPoint::StopTracing + } else { + // ...but in a different frame. + #[cfg(target_arch = "x86_64")] + { + // If this assert fails, we've fallen through to a + // caller, which is UB. + assert!(*tracing_frameaddr > frameaddr); + } + // We're inlining. + TransitionControlPoint::NoAction + } } } _ => { @@ -746,7 +765,7 @@ impl MT { self: &Arc, parent: Arc, gidx: GuardIdx, - frameaddr: *const c_void, + frameaddr: *mut c_void, ) { match self.transition_guard_failure(parent, gidx) { TransitionGuardFailure::NoAction => (), @@ -770,6 +789,40 @@ impl MT { } } } + + pub fn early_return(self: &Arc, frameaddr: *mut c_void) { + MTThread::with(|mtt| { + let mut abort = false; + if let MTThreadState::Tracing { + frameaddr: tracing_frameaddr, + .. + } = &*mtt.tstate.borrow() + { + if frameaddr <= *tracing_frameaddr { + abort = true; + } + } + + if abort { + match mtt.tstate.replace(MTThreadState::Interpreting) { + MTThreadState::Tracing { + thread_tracer, hl, .. + } => { + // We don't care if the thread tracer went wrong: we're not going to use + // its result anyway. + thread_tracer.stop().unwrap(); + let mut lk = hl.lock(); + lk.tracecompilation_error(self); + drop(lk); + self.stats.timing_state(TimingState::None); + self.log + .log(Verbosity::JITEvent, "stop-tracing-early-return"); + } + _ => unreachable!(), + } + } + }); + } } #[cfg(target_arch = "x86_64")] @@ -826,7 +879,7 @@ enum MTThreadState { promotions: Vec, /// The `frameaddr` when tracing started. This allows us to tell if we're finishing tracing /// at the same point that we started. - frameaddr: *const c_void, + frameaddr: *mut c_void, }, /// This thread is executing a trace. Note that the `dyn CompiledTrace` serves two different purposes: /// @@ -1033,7 +1086,9 @@ mod tests { } fn expect_start_tracing(mt: &Arc, loc: &Location) { - let TransitionControlPoint::StartTracing(hl) = mt.transition_control_point(loc) else { + let TransitionControlPoint::StartTracing(hl) = + mt.transition_control_point(loc, 0 as *mut c_void) + else { panic!() }; MTThread::with(|mtt| { @@ -1041,13 +1096,15 @@ mod tests { hl, thread_tracer: Box::new(DummyTraceRecorder), promotions: Vec::new(), - frameaddr: 0 as *const c_void, + frameaddr: 0 as *mut c_void, }; }); } fn expect_stop_tracing(mt: &Arc, loc: &Location) { - let TransitionControlPoint::StopTracing = mt.transition_control_point(loc) else { + let TransitionControlPoint::StopTracing = + mt.transition_control_point(loc, 0 as *mut c_void) + else { panic!() }; MTThread::with(|mtt| { @@ -1066,7 +1123,7 @@ mod tests { hl, thread_tracer: Box::new(DummyTraceRecorder), promotions: Vec::new(), - frameaddr: 0 as *const c_void, + frameaddr: 0 as *mut c_void, }; }); } @@ -1080,7 +1137,7 @@ mod tests { let loc = Location::new(); for i in 0..mt.hot_threshold() { assert_eq!( - mt.transition_control_point(&loc), + mt.transition_control_point(&loc, 0 as *mut c_void), TransitionControlPoint::NoAction ); assert_eq!(loc.count(), Some(i + 1)); @@ -1100,12 +1157,12 @@ mod tests { ))); loc.hot_location().unwrap().lock().kind = HotLocationKind::Compiled(ctr.clone()); assert!(matches!( - mt.transition_control_point(&loc), + mt.transition_control_point(&loc, 0 as *mut c_void), TransitionControlPoint::Execute(_) )); expect_start_side_tracing(&mt, ctr); - match mt.transition_control_point(&loc) { + match mt.transition_control_point(&loc, 0 as *mut c_void) { TransitionControlPoint::StopSideTracing { .. } => { MTThread::with(|mtt| { *mtt.tstate.borrow_mut() = MTThreadState::Interpreting; @@ -1118,7 +1175,7 @@ mod tests { _ => unreachable!(), } assert!(matches!( - mt.transition_control_point(&loc), + mt.transition_control_point(&loc, 0 as *mut c_void), TransitionControlPoint::Execute(_) )); } @@ -1142,25 +1199,25 @@ mod tests { // otherwise tracing will start, and the assertions will fail. for _ in 0..hot_thrsh / (num_threads * 4) { assert_eq!( - mt.transition_control_point(&loc), + mt.transition_control_point(&loc, 0 as *mut c_void), TransitionControlPoint::NoAction ); let c1 = loc.count(); assert!(c1.is_some()); assert_eq!( - mt.transition_control_point(&loc), + mt.transition_control_point(&loc, 0 as *mut c_void), TransitionControlPoint::NoAction ); let c2 = loc.count(); assert!(c2.is_some()); assert_eq!( - mt.transition_control_point(&loc), + mt.transition_control_point(&loc, 0 as *mut c_void), TransitionControlPoint::NoAction ); let c3 = loc.count(); assert!(c3.is_some()); assert_eq!( - mt.transition_control_point(&loc), + mt.transition_control_point(&loc, 0 as *mut c_void), TransitionControlPoint::NoAction ); let c4 = loc.count(); @@ -1180,7 +1237,7 @@ mod tests { // at or below the threshold: it could even be (although it's rather unlikely) 0! assert!(loc.count().is_some()); loop { - match mt.transition_control_point(&loc) { + match mt.transition_control_point(&loc, 0 as *mut c_void) { TransitionControlPoint::NoAction => (), TransitionControlPoint::StartTracing(hl) => { MTThread::with(|mtt| { @@ -1188,7 +1245,7 @@ mod tests { hl, thread_tracer: Box::new(DummyTraceRecorder), promotions: Vec::new(), - frameaddr: 0 as *const c_void, + frameaddr: 0 as *mut c_void, }; }); break; @@ -1213,7 +1270,7 @@ mod tests { // Get the location to the point of being hot. for _ in 0..THRESHOLD { assert_eq!( - mt.transition_control_point(&loc), + mt.transition_control_point(&loc, 0 as *mut c_void), TransitionControlPoint::NoAction ); } @@ -1245,7 +1302,7 @@ mod tests { HotLocationKind::Tracing )); assert_eq!( - mt.transition_control_point(&loc), + mt.transition_control_point(&loc, 0 as *mut c_void), TransitionControlPoint::NoAction ); assert!(matches!( @@ -1266,7 +1323,7 @@ mod tests { // Get the location to the point of being hot. for _ in 0..THRESHOLD { assert_eq!( - mt.transition_control_point(&loc), + mt.transition_control_point(&loc, 0 as *mut c_void), TransitionControlPoint::NoAction ); } @@ -1324,17 +1381,17 @@ mod tests { for _ in 0..THRESHOLD { assert_eq!( - mt.transition_control_point(&loc1), + mt.transition_control_point(&loc1, 0 as *mut c_void), TransitionControlPoint::NoAction ); assert_eq!( - mt.transition_control_point(&loc2), + mt.transition_control_point(&loc2, 0 as *mut c_void), TransitionControlPoint::NoAction ); } expect_start_tracing(&mt, &loc1); assert_eq!( - mt.transition_control_point(&loc2), + mt.transition_control_point(&loc2, 0 as *mut c_void), TransitionControlPoint::NoAction ); assert!(matches!( @@ -1371,7 +1428,7 @@ mod tests { let num_starts = Arc::clone(&num_starts); thrs.push(thread::spawn(move || { for _ in 0..THRESHOLD { - match mt.transition_control_point(&loc) { + match mt.transition_control_point(&loc, 0 as *mut c_void) { TransitionControlPoint::NoAction => (), TransitionControlPoint::Execute(_) => (), TransitionControlPoint::StartTracing(hl) => { @@ -1381,7 +1438,7 @@ mod tests { hl, thread_tracer: Box::new(DummyTraceRecorder), promotions: Vec::new(), - frameaddr: 0 as *const c_void, + frameaddr: 0 as *mut c_void, }; }); assert!(matches!( @@ -1394,7 +1451,7 @@ mod tests { HotLocationKind::Compiling )); assert_eq!( - mt.transition_control_point(&loc), + mt.transition_control_point(&loc, 0 as *mut c_void), TransitionControlPoint::NoAction ); assert!(matches!( @@ -1406,7 +1463,7 @@ mod tests { ); loop { if let TransitionControlPoint::Execute(_) = - mt.transition_control_point(&loc) + mt.transition_control_point(&loc, 0 as *mut c_void) { break; } @@ -1441,11 +1498,11 @@ mod tests { for _ in 0..THRESHOLD { assert_eq!( - mt.transition_control_point(&loc1), + mt.transition_control_point(&loc1, 0 as *mut c_void), TransitionControlPoint::NoAction ); assert_eq!( - mt.transition_control_point(&loc2), + mt.transition_control_point(&loc2, 0 as *mut c_void), TransitionControlPoint::NoAction ); } @@ -1460,7 +1517,7 @@ mod tests { expect_start_tracing(&mt, &loc2); assert_eq!( - mt.transition_control_point(&loc1), + mt.transition_control_point(&loc1, 0 as *mut c_void), TransitionControlPoint::NoAction ); expect_stop_tracing(&mt, &loc2); @@ -1482,7 +1539,7 @@ mod tests { fn to_compiled(mt: &Arc, loc: &Location) -> Arc { for _ in 0..THRESHOLD { assert_eq!( - mt.transition_control_point(loc), + mt.transition_control_point(loc, 0 as *mut c_void), TransitionControlPoint::NoAction ); } @@ -1516,11 +1573,11 @@ mod tests { expect_start_side_tracing(&mt, ctr2); assert!(matches!( - dbg!(mt.transition_control_point(&loc1)), + dbg!(mt.transition_control_point(&loc1, 0 as *mut c_void)), TransitionControlPoint::Execute(_) )); assert!(matches!( - mt.transition_control_point(&loc2), + mt.transition_control_point(&loc2, 0 as *mut c_void), TransitionControlPoint::StopSideTracing { .. } )); } @@ -1531,7 +1588,7 @@ mod tests { let loc = Location::new(); b.iter(|| { for _ in 0..100000 { - black_box(mt.transition_control_point(&loc)); + black_box(mt.transition_control_point(&loc, 0 as *mut c_void)); } }); } @@ -1547,7 +1604,7 @@ mod tests { let mt = Arc::clone(&mt); thrs.push(thread::spawn(move || { for _ in 0..100 { - black_box(mt.transition_control_point(&loc)); + black_box(mt.transition_control_point(&loc, 0 as *mut c_void)); } })); } @@ -1578,14 +1635,14 @@ mod tests { // https://github.com/ykjit/yk/issues/519 expect_start_tracing(&mt, &loc2); assert!(matches!( - mt.transition_control_point(&loc1), + mt.transition_control_point(&loc1, 0 as *mut c_void), TransitionControlPoint::NoAction )); // But once we stop tracing for `loc2`, we should be able to execute the trace for `loc1`. expect_stop_tracing(&mt, &loc2); assert!(matches!( - mt.transition_control_point(&loc1), + mt.transition_control_point(&loc1, 0 as *mut c_void), TransitionControlPoint::Execute(_) )); }