ruby-changes:73201
From: Maxime <ko1@a...>
Date: Tue, 30 Aug 2022 01:00:24 +0900 (JST)
Subject: [ruby-changes:73201] 90137f5194 (master): Implement PosMarker instruction (https://github.com/Shopify/ruby/pull/328)
https://git.ruby-lang.org/ruby.git/commit/?id=90137f5194 From 90137f519459764a78ae8eb777e3f396f7cffd98 Mon Sep 17 00:00:00 2001 From: Maxime Chevalier-Boisvert <maximechevalierb@g...> 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 <XrXr@u...> --- .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: https://github.com/ruby/ruby/blob/trunk/.cirrus.yml#L116 --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 https://github.com/ruby/ruby/blob/trunk/yjit/src/backend/arm64/mod.rs#L81 /// 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 https://github.com/ruby/ruby/blob/trunk/yjit/src/backend/arm64/mod.rs#L103 (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 https://github.com/ruby/ruby/blob/trunk/yjit/src/backend/arm64/mod.rs#L146 } }).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 https://github.com/ruby/ruby/blob/trunk/yjit/src/backend/arm64/mod.rs#L250 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 https://github.com/ruby/ruby/blob/trunk/yjit/src/backend/arm64/mod.rs#L402 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 https://github.com/ruby/ruby/blob/trunk/yjit/src/backend/ir.rs#L35 // 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<CodePtr> for Target { https://github.com/ruby/ruby/blob/trunk/yjit/src/backend/ir.rs#L345 } } +type PosMarkerFn = Box<dyn Fn(CodePtr)>; + /// YJIT IR instruction -#[derive(Clone)] pub struct Insn { // Opcode for the instruction @@ -361,9 +365,9 @@ pub struct Insn https://github.com/ruby/ruby/blob/trunk/yjit/src/backend/ir.rs#L365 // List of branch targets (branch instructions only) pub(super) target: Option<Target>, - // Position in the generated machine code - // Useful for comments and for patching jumps - pub(super) pos: Option<CodePtr>, + // Callback to mark the position of this instruction + // in the generated code + pub(super) pos_marker: Option<PosMarkerFn>, } impl fmt::Debug for Insn { @@ -387,9 +391,6 @@ impl fmt::Debug for Insn { https://github.com/ruby/ruby/blob/trunk/yjit/src/backend/ir.rs#L391 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 https://github.com/ruby/ruby/blob/trunk/yjit/src/backend/ir.rs#L421 } /// Append an instruction to the list - pub(super) fn push_insn(&mut self, op: Op, opnds: Vec<Opnd>, target: Option<Target>, text: Option<String>) -> Opnd + pub(super) fn push_insn( + &mut self, + op: Op, + opnds: Vec<Opnd>, + target: Option<Target>, + text: Option<String>, + pos_marker: Option<PosMarkerFn> + ) -> Opnd { // Index of this instruction let insn_idx = self.insns.len(); @@ -471,7 +479,7 @@ impl Assembler https://github.com/ruby/ruby/blob/trunk/yjit/src/backend/ir.rs#L479 opnds, out: out_opnd, target, - pos: None + pos_marker, }; self.insns.push(insn); @@ -490,7 +498,7 @@ impl Assembler https://github.com/ruby/ruby/blob/trunk/yjit/src/backend/ir.rs#L498 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 https://github.com/ruby/ruby/blob/trunk/yjit/src/backend/ir.rs#L513 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 https://github.com/ruby/ruby/blob/trunk/yjit/src/backend/ir.rs#L522 /// 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 https://github.com/ruby/ruby/blob/trunk/yjit/src/backend/ir.rs#L546 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 https://github.com/ruby/ruby/blob/trunk/yjit/src/backend/ir.rs#L554 /// Transform input instructions, consumes the input assembler pub(super) fn forward_pass<F>(mut self, mut map_insn: F) -> Assembler - where F: FnMut(&mut Assembler, usize, Op, Vec<Opnd>, Option<Target>, Option<String>) + where F: FnMut(&mut Assembler, usize, Op, Vec<Opnd>, Option<Target>, Option<String>, Option<PosMarkerFn>) { let mut asm = Assembler { insns: Vec::default(), @@ -582,7 +590,7 @@ impl Assembler https://github.com/ruby/ruby/blob/trunk/yjit/src/backend/ir.rs#L590 asm.comment(insn.text.unwrap().as_str()); }, _ => { - map_insn(&mut asm, index, insn. (... truncated) -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/