Skip to content

Commit

Permalink
Merge pull request #1464 from ltratt/dekind
Browse files Browse the repository at this point in the history 
Pass around the root trace rather than fishing it out of `HotLocationKind`.
  • Loading branch information
@ptersilie
ptersilie authored Nov 13, 2024
2 parents 285ecbc + e6abca8 commit c1fc7b4
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 28 deletions.
26 changes: 7 additions & 19 deletions ykrt/src/compile/jitc_yk/codegen/x64/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ use crate::{
},
CompiledTrace, Guard, GuardIdx, SideTraceInfo,
},
location::{HotLocation, HotLocationKind},
location::HotLocation,
mt::MT,
};
use dynasmrt::{
Expand Down Expand Up @@ -2397,7 +2397,12 @@ impl CompiledTrace for X64CompiledTrace {
self.buf.ptr(AssemblyOffset(0)) as *const libc::c_void
}

fn sidetraceinfo(&self, gidx: GuardIdx) -> Arc<dyn SideTraceInfo> {
fn sidetraceinfo(
&self,
root_ctr: Arc<dyn CompiledTrace>,
gidx: GuardIdx,
) -> Arc<dyn SideTraceInfo> {
let root_ctr = root_ctr.as_any().downcast::<X64CompiledTrace>().unwrap();
// FIXME: Can we reference these instead of copying them, e.g. by passing in a reference to
// the `CompiledTrace` and `gidx` or better a reference to `DeoptInfo`?
let deoptinfo = &self.deoptinfo[&usize::from(gidx)];
Expand All @@ -2408,23 +2413,6 @@ impl CompiledTrace for X64CompiledTrace {
.collect();
let callframes = deoptinfo.inlined_frames.clone();

// Get the root trace from the HotLocation.
// FIXME: It might be better if we had a consistent mechanism for doing this rather than
// fishing it out of the HotLocationKind`.
let hlarc = self.hl.upgrade().unwrap();
let hl = hlarc.lock();
let root_ctr = match &hl.kind {
HotLocationKind::Compiled(root_ctr) => Arc::clone(root_ctr)
.as_any()
.downcast::<X64CompiledTrace>()
.unwrap(),
HotLocationKind::SideTracing { root_ctr, .. } => Arc::clone(root_ctr)
.as_any()
.downcast::<X64CompiledTrace>()
.unwrap(),
_ => panic!("Unexpected HotLocationKind"),
};

// Calculate the address inside the root trace we want side-traces to jump to. Currently
// this is directly after the prologue. Later we will change this to jump to after the
// preamble and before the peeled loop.
Expand Down
18 changes: 15 additions & 3 deletions ykrt/src/compile/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,11 @@ pub(crate) trait CompiledTrace: fmt::Debug + Send + Sync {
/// upcasting in Rust is incomplete.
fn as_any(self: Arc<Self>) -> Arc<dyn std::any::Any + Send + Sync + 'static>;

fn sidetraceinfo(&self, gidx: GuardIdx) -> Arc<dyn SideTraceInfo>;
fn sidetraceinfo(
&self,
root_ctr: Arc<dyn CompiledTrace>,
gidx: GuardIdx,
) -> Arc<dyn SideTraceInfo>;

/// Return a reference to the guard `id`.
fn guard(&self, gidx: GuardIdx) -> &Guard;
Expand Down Expand Up @@ -113,7 +117,11 @@ mod compiled_trace_testing {
panic!();
}

fn sidetraceinfo(&self, _gidx: GuardIdx) -> Arc<dyn SideTraceInfo> {
fn sidetraceinfo(
&self,
_root_ctr: Arc<dyn CompiledTrace>,
_gidx: GuardIdx,
) -> Arc<dyn SideTraceInfo> {
panic!();
}

Expand Down Expand Up @@ -156,7 +164,11 @@ mod compiled_trace_testing {
panic!();
}

fn sidetraceinfo(&self, _gidx: GuardIdx) -> Arc<dyn SideTraceInfo> {
fn sidetraceinfo(
&self,
_root_ctr: Arc<dyn CompiledTrace>,
_gidx: GuardIdx,
) -> Arc<dyn SideTraceInfo> {
panic!();
}

Expand Down
22 changes: 16 additions & 6 deletions ykrt/src/mt.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ impl MT {
self: &Arc<Self>,
trace_iter: (Box<dyn AOTTraceIterator>, Box<[u8]>),
hl_arc: Arc<Mutex<HotLocation>>,
sidetrace: Option<(GuardIdx, Arc<dyn CompiledTrace>)>,
sidetrace: Option<(Arc<dyn CompiledTrace>, GuardIdx, Arc<dyn CompiledTrace>)>,
) {
self.stats.trace_recorded_ok();
let mt = Arc::clone(self);
Expand All @@ -271,8 +271,11 @@ impl MT {
Arc::clone(&*lk)
};
mt.stats.timing_state(TimingState::Compiling);
let (sti, guardid) = if let Some((guardid, ctr)) = &sidetrace {
(Some(ctr.sidetraceinfo(*guardid)), Some(*guardid))
let (sti, guardid) = if let Some((root_ctr, guardid, ctr)) = &sidetrace {
(
Some(ctr.sidetraceinfo(Arc::clone(root_ctr), *guardid)),
Some(*guardid),
)
} else {
(None, None)
};
Expand All @@ -287,7 +290,7 @@ impl MT {
trace_iter.1,
) {
Ok(ct) => {
if let Some((_, parent_ctr)) = sidetrace {
if let Some((_root_ctr, _, parent_ctr)) = sidetrace {
parent_ctr.guard(guardid.unwrap()).set_ctr(ct);
} else {
let mut hl = hl_arc.lock();
Expand Down Expand Up @@ -416,6 +419,7 @@ impl MT {
TransitionControlPoint::StopSideTracing {
gidx: guardid,
parent_ctr,
root_ctr,
} => {
// Assuming no bugs elsewhere, the `unwrap`s cannot fail, because
// `StartSideTracing` will have put a `Some` in the `Rc`.
Expand All @@ -438,7 +442,7 @@ impl MT {
self.queue_compile_job(
(utrace, promotions.into_boxed_slice()),
hl,
Some((guardid, parent_ctr)),
Some((root_ctr, guardid, parent_ctr)),
);
}
Err(e) => {
Expand Down Expand Up @@ -572,9 +576,14 @@ impl MT {
// ...and it's this location: we have therefore finished
// tracing the loop.
let parent_ctr = Arc::clone(parent_ctr);
let root_ctr_cl = Arc::clone(root_ctr);
lk.kind = HotLocationKind::Compiled(Arc::clone(root_ctr));
drop(lk);
TransitionControlPoint::StopSideTracing { gidx, parent_ctr }
TransitionControlPoint::StopSideTracing {
gidx,
parent_ctr,
root_ctr: root_ctr_cl,
}
}
}
_ => {
Expand Down Expand Up @@ -899,6 +908,7 @@ enum TransitionControlPoint {
StopSideTracing {
gidx: GuardIdx,
parent_ctr: Arc<dyn CompiledTrace>,
root_ctr: Arc<dyn CompiledTrace>,
},
}

Expand Down

0 comments on commit c1fc7b4

Please sign in to comment.