From 756d0c0ba3e99c0537e6b8346ac77bacaa1f10f9 Mon Sep 17 00:00:00 2001 From: Faith Ekstrand Date: Fri, 16 Jun 2023 14:26:32 -0500 Subject: [PATCH] nak: Add a new VecPair type This is a pair of vectors but it acts a bit like a vector of a pair. Everything is inserted, removed, iterated, etc. in tandem to ensure that we never mismatch between the two vectors. However, because they're two separate vectors, we ensure that it can be used to store Src and Dst and keep everything contiguous. Of particular interest is the new retain() method which is equivalent to Vec::retain() which allows filtering a VecPair. This tricky operation is performed three different times by DCE and will be needed in spilling as well. Part-of: --- src/nouveau/compiler/nak_assign_regs.rs | 7 +- src/nouveau/compiler/nak_from_nir.rs | 8 +- src/nouveau/compiler/nak_ir.rs | 175 +++++++++++-------- src/nouveau/compiler/nak_lower_par_copies.rs | 8 +- src/nouveau/compiler/nak_opt_copy_prop.rs | 2 +- src/nouveau/compiler/nak_opt_dce.rs | 45 +---- 6 files changed, 121 insertions(+), 124 deletions(-) diff --git a/src/nouveau/compiler/nak_assign_regs.rs b/src/nouveau/compiler/nak_assign_regs.rs index 5221005674b..831c0e0c19d 100644 --- a/src/nouveau/compiler/nak_assign_regs.rs +++ b/src/nouveau/compiler/nak_assign_regs.rs @@ -483,8 +483,7 @@ impl<'a> PinnedRegAllocator<'a> { } fn finish(mut self, pcopy: &mut OpParCopy) { - pcopy.srcs.append(&mut self.pcopy.srcs); - pcopy.dsts.append(&mut self.pcopy.dsts); + pcopy.dsts_srcs.append(&mut self.pcopy.dsts_srcs); if !self.evicted.is_empty() { /* Sort so we get determinism, even if the hash map order changes @@ -845,7 +844,7 @@ impl AssignRegsBlock { None } Op::PhiSrcs(phi) => { - for (id, src) in phi.iter() { + for (id, src) in phi.srcs.iter() { assert!(src.src_mod.is_none()); if let SrcRef::SSA(ssa) = src.src_ref { assert!(ssa.comps() == 1); @@ -862,7 +861,7 @@ impl AssignRegsBlock { Op::PhiDsts(phi) => { assert!(instr.pred.is_true()); - for (id, dst) in phi.iter() { + for (id, dst) in phi.dsts.iter() { if let Dst::SSA(ssa) = dst { assert!(ssa.comps() == 1); let ra = &mut self.ra[ssa.file()]; diff --git a/src/nouveau/compiler/nak_from_nir.rs b/src/nouveau/compiler/nak_from_nir.rs index c73b20c378b..acc87c6e939 100644 --- a/src/nouveau/compiler/nak_from_nir.rs +++ b/src/nouveau/compiler/nak_from_nir.rs @@ -1551,7 +1551,7 @@ impl<'a> ShaderFromNir<'a> { let dst = alloc_ssa_for_nir(&mut b, np.def.as_def()); for (i, dst) in dst.iter().enumerate() { let phi_id = self.get_phi_id(np, i.try_into().unwrap()); - phi.push(phi_id, (*dst).into()); + phi.dsts.push(phi_id, (*dst).into()); } self.set_ssa(np.def.as_def(), dst); } else { @@ -1559,7 +1559,7 @@ impl<'a> ShaderFromNir<'a> { } } - if !phi.ids.is_empty() { + if !phi.dsts.is_empty() { b.push_op(phi); } @@ -1609,14 +1609,14 @@ impl<'a> ShaderFromNir<'a> { for (i, src) in src.iter().enumerate() { let phi_id = self.get_phi_id(np, i.try_into().unwrap()); - phi.push(phi_id, (*src).into()); + phi.srcs.push(phi_id, (*src).into()); } break; } } } - if !phi.ids.is_empty() { + if !phi.srcs.is_empty() { b.push_op(phi); } } diff --git a/src/nouveau/compiler/nak_ir.rs b/src/nouveau/compiler/nak_ir.rs index 88094b82b47..3a7e1d966a9 100644 --- a/src/nouveau/compiler/nak_ir.rs +++ b/src/nouveau/compiler/nak_ir.rs @@ -3007,46 +3007,104 @@ impl fmt::Display for OpUndef { } } +pub struct VecPair { + a: Vec, + b: Vec, +} + +impl VecPair { + pub fn append(&mut self, other: &mut VecPair) { + self.a.append(&mut other.a); + self.b.append(&mut other.b); + } + + pub fn is_empty(&self) -> bool { + debug_assert!(self.a.len() == self.b.len()); + self.a.is_empty() + } + + pub fn iter(&self) -> Zip, slice::Iter<'_, B>> { + debug_assert!(self.a.len() == self.b.len()); + self.a.iter().zip(self.b.iter()) + } + + pub fn len(&self) -> usize { + debug_assert!(self.a.len() == self.b.len()); + self.a.len() + } + + pub fn new() -> Self { + Self { + a: Vec::new(), + b: Vec::new(), + } + } + + pub fn push(&mut self, a: A, b: B) { + debug_assert!(self.a.len() == self.b.len()); + self.a.push(a); + self.b.push(b); + } +} + +impl VecPair { + pub fn retain(&mut self, f: impl Fn(&A, &B) -> bool) { + debug_assert!(self.a.len() == self.b.len()); + let len = self.a.len(); + let mut i = 0_usize; + while i < len { + if !f(&self.a[i], &self.b[i]) { + break; + } + i += 1; + } + + let mut new_len = i; + + /* Don't check this one twice. */ + i += 1; + + while i < len { + /* This could be more efficient but it's good enough for our + * purposes since everything we're storing is small and has a + * trivial Drop. + */ + if f(&self.a[i], &self.b[i]) { + self.a[new_len] = self.a[i].clone(); + self.b[new_len] = self.b[i].clone(); + new_len += 1; + } + i += 1; + } + + if new_len < len { + self.a.truncate(new_len); + self.b.truncate(new_len); + } + } +} + #[repr(C)] #[derive(DstsAsSlice)] pub struct OpPhiSrcs { - pub srcs: Vec, - pub ids: Vec, + pub srcs: VecPair, } impl OpPhiSrcs { pub fn new() -> OpPhiSrcs { OpPhiSrcs { - srcs: Vec::new(), - ids: Vec::new(), + srcs: VecPair::new(), } } - - #[allow(dead_code)] - pub fn is_empty(&self) -> bool { - assert!(self.ids.len() == self.srcs.len()); - self.ids.is_empty() - } - - pub fn iter(&self) -> Zip, slice::Iter<'_, Src>> { - assert!(self.ids.len() == self.srcs.len()); - self.ids.iter().zip(self.srcs.iter()) - } - - pub fn push(&mut self, id: u32, src: Src) { - assert!(self.ids.len() == self.srcs.len()); - self.ids.push(id); - self.srcs.push(src); - } } impl SrcsAsSlice for OpPhiSrcs { fn srcs_as_slice(&self) -> &[Src] { - &self.srcs + &self.srcs.b } fn srcs_as_mut_slice(&mut self) -> &mut [Src] { - &mut self.srcs + &mut self.srcs.b } fn src_types(&self) -> SrcTypeList { @@ -3057,12 +3115,11 @@ impl SrcsAsSlice for OpPhiSrcs { impl fmt::Display for OpPhiSrcs { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "PHI_SRC {{")?; - assert!(self.ids.len() == self.srcs.len()); - for i in 0..self.ids.len() { + for (i, (id, src)) in self.srcs.iter().enumerate() { if i > 0 { write!(f, ",")?; } - write!(f, " {} <- {}", self.ids[i], self.srcs[i])?; + write!(f, " {} <- {}", id, src)?; } write!(f, " }}") } @@ -3071,55 +3128,35 @@ impl fmt::Display for OpPhiSrcs { #[repr(C)] #[derive(SrcsAsSlice)] pub struct OpPhiDsts { - pub ids: Vec, - pub dsts: Vec, + pub dsts: VecPair, } impl OpPhiDsts { pub fn new() -> OpPhiDsts { OpPhiDsts { - ids: Vec::new(), - dsts: Vec::new(), + dsts: VecPair::new(), } } - - #[allow(dead_code)] - pub fn is_empty(&self) -> bool { - assert!(self.ids.len() == self.dsts.len()); - self.ids.is_empty() - } - - pub fn iter(&self) -> Zip, slice::Iter<'_, Dst>> { - assert!(self.ids.len() == self.dsts.len()); - self.ids.iter().zip(self.dsts.iter()) - } - - pub fn push(&mut self, id: u32, dst: Dst) { - assert!(self.ids.len() == self.dsts.len()); - self.ids.push(id); - self.dsts.push(dst); - } } impl DstsAsSlice for OpPhiDsts { fn dsts_as_slice(&self) -> &[Dst] { - &self.dsts + &self.dsts.b } fn dsts_as_mut_slice(&mut self) -> &mut [Dst] { - &mut self.dsts + &mut self.dsts.b } } impl fmt::Display for OpPhiDsts { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "PHI_DST {{")?; - assert!(self.ids.len() == self.dsts.len()); - for i in 0..self.ids.len() { + for (i, (id, dst)) in self.dsts.iter().enumerate() { if i > 0 { write!(f, ",")?; } - write!(f, " {} <- {}", self.dsts[i], self.ids[i])?; + write!(f, " {} <- {}", dst, id)?; } write!(f, " }}") } @@ -3144,42 +3181,32 @@ impl fmt::Display for OpSwap { #[repr(C)] pub struct OpParCopy { - pub dsts: Vec, - pub srcs: Vec, + pub dsts_srcs: VecPair, } impl OpParCopy { pub fn new() -> OpParCopy { OpParCopy { - dsts: Vec::new(), - srcs: Vec::new(), + dsts_srcs: VecPair::new(), } } pub fn is_empty(&self) -> bool { - assert!(self.srcs.len() == self.dsts.len()); - self.srcs.is_empty() - } - - pub fn iter(&self) -> Zip, slice::Iter<'_, Src>> { - assert!(self.srcs.len() == self.dsts.len()); - self.dsts.iter().zip(&self.srcs) + self.dsts_srcs.is_empty() } pub fn push(&mut self, dst: Dst, src: Src) { - assert!(self.srcs.len() == self.dsts.len()); - self.srcs.push(src); - self.dsts.push(dst); + self.dsts_srcs.push(dst, src); } } impl SrcsAsSlice for OpParCopy { fn srcs_as_slice(&self) -> &[Src] { - &self.srcs + &self.dsts_srcs.b } fn srcs_as_mut_slice(&mut self) -> &mut [Src] { - &mut self.srcs + &mut self.dsts_srcs.b } fn src_types(&self) -> SrcTypeList { @@ -3189,23 +3216,22 @@ impl SrcsAsSlice for OpParCopy { impl DstsAsSlice for OpParCopy { fn dsts_as_slice(&self) -> &[Dst] { - &self.dsts + &self.dsts_srcs.a } fn dsts_as_mut_slice(&mut self) -> &mut [Dst] { - &mut self.dsts + &mut self.dsts_srcs.a } } impl fmt::Display for OpParCopy { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "PAR_COPY {{")?; - assert!(self.srcs.len() == self.dsts.len()); - for i in 0..self.srcs.len() { + for (i, (dst, src)) in self.dsts_srcs.iter().enumerate() { if i > 0 { write!(f, ",")?; } - write!(f, " {} <- {}", self.dsts[i], self.srcs[i])?; + write!(f, " {} <- {}", dst, src)?; } write!(f, " }}") } @@ -4055,8 +4081,7 @@ impl Shader { for (i, src) in out.srcs.iter().enumerate() { let dst = RegRef::new(RegFile::GPR, i.try_into().unwrap(), 1); - pcopy.srcs.push(*src); - pcopy.dsts.push(dst.into()); + pcopy.push(dst.into(), *src); } MappedInstrs::One(Instr::new_boxed(pcopy)) } diff --git a/src/nouveau/compiler/nak_lower_par_copies.rs b/src/nouveau/compiler/nak_lower_par_copies.rs index 3645d4fd614..da991617e1f 100644 --- a/src/nouveau/compiler/nak_lower_par_copies.rs +++ b/src/nouveau/compiler/nak_lower_par_copies.rs @@ -65,7 +65,7 @@ fn lower_par_copy(pc: OpParCopy) -> MappedInstrs { let mut vals = Vec::new(); let mut reg_to_idx = HashMap::new(); - for (i, dst) in pc.dsts.iter().enumerate() { + for (i, (dst, _)) in pc.dsts_srcs.iter().enumerate() { /* Destinations must be pairwise unique */ let reg = dst.as_reg().unwrap(); assert!(reg_to_idx.get(reg).is_none()); @@ -79,7 +79,7 @@ fn lower_par_copy(pc: OpParCopy) -> MappedInstrs { reg_to_idx.insert(*reg, i); } - for (dst_idx, src) in pc.srcs.iter().enumerate() { + for (dst_idx, (_, src)) in pc.dsts_srcs.iter().enumerate() { assert!(src.src_mod.is_none()); let src = src.src_ref; @@ -113,7 +113,7 @@ fn lower_par_copy(pc: OpParCopy) -> MappedInstrs { let mut b = InstrBuilder::new(); let mut ready = Vec::new(); - for i in 0..pc.dsts.len() { + for i in 0..pc.dsts_srcs.len() { if graph.num_reads(i) == 0 { ready.push(i); } @@ -156,7 +156,7 @@ fn lower_par_copy(pc: OpParCopy) -> MappedInstrs { * * QED */ - for i in 0..pc.dsts.len() { + for i in 0..pc.dsts_srcs.len() { loop { if let Some(j) = graph.src(i) { /* We're part of a cycle so j also has a source */ diff --git a/src/nouveau/compiler/nak_opt_copy_prop.rs b/src/nouveau/compiler/nak_opt_copy_prop.rs index 1d5780ee977..4914e26edf2 100644 --- a/src/nouveau/compiler/nak_opt_copy_prop.rs +++ b/src/nouveau/compiler/nak_opt_copy_prop.rs @@ -415,7 +415,7 @@ impl CopyPropPass { self.add_copy(dst[0], SrcType::I32, neg.src.ineg()); } Op::ParCopy(pcopy) => { - for (dst, src) in pcopy.iter() { + for (dst, src) in pcopy.dsts_srcs.iter() { let dst = dst.as_ssa().unwrap(); assert!(dst.comps() == 1); self.add_copy(dst[0], SrcType::GPR, *src); diff --git a/src/nouveau/compiler/nak_opt_dce.rs b/src/nouveau/compiler/nak_opt_dce.rs index 8bfc52eb93f..c05c75fc6bf 100644 --- a/src/nouveau/compiler/nak_opt_dce.rs +++ b/src/nouveau/compiler/nak_opt_dce.rs @@ -81,7 +81,7 @@ impl DeadCodePass { match &instr.op { Op::PhiSrcs(phi) => { assert!(instr.pred.is_true()); - for (id, src) in phi.iter() { + for (id, src) in phi.srcs.iter() { if self.is_phi_live(*id) { self.mark_src_live(src); } else { @@ -91,7 +91,7 @@ impl DeadCodePass { } Op::PhiDsts(phi) => { assert!(instr.pred.is_true()); - for (id, dst) in phi.iter() { + for (id, dst) in phi.dsts.iter() { if self.is_dst_live(dst) { self.mark_phi_live(*id); } else { @@ -101,7 +101,7 @@ impl DeadCodePass { } Op::ParCopy(pcopy) => { assert!(instr.pred.is_true()); - for (dst, src) in pcopy.iter() { + for (dst, src) in pcopy.dsts_srcs.iter() { if self.is_dst_live(dst) { self.mark_src_live(src); } else { @@ -128,43 +128,16 @@ impl DeadCodePass { fn map_instr(&self, mut instr: Box) -> MappedInstrs { let is_live = match &mut instr.op { Op::PhiSrcs(phi) => { - assert!(phi.ids.len() == phi.srcs.len()); - let mut i = 0; - while i < phi.ids.len() { - if self.is_phi_live(phi.ids[i]) { - i += 1; - } else { - phi.srcs.remove(i); - phi.ids.remove(i); - } - } - i > 0 + phi.srcs.retain(|id, _| self.is_phi_live(*id)); + !phi.srcs.is_empty() } Op::PhiDsts(phi) => { - assert!(phi.ids.len() == phi.dsts.len()); - let mut i = 0; - while i < phi.ids.len() { - if self.is_dst_live(&phi.dsts[i]) { - i += 1; - } else { - phi.ids.remove(i); - phi.dsts.remove(i); - } - } - i > 0 + phi.dsts.retain(|_, dst| self.is_dst_live(dst)); + !phi.dsts.is_empty() } Op::ParCopy(pcopy) => { - assert!(pcopy.srcs.len() == pcopy.dsts.len()); - let mut i = 0; - while i < pcopy.dsts.len() { - if self.is_dst_live(&pcopy.dsts[i]) { - i += 1; - } else { - pcopy.srcs.remove(i); - pcopy.dsts.remove(i); - } - } - i > 0 + pcopy.dsts_srcs.retain(|dst, _| self.is_dst_live(dst)); + !pcopy.dsts_srcs.is_empty() } _ => self.is_instr_live(&instr), };