Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unbreak YKD_OPT=0. #1523

Merged
merged 1 commit into from
Dec 20, 2024
Merged
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
47 changes: 47 additions & 0 deletions tests/c/ykd_opt_off.c
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
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
Loading