Skip to content
Merged

Depub #309

Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion internal_ws/ykcompile/src/arch/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,4 @@
compile_error!("Currently only linux x86_64 is supported.");

#[cfg(all(target_arch = "x86_64", target_os = "linux"))]
pub(crate) mod x86_64;
pub(super) mod x86_64;
4 changes: 2 additions & 2 deletions internal_ws/ykcompile/src/arch/x86_64/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,15 @@ use yktrace::sir::SIR;

impl TraceCompiler {
/// Store the value in `src_loc` into `dst_loc`.
pub(crate) fn store(&mut self, dst_ip: &IRPlace, src_ip: &IRPlace) {
pub(super) fn store(&mut self, dst_ip: &IRPlace, src_ip: &IRPlace) {
let dst_loc = self.irplace_to_location(dst_ip);
let src_loc = self.irplace_to_location(src_ip);
debug_assert!(SIR.ty(&dst_ip.ty()).size() == SIR.ty(&src_ip.ty()).size());
self.store_raw(&dst_loc, &src_loc, SIR.ty(&dst_ip.ty()).size());
}

/// Stores src_loc into dst_loc.
pub(crate) fn store_raw(&mut self, dst_loc: &Location, src_loc: &Location, size: u64) {
pub(super) fn store_raw(&mut self, dst_loc: &Location, src_loc: &Location, size: u64) {
// This is the one place in the compiler where we allow an explosion of cases over the
// variants of `Location`. If elsewhere you find yourself matching over a pair of locations
// you should try and re-work you code so it calls this.
Expand Down
6 changes: 3 additions & 3 deletions internal_ws/ykcompile/src/stack_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,15 @@ use dynasmrt::{x64::Rq::RBP, Register};
use std::convert::{TryFrom, TryInto};

#[derive(Default, Debug)]
pub(crate) struct StackBuilder {
pub(super) struct StackBuilder {
/// Keeps track of how many bytes have been allocated.
stack_top: u64,
}

impl StackBuilder {
/// Allocate an object of given size and alignment on the stack, returning a `Location::Mem`
/// describing the position of the allocation. The stack is assumed to grow down.
pub(crate) fn alloc(&mut self, size: u64, align: u64) -> Location {
pub(super) fn alloc(&mut self, size: u64, align: u64) -> Location {
self.align(align);
self.stack_top += size;
Location::new_mem(RBP.code(), -i32::try_from(self.stack_top).unwrap())
Expand All @@ -32,7 +32,7 @@ impl StackBuilder {
}

/// Total allocated stack size in bytes.
pub(crate) fn size(&self) -> u32 {
pub(super) fn size(&self) -> u32 {
self.stack_top.try_into().unwrap()
}
}
Expand Down
6 changes: 3 additions & 3 deletions internal_ws/ykpack/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ pub struct TupleTy {
}

impl TupleTy {
pub fn is_unit(&self) -> bool {
fn is_unit(&self) -> bool {
self.fields.offsets.is_empty()
}
}
Expand Down Expand Up @@ -306,7 +306,7 @@ pub struct Body {
pub blocks: Vec<BasicBlock>,
pub flags: BodyFlags,
pub local_decls: Vec<LocalDecl>,
pub num_args: usize,
pub(super) num_args: usize,
pub layout: (usize, usize),
pub offsets: Vec<usize>,
}
Expand Down Expand Up @@ -593,7 +593,7 @@ pub enum ConstantInt {
impl ConstantInt {
/// Returns an i64 value suitable for loading into a register.
/// If the constant is signed, then it will be sign-extended.
pub fn i64_cast(&self) -> i64 {
fn i64_cast(&self) -> i64 {
match self {
ConstantInt::UnsignedInt(ui) => match ui {
UnsignedInt::U8(i) => *i as i64,
Expand Down
3 changes: 2 additions & 1 deletion internal_ws/yksg/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ pub struct FrameInfo {

/// Heap allocated memory for writing and reading locals of a stack frame.
#[derive(Debug)]
pub struct LocalMem {
pub(crate) struct LocalMem {
/// Pointer to allocated memory containing a frame's locals.
locals: *mut u8,
/// The offset of each Local into locals.
Expand Down Expand Up @@ -198,6 +198,7 @@ pub struct StopgapInterpreter {

impl StopgapInterpreter {
/// Initialise the interpreter from a symbol name.
#[cfg(feature = "testing")]
pub fn from_symbol(sym: String) -> Self {
let frame = StopgapInterpreter::create_frame(&sym);
StopgapInterpreter {
Expand Down
2 changes: 1 addition & 1 deletion internal_ws/yktrace/src/swt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ impl ThreadTracerImpl for SWTThreadTracer {
}
}

pub(crate) fn start_tracing() -> ThreadTracer {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if this is right. Why would starting tracing be a testing feature?

It's probably because your tool build with hardware tracing rustflags (and not software tracing)?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly for the other bits in this file.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's probably because your tool build with hardware tracing rustflags (and not software tracing)?

Yes, I used hw. I didn't realise that there'd be a difference here with sw? Would a bors try flush this out?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't realise that there'd be a difference here with sw?

My guess is that compile-time guards like this would give inaccurate results if the oracle uses only -C tracer=hw.

Ideally your oracle would include both kinds of tracing, but read on...

Would a bors try flush this out?

No, because we don't yet test software tracing with cranelift. I'm not sure it's ready yet, but @bjorn3 is the person to ask.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having looked at this again, I agree that applying this to swt.rs is incorrect. Fixed in 0703d94.

pub(super) fn start_tracing() -> ThreadTracer {
TRACE_BUF.with(|trace_buf| {
assert!(trace_buf.is_empty());
});
Expand Down
24 changes: 12 additions & 12 deletions tests/src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@ use regex::Regex;
use ykshim_client::{sir_body_ret_ty, TirTrace, TypeId};

extern "C" {
pub fn add6(a: u64, b: u64, c: u64, d: u64, e: u64, f: u64) -> u64;
pub(super) fn add6(a: u64, b: u64, c: u64, d: u64, e: u64, f: u64) -> u64;
}
extern "C" {
pub fn add_some(a: u64, b: u64, c: u64, d: u64, e: u64) -> u64;
pub(super) fn add_some(a: u64, b: u64, c: u64, d: u64, e: u64) -> u64;
}

/// Fuzzy matches the textual TIR for the trace `tt` with the pattern `ptn`.
pub fn assert_tir(ptn: &str, tt: &TirTrace) {
pub(super) fn assert_tir(ptn: &str, tt: &TirTrace) {
let ptn_re = Regex::new(r"%.+?\b").unwrap(); // Names are words prefixed with `%`.
let text_re = Regex::new(r"\$?.+?\b").unwrap(); // Any word optionally prefixed with `$`.
let matcher = FMBuilder::new(ptn)
Expand All @@ -28,7 +28,7 @@ pub fn assert_tir(ptn: &str, tt: &TirTrace) {
}
}

pub fn neg_assert_tir(ptn: &str, tt: &TirTrace) {
pub(super) fn neg_assert_tir(ptn: &str, tt: &TirTrace) {
let ptn_re = Regex::new(r"%.+?\b").unwrap(); // Names are words prefixed with `%`.
let text_re = Regex::new(r"\$?.+?\b").unwrap(); // Any word optionally prefixed with `$`.
let matcher = FMBuilder::new(ptn)
Expand All @@ -46,17 +46,17 @@ pub fn neg_assert_tir(ptn: &str, tt: &TirTrace) {

/// Types IDs that we need for tests.
#[repr(C)]
pub struct TestTypes {
pub t_u8: TypeId,
pub t_i64: TypeId,
pub t_string: TypeId,
pub t_tiny_struct: TypeId,
pub t_tiny_array: TypeId,
pub t_tiny_tuple: TypeId,
pub(super) struct TestTypes {
pub(super) t_u8: TypeId,
pub(super) t_i64: TypeId,
pub(super) t_string: TypeId,
pub(super) t_tiny_struct: TypeId,
pub(super) t_tiny_array: TypeId,
pub(super) t_tiny_tuple: TypeId,
}

impl TestTypes {
pub fn new() -> TestTypes {
pub(super) fn new() -> TestTypes {
// We can't know the type ID of any given type, so this works by defining unmangled
// functions with known return types and then looking them up by name in the SIR.
#[no_mangle]
Expand Down
2 changes: 1 addition & 1 deletion tests/src/tracing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ fn trace_twice() {

/// Test that tracing in different threads works.
#[test]
pub(crate) fn trace_concurrent() {
fn trace_concurrent() {
#[cfg(tracermode = "hw")]
let kind = TracingKind::HardwareTracing;
#[cfg(tracermode = "sw")]
Expand Down
2 changes: 1 addition & 1 deletion ykrt/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
#![cfg_attr(test, feature(test))]

mod location;
pub mod mt;
pub(crate) mod mt;

pub use self::location::Location;
pub use self::mt::{MTBuilder, MT};
Expand Down
56 changes: 28 additions & 28 deletions ykrt/src/location.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,14 +109,14 @@ impl<I> Location<I> {
}

/// Return this Location's internal state.
pub(crate) fn load(&self, order: Ordering) -> State<I> {
pub(super) fn load(&self, order: Ordering) -> State<I> {
State {
x: self.state.load(order),
marker: PhantomData,
}
}

pub(crate) fn compare_exchange_weak(
pub(super) fn compare_exchange_weak(
&self,
current: State<I>,
new: State<I>,
Expand All @@ -140,7 +140,7 @@ impl<I> Location<I> {

/// Locks this `State` with `Acquire` ordering. If the location was in, or moves to, the
/// Counting state this will return `Err`.
pub(crate) fn lock(&self) -> Result<State<I>, ()> {
pub(super) fn lock(&self) -> Result<State<I>, ()> {
{
let ls = self.load(Ordering::Relaxed);
if ls.is_counting() {
Expand Down Expand Up @@ -227,7 +227,7 @@ impl<I> Location<I> {
}

/// Unlocks this `State` with `Release` ordering.
pub(crate) fn unlock(&self) {
pub(super) fn unlock(&self) {
let ls = self.load(Ordering::Relaxed);
debug_assert!(ls.is_locked());
debug_assert!(!ls.is_counting());
Expand Down Expand Up @@ -271,7 +271,7 @@ impl<I> Location<I> {
}

/// Try obtaining the lock, returning the new `State` if successful.
pub(crate) fn try_lock(&self) -> Result<State<I>, ()> {
pub(super) fn try_lock(&self) -> Result<State<I>, ()> {
let mut ls = self.load(Ordering::Relaxed);
loop {
if ls.is_locked() {
Expand Down Expand Up @@ -300,20 +300,20 @@ impl<I> Drop for Location<I> {
}

#[cfg(target_pointer_width = "64")]
pub(crate) const STATE_TAG: usize = 0b111; // All of the other tag data must fit in this.
const STATE_TAG: usize = 0b111; // All of the other tag data must fit in this.
#[cfg(target_pointer_width = "64")]
pub(crate) const STATE_NUM_BITS: usize = 3;
const STATE_NUM_BITS: usize = 3;

pub(crate) const STATE_IS_LOCKED: usize = 0b001;
pub(crate) const STATE_IS_PARKED: usize = 0b010;
pub(crate) const STATE_IS_COUNTING: usize = 0b100;
const STATE_IS_LOCKED: usize = 0b001;
const STATE_IS_PARKED: usize = 0b010;
const STATE_IS_COUNTING: usize = 0b100;

pub(crate) const TOKEN_NORMAL: UnparkToken = UnparkToken(0);
pub(crate) const TOKEN_HANDOFF: UnparkToken = UnparkToken(1);
const TOKEN_NORMAL: UnparkToken = UnparkToken(0);
const TOKEN_HANDOFF: UnparkToken = UnparkToken(1);

#[derive(PartialEq, Eq, Debug)]
pub(crate) struct State<I> {
pub(crate) x: usize,
pub(super) struct State<I> {
x: usize,
marker: PhantomData<I>,
}

Expand All @@ -327,7 +327,7 @@ impl<I> Clone for State<I> {

impl<I> State<I> {
/// Return a new `State` in the counting phase with a count 0.
pub(crate) fn new() -> Self {
pub(super) fn new() -> Self {
State {
x: STATE_IS_COUNTING,
marker: PhantomData,
Expand All @@ -341,21 +341,21 @@ impl<I> State<I> {
}
}

pub(crate) fn is_locked(&self) -> bool {
pub(super) fn is_locked(&self) -> bool {
self.x & STATE_IS_LOCKED != 0
}

pub(crate) fn is_parked(&self) -> bool {
pub(super) fn is_parked(&self) -> bool {
self.x & STATE_IS_PARKED != 0
}

/// Is the Location in the counting or a non-counting state?
pub(crate) fn is_counting(&self) -> bool {
pub(super) fn is_counting(&self) -> bool {
self.x & STATE_IS_COUNTING != 0
}

/// If, and only if, the Location is in the counting state, return the current count.
pub(crate) fn count(&self) -> HotThreshold {
pub(super) fn count(&self) -> HotThreshold {
debug_assert!(self.is_counting());
debug_assert!(!self.is_locked());
u32::try_from(self.x >> STATE_NUM_BITS).unwrap()
Expand All @@ -364,38 +364,38 @@ impl<I> State<I> {
/// If this `State` is not counting, return its `HotLocation`. It is undefined behaviour to
/// call this function if this `State` is in the counting phase and/or if this `State` is not
/// locked.
pub(crate) unsafe fn hot_location(&self) -> &mut HotLocation<I> {
pub(super) unsafe fn hot_location(&self) -> &mut HotLocation<I> {
debug_assert!(!self.is_counting());
//debug_assert!(self.is_locked());
&mut *((self.x & !STATE_TAG) as *mut _)
}

/// Return a version of this `State` with the locked bit set.
pub(crate) fn with_lock(&self) -> State<I> {
pub(super) fn with_lock(&self) -> State<I> {
State {
x: self.x | STATE_IS_LOCKED,
marker: PhantomData,
}
}

/// Return a version of this `State` with the locked bit unset.
pub(crate) fn with_unlock(&self) -> State<I> {
fn with_unlock(&self) -> State<I> {
State {
x: self.x & !STATE_IS_LOCKED,
marker: PhantomData,
}
}

/// Return a version of this `State` with the parked bit set.
pub(crate) fn with_parked(&self) -> State<I> {
fn with_parked(&self) -> State<I> {
State {
x: self.x | STATE_IS_PARKED,
marker: PhantomData,
}
}

/// Return a version of this `State` with the parked bit unset.
pub(crate) fn with_unparked(&self) -> State<I> {
fn with_unparked(&self) -> State<I> {
State {
x: self.x & !STATE_IS_PARKED,
marker: PhantomData,
Expand All @@ -404,7 +404,7 @@ impl<I> State<I> {

/// Return a version of this `State` with the count set to `count`. It is undefined behaviour
/// to call this function if this `State` is not in the counting phase.
pub(crate) fn with_count(&self, count: HotThreshold) -> Self {
pub(super) fn with_count(&self, count: HotThreshold) -> Self {
debug_assert!(self.is_counting());
debug_assert_eq!(count << STATE_NUM_BITS >> STATE_NUM_BITS, count);
State {
Expand All @@ -415,7 +415,7 @@ impl<I> State<I> {

/// Set this `State`'s `HotLocation`. It is undefined behaviour for this `State` to already
/// have a `HotLocation`.
pub(crate) fn with_hotlocation(&self, hl_ptr: *mut HotLocation<I>) -> Self {
pub(super) fn with_hotlocation(&self, hl_ptr: *mut HotLocation<I>) -> Self {
debug_assert!(self.is_counting());
let hl_ptr = hl_ptr as usize;
debug_assert_eq!(hl_ptr & !STATE_TAG, hl_ptr);
Expand All @@ -428,11 +428,11 @@ impl<I> State<I> {

/// An opaque struct used by `MTThreadInner` to help identify if a thread that started a trace is
/// still active.
pub(crate) struct ThreadIdInner;
pub(super) struct ThreadIdInner;

/// A `Location`'s non-counting states.
#[derive(EnumDiscriminants)]
pub(crate) enum HotLocation<I> {
pub(super) enum HotLocation<I> {
Compiled(Box<CompiledTrace<I>>),
Compiling(Arc<Mutex<Option<Box<CompiledTrace<I>>>>>),
DontTrace,
Expand Down
5 changes: 4 additions & 1 deletion ykshim_client/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,12 @@ pub type RawStopgapInterpreter = c_void;
pub type LocalIndex = u32;
#[derive(PartialEq, Eq, Debug, Clone, Copy, Hash)]
#[repr(C)]
#[cfg(feature = "testing")]
pub struct Local(pub LocalIndex);
#[derive(Debug, Clone, Copy)]
#[repr(C)]
pub struct TyIndex(pub u32);
#[cfg(feature = "testing")]
pub(crate) struct TyIndex(pub(crate) u32);

extern "C" {
fn __ykshim_start_tracing(tracing_kind: u8) -> *mut RawThreadTracer;
Expand Down Expand Up @@ -149,6 +151,7 @@ impl<I> CompiledTrace<I> {
}

/// Execute the trace with the given interpreter context.
#[cfg(feature = "testing")]
pub unsafe fn execute(&self, ctx: &mut I) -> *mut RawStopgapInterpreter {
let f = mem::transmute::<_, fn(&mut I) -> *mut RawStopgapInterpreter>(self.ptr());
#[cfg(not(debug_assertions))]
Expand Down
Loading