Skip to content

Commit

Permalink
Merge pull request #1523 from ltratt/unbreak_ykd_opt_zero
Browse files Browse the repository at this point in the history 
Unbreak `YKD_OPT=0`.
  • Loading branch information
@vext01
vext01 authored Dec 20, 2024
2 parents 40f5cad + 3508eca commit 7565ece
Show file tree
Hide file tree
Showing 6 changed files with 192 additions and 78 deletions.
47 changes: 47 additions & 0 deletions tests/c/ykd_opt_off.c
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// Run-time:
// env-var: YKD_OPT=0
// env-var: YKD_SERIALISE_COMPILATION=1
// stdout:
// 10
// 9
// 8
// 7
// 7: 6
// 7: 5
// 7: 4
// 7: 4: 3
// 7: 4: 2
// 7: 4: 1


// Check that basic trace compilation works.

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

int main(int argc, char **argv) {
YkMT *mt = yk_mt_new(NULL);
yk_mt_hot_threshold_set(mt, 0);
yk_mt_sidetrace_threshold_set(mt, 1);
YkLocation loc = yk_location_new();

int i = 10;
NOOPT_VAL(loc);
NOOPT_VAL(i);
while (i > 0) {
yk_mt_control_point(mt, &loc);
if (i < 7)
printf("7: ");
if (i < 4)
printf("4: ");
printf("%d\n", i);
i--;
}
yk_location_drop(loc);
yk_mt_shutdown(mt);
return (EXIT_SUCCESS);
}
160 changes: 91 additions & 69 deletions ykrt/src/compile/jitc_yk/codegen/x64/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
use super::{
super::{
int_signs::{SignExtend, Truncate},
jit_ir::{self, BinOp, FloatTy, Inst, InstIdx, Module, Operand, PtrAddInst, Ty},
jit_ir::{self, BinOp, FloatTy, Inst, InstIdx, Module, Operand, PtrAddInst, TraceKind, Ty},
CompilationError,
},
reg_alloc::{self, VarLocation},
Expand Down Expand Up @@ -519,7 +519,7 @@ impl<'a> Assemble<'a> {
jit_ir::Inst::Guard(i) => self.cg_guard(iidx, i),
jit_ir::Inst::TraceHeaderStart => self.cg_header_start(),
jit_ir::Inst::TraceHeaderEnd => {
self.cg_header_end();
self.cg_header_end(iidx);
in_header = false;
}
jit_ir::Inst::TraceBodyStart => self.cg_body_start(),
Expand Down Expand Up @@ -1791,12 +1791,11 @@ impl<'a> Assemble<'a> {
/// # Arguments
///
/// * `tgt_vars` - The target locations. If `None` use `self.loop_start_locs` instead.
fn write_jump_vars(&mut self, iidx: InstIdx, is_sidetrace: bool) {
let (tgt_vars, src_ops) = if is_sidetrace {
// Side-traces don't have a body and store these variables in `trace_header_end`.
(self.m.root_entry_vars(), self.m.trace_header_end())
} else {
(self.body_start_locs.as_slice(), self.m.trace_body_end())
fn write_jump_vars(&mut self, iidx: InstIdx) {
let (tgt_vars, src_ops) = match self.m.tracekind() {
TraceKind::HeaderOnly => (self.header_start_locs.as_slice(), self.m.trace_header_end()),
TraceKind::HeaderAndBody => (self.body_start_locs.as_slice(), self.m.trace_body_end()),
TraceKind::Sidetrace => (self.m.root_entry_vars(), self.m.trace_header_end()),
};
// If we pass in `None` use `self.loop_start_locs` instead. We need to do this since we
// can't pass in `&self.loop_start_locs` directly due to borrowing restrictions.
Expand Down Expand Up @@ -1889,13 +1888,14 @@ impl<'a> Assemble<'a> {
}

fn cg_body_end(&mut self, iidx: InstIdx) {
debug_assert_eq!(self.m.tracekind(), TraceKind::HeaderAndBody);
// Loop the JITted code if the `tloop_start` label is present (not relevant for IR created
// by a test or a side-trace).
let label = StaticLabel::global("tloop_start");
match self.asm.labels().resolve_static(&label) {
Ok(_) => {
// Found the label, emit a jump to it.
self.write_jump_vars(iidx, false);
self.write_jump_vars(iidx);
dynasm!(self.asm; jmp ->tloop_start);
}
Err(DynasmError::UnknownLabel(_)) => {
Expand All @@ -1917,9 +1917,10 @@ impl<'a> Assemble<'a> {
}

fn cg_sidetrace_end(&mut self, iidx: InstIdx, addr: *const libc::c_void) {
debug_assert_eq!(self.m.tracekind(), TraceKind::Sidetrace);
// The end of a side-trace. Map live variables of this side-trace to the entry variables of
// the root parent trace, then jump to it.
self.write_jump_vars(iidx, true);
self.write_jump_vars(iidx);
self.ra.align_stack(SYSV_CALL_STACK_ALIGN);

dynasm!(self.asm
Expand All @@ -1944,60 +1945,80 @@ impl<'a> Assemble<'a> {
};
self.header_start_locs.push(loc);
}
dynasm!(self.asm; ->reentry:);
match self.m.tracekind() {
TraceKind::HeaderOnly => {
dynasm!(self.asm; ->tloop_start:);
}
TraceKind::HeaderAndBody => {
dynasm!(self.asm; ->reentry:);
}
TraceKind::Sidetrace => todo!(),
}
self.prologue_offset = self.asm.offset();
}

fn cg_header_end(&mut self) {
// FIXME: This is a bit of a roundabout way of doing things. Especially, since it means
// that the [ParamInst]s in the trace body are just placeholders. While, since a recent
// change, the register allocator makes sure the values automatically end up in the
// [VarLocation]s expected by the loop start, this only works for registers right now. We
// can extend this to spill locations as well, but won't be able to do so for variables
// that have become constants during the trace header. So we will always have to either
// update the [ParamInst]s of the trace body, which isn't ideal since it requires the
// [Module] the be mutable. Or we do what we do below just for constants.
let mut varlocs = Vec::new();
for var in self.m.trace_header_end().iter() {
let varloc = self.op_to_var_location(var.unpack(self.m));
varlocs.push(varloc);
}
// Reset the register allocator before priming it with information about the trace body
// inputs.
self.ra.reset();
for (i, op) in self.m.trace_body_start().iter().enumerate() {
// By definition these can only be variables.
let iidx = match op.unpack(self.m) {
Operand::Var(iidx) => iidx,
_ => panic!(),
};
let varloc = varlocs[i];

// Write the varlocations from the head jump to the body start.
// FIXME: This is copied verbatim from `cg_param` and can be reused.
match varloc {
VarLocation::Register(reg_alloc::Register::GP(reg)) => {
self.ra.force_assign_inst_gp_reg(&mut self.asm, iidx, reg);
}
VarLocation::Register(reg_alloc::Register::FP(reg)) => {
self.ra.force_assign_inst_fp_reg(iidx, reg);
}
VarLocation::Direct { frame_off, size: _ } => {
self.ra.force_assign_inst_direct(iidx, frame_off);
}
VarLocation::Stack { frame_off, size: _ } => {
self.ra
.force_assign_inst_indirect(iidx, i32::try_from(frame_off).unwrap());
fn cg_header_end(&mut self, iidx: InstIdx) {
match self.m.tracekind() {
TraceKind::HeaderOnly => {
self.write_jump_vars(iidx);
dynasm!(self.asm; jmp ->tloop_start);
}
TraceKind::HeaderAndBody => {
// FIXME: This is a bit of a roundabout way of doing things. Especially, since it means
// that the [ParamInst]s in the trace body are just placeholders. While, since a recent
// change, the register allocator makes sure the values automatically end up in the
// [VarLocation]s expected by the loop start, this only works for registers right now. We
// can extend this to spill locations as well, but won't be able to do so for variables
// that have become constants during the trace header. So we will always have to either
// update the [ParamInst]s of the trace body, which isn't ideal since it requires the
// [Module] the be mutable. Or we do what we do below just for constants.
let mut varlocs = Vec::new();
for var in self.m.trace_header_end().iter() {
let varloc = self.op_to_var_location(var.unpack(self.m));
varlocs.push(varloc);
}
VarLocation::ConstInt { bits, v } => {
self.ra.assign_const(iidx, bits, v);
// Reset the register allocator before priming it with information about the trace body
// inputs.
self.ra.reset();
for (i, op) in self.m.trace_body_start().iter().enumerate() {
// By definition these can only be variables.
let iidx = match op.unpack(self.m) {
Operand::Var(iidx) => iidx,
_ => panic!(),
};
let varloc = varlocs[i];

// Write the varlocations from the head jump to the body start.
// FIXME: This is copied verbatim from `cg_param` and can be reused.
match varloc {
VarLocation::Register(reg_alloc::Register::GP(reg)) => {
self.ra.force_assign_inst_gp_reg(&mut self.asm, iidx, reg);
}
VarLocation::Register(reg_alloc::Register::FP(reg)) => {
self.ra.force_assign_inst_fp_reg(iidx, reg);
}
VarLocation::Direct { frame_off, size: _ } => {
self.ra.force_assign_inst_direct(iidx, frame_off);
}
VarLocation::Stack { frame_off, size: _ } => {
self.ra.force_assign_inst_indirect(
iidx,
i32::try_from(frame_off).unwrap(),
);
}
VarLocation::ConstInt { bits, v } => {
self.ra.assign_const(iidx, bits, v);
}
e => panic!("{:?}", e),
}
}
e => panic!("{:?}", e),
}
TraceKind::Sidetrace => todo!(),
}
}

fn cg_body_start(&mut self) {
debug_assert_eq!(self.m.tracekind(), TraceKind::HeaderAndBody);
debug_assert_eq!(self.body_start_locs.len(), 0);
// Remember the locations of the live variables at the beginning of the trace loop. When we
// loop back around here we need to write the live variables back into these same
Expand Down Expand Up @@ -2495,7 +2516,7 @@ enum Immediate {
mod tests {
use super::{Assemble, X64CompiledTrace};
use crate::compile::{
jitc_yk::jit_ir::{self, Inst, Module, ParamIdx},
jitc_yk::jit_ir::{self, Inst, Module, ParamIdx, TraceKind},
CompiledTrace,
};
use crate::location::{HotLocation, HotLocationKind};
Expand Down Expand Up @@ -3984,12 +4005,13 @@ mod tests {
);
}

#[should_panic]
#[test]
fn unterminated_trace() {
codegen_and_test(
"
entry:
body_end []
header_end []
",
"
...
Expand All @@ -4006,13 +4028,13 @@ mod tests {
codegen_and_test(
"
entry:
body_start []
body_end []
header_start []
header_end []
",
"
...
; body_start []
; body_end []
; header_start []
; header_end []
jmp {{target}}
",
false,
Expand All @@ -4025,20 +4047,20 @@ mod tests {
"
entry:
%0: i8 = param 0
body_start [%0]
header_start [%0]
%2: i8 = add %0, %0
black_box %2
body_end [%0]
header_end [%0]
",
"
...
; %0: i8 = param ...
...
; body_start [%0]
; header_start [%0]
; %2: i8 = add %0, %0
{{_}} {{off}}: ...
...
; body_end [%0]
; header_end [%0]
...
{{_}} {{_}}: jmp 0x00000000{{off}}
",
Expand Down Expand Up @@ -4531,16 +4553,16 @@ mod tests {
"
entry:
%0: i8 = param 0
body_start [%0]
header_start [%0]
%2: i8 = 42i8
body_end [%2]
header_end [%2]
",
"
...
; %0: i8 = param ...
...
; body_start [%0]
; body_end [42i8]
; header_start [%0]
; header_end [42i8]
mov r.64.x, 0x2a
jmp ...
",
Expand Down Expand Up @@ -4578,7 +4600,7 @@ mod tests {

#[test]
fn cg_aliasing_params() {
let mut m = jit_ir::Module::new(0, 0).unwrap();
let mut m = jit_ir::Module::new(TraceKind::HeaderOnly, 0, 0).unwrap();

// Create two trace paramaters whose locations alias.
let loc = yksmp::Location::Register(13, 1, [].into());
Expand Down
Loading

0 comments on commit 7565ece

Please sign in to comment.