ruby-changes:73312
From: Kevin <ko1@a...>
Date: Tue, 30 Aug 2022 01:10:02 +0900 (JST)
Subject: [ruby-changes:73312] 1c67e90bde (master): More work toward instruction enum (https://github.com/Shopify/ruby/pull/421)
https://git.ruby-lang.org/ruby.git/commit/?id=1c67e90bde From 1c67e90bdecf9aec97eb3185b237d879207db465 Mon Sep 17 00:00:00 2001 From: Kevin Newton <kddnewton@g...> Date: Thu, 18 Aug 2022 13:04:11 -0400 Subject: More work toward instruction enum (https://github.com/Shopify/ruby/pull/421) * Operand iterators There are a couple of times when we're dealing with instructions that we need to iterate through their operands. At the moment this is relatively easy because there's an opnds field and we can work with it directly. When the instructions become enums, however, the shape of each variant will be different so we'll need an iterator to make sense of the shape. This commit introduces two new iterators that are created from an instruction. One iterates over references to each operand (for instances where they don't need to be mutable like updating live ranges) and one iterates over mutable references to each operand (for instances where you need to mutate them like loading values in arm64). Note that because iterators can't have generic items (i.e., be associated with lifetimes) the mutable iterator forces you to use the `while let Some` syntax as opposed to the for-loop like we did with instructions. This commit eliminates the last reference to insn.opnds, which is going to make it much easier to transition to an enum. * Consolidate output operand fetching Currently we always look at the .out field on instructions whenever we want to access the output operand. When the instructions become an enum, this is not going to be possible since the shape of the variants will be different. Instead, this commit introduces two functions on Insn: out_opnd() and out_opnd_mut(). These return an Option containing a reference to the output operand and a mutable reference to the output operand, respectively. This commit then uses those functions to replace all instances of accessing the output operand. For the most part this was straightforward; when we previously checked if it was Opnd::None we now check that it's None, when we assumed there was an output operand we now unwrap. --- yjit/src/backend/arm64/mod.rs | 16 +- yjit/src/backend/ir.rs | 446 +++++++++++++++++++++++++++++++++++------ yjit/src/backend/tests.rs | 12 +- yjit/src/backend/x86_64/mod.rs | 2 +- 4 files changed, 403 insertions(+), 73 deletions(-) diff --git a/yjit/src/backend/arm64/mod.rs b/yjit/src/backend/arm64/mod.rs index c1d8b773f1..a32be6a6b2 100644 --- a/yjit/src/backend/arm64/mod.rs +++ b/yjit/src/backend/arm64/mod.rs @@ -192,12 +192,15 @@ impl Assembler https://github.com/ruby/ruby/blob/trunk/yjit/src/backend/arm64/mod.rs#L192 // such that only the Op::Load instruction needs to handle that // case. If the values aren't heap objects then we'll treat them as // if they were just unsigned integer. - for opnd in &mut insn.opnds { + let skip_load = matches!(insn, Insn { op: Op::Load, .. }); + let mut opnd_iter = insn.opnd_iter_mut(); + + while let Some(opnd) = opnd_iter.next() { match opnd { Opnd::Value(value) => { if value.special_const_p() { *opnd = Opnd::UImm(value.as_u64()); - } else if insn.op != Op::Load { + } else if !skip_load { *opnd = asm.load(*opnd); } }, @@ -400,9 +403,14 @@ impl Assembler https://github.com/ruby/ruby/blob/trunk/yjit/src/backend/arm64/mod.rs#L403 asm.test(opnd0, opnd1); }, _ => { - if insn.out.is_some() { - insn.out = asm.next_opnd_out(&insn.opnds); + // If we have an output operand, then we need to replace it + // with a new output operand from the new assembler. + if insn.out_opnd().is_some() { + let out_num_bits = Opnd::match_num_bits_iter(insn.opnd_iter()); + let out = insn.out_opnd_mut().unwrap(); + *out = asm.next_opnd_out(out_num_bits); } + asm.push_insn(insn); } }; diff --git a/yjit/src/backend/ir.rs b/yjit/src/backend/ir.rs index db2bc7622c..cea8dfb227 100644 --- a/yjit/src/backend/ir.rs +++ b/yjit/src/backend/ir.rs @@ -318,9 +318,13 @@ impl Opnd https://github.com/ruby/ruby/blob/trunk/yjit/src/backend/ir.rs#L318 } } - /// Determine the size in bits of the slice of the given operands. If any of - /// them are different sizes this will panic. - fn match_num_bits(opnds: &[Opnd]) -> u8 { + /// When there aren't any operands to check against, this is the number of + /// bits that should be used for any given output variable. + const DEFAULT_NUM_BITS: u8 = 64; + + /// Determine the size in bits from the iterator of operands. If any of them + /// are different sizes this will panic. + pub fn match_num_bits_iter<'a>(opnds: impl Iterator<Item = &'a Opnd>) -> u8 { let mut value: Option<u8> = None; for opnd in opnds { @@ -336,7 +340,13 @@ impl Opnd https://github.com/ruby/ruby/blob/trunk/yjit/src/backend/ir.rs#L340 } } - value.unwrap_or(64) + value.unwrap_or(Self::DEFAULT_NUM_BITS) + } + + /// Determine the size in bits of the slice of the given operands. If any of + /// them are different sizes this will panic. + pub fn match_num_bits(opnds: &[Opnd]) -> u8 { + Self::match_num_bits_iter(opnds.iter()) } } @@ -441,12 +451,287 @@ pub struct Insn https://github.com/ruby/ruby/blob/trunk/yjit/src/backend/ir.rs#L451 pub(super) pos_marker: Option<PosMarkerFn>, } +impl Insn { + /// Create an iterator that will yield a non-mutable reference to each + /// operand in turn for this instruction. + pub(super) fn opnd_iter(&self) -> InsnOpndIterator { + InsnOpndIterator::new(self) + } + + /// Create an iterator that will yield a mutable reference to each operand + /// in turn for this instruction. + pub(super) fn opnd_iter_mut(&mut self) -> InsnOpndMutIterator { + InsnOpndMutIterator::new(self) + } + + /// Return a non-mutable reference to the out operand for this instruction + /// if it has one. + pub fn out_opnd(&self) -> Option<&Opnd> { + match self { + Insn { op: Op::Add, out, .. } | + Insn { op: Op::And, out, .. } | + Insn { op: Op::CCall, out, .. } | + Insn { op: Op::CPop, out, .. } | + Insn { op: Op::CSelE, out, .. } | + Insn { op: Op::CSelG, out, .. } | + Insn { op: Op::CSelGE, out, .. } | + Insn { op: Op::CSelL, out, .. } | + Insn { op: Op::CSelLE, out, .. } | + Insn { op: Op::CSelNE, out, .. } | + Insn { op: Op::CSelNZ, out, .. } | + Insn { op: Op::CSelZ, out, .. } | + Insn { op: Op::Lea, out, .. } | + Insn { op: Op::LeaLabel, out, .. } | + Insn { op: Op::LiveReg, out, .. } | + Insn { op: Op::Load, out, .. } | + Insn { op: Op::LoadSExt, out, .. } | + Insn { op: Op::LShift, out, .. } | + Insn { op: Op::Not, out, .. } | + Insn { op: Op::Or, out, .. } | + Insn { op: Op::RShift, out, .. } | + Insn { op: Op::Sub, out, .. } | + Insn { op: Op::URShift, out, .. } | + Insn { op: Op::Xor, out, .. } => Some(out), + _ => None + } + } + + /// Return a mutable reference to the out operand for this instruction if it + /// has one. + pub fn out_opnd_mut(&mut self) -> Option<&mut Opnd> { + match self { + Insn { op: Op::Add, out, .. } | + Insn { op: Op::And, out, .. } | + Insn { op: Op::CCall, out, .. } | + Insn { op: Op::CPop, out, .. } | + Insn { op: Op::CSelE, out, .. } | + Insn { op: Op::CSelG, out, .. } | + Insn { op: Op::CSelGE, out, .. } | + Insn { op: Op::CSelL, out, .. } | + Insn { op: Op::CSelLE, out, .. } | + Insn { op: Op::CSelNE, out, .. } | + Insn { op: Op::CSelNZ, out, .. } | + Insn { op: Op::CSelZ, out, .. } | + Insn { op: Op::Lea, out, .. } | + Insn { op: Op::LeaLabel, out, .. } | + Insn { op: Op::LiveReg, out, .. } | + Insn { op: Op::Load, out, .. } | + Insn { op: Op::LoadSExt, out, .. } | + Insn { op: Op::LShift, out, .. } | + Insn { op: Op::Not, out, .. } | + Insn { op: Op::Or, out, .. } | + Insn { op: Op::RShift, out, .. } | + Insn { op: Op::Sub, out, .. } | + Insn { op: Op::URShift, out, .. } | + Insn { op: Op::Xor, out, .. } => Some(out), + _ => None + } + } +} + +/// An iterator that will yield a non-mutable reference to each operand in turn +/// for the given instruction. +pub(super) struct InsnOpndIterator<'a> { + insn: &'a Insn, + idx: usize, +} + +impl<'a> InsnOpndIterator<'a> { + fn new(insn: &'a Insn) -> Self { + Self { insn, idx: 0 } + } +} + +impl<'a> Iterator for InsnOpndIterator<'a> { + type Item = &'a Opnd; + + fn next(&mut self) -> Option<Self::Item> { + match self.insn { + Insn { op: Op::BakeString, .. } | + Insn { op: Op::Breakpoint, .. } | + Insn { op: Op::Comment, .. } | + Insn { op: Op::CPop, .. } | + Insn { op: Op::CPopAll, .. } | + Insn { op: Op::CPushAll, .. } | + Insn { op: Op::FrameSetup, .. } | + Insn { op: Op::FrameTeardown, .. } | + Insn { op: Op::Jbe, .. } | + Insn { op: Op::Je, .. } | + Insn { op: Op::Jl, .. } | + Insn { op: (... truncated) -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/