From 90137f519459764a78ae8eb777e3f396f7cffd98 Mon Sep 17 00:00:00 2001 From: Maxime Chevalier-Boisvert Date: Wed, 20 Jul 2022 10:43:14 -0400 Subject: Implement PosMarker instruction (https://github.com/Shopify/ruby/pull/328) * Implement PosMarker instruction * Implement PosMarker in the arm backend * Make bindgen run only for clang image * Fix if-else in cirrus CI file * Add missing semicolon * Try removing trailing semicolon * Try to fix shell/YAML syntax Co-authored-by: Alan Wu --- .cirrus.yml | 7 ++++- yjit/src/backend/arm64/mod.rs | 21 ++++++++++---- yjit/src/backend/ir.rs | 64 +++++++++++++++++++++++++----------------- yjit/src/backend/x86_64/mod.rs | 15 +++++++--- yjit/src/codegen.rs | 19 +++++++------ yjit/src/core.rs | 20 +++++++++++-- 6 files changed, 99 insertions(+), 47 deletions(-) diff --git a/.cirrus.yml b/.cirrus.yml index 3e03d0adc3..293873af5b 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -116,7 +116,12 @@ yjit_task: --prefix="$RUBY_PREFIX" --enable-yjit=dev make_miniruby_script: source $HOME/.cargo/env && make -j miniruby - make_bindgen_script: source $HOME/.cargo/env && make -j yjit-bindgen + make_bindgen_script: | + if [[ "$CC" = "clang-12" ]]; then + source $HOME/.cargo/env && make -j yjit-bindgen + else + echo "only running bindgen on clang image" + fi boot_miniruby_script: RUST_BACKTRACE=1 ./miniruby --yjit-call-threshold=1 -e0 test_dump_insns_script: RUST_BACKTRACE=1 ./miniruby --yjit-call-threshold=1 --yjit-dump-insns -e0 # output_stats_script: RUST_BACKTRACE=1 ./miniruby --yjit-call-threshold=1 --yjit-stats -e0 diff --git a/yjit/src/backend/arm64/mod.rs b/yjit/src/backend/arm64/mod.rs index df4fcceec6..be329f61cf 100644 --- a/yjit/src/backend/arm64/mod.rs +++ b/yjit/src/backend/arm64/mod.rs @@ -81,7 +81,7 @@ impl Assembler /// have no memory operands. fn arm64_split(mut self) -> Assembler { - self.forward_pass(|asm, index, op, opnds, target, text| { + self.forward_pass(|asm, index, op, opnds, target, text, pos_marker| { // Load all Value operands into registers that aren't already a part // of Load instructions. let opnds = match op { @@ -103,15 +103,15 @@ impl Assembler (Opnd::Mem(_), Opnd::Mem(_)) => { let opnd0 = asm.load(opnds[0]); let opnd1 = asm.load(opnds[1]); - asm.push_insn(op, vec![opnd0, opnd1], target, text); + asm.push_insn(op, vec![opnd0, opnd1], target, text, pos_marker); }, (mem_opnd @ Opnd::Mem(_), other_opnd) | (other_opnd, mem_opnd @ Opnd::Mem(_)) => { let opnd0 = asm.load(mem_opnd); - asm.push_insn(op, vec![opnd0, other_opnd], target, text); + asm.push_insn(op, vec![opnd0, other_opnd], target, text, pos_marker); }, _ => { - asm.push_insn(op, opnds, target, text); + asm.push_insn(op, opnds, target, text, pos_marker); } } }, @@ -146,7 +146,7 @@ impl Assembler } }).collect(); - asm.push_insn(op, new_opnds, target, text); + asm.push_insn(op, new_opnds, target, text, pos_marker); }, Op::IncrCounter => { // Every operand to the IncrCounter instruction need to be a @@ -250,7 +250,7 @@ impl Assembler asm.test(opnd0, opnds[1]); }, _ => { - asm.push_insn(op, opnds, target, text); + asm.push_insn(op, opnds, target, text, pos_marker); } }; }) @@ -402,9 +402,18 @@ impl Assembler cb.add_comment(&insn.text.as_ref().unwrap()); } }, + Op::Label => { cb.write_label(insn.target.unwrap().unwrap_label_idx()); }, + + // Report back the current position in the generated code + Op::PosMarker => { + let pos = cb.get_write_ptr(); + let pos_marker_fn = insn.pos_marker.as_ref().unwrap(); + pos_marker_fn(pos); + } + Op::BakeString => { let str = insn.text.as_ref().unwrap(); for byte in str.as_bytes() { diff --git a/yjit/src/backend/ir.rs b/yjit/src/backend/ir.rs index de59b420c1..13a5c5c3d3 100644 --- a/yjit/src/backend/ir.rs +++ b/yjit/src/backend/ir.rs @@ -35,6 +35,9 @@ pub enum Op // Add a label into the IR at the point that this instruction is added. Label, + // Mark a position in the generated code + PosMarker, + // Bake a string directly into the instruction stream. BakeString, @@ -342,8 +345,9 @@ impl From for Target { } } +type PosMarkerFn = Box; + /// YJIT IR instruction -#[derive(Clone)] pub struct Insn { // Opcode for the instruction @@ -361,9 +365,9 @@ pub struct Insn // List of branch targets (branch instructions only) pub(super) target: Option, - // Position in the generated machine code - // Useful for comments and for patching jumps - pub(super) pos: Option, + // Callback to mark the position of this instruction + // in the generated code + pub(super) pos_marker: Option, } impl fmt::Debug for Insn { @@ -387,9 +391,6 @@ impl fmt::Debug for Insn { if let Some(target) = self.target { write!(fmt, " target={target:?}")?; } - if let Some(pos) = self.pos { - write!(fmt, " pos={pos:?}")?; - } write!(fmt, " -> {:?}", self.out) } @@ -420,7 +421,14 @@ impl Assembler } /// Append an instruction to the list - pub(super) fn push_insn(&mut self, op: Op, opnds: Vec, target: Option, text: Option) -> Opnd + pub(super) fn push_insn( + &mut self, + op: Op, + opnds: Vec, + target: Option, + text: Option, + pos_marker: Option + ) -> Opnd { // Index of this instruction let insn_idx = self.insns.len(); @@ -471,7 +479,7 @@ impl Assembler opnds, out: out_opnd, target, - pos: None + pos_marker, }; self.insns.push(insn); @@ -490,7 +498,7 @@ impl Assembler opnds: vec![], out: Opnd::None, target: None, - pos: None + pos_marker: None, }; self.insns.push(insn); self.live_ranges.push(self.insns.len()); @@ -505,7 +513,7 @@ impl Assembler opnds: vec![], out: Opnd::None, target: None, - pos: None + pos_marker: None, }; self.insns.push(insn); self.live_ranges.push(self.insns.len()); @@ -514,7 +522,7 @@ impl Assembler /// Load an address relative to the given label. #[must_use] pub fn lea_label(&mut self, target: Target) -> Opnd { - self.push_insn(Op::LeaLabel, vec![], Some(target), None) + self.push_insn(Op::LeaLabel, vec![], Some(target), None, None) } /// Create a new label instance that we can jump to @@ -538,7 +546,7 @@ impl Assembler opnds: vec![], out: Opnd::None, target: Some(label), - pos: None + pos_marker: None, }; self.insns.push(insn); self.live_ranges.push(self.insns.len()); @@ -546,7 +554,7 @@ impl Assembler /// Transform input instructions, consumes the input assembler pub(super) fn forward_pass(mut self, mut map_insn: F) -> Assembler - where F: FnMut(&mut Assembler, usize, Op, Vec, Option, Option) + where F: FnMut(&mut Assembler, usize, Op, Vec, Option, Option, Option) { let mut asm = Assembler { insns: Vec::default(), @@ -582,7 +590,7 @@ impl Assembler asm.comment(insn.text.unwrap().as_str()); }, _ => { - map_insn(&mut asm, index, insn.op, opnds, insn.target, insn.text); + map_insn(&mut asm, index, insn.op, opnds, insn.target, insn.text, insn.pos_marker); } }; @@ -644,7 +652,7 @@ impl Assembler let live_ranges: Vec = std::mem::take(&mut self.live_ranges); - let asm = self.forward_pass(|asm, index, op, opnds, target, text| { + let asm = self.forward_pass(|asm, index, op, opnds, target, text, pos_marker| { // Check if this is the last instruction that uses an operand that // spans more than one instruction. In that case, return the // allocated register to the pool. @@ -725,7 +733,7 @@ impl Assembler } ).collect(); - asm.push_insn(op, reg_opnds, target, text); + asm.push_insn(op, reg_opnds, target, text, pos_marker); // Set the output register for this instruction let num_insns = asm.insns.len(); @@ -776,7 +784,13 @@ impl Assembler pub fn ccall(&mut self, fptr: *const u8, opnds: Vec) -> Opnd { let target = Target::FunPtr(fptr); - self.push_insn(Op::CCall, opnds, Some(target), None) + self.push_insn(Op::CCall, opnds, Some(target), None, None) + } + + //pub fn pos_marker(&mut self, marker_fn: F) + pub fn pos_marker(&mut self, marker_fn: PosMarkerFn) + { + self.push_insn(Op::PosMarker, vec![], None, None, Some(marker_fn)); } } @@ -786,7 +800,7 @@ macro_rules! def_push_jcc { { pub fn $op_name(&mut self, target: Target) { - self.push_insn($opcode, vec![], Some(target), None); + self.push_insn($opcode, vec![], Some(target), None, None); } } }; @@ -799,7 +813,7 @@ macro_rules! def_push_0_opnd { #[must_use] pub fn $op_name(&mut self) -> Opnd { - self.push_insn($opcode, vec![], None, None) + self.push_insn($opcode, vec![], None, None, None) } } }; @@ -811,7 +825,7 @@ macro_rules! def_push_0_opnd_no_out { { pub fn $op_name(&mut self) { - self.push_insn($opcode, vec![], None, None); + self.push_insn($opcode, vec![], None, None, None); } } }; @@ -824,7 +838,7 @@ macro_rules! def_push_1_opnd { #[must_use] pub fn $op_name(&mut self, opnd0: Opnd) -> Opnd { - self.push_insn($opcode, vec![opnd0], None, None) + self.push_insn($opcode, vec![opnd0], None, None, None) } } }; @@ -836,7 +850,7 @@ macro_rules! def_push_1_opnd_no_out { { pub fn $op_name(&mut self, opnd0: Opnd) { - self.push_insn($opcode, vec![opnd0], None, None); + self.push_insn($opcode, vec![opnd0], None, None, None); } } }; @@ -849,7 +863,7 @@ macro_rules! def_push_2_opnd { #[must_use] pub fn $op_name(&mut self, opnd0: Opnd, opnd1: Opnd) -> Opnd { - self.push_insn($opcode, vec![opnd0, opnd1], None, None) + self.push_insn($opcode, vec![opnd0, opnd1], None, None, None) } } }; @@ -861,7 +875,7 @@ macro_rules! def_push_2_opnd_no_out { { pub fn $op_name(&mut self, opnd0: Opnd, opnd1: Opnd) { - self.push_insn($opcode, vec![opnd0, opnd1], None, None); + self.push_insn($opcode, vec![opnd0, opnd1], None, None, None); } } }; diff --git a/yjit/src/backend/x86_64/mod.rs b/yjit/src/backend/x86_64/mod.rs index 4cd48ee3c9..2fb7e39346 100644 --- a/yjit/src/backend/x86_64/mod.rs +++ b/yjit/src/backend/x86_64/mod.rs @@ -94,7 +94,7 @@ impl Assembler { let live_ranges: Vec = std::mem::take(&mut self.live_ranges); - self.forward_pass(|asm, index, op, opnds, target, text| { + self.forward_pass(|asm, index, op, opnds, target, text, pos_marker| { // Load heap object operands into registers because most // instructions can't directly work with 64-bit constants let opnds = match op { @@ -140,7 +140,7 @@ impl Assembler _ => (opnds[0], opnds[1]) }; - asm.push_insn(op, vec![opnd0, opnd1], target, text); + asm.push_insn(op, vec![opnd0, opnd1], target, text, pos_marker); }, Op::CSelZ | Op::CSelNZ | Op::CSelE | Op::CSelNE | Op::CSelL | Op::CSelLE | Op::CSelG | Op::CSelGE => { @@ -151,7 +151,7 @@ impl Assembler } }).collect(); - asm.push_insn(op, new_opnds, target, text); + asm.push_insn(op, new_opnds, target, text, pos_marker); }, Op::Mov => { match (opnds[0], opnds[1]) { @@ -194,7 +194,7 @@ impl Assembler asm.not(opnd0); }, _ => { - asm.push_insn(op, opnds, target, text); + asm.push_insn(op, opnds, target, text, pos_marker); } }; }) @@ -222,6 +222,13 @@ impl Assembler cb.write_label(insn.target.unwrap().unwrap_label_idx()); }, + // Report back the current position in the generated code + Op::PosMarker => { + let pos = cb.get_write_ptr(); + let pos_marker_fn = insn.pos_marker.as_ref().unwrap(); + pos_marker_fn(pos); + } + Op::BakeString => { for byte in insn.text.as_ref().unwrap().as_bytes() { cb.write_byte(*byte); diff --git a/yjit/src/codegen.rs b/yjit/src/codegen.rs index fa6b4e41b0..d8f663fa05 100644 --- a/yjit/src/codegen.rs +++ b/yjit/src/codegen.rs @@ -297,8 +297,10 @@ fn jit_prepare_routine_call( /// Record the current codeblock write position for rewriting into a jump into /// the outlined block later. Used to implement global code invalidation. -fn record_global_inval_patch(cb: &mut CodeBlock, outline_block_target_pos: CodePtr) { - CodegenGlobals::push_global_inval_patch(cb.get_write_ptr(), outline_block_target_pos); +fn record_global_inval_patch(asm: &mut Assembler, outline_block_target_pos: CodePtr) { + asm.pos_marker(Box::new(move |code_ptr| { + CodegenGlobals::push_global_inval_patch(code_ptr, outline_block_target_pos); + })); } /// Verify the ctx's types and mappings against the compile-time stack, self, @@ -681,7 +683,7 @@ fn gen_check_ints(asm: &mut Assembler, side_exit: CodePtr) { fn jump_to_next_insn( jit: &mut JITState, current_context: &Context, - cb: &mut CodeBlock, + asm: &mut Assembler, ocb: &mut OutlinedCb, ) { // Reset the depth since in current usages we only ever jump to to @@ -704,10 +706,13 @@ fn jump_to_next_insn( record_global_inval_patch(cb, exit_pos); jit.record_boundary_patch_point = false; */ + + + } // Generate the jump instruction - gen_direct_jump(jit, &reset_depth, jump_block, cb); + gen_direct_jump(jit, &reset_depth, jump_block, asm); } // Compile a sequence of bytecode instructions for a given basic block version. @@ -763,7 +768,7 @@ pub fn gen_single_block( // opt_getinlinecache wants to be in a block all on its own. Cut the block short // if we run into it. See gen_opt_getinlinecache() for details. if opcode == YARVINSN_opt_getinlinecache.as_usize() && insn_idx > starting_insn_idx { - jump_to_next_insn(&mut jit, &ctx, cb, ocb); + jump_to_next_insn(&mut jit, &ctx, &mut asm, ocb); break; } @@ -775,11 +780,9 @@ pub fn gen_single_block( // If previous instruction requested to record the boundary if jit.record_boundary_patch_point { - // FIXME: is this sound with the new assembler? - // Generate an exit to this instruction and record it let exit_pos = gen_outlined_exit(jit.pc, &ctx, ocb); - record_global_inval_patch(cb, exit_pos); + record_global_inval_patch(&mut asm, exit_pos); jit.record_boundary_patch_point = false; } diff --git a/yjit/src/core.rs b/yjit/src/core.rs index 6c7044c843..63c373b70a 100644 --- a/yjit/src/core.rs +++ b/yjit/src/core.rs @@ -1861,7 +1861,7 @@ fn gen_jump_branch( } } -pub fn gen_direct_jump(jit: &JITState, ctx: &Context, target0: BlockId, cb: &mut CodeBlock) { +pub fn gen_direct_jump(jit: &JITState, ctx: &Context, target0: BlockId, asm: &mut Assembler) { let branchref = make_branch_entry(&jit.get_block(), ctx, gen_jump_branch); let mut branch = branchref.borrow_mut(); @@ -1897,6 +1897,17 @@ pub fn gen_direct_jump(jit: &JITState, ctx: &Context, target0: BlockId, cb: &mut + asm.pos_marker(Box::new(move |code_ptr| { + let mut branch = branchref.borrow_mut(); + branch.start_addr = Some(code_ptr); + })); + + + + + + + } else { @@ -1904,8 +1915,11 @@ pub fn gen_direct_jump(jit: &JITState, ctx: &Context, target0: BlockId, cb: &mut // target block right after this one (fallthrough). branch.dst_addrs[0] = None; branch.shape = BranchShape::Next0; - branch.start_addr = Some(cb.get_write_ptr()); - branch.end_addr = Some(cb.get_write_ptr()); + + todo!(); + + //branch.start_addr = Some(cb.get_write_ptr()); + //branch.end_addr = Some(cb.get_write_ptr()); } } -- cgit v1.2.1