From e32c82d0f52afe77b9ef545adb27ba12057e0a6a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Se=C3=A1n=20de=20B=C3=BArca?= Date: Thu, 3 Apr 2025 18:36:34 -0700 Subject: [PATCH] nak: use standard methods and macros to improve readability v2: Leave `Op::is_branch()` and `Op::no_scoreboard()` matches alone v3: Revert additional changes with unclear readability Reviewed-by: Faith Ekstrand Part-of: --- src/nouveau/compiler/nak/assign_regs.rs | 2 +- src/nouveau/compiler/nak/calc_instr_deps.rs | 17 +++-------------- src/nouveau/compiler/nak/from_nir.rs | 8 +++----- src/nouveau/compiler/nak/ir.rs | 19 ++++++++----------- src/nouveau/compiler/nak/legalize.rs | 2 +- src/nouveau/compiler/nak/lower_par_copies.rs | 2 +- src/nouveau/compiler/nak/opt_dce.rs | 4 ++-- src/nouveau/compiler/nak/sph.rs | 4 ++-- src/nouveau/compiler/nak/spill_values.rs | 2 +- src/nouveau/compiler/nak/ssa_value.rs | 6 +++--- src/nouveau/compiler/nak/to_cssa.rs | 3 +-- 11 files changed, 26 insertions(+), 43 deletions(-) diff --git a/src/nouveau/compiler/nak/assign_regs.rs b/src/nouveau/compiler/nak/assign_regs.rs index c662877cb9d..6c2f12d9d6b 100644 --- a/src/nouveau/compiler/nak/assign_regs.rs +++ b/src/nouveau/compiler/nak/assign_regs.rs @@ -563,7 +563,7 @@ impl<'a> VecRegAllocator<'a> { fn move_ssa_to_reg(&mut self, ssa: SSAValue, new_reg: u32) { if let Some(old_reg) = self.ra.try_get_reg(ssa) { - assert!(self.evicted.get(&ssa).is_none()); + assert!(!self.evicted.contains_key(&ssa)); assert!(!self.reg_is_pinned(old_reg)); if new_reg == old_reg { diff --git a/src/nouveau/compiler/nak/calc_instr_deps.rs b/src/nouveau/compiler/nak/calc_instr_deps.rs index 21243d024ba..2a1cc631a3c 100644 --- a/src/nouveau/compiler/nak/calc_instr_deps.rs +++ b/src/nouveau/compiler/nak/calc_instr_deps.rs @@ -209,12 +209,7 @@ impl BarAlloc { } pub fn try_find_free_bar(&self) -> Option { - for bar in 0..self.num_bars { - if self.bar_is_free(bar) { - return Some(bar); - } - } - None + (0..self.num_bars).find(|&bar| self.bar_is_free(bar)) } pub fn free_some_bar(&mut self) -> u8 { @@ -230,12 +225,7 @@ impl BarAlloc { } pub fn get_bar_for_dep(&self, dep: usize) -> Option { - for bar in 0..self.num_bars { - if self.bar_dep[usize::from(bar)] == dep { - return Some(bar); - } - } - None + (0..self.num_bars).find(|&bar| self.bar_dep[usize::from(bar)] == dep) } } @@ -347,8 +337,7 @@ fn calc_delays(f: &mut Function, sm: &dyn ShaderModel) -> u32 { let mut cycle = 0_u32; // Vector mapping IP to start cycle - let mut instr_cycle = Vec::new(); - instr_cycle.resize(b.instrs.len(), 0_u32); + let mut instr_cycle = vec![0; b.instrs.len()]; // Maps registers to RegUse. Predicates are // represented by src_idx = usize::MAX. diff --git a/src/nouveau/compiler/nak/from_nir.rs b/src/nouveau/compiler/nak/from_nir.rs index 9025f18e028..a51521ac809 100644 --- a/src/nouveau/compiler/nak/from_nir.rs +++ b/src/nouveau/compiler/nak/from_nir.rs @@ -502,9 +502,7 @@ impl<'a> ShaderFromNir<'a> { } fn alu_src_is_saturated(&self, src: &nir_alu_src) -> bool { - self.saturated - .get(&(src.src.as_def() as *const _)) - .is_some() + self.saturated.contains(&(src.src.as_def() as *const _)) } fn parse_alu(&mut self, b: &mut impl SSABuilder, alu: &nir_alu_instr) { @@ -3783,7 +3781,7 @@ impl<'a> ShaderFromNir<'a> { /* Next block in the NIR CF list */ let next_block = nb.cf_node.next().unwrap().as_block().unwrap(); - if else_target as *const _ == next_block as *const _ { + if std::ptr::eq(else_target, next_block) { self.emit_pred_jump( &mut b, nb, @@ -3792,7 +3790,7 @@ impl<'a> ShaderFromNir<'a> { target, else_target, ); - } else if target as *const _ == next_block as *const _ { + } else if std::ptr::eq(target, next_block) { self.emit_pred_jump( &mut b, nb, diff --git a/src/nouveau/compiler/nak/ir.rs b/src/nouveau/compiler/nak/ir.rs index b3adedf36f5..cbc090017aa 100644 --- a/src/nouveau/compiler/nak/ir.rs +++ b/src/nouveau/compiler/nak/ir.rs @@ -583,10 +583,9 @@ impl SrcRef { pub fn is_bindless_cbuf(&self) -> bool { match self { - SrcRef::CBuf(cbuf) => match cbuf.buf { - CBuf::BindlessSSA(_) | CBuf::BindlessUGPR(_) => true, - _ => false, - }, + SrcRef::CBuf(cbuf) => { + matches!(cbuf.buf, CBuf::BindlessSSA(_) | CBuf::BindlessUGPR(_)) + } _ => false, } } @@ -1264,7 +1263,7 @@ fn all_dsts_uniform(dsts: &[Dst]) -> bool { Dst::Reg(r) => r.is_uniform(), Dst::SSA(r) => r.file().unwrap().is_uniform(), }; - assert!(uniform == None || uniform == Some(dst_uniform)); + assert!(uniform.is_none() || uniform == Some(dst_uniform)); uniform = Some(dst_uniform); } uniform == Some(true) @@ -3303,7 +3302,7 @@ pub struct OpIAbs { impl Foldable for OpIAbs { fn fold(&self, _sm: &dyn ShaderModel, f: &mut OpFoldData<'_>) { let src = f.get_u32_src(self, &self.src); - let dst = (src as i32).abs() as u32; + let dst = (src as i32).unsigned_abs(); f.set_u32_dst(self, &self.dst, dst); } } @@ -4609,12 +4608,10 @@ impl OpPrmt { return None; } - if let Some(sel) = self.sel.as_u32(SrcType::ALU) { + self.sel.as_u32(SrcType::ALU).map(|sel| { // The top 16 bits are ignored - Some(PrmtSel(sel as u16)) - } else { - None - } + PrmtSel(sel as u16) + }) } /// Reduces the sel immediate, if any. diff --git a/src/nouveau/compiler/nak/legalize.rs b/src/nouveau/compiler/nak/legalize.rs index a91c8805709..8bc28aba556 100644 --- a/src/nouveau/compiler/nak/legalize.rs +++ b/src/nouveau/compiler/nak/legalize.rs @@ -462,7 +462,7 @@ fn legalize_instr( // vector sources or as multiple components in the same // source, we need to make a copy so it can get assigned to // multiple different registers. - if vec_comps.get(&ssa).is_some() { + if vec_comps.contains(&ssa) { let copy = b.alloc_ssa(ssa.file()); b.copy_to(copy.into(), ssa.into()); new_vec[usize::from(c)] = copy; diff --git a/src/nouveau/compiler/nak/lower_par_copies.rs b/src/nouveau/compiler/nak/lower_par_copies.rs index d3a5e50e2a0..8fcf18b1729 100644 --- a/src/nouveau/compiler/nak/lower_par_copies.rs +++ b/src/nouveau/compiler/nak/lower_par_copies.rs @@ -98,7 +98,7 @@ fn lower_par_copy(pc: OpParCopy, sm: &dyn ShaderModel) -> MappedInstrs { 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()); + assert!(!reg_to_idx.contains_key(reg)); // Everything must be scalar assert!(reg.comps() == 1); diff --git a/src/nouveau/compiler/nak/opt_dce.rs b/src/nouveau/compiler/nak/opt_dce.rs index 9c12b2cfaa1..280a8d168ef 100644 --- a/src/nouveau/compiler/nak/opt_dce.rs +++ b/src/nouveau/compiler/nak/opt_dce.rs @@ -43,7 +43,7 @@ impl DeadCodePass { match dst { Dst::SSA(ssa) => { for val in ssa.iter() { - if self.live_ssa.get(val).is_some() { + if self.live_ssa.contains(val) { return true; } } @@ -55,7 +55,7 @@ impl DeadCodePass { } fn is_phi_live(&self, id: u32) -> bool { - self.live_phi.get(&id).is_some() + self.live_phi.contains(&id) } fn is_instr_live(&self, instr: &Instr) -> bool { diff --git a/src/nouveau/compiler/nak/sph.rs b/src/nouveau/compiler/nak/sph.rs index 3c0e76d595a..ffca4c6d515 100644 --- a/src/nouveau/compiler/nak/sph.rs +++ b/src/nouveau/compiler/nak/sph.rs @@ -517,7 +517,7 @@ pub fn encode_header( } let uses_underestimate = - fs_key.map_or(false, |key| key.uses_underestimate); + fs_key.is_some_and(|key| key.uses_underestimate); // This isn't so much a "Do we write multiple render targets?" bit // as a "Should color0 be broadcast to all render targets?" bit. In @@ -541,7 +541,7 @@ pub fn encode_header( match &shader_info.stage { ShaderStageInfo::Fragment(stage) => { - let zs_self_dep = fs_key.map_or(false, |key| key.zs_self_dep); + let zs_self_dep = fs_key.is_some_and(|key| key.zs_self_dep); sph.set_kills_pixels(stage.uses_kill || zs_self_dep); sph.set_does_interlock(stage.does_interlock); } diff --git a/src/nouveau/compiler/nak/spill_values.rs b/src/nouveau/compiler/nak/spill_values.rs index f334c3730ce..091ae0a55df 100644 --- a/src/nouveau/compiler/nak/spill_values.rs +++ b/src/nouveau/compiler/nak/spill_values.rs @@ -1034,7 +1034,7 @@ fn spill_values( let ip = pb .phi_srcs_ip() .or_else(|| pb.branch_ip()) - .unwrap_or_else(|| pb.instrs.len()); + .unwrap_or(pb.instrs.len()); pb.instrs.splice(ip..ip, instrs.into_iter()); } } diff --git a/src/nouveau/compiler/nak/ssa_value.rs b/src/nouveau/compiler/nak/ssa_value.rs index ac014c6aaa6..20a87d13a87 100644 --- a/src/nouveau/compiler/nak/ssa_value.rs +++ b/src/nouveau/compiler/nak/ssa_value.rs @@ -79,9 +79,9 @@ impl SSAValueArray { packed: NonZeroU32::MAX, }; SIZE], }; - for i in 0..comps.len() { - r.v[i] = comps[i]; - } + + r.v[..comps.len()].copy_from_slice(comps); + if comps.len() < SIZE { r.v[SIZE - 1].packed = (comps.len() as u32).wrapping_neg().try_into().unwrap(); diff --git a/src/nouveau/compiler/nak/to_cssa.rs b/src/nouveau/compiler/nak/to_cssa.rs index e85768dc1dc..0188afe8e50 100644 --- a/src/nouveau/compiler/nak/to_cssa.rs +++ b/src/nouveau/compiler/nak/to_cssa.rs @@ -238,9 +238,8 @@ impl<'a> CoalesceGraph<'a> { let nodes = MergedIter::new(a_nodes.into_iter(), b_nodes.into_iter()); self.sets[a].nodes = nodes - .map(|n| { + .inspect(|&n| { self.nodes[n].set = a; - n }) .collect();