ruby-changes:73290
From: Kevin <ko1@a...>
Date: Tue, 30 Aug 2022 01:07:56 +0900 (JST)
Subject: [ruby-changes:73290] d57a9f61a0 (master): Build output operands explicitly (https://github.com/Shopify/ruby/pull/411)
https://git.ruby-lang.org/ruby.git/commit/?id=d57a9f61a0 From d57a9f61a065418ef99fcbbb65eca4f34f33f1c8 Mon Sep 17 00:00:00 2001 From: Kevin Newton <kddnewton@g...> Date: Wed, 17 Aug 2022 12:59:08 -0400 Subject: Build output operands explicitly (https://github.com/Shopify/ruby/pull/411) When we're pushing instructions onto the assembler, we previously would iterate through the instruction's operands and then assign the output operand to it through the push_insn function. This is easy when all instructions have a vector of operands, but is much more difficult when the shape differs in an enum. This commit changes it so that we explicitly define the output operand for each instruction before it gets pushed onto the assembler. This has the added benefit of changing the definition of push_insn to no longer require a mutable instruction. This paves the way to make the out field on the instructions an Option<Opnd> instead which is going to more accurately reflect the behavior we're going to have once we switch the instructions over to an enum instead of a struct. --- yjit/src/backend/ir.rs | 181 +++++++++++++++++++++++++++++++------------------ 1 file changed, 116 insertions(+), 65 deletions(-) diff --git a/yjit/src/backend/ir.rs b/yjit/src/backend/ir.rs index 481d447c5c..d2b90c3373 100644 --- a/yjit/src/backend/ir.rs +++ b/yjit/src/backend/ir.rs @@ -466,8 +466,41 @@ impl Assembler https://github.com/ruby/ruby/blob/trunk/yjit/src/backend/ir.rs#L466 } } - /// Append an instruction onto the current list of instructions. - pub(super) fn push_insn(&mut self, mut insn: Insn) -> Opnd { + /// Build an Opnd::InsnOut from the current index of the assembler and the + /// given slice of operands. The operands are given to determine the number + /// of bits necessary for the output operand. They should all be the same + /// size. + fn next_opnd_out(&self, opnds: &[Opnd]) -> Opnd { + let mut out_num_bits: Option<u8> = None; + + for opnd in opnds { + match opnd { + Opnd::InsnOut { num_bits, .. } | + Opnd::Mem(Mem { num_bits, .. }) | + Opnd::Reg(Reg { num_bits, .. }) => { + match out_num_bits { + None => { + out_num_bits = Some(*num_bits); + }, + Some(out_num_bits) => { + assert_eq!(out_num_bits, *num_bits, "operands of incompatible sizes"); + } + }; + } + _ => {} + } + } + + Opnd::InsnOut { + idx: self.insns.len(), + num_bits: out_num_bits.unwrap_or(64) + } + } + + /// Append an instruction onto the current list of instructions and update + /// the live ranges of any instructions whose outputs are being used as + /// operands to this instruction. + pub(super) fn push_insn(&mut self, insn: Insn) { // Index of this instruction let insn_idx = self.insns.len(); @@ -476,51 +509,25 @@ impl Assembler https://github.com/ruby/ruby/blob/trunk/yjit/src/backend/ir.rs#L509 // one. for opnd in &insn.opnds { match opnd { - Opnd::InsnOut{ idx, .. } => { + Opnd::InsnOut { idx, .. } => { + assert!(*idx < self.insns.len()); self.live_ranges[*idx] = insn_idx; } Opnd::Mem(Mem { base: MemBase::InsnOut(idx), .. }) => { + assert!(*idx < self.insns.len()); self.live_ranges[*idx] = insn_idx; } _ => {} } } - let mut out_num_bits: u8 = 0; - - for opnd in &insn.opnds { - match *opnd { - Opnd::InsnOut{ num_bits, .. } | - Opnd::Mem(Mem { num_bits, .. }) | - Opnd::Reg(Reg { num_bits, .. }) => { - if out_num_bits == 0 { - out_num_bits = num_bits - } - else if out_num_bits != num_bits { - panic!("operands of incompatible sizes"); - } - } - _ => {} - } - } - - if out_num_bits == 0 { - out_num_bits = 64; - } - - // Operand for the output of this instruction - let out_opnd = Opnd::InsnOut{ idx: insn_idx, num_bits: out_num_bits }; - insn.out = out_opnd; - self.insns.push(insn); self.live_ranges.push(insn_idx); - - // Return an operand for the output of this instruction - out_opnd } /// Append an instruction to the list by creating a new instruction from the - /// component parts given to this function. + /// component parts given to this function. This will also create a new + /// output operand from the given operands for the new instruction. pub(super) fn push_insn_parts( &mut self, op: Op, @@ -530,14 +537,9 @@ impl Assembler https://github.com/ruby/ruby/blob/trunk/yjit/src/backend/ir.rs#L537 pos_marker: Option<PosMarkerFn> ) -> Opnd { - self.push_insn(Insn { - op, - text, - opnds, - out: Opnd::None, - target, - pos_marker, - }) + let out = self.next_opnd_out(&opnds); + self.push_insn(Insn { op, text, opnds, out, target, pos_marker }); + out } /// Create a new label instance that we can jump to @@ -860,12 +862,16 @@ impl fmt::Debug for Assembler { https://github.com/ruby/ruby/blob/trunk/yjit/src/backend/ir.rs#L862 impl Assembler { #[must_use] pub fn add(&mut self, left: Opnd, right: Opnd) -> Opnd { - self.push_insn(Insn { op: Op::Add, opnds: vec![left, right], out: Opnd::None, text: None, target: None, pos_marker: None }) + let out = self.next_opnd_out(&[left, right]); + self.push_insn(Insn { op: Op::Add, opnds: vec![left, right], out, text: None, target: None, pos_marker: None }); + out } #[must_use] pub fn and(&mut self, left: Opnd, right: Opnd) -> Opnd { - self.push_insn(Insn { op: Op::And, opnds: vec![left, right], out: Opnd::None, text: None, target: None, pos_marker: None }) + let out = self.next_opnd_out(&[left, right]); + self.push_insn(Insn { op: Op::And, opnds: vec![left, right], out, text: None, target: None, pos_marker: None }); + out } pub fn bake_string(&mut self, text: &str) { @@ -878,7 +884,9 @@ impl Assembler { https://github.com/ruby/ruby/blob/trunk/yjit/src/backend/ir.rs#L884 #[must_use] pub fn ccall(&mut self, fptr: *const u8, opnds: Vec<Opnd>) -> Opnd { - self.push_insn(Insn { op: Op::CCall, opnds, out: Opnd::None, text: None, target: Some(Target::FunPtr(fptr)), pos_marker: None }) + let out = self.next_opnd_out(&opnds); + self.push_insn(Insn { op: Op::CCall, opnds, out, text: None, target: Some(Target::FunPtr(fptr)), pos_marker: None }); + out } pub fn cmp(&mut self, left: Opnd, right: Opnd) { @@ -889,8 +897,11 @@ impl Assembler { https://github.com/ruby/ruby/blob/trunk/yjit/src/backend/ir.rs#L897 self.push_insn(Insn { op: Op::Comment, opnds: vec![], out: Opnd::None, text: Some(text.to_string()), target: None, pos_marker: None }); } + #[must_use] pub fn cpop(&mut self) -> Opnd { - self.push_insn(Insn { op: Op::CPop, opnds: vec![], out: Opnd::None, text: None, target: None, pos_marker: None }) + let out = self.next_opnd_out(&[]); + self.push_insn(Insn { op: Op::CPop, opnds: vec![], out, text: None, target: None, pos_marker: None }); + out } pub fn cpop_all(&mut self) { @@ -915,42 +926,58 @@ impl Assembler { https://github.com/ruby/ruby/blob/trunk/yjit/src/backend/ir.rs#L926 #[must_use] pub fn csel_e(&mut self, truthy: Opnd, falsy: Opnd) -> Opnd { - self.push_insn(Insn { op: Op::CSelE, opnds: vec![truthy, falsy], out: Opnd::None, text: None, target: None, pos_marker: None }) + let out = self.next_opnd_out(&[truthy, falsy]); + self.push_insn(Insn { op: Op::CSelE, opnds: vec![truthy, falsy], out, text: None, target: None, pos_marker: None }); + out } #[must_use] pub fn csel_g(&mut self, truthy: Opnd, falsy: Opnd) -> Opnd { - self.push_insn(Insn { op: Op::CSelG, opnds: vec![truthy, falsy], out: Opnd::None, text: None, target: None, pos_marker: None }) + let out = self.next_opnd_out(&[truthy, falsy]); + self.push_insn(Insn { op: Op::CSelG, opnds: vec![truthy, falsy], out, text: None, target: None, pos_marker: None }); + out } #[must_use] pub fn csel_ge(&mut self, truthy: Opnd, falsy: Opnd) -> Opnd { - self.push_insn(Insn { op: Op::CSelGE, opnds: vec![truthy, falsy], out: Opnd::None, text: None, target: None, pos_marker: None }) + let out = self.next_opnd_out(&[truthy, falsy]); + self.push_insn(Insn { op: Op::CSelGE, opnds: vec![truthy, falsy], out, text: None, target: None, pos_marker: None }); + out } #[must_use] pub fn csel_l(&mut self, truthy: Opnd, falsy: Opnd) -> Opnd { - self.push_insn(Insn { op: Op::CSelL, opnds: vec![truthy, falsy], out: Opnd::None, text: None, target: None, pos_marker: None }) + let out = self.next_opnd_out(&[truthy, falsy]); + self.push_insn(Insn { op: Op::CSelL, opnds: vec![truthy, falsy], out, text: None, target: None, pos_marker: None }); + out } #[must_use] pub fn csel_le(&mut self, truthy: Opnd, falsy: Opnd) -> Opnd { - self.push_insn(Insn { op: Op::CSelLE, opnds: vec![truthy, falsy], out: Opnd::None, text: None, target: None, pos_marker: None }) + let out = self.next_opnd_out(&[truthy, falsy]); + self.push_insn(Insn { op: Op::CSe (... truncated) -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/