From 495b64be2b17d7046a383f078953a9bdac7dade4 Mon Sep 17 00:00:00 2001 From: Faith Ekstrand Date: Mon, 30 Jan 2023 20:53:19 -0600 Subject: [PATCH] nak: Make Dst its own type Instead of having Src::src_ref and Dst share a type, make them separate enums. We're not really getting any benefit from them being the same enum type anymore and this means we avoid things like immediates in destinations. We can't make the type system do all our work for us but this seems tractable. While we're reworking things we also implement From<> for stuff instead of having quite as many new_*() constructor variants. Part-of: --- src/nouveau/compiler/nak_assign_regs.rs | 16 +- src/nouveau/compiler/nak_calc_instr_deps.rs | 2 +- src/nouveau/compiler/nak_encode_sm75.rs | 22 +-- src/nouveau/compiler/nak_from_nir.rs | 26 +-- src/nouveau/compiler/nak_ir.rs | 171 ++++++++++++-------- src/nouveau/compiler/nak_opt_copy_prop.rs | 4 +- src/nouveau/compiler/nak_opt_dce.rs | 6 +- 7 files changed, 144 insertions(+), 103 deletions(-) diff --git a/src/nouveau/compiler/nak_assign_regs.rs b/src/nouveau/compiler/nak_assign_regs.rs index 3bf16d609b7..2e43de2a395 100644 --- a/src/nouveau/compiler/nak_assign_regs.rs +++ b/src/nouveau/compiler/nak_assign_regs.rs @@ -66,14 +66,6 @@ impl TrivialRegAlloc { } } - pub fn rewrite_ref(&mut self, r: Ref) -> Ref { - if let Ref::SSA(ssa) = r { - Ref::Reg(self.rewrite_ssa(ssa)) - } else { - r - } - } - pub fn do_alloc(&mut self, s: &mut Shader) { for f in &mut s.functions { for b in &mut f.blocks { @@ -82,10 +74,14 @@ impl TrivialRegAlloc { instr.pred = Pred::Reg(self.rewrite_ssa(ssa)); } for dst in instr.dsts_mut() { - *dst = self.rewrite_ref(*dst); + if let Dst::SSA(ssa) = dst { + *dst = self.rewrite_ssa(*ssa).into(); + } } for src in instr.srcs_mut() { - src.src_ref = self.rewrite_ref(src.src_ref); + if let SrcRef::SSA(ssa) = src.src_ref { + src.src_ref = self.rewrite_ssa(ssa).into(); + } } } } diff --git a/src/nouveau/compiler/nak_calc_instr_deps.rs b/src/nouveau/compiler/nak_calc_instr_deps.rs index 250658c0d05..dba543ec0c1 100644 --- a/src/nouveau/compiler/nak_calc_instr_deps.rs +++ b/src/nouveau/compiler/nak_calc_instr_deps.rs @@ -200,7 +200,7 @@ impl CalcDelay { .dsts() .iter() .map(|dst| match dst { - Dst::Zero => 0, + Dst::None => 0, Dst::Reg(reg) => self.reg_ready(reg), _ => panic!("Should be run after RA"), }) diff --git a/src/nouveau/compiler/nak_encode_sm75.rs b/src/nouveau/compiler/nak_encode_sm75.rs index 99d5a6c3acc..d8c43d11bb4 100644 --- a/src/nouveau/compiler/nak_encode_sm75.rs +++ b/src/nouveau/compiler/nak_encode_sm75.rs @@ -31,7 +31,7 @@ enum ALUSrc { impl ALUSrc { fn from_nonzero_src(src: &Src) -> ALUSrc { match src.src_ref { - Ref::Reg(reg) => { + SrcRef::Reg(reg) => { assert!(reg.comps() == 1); let alu_ref = ALURegRef { reg: reg, @@ -44,11 +44,11 @@ impl ALUSrc { _ => panic!("Invalid ALU register file"), } } - Ref::Imm(i) => { + SrcRef::Imm(i) => { assert!(src.src_mod.is_none()); ALUSrc::Imm(i) } - Ref::CBuf(cb) => { + SrcRef::CBuf(cb) => { let alu_ref = ALUCBufRef { cb: cb, abs: src.src_mod.has_abs(), @@ -62,7 +62,7 @@ impl ALUSrc { fn zero(file: RegFile) -> ALUSrc { let src = Src { - src_ref: Ref::Reg(RegRef::zero(file, 1)), + src_ref: SrcRef::Reg(RegRef::zero(file, 1)), /* Modifiers don't matter for zero */ src_mod: SrcMod::None, }; @@ -71,7 +71,7 @@ impl ALUSrc { pub fn from_src(src: &Src) -> ALUSrc { match src.src_ref { - Ref::Zero => ALUSrc::zero(RegFile::GPR), + SrcRef::Zero => ALUSrc::zero(RegFile::GPR), _ => ALUSrc::from_nonzero_src(src), } } @@ -79,7 +79,7 @@ impl ALUSrc { pub fn from_usrc(src: &Src) -> ALUSrc { assert!(src.is_uniform()); match src.src_ref { - Ref::Zero => ALUSrc::zero(RegFile::UGPR), + SrcRef::Zero => ALUSrc::zero(RegFile::UGPR), _ => ALUSrc::from_nonzero_src(src), } } @@ -140,15 +140,15 @@ impl SM75Instr { fn set_reg_src(&mut self, range: Range, src: Src) { assert!(src.src_mod.is_none()); match src.src_ref { - Ref::Zero => self.set_reg(range, RegRef::zero(RegFile::GPR, 1)), - Ref::Reg(reg) => self.set_reg(range, reg), + SrcRef::Zero => self.set_reg(range, RegRef::zero(RegFile::GPR, 1)), + SrcRef::Reg(reg) => self.set_reg(range, reg), _ => panic!("Not a register"), } } fn set_pred_dst(&mut self, range: Range, dst: Dst) { match dst { - Dst::Zero => { + Dst::None => { self.set_pred_reg(range, RegRef::zero(RegFile::Pred, 1)); } Dst::Reg(reg) => self.set_pred_reg(range, reg), @@ -158,10 +158,10 @@ impl SM75Instr { fn set_pred_src(&mut self, range: Range, not_bit: isize, src: Src) { match src.src_ref { - Ref::Zero => { + SrcRef::Zero => { self.set_pred_reg(range, RegRef::zero(RegFile::Pred, 1)); } - Ref::Reg(reg) => self.set_pred_reg(range, reg), + SrcRef::Reg(reg) => self.set_pred_reg(range, reg), _ => panic!("Not a register"), } if not_bit >= 0 { diff --git a/src/nouveau/compiler/nak_from_nir.rs b/src/nouveau/compiler/nak_from_nir.rs index e92c5ad11e4..79674aab24f 100644 --- a/src/nouveau/compiler/nak_from_nir.rs +++ b/src/nouveau/compiler/nak_from_nir.rs @@ -33,27 +33,27 @@ impl<'a> ShaderFromNir<'a> { } } - pub fn alloc_ssa(&mut self, file: RegFile, comps: u8) -> Ref { + pub fn alloc_ssa(&mut self, file: RegFile, comps: u8) -> SSAValue { self.func.as_mut().unwrap().alloc_ssa(file, comps) } - fn ref_for_nir_def(&self, def: &nir_def) -> Ref { + fn get_ssa(&self, def: &nir_def) -> SSAValue { if def.bit_size == 1 { - Ref::new_ssa(RegFile::Pred, def.index, def.num_components) + SSAValue::new(RegFile::Pred, def.index, def.num_components) } else { assert!(def.bit_size == 32 || def.bit_size == 64); let dwords = (def.bit_size / 32) * def.num_components; //Src::new_ssa(def.index, dwords, !def.divergent) - Ref::new_ssa(RegFile::GPR, def.index, dwords) + SSAValue::new(RegFile::GPR, def.index, dwords) } } fn get_src(&self, src: &nir_src) -> Src { - self.ref_for_nir_def(&src.as_def()).into() + self.get_ssa(&src.as_def()).into() } - fn get_dst(&self, def: &nir_def) -> Dst { - self.ref_for_nir_def(def).as_dst() + fn get_dst(&self, dst: &nir_def) -> Dst { + self.get_ssa(dst).into() } fn get_alu_src(&mut self, alu_src: &nir_alu_src) -> Src { @@ -67,9 +67,9 @@ impl<'a> ShaderFromNir<'a> { let mut dsts = Vec::new(); for c in 0..alu_src.src.num_components() { if c == alu_src.swizzle[0] { - dsts.push(comp.as_dst()); + dsts.push(comp.into()); } else { - dsts.push(Dst::Zero); + dsts.push(Dst::None); } } self.instrs.push(Instr::new_split(&dsts, vec_src)); @@ -157,7 +157,7 @@ impl<'a> ShaderFromNir<'a> { nir_op_fsign => { let lz = self.alloc_ssa(RegFile::GPR, 1); self.instrs.push(Instr::new_fset( - lz, + lz.into(), FloatCmpOp::OrdLt, srcs[0], Src::new_zero(), @@ -165,7 +165,7 @@ impl<'a> ShaderFromNir<'a> { let gz = self.alloc_ssa(RegFile::GPR, 1); self.instrs.push(Instr::new_fset( - gz, + gz.into(), FloatCmpOp::OrdGt, srcs[0], Src::new_zero(), @@ -416,7 +416,7 @@ impl<'a> ShaderFromNir<'a> { for c in 0..intrin.num_components { let tmp = self.alloc_ssa(RegFile::GPR, 1); self.fs_out_regs[(base + c) as usize] = tmp.into(); - dsts.push(tmp); + dsts.push(Dst::from(tmp)); } self.instrs.push(Instr::new_split(&dsts, data)) } else { @@ -432,7 +432,7 @@ impl<'a> ShaderFromNir<'a> { } fn parse_load_const(&mut self, load_const: &nir_load_const_instr) { - let dst = self.ref_for_nir_def(&load_const.def); + let dst = self.get_dst(&load_const.def); let mut srcs = Vec::new(); for c in 0..load_const.def.num_components { assert!(load_const.def.bit_size == 32); diff --git a/src/nouveau/compiler/nak_ir.rs b/src/nouveau/compiler/nak_ir.rs index 199a988ea20..25bfc8cbbaf 100644 --- a/src/nouveau/compiler/nak_ir.rs +++ b/src/nouveau/compiler/nak_ir.rs @@ -202,6 +202,52 @@ impl fmt::Display for RegRef { } } +#[derive(Clone, Copy)] +pub enum Dst { + None, + SSA(SSAValue), + Reg(RegRef), +} + +impl Dst { + pub fn as_reg(&self) -> Option<&RegRef> { + match self { + Dst::Reg(r) => Some(r), + _ => None, + } + } + + pub fn as_ssa(&self) -> Option<&SSAValue> { + match self { + Dst::SSA(r) => Some(r), + _ => None, + } + } +} + +impl From for Dst { + fn from(reg: RegRef) -> Dst { + Dst::Reg(reg) + } +} + +impl From for Dst { + fn from(ssa: SSAValue) -> Dst { + Dst::SSA(ssa) + } +} + +impl fmt::Display for Dst { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Dst::None => write!(f, "NULL")?, + Dst::SSA(v) => v.fmt(f)?, + Dst::Reg(r) => r.fmt(f)?, + } + Ok(()) + } +} + #[derive(Clone, Copy)] pub enum CBuf { Binding(u8), @@ -216,7 +262,7 @@ pub struct CBufRef { } #[derive(Clone, Copy)] -pub enum Ref { +pub enum SrcRef { Zero, Imm(Immediate), CBuf(CBufRef), @@ -224,62 +270,62 @@ pub enum Ref { Reg(RegRef), } -impl Ref { - pub fn new_ssa(file: RegFile, idx: u32, comps: u8) -> Ref { - Ref::SSA(SSAValue::new(file, idx, comps)) - } - - pub fn new_reg(file: RegFile, idx: u8, comps: u8) -> Ref { - Ref::Reg(RegRef::new(file, idx, comps)) - } - - pub fn as_dst(&self) -> Dst { - *self - } - +impl SrcRef { pub fn as_reg(&self) -> Option<&RegRef> { match self { - Ref::Reg(r) => Some(r), + SrcRef::Reg(r) => Some(r), _ => None, } } pub fn as_ssa(&self) -> Option<&SSAValue> { match self { - Ref::SSA(r) => Some(r), + SrcRef::SSA(r) => Some(r), _ => None, } } pub fn get_reg(&self) -> Option<&RegRef> { match self { - Ref::Zero | Ref::Imm(_) | Ref::SSA(_) => None, - Ref::CBuf(cb) => match &cb.buf { + SrcRef::Zero | SrcRef::Imm(_) | SrcRef::SSA(_) => None, + SrcRef::CBuf(cb) => match &cb.buf { CBuf::Binding(_) | CBuf::BindlessSSA(_) => None, CBuf::BindlessGPR(reg) => Some(reg), }, - Ref::Reg(reg) => Some(reg), + SrcRef::Reg(reg) => Some(reg), } } pub fn get_ssa(&self) -> Option<&SSAValue> { match self { - Ref::Zero | Ref::Imm(_) | Ref::Reg(_) => None, - Ref::CBuf(cb) => match &cb.buf { + SrcRef::Zero | SrcRef::Imm(_) | SrcRef::Reg(_) => None, + SrcRef::CBuf(cb) => match &cb.buf { CBuf::Binding(_) | CBuf::BindlessGPR(_) => None, CBuf::BindlessSSA(ssa) => Some(ssa), }, - Ref::SSA(ssa) => Some(ssa), + SrcRef::SSA(ssa) => Some(ssa), } } } -impl fmt::Display for Ref { +impl From for SrcRef { + fn from(reg: RegRef) -> SrcRef { + SrcRef::Reg(reg) + } +} + +impl From for SrcRef { + fn from(ssa: SSAValue) -> SrcRef { + SrcRef::SSA(ssa) + } +} + +impl fmt::Display for SrcRef { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { - Ref::Zero => write!(f, "ZERO")?, - Ref::Imm(x) => write!(f, "{:#x}", x.u)?, - Ref::CBuf(r) => { + SrcRef::Zero => write!(f, "ZERO")?, + SrcRef::Imm(x) => write!(f, "{:#x}", x.u)?, + SrcRef::CBuf(r) => { match r.buf { CBuf::Binding(idx) => write!(f, "c[{:#x}]", idx)?, CBuf::BindlessSSA(v) => write!(f, "cx[{}]", v)?, @@ -287,15 +333,13 @@ impl fmt::Display for Ref { } write!(f, "[{:#x}]", r.offset)?; } - Ref::SSA(v) => v.fmt(f)?, - Ref::Reg(r) => r.fmt(f)?, + SrcRef::SSA(v) => v.fmt(f)?, + SrcRef::Reg(r) => r.fmt(f)?, } Ok(()) } } -pub type Dst = Ref; - #[derive(Clone, Copy)] pub enum SrcMod { None, @@ -367,21 +411,21 @@ impl SrcMod { #[derive(Clone, Copy)] pub struct Src { - pub src_ref: Ref, + pub src_ref: SrcRef, pub src_mod: SrcMod, } impl Src { pub fn new_zero() -> Src { - Ref::Zero.into() + SrcRef::Zero.into() } pub fn new_imm_u32(u: u32) -> Src { - Ref::Imm(Immediate { u: u }).into() + SrcRef::Imm(Immediate { u: u }).into() } pub fn new_cbuf(idx: u8, offset: u16) -> Src { - Ref::CBuf(CBufRef { + SrcRef::CBuf(CBufRef { buf: CBuf::Binding(idx), offset: offset, }) @@ -419,29 +463,29 @@ impl Src { pub fn is_uniform(&self) -> bool { match self.src_ref { - Ref::Zero | Ref::Imm(_) | Ref::CBuf(_) => true, - Ref::SSA(ssa) => ssa.is_uniform(), - Ref::Reg(reg) => reg.is_uniform(), + SrcRef::Zero | SrcRef::Imm(_) | SrcRef::CBuf(_) => true, + SrcRef::SSA(ssa) => ssa.is_uniform(), + SrcRef::Reg(reg) => reg.is_uniform(), } } pub fn is_zero(&self) -> bool { match self.src_ref { - Ref::Zero => true, + SrcRef::Zero => true, _ => false, } } pub fn is_reg_or_zero(&self) -> bool { match self.src_ref { - Ref::Zero | Ref::SSA(_) | Ref::Reg(_) => true, - Ref::Imm(_) | Ref::CBuf(_) => false, + SrcRef::Zero | SrcRef::SSA(_) | SrcRef::Reg(_) => true, + SrcRef::Imm(_) | SrcRef::CBuf(_) => false, } } } -impl From for Src { - fn from(src_ref: Ref) -> Src { +impl From for Src { + fn from(src_ref: SrcRef) -> Src { Src { src_ref: src_ref, src_mod: SrcMod::None, @@ -449,6 +493,18 @@ impl From for Src { } } +impl From for Src { + fn from(reg: RegRef) -> Src { + SrcRef::from(reg).into() + } +} + +impl From for Src { + fn from(ssa: SSAValue) -> Src { + SrcRef::from(ssa).into() + } +} + impl fmt::Display for Src { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self.src_mod { @@ -1291,14 +1347,6 @@ pub enum Pred { } impl Pred { - pub fn new_ssa(file: RegFile, idx: u32, comps: u8) -> Pred { - Pred::SSA(SSAValue::new(file, idx, comps)) - } - - pub fn new_reg(file: RegFile, idx: u8, comps: u8) -> Pred { - Pred::Reg(RegRef::new(file, idx, comps)) - } - pub fn as_reg(&self) -> Option<&RegRef> { match self { Pred::Reg(r) => Some(r), @@ -1476,7 +1524,7 @@ impl Instr { pub fn new_iadd(dst: Dst, x: Src, y: Src) -> Instr { Instr::new(Op::IAdd3(OpIAdd3 { dst: dst, - overflow: Dst::Zero, + overflow: Dst::None, srcs: [Src::new_zero(), x, y], carry: Src::new_zero(), })) @@ -1734,10 +1782,10 @@ impl Function { } } - pub fn alloc_ssa(&mut self, file: RegFile, comps: u8) -> Ref { + pub fn alloc_ssa(&mut self, file: RegFile, comps: u8) -> SSAValue { let idx = self.ssa_count; self.ssa_count += 1; - Ref::new_ssa(file, idx, comps) + SSAValue::new(file, idx, comps) } pub fn map_instrs Vec>(&mut self, map: &F) { @@ -1789,7 +1837,7 @@ impl Shader { Op::IMov(mov) => { vec![Instr::new(Op::IAdd3(OpIAdd3 { dst: mov.dst, - overflow: Dst::Zero, + overflow: Dst::None, srcs: [Src::new_zero(), mov.src, Src::new_zero()], carry: Src::new_zero(), }))] @@ -1823,12 +1871,12 @@ impl Shader { let vec_src = split.src.src_ref.as_reg().unwrap(); assert!(comps == vec_src.comps()); for i in 0..comps { - let src = Dst::Reg(vec_src.as_comp(i).unwrap()); + let src = vec_src.as_comp(i).unwrap(); let dst = split.dsts[usize::from(i)]; - if let Dst::Zero = dst { + if let Dst::None = dst { continue; } - instrs.push(Instr::new_mov(dst, src.into())); + instrs.push(Instr::new_mov(dst.into(), src.into())); } } instrs @@ -1836,12 +1884,9 @@ impl Shader { Op::FSOut(out) => { let mut instrs = Vec::new(); for (i, src) in out.srcs.iter().enumerate() { - let dst = Ref::new_reg( - RegFile::GPR, - i.try_into().unwrap(), - 1, - ); - instrs.push(Instr::new_mov(dst, *src)); + let dst = + RegRef::new(RegFile::GPR, i.try_into().unwrap(), 1); + instrs.push(Instr::new_mov(dst.into(), *src)); } instrs } diff --git a/src/nouveau/compiler/nak_opt_copy_prop.rs b/src/nouveau/compiler/nak_opt_copy_prop.rs index 2bfbc478a30..ed004e93c72 100644 --- a/src/nouveau/compiler/nak_opt_copy_prop.rs +++ b/src/nouveau/compiler/nak_opt_copy_prop.rs @@ -54,14 +54,14 @@ impl CopyPropPass { if let Pred::SSA(src_ssa) = &instr.pred { if let Some(src_vec) = self.get_copy(&src_ssa) { assert!(src_vec[0].src_mod.is_none()); - if let Ref::SSA(ssa) = src_vec[0].src_ref { + if let SrcRef::SSA(ssa) = src_vec[0].src_ref { instr.pred = Pred::SSA(ssa); } } } for src in instr.srcs_mut() { - if let Ref::SSA(src_ssa) = src.src_ref { + if let SrcRef::SSA(src_ssa) = src.src_ref { if src_ssa.comps() == 1 { if let Some(src_vec) = self.get_copy(&src_ssa) { assert!(src_vec[0].src_mod.is_none()); diff --git a/src/nouveau/compiler/nak_opt_dce.rs b/src/nouveau/compiler/nak_opt_dce.rs index 2000b0df801..d4d421ea264 100644 --- a/src/nouveau/compiler/nak_opt_dce.rs +++ b/src/nouveau/compiler/nak_opt_dce.rs @@ -28,7 +28,7 @@ impl DeadCodePass { } for src in instr.srcs() { - if let Ref::SSA(ssa) = &src.src_ref { + if let SrcRef::SSA(ssa) = &src.src_ref { self.mark_ssa_live(ssa); } } @@ -36,8 +36,8 @@ impl DeadCodePass { fn is_dst_live(&self, dst: &Dst) -> bool { match dst { - Ref::SSA(ssa) => self.live_ssa.get(ssa).is_some(), - Ref::Zero => false, + Dst::SSA(ssa) => self.live_ssa.get(ssa).is_some(), + Dst::None => false, _ => panic!("Invalid SSA destination"), } }