Skip to content

Commit

Permalink
Track nested meta-tracing states.
Browse files Browse the repository at this point in the history
Previously we -- well, this one is entirely my fault, so "I"! -- tracked
per-thread meta-tracing state as a single `MTThreadState` that we updates as
necessary. This doesn't work when we have nested execution, tracing and
the like, as the bug in #1571 highlighted.

This commit moves us to a stack of `MTThreadState`s. The basic idea is
that the stack always has at least one element: `Interpreting`. As we go
through other states, we push / pop as appropriate. The test below (from
Edd and Lukas) fails on `master` but is fixed by this commit.

The implementation is a bit more awkward than one might hope as naive implementations either:

  1. Spread lots of knowledge about the stack around the code. That's a
     disaster waiting to happen.
  2. Run into borrow checker problems.

This commit gets around this in two phases:

  1. We pass closures to `MTThread` which can peek at the current
     `MTThreadState` (which isn't `Copy`!).
  2. Those closures return the "things" we need to update outside that
     context.

This is a bit awkward, and perhaps there's a better API, but this one is
at least safe.

Co-authored-by: Edd Barrett <[email protected]>
Co-authored-by: Lukas Diekmann <[email protected]>
  • Loading branch information
3 people committed Feb 5, 2025
1 parent de8dd26 commit bfbcbdb
Show file tree
Hide file tree
Showing 3 changed files with 319 additions and 125 deletions.
134 changes: 134 additions & 0 deletions tests/c/nested_execution.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
// Run-time:
// env-var: YKD_SERIALISE_COMPILATION=1
// env-var: YKD_LOG_IR=jit-pre-opt
// env-var: YK_LOG=4
// stderr:
// enter
// yk-jit-event: start-tracing
// 6
// enter
// 5
// 4
// 3
// 2
// 1
// return
// yk-jit-event: stop-tracing
// --- Begin jit-pre-opt ---
// ...
// guard true, ...
// ...
// guard false, ...
// ...
// guard false, ...
// ...
// guard false, ...
// ...
// guard true, ...
// ...
// guard true, ...
// ...
// --- End jit-pre-opt ---
// 5
// enter
// yk-jit-event: start-tracing
// 4
// yk-jit-event: stop-tracing
// --- Begin jit-pre-opt ---
// ...
// guard false, ...
// ...
// guard false, ...
// ...
// guard true, ...
// ...
// --- End jit-pre-opt ---
// 3
// yk-jit-event: enter-jit-code
// 2
// 1
// yk-jit-event: deoptimise
// return
// yk-jit-event: enter-jit-code
// 4
// enter
// yk-jit-event: enter-jit-code
// 3
// 2
// 1
// yk-jit-event: deoptimise
// return
// yk-jit-event: deoptimise
// c
// 3
// enter
// yk-jit-event: enter-jit-code
// 2
// 1
// yk-jit-event: deoptimise
// return
// yk-jit-event: enter-jit-code
// yk-jit-event: deoptimise
// b
// 2
// enter
// yk-jit-event: enter-jit-code
// 1
// yk-jit-event: deoptimise
// return
// yk-jit-event: enter-jit-code
// yk-jit-event: deoptimise
// a
// 1
// enter
// return
// return

// Check that recursive execution finds the right guards.

#include <assert.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <yk.h>
#include <yk_testing.h>

void f(YkMT *mt, int who, YkLocation *loc1, YkLocation *loc2, int i) {
fprintf(stderr, "enter\n");
while (i > 0) {
yk_mt_control_point(mt, loc1);
if (who) {
if (i == 1) {
fprintf(stderr, "a\n");
}
if (i == 2) {
fprintf(stderr, "b\n");
}
if (i == 3) {
fprintf(stderr, "c\n");
}
}
fprintf(stderr, "%d\n", i);
i -= 1;
if (loc2 != NULL) {
f(mt, 0, loc2, NULL, i);
}
}
fprintf(stderr, "return\n");
}

int main(int argc, char **argv) {
YkMT *mt = yk_mt_new(NULL);
yk_mt_hot_threshold_set(mt, 0);
YkLocation loc1 = yk_location_new();
YkLocation loc2 = yk_location_new();
int i = 6;
NOOPT_VAL(loc1);
NOOPT_VAL(loc2);
NOOPT_VAL(i);
f(mt, 1, &loc1, &loc2, i);
yk_location_drop(loc1);
yk_location_drop(loc2);
yk_mt_shutdown(mt);
return (EXIT_SUCCESS);
}
9 changes: 4 additions & 5 deletions ykrt/src/compile/jitc_yk/codegen/x64/deopt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,8 +101,8 @@ pub(crate) extern "C" fn __yk_deopt(
let info = &ctr.deoptinfo[&usize::from(gidx)];
let mt = Arc::clone(&ctr.mt);

ctr.mt
.stats
mt.deopt();
mt.stats
.timing_state(crate::log::stats::TimingState::Deopting);
mt.log.log(Verbosity::JITEvent, "deoptimise");

Expand Down Expand Up @@ -352,9 +352,8 @@ pub(crate) extern "C" fn __yk_deopt(

// The `clone` should really be `Arc::clone(&ctr)` but that doesn't play well with type
// inference in this (unusual) case.
ctr.mt.guard_failure(ctr.clone(), gidx, frameaddr);
ctr.mt
.stats
mt.guard_failure(ctr.clone(), gidx, frameaddr);
mt.stats
.timing_state(crate::log::stats::TimingState::OutsideYk);

// Since we won't return from this function, drop `ctr` manually.
Expand Down
Loading

0 comments on commit bfbcbdb

Please sign in to comment.