AlexIndustrial/mesa
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

nak: Add a Phi struct type

Browse Source 
Instead of passing raw u32's arround, this adds a new Phi wrapper struct
which is treated as opaque by most of the rest of the compiler.  This is
similar to what we're already doing with Label and SSAValue.  This also
gives us the opportunity to properly document NAK's phi model.

Reviewed-by: Mel Henning <mhenning@darkrefraction.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/34994>
This commit is contained in:
Faith Ekstrand
2025-05-15 18:04:27 -04:00
parent 531070cf91
commit 52dcb1c9c1
8 changed files with 152 additions and 91 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/nouveau/compiler/nak/assign_regs.rs
+5 -5
View File
@@ -181,7 +181,7 @@ impl PhiWebs {
let Some(phi_dsts) = f.blocks[b_idx].phi_dsts() else {
continue;
};
let dsts: FxHashMap<u32, &SSARef> = phi_dsts
let dsts: FxHashMap<Phi, &SSARef> = phi_dsts
.dsts
.iter()
.map(|(idx, dst)| {
@@ -225,7 +225,7 @@ impl PhiWebs {
#[derive(Clone, Copy, Eq, Hash, PartialEq)]
enum LiveRef {
SSA(SSAValue),
Phi(u32),
Phi(Phi),
}
#[derive(Clone, Copy, Eq, Hash, PartialEq)]
@@ -922,7 +922,7 @@ struct AssignRegsBlock {
ra: PerRegFile<RegAllocator>,
pcopy_tmp_gprs: u8,
live_in: Vec<LiveValue>,
phi_out: FxHashMap<u32, SrcRef>,
phi_out: FxHashMap<Phi, SrcRef>,
}
impl AssignRegsBlock {
@@ -1031,12 +1031,12 @@ impl AssignRegsBlock {
Op::PhiDsts(op) => {
assert!(instr.pred.is_true());
for (id, dst) in op.dsts.iter() {
for (phi, dst) in op.dsts.iter() {
if let Dst::SSA(ssa) = dst {
assert!(ssa.comps() == 1);
let reg = self.alloc_scalar(ip, sum, phi_webs, ssa[0]);
self.live_in.push(LiveValue {
live_ref: LiveRef::Phi(*id),
live_ref: LiveRef::Phi(*phi),
reg_ref: reg,
});
}
src/nouveau/compiler/nak/from_nir.rs
+5 -5
View File
@@ -192,7 +192,7 @@ fn alloc_ssa_for_nir(b: &mut impl SSABuilder, ssa: &nir_def) -> Vec<SSAValue> {
struct PhiAllocMap<'a> {
alloc: &'a mut PhiAllocator,
map: FxHashMap<(u32, u8), u32>,
map: FxHashMap<(u32, u8), Phi>,
}
impl<'a> PhiAllocMap<'a> {
@@ -203,10 +203,10 @@ impl<'a> PhiAllocMap<'a> {
}
}
fn get_phi_id(&mut self, phi: &nir_phi_instr, comp: u8) -> u32 {
fn get_phi(&mut self, nphi: &nir_phi_instr, comp: u8) -> Phi {
*self
.map
.entry((phi.def.index, comp))
.entry((nphi.def.index, comp))
.or_insert_with(|| self.alloc.alloc())
}
}
@@ -3638,7 +3638,7 @@ impl<'a> ShaderFromNir<'a> {
let mut b = UniformBuilder::new(&mut b, uniform);
let dst = alloc_ssa_for_nir(&mut b, np.def.as_def());
for i in 0..dst.len() {
let phi_id = phi_map.get_phi_id(np, i.try_into().unwrap());
let phi_id = phi_map.get_phi(np, i.try_into().unwrap());
phi.dsts.push(phi_id, dst[i].into());
}
self.set_ssa(np.def.as_def(), dst);
@@ -3757,7 +3757,7 @@ impl<'a> ShaderFromNir<'a> {
let src = src.as_ssa().unwrap();
for (i, src) in src.iter().enumerate() {
let phi_id =
phi_map.get_phi_id(np, i.try_into().unwrap());
phi_map.get_phi(np, i.try_into().unwrap());
phi.srcs.push(phi_id, (*src).into());
}
break;
src/nouveau/compiler/nak/ir.rs
+79 -16
View File
@@ -6130,26 +6130,88 @@ impl<A: Clone, B: Clone> VecPair<A, B> {
}
}
pub struct PhiAllocator {
count: u32,
}
mod phi {
#[allow(unused_imports)]
use crate::ir::{OpPhiDsts, OpPhiSrcs};
use compiler::bitset::IntoBitIndex;
use std::fmt;
impl PhiAllocator {
pub fn new() -> PhiAllocator {
PhiAllocator { count: 0 }
/// A phi node
///
/// Phis in NAK are implemented differently from NIR and similar IRs.
/// Instead of having a single phi instruction which lives in the successor
/// block, each `Phi` represents a single merged 32-bit (or 1-bit for
/// predicates) value and we have separate [`OpPhiSrcs`] and [`OpPhiDsts`]
/// instructions which map phis to sources and destinations.
///
/// One of the problems fundamental to phis is that they really live on the
/// edges between blocks. Regardless of where the phi instruction lives in
/// the IR data structures, its sources are consumed at the end of the
/// predecessor block and its destinations are defined at the start of the
/// successor block and all phi sources and destinations get consumed and go
/// live simultaneously for any given CFG edge. For a phi that participates
/// in a back-edge, this means that the source of the phi may be consumed
/// after (in block order) the destination goes live.
///
/// In NIR, this has caused no end of headaches. Most passes which need to
/// process phis ignore phis when first processing a block and then have a
/// special case at the end of each block which walks the successors and
/// processes the successor's phis, looking only at the phi sources whose
/// predecessor matches the block. This is clunky and often forgotten by
/// optimization and lowering pass authors. It's also easy to get missed by
/// testing since it only really breaks if you have a phi which participates
/// in a back-edge so it often gets found later when something breaks in the
/// wild.
///
/// To work around this (and also make things a little more Rust-friendly),
/// NAK places the instruction which consumes phi sources at the end of the
/// predecessor block and the instruction which defines phi destinations at
/// the start of the successor block. This structurally eliminates the
/// problem that has plagued NIR for years. The cost to this solution is
/// that we have to create maps from phis to/from SSA values whenever we
/// want to optimize the phis themselves. However, this affects few enough
/// passes that the benefits to the rest of the IR are worth the trade-off,
/// at least for a back-end compiler.
#[derive(Clone, Copy, Eq, Hash, PartialEq)]
pub struct Phi {
idx: u32,
}
pub fn alloc(&mut self) -> u32 {
let idx = self.count;
self.count = idx + 1;
idx
impl IntoBitIndex for Phi {
fn into_bit_index(self) -> usize {
self.idx.try_into().unwrap()
}
}
impl fmt::Display for Phi {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "φ{}", self.idx)
}
}
pub struct PhiAllocator {
count: u32,
}
impl PhiAllocator {
pub fn new() -> PhiAllocator {
PhiAllocator { count: 0 }
}
pub fn alloc(&mut self) -> Phi {
let idx = self.count;
self.count = idx + 1;
Phi { idx }
}
}
}
pub use phi::{Phi, PhiAllocator};
/// An instruction which maps [Phi]s to sources in the predecessor block
#[repr(C)]
#[derive(DstsAsSlice)]
pub struct OpPhiSrcs {
pub srcs: VecPair<u32, Src>,
pub srcs: VecPair<Phi, Src>,
}
impl OpPhiSrcs {
@@ -6183,21 +6245,22 @@ impl DisplayOp for OpPhiSrcs {
fn fmt_op(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "phi_src ")?;
for (i, (id, src)) in self.srcs.iter().enumerate() {
for (i, (phi, src)) in self.srcs.iter().enumerate() {
if i > 0 {
write!(f, ", ")?;
}
write!(f, "φ{} = {}", id, src)?;
write!(f, "{phi} = {src}")?;
}
Ok(())
}
}
impl_display_for_op!(OpPhiSrcs);
/// An instruction which maps [Phi]s to destinations in the succeessor block
#[repr(C)]
#[derive(SrcsAsSlice)]
pub struct OpPhiDsts {
pub dsts: VecPair<u32, Dst>,
pub dsts: VecPair<Phi, Dst>,
}
impl OpPhiDsts {
@@ -6231,11 +6294,11 @@ impl DisplayOp for OpPhiDsts {
fn fmt_op(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "phi_dst ")?;
for (i, (id, dst)) in self.dsts.iter().enumerate() {
for (i, (phi, dst)) in self.dsts.iter().enumerate() {
if i > 0 {
write!(f, ", ")?;
}
write!(f, "{} = φ{}", dst, id)?;
write!(f, "{dst} = {phi}")?;
}
Ok(())
}
src/nouveau/compiler/nak/opt_bar_prop.rs
+25 -27
View File
@@ -8,8 +8,8 @@ use compiler::bitset::BitSet;
use rustc_hash::FxHashMap;
struct PhiMap {
phi_ssa: FxHashMap<u32, Vec<SSAValue>>,
ssa_phi: FxHashMap<SSAValue, u32>,
phi_ssa: FxHashMap<Phi, Vec<SSAValue>>,
ssa_phi: FxHashMap<SSAValue, Phi>,
}
impl PhiMap {
@@ -21,29 +21,29 @@ impl PhiMap {
}
fn add_phi_srcs(&mut self, op: &OpPhiSrcs) {
for (idx, src) in op.srcs.iter() {
for (phi, src) in op.srcs.iter() {
if let SrcRef::SSA(ssa) = &src.src_ref {
assert!(ssa.comps() == 1);
let phi_srcs = self.phi_ssa.entry(*idx).or_default();
let phi_srcs = self.phi_ssa.entry(*phi).or_default();
phi_srcs.push(ssa[0]);
}
}
}
fn add_phi_dsts(&mut self, op: &OpPhiDsts) {
for (idx, dst) in op.dsts.iter() {
for (phi, dst) in op.dsts.iter() {
if let Dst::SSA(ssa) = dst {
assert!(ssa.comps() == 1);
self.ssa_phi.insert(ssa[0], *idx);
self.ssa_phi.insert(ssa[0], *phi);
}
}
}
fn get_phi(&self, ssa: &SSAValue) -> Option<&u32> {
fn get_phi(&self, ssa: &SSAValue) -> Option<&Phi> {
self.ssa_phi.get(ssa)
}
fn phi_srcs(&self, idx: &u32) -> &[SSAValue] {
fn phi_srcs(&self, idx: &Phi) -> &[SSAValue] {
self.phi_ssa.get(idx).unwrap()
}
}
@@ -51,8 +51,8 @@ impl PhiMap {
#[derive(Default)]
struct BarPropPass {
ssa_map: FxHashMap<SSAValue, SSAValue>,
phi_is_bar: BitSet,
phi_is_not_bar: BitSet,
phi_is_bar: BitSet<Phi>,
phi_is_not_bar: BitSet<Phi>,
}
impl BarPropPass {
@@ -89,19 +89,19 @@ impl BarPropPass {
fn phi_can_be_bar_recur(
&mut self,
phi_map: &PhiMap,
seen: &mut BitSet,
phi: u32,
seen: &mut BitSet<Phi>,
phi: Phi,
) -> bool {
if self.phi_is_not_bar.contains(phi.try_into().unwrap()) {
if self.phi_is_not_bar.contains(phi) {
return false;
}
if seen.contains(phi.try_into().unwrap()) {
if seen.contains(phi) {
// If we've hit a cycle, that's not a fail
return true;
}
seen.insert(phi.try_into().unwrap());
seen.insert(phi);
for src_ssa in phi_map.phi_srcs(&phi) {
if self.is_bar(src_ssa) {
@@ -114,7 +114,7 @@ impl BarPropPass {
}
}
self.phi_is_not_bar.insert(phi.try_into().unwrap());
self.phi_is_not_bar.insert(phi);
return false;
}
@@ -125,18 +125,18 @@ impl BarPropPass {
&mut self,
ssa_alloc: &mut SSAValueAllocator,
phi_map: &PhiMap,
needs_bar: &mut BitSet,
phi: u32,
needs_bar: &mut BitSet<Phi>,
phi: Phi,
ssa: SSAValue,
) {
if !needs_bar.contains(phi.try_into().unwrap()) {
if !needs_bar.contains(phi) {
return;
}
let bar = ssa_alloc.alloc(RegFile::Bar);
self.ssa_map.insert(ssa, bar);
self.phi_is_bar.insert(phi.try_into().unwrap());
needs_bar.remove(phi.try_into().unwrap());
self.phi_is_bar.insert(phi);
needs_bar.remove(phi);
for src_ssa in phi_map.phi_srcs(&phi) {
if let Some(src_phi) = phi_map.get_phi(src_ssa) {
@@ -151,10 +151,10 @@ impl BarPropPass {
&mut self,
ssa_alloc: &mut SSAValueAllocator,
phi_map: &PhiMap,
phi: u32,
phi: Phi,
ssa: SSAValue,
) {
if self.phi_is_bar.contains(phi.try_into().unwrap()) {
if self.phi_is_bar.contains(phi) {
return;
}
@@ -203,8 +203,7 @@ impl BarPropPass {
match &mut instr.op {
Op::PhiSrcs(op) => {
for (idx, src) in op.srcs.iter_mut() {
if self.phi_is_bar.contains((*idx).try_into().unwrap())
{
if self.phi_is_bar.contains(*idx) {
// Barrier immediates don't exist
let ssa = src.as_ssa().unwrap();
let bar = *self.map_bar(&ssa[0]).unwrap();
@@ -216,8 +215,7 @@ impl BarPropPass {
Op::PhiDsts(op) => {
let mut bmovs = Vec::new();
for (idx, dst) in op.dsts.iter_mut() {
if self.phi_is_bar.contains((*idx).try_into().unwrap())
{
if self.phi_is_bar.contains(*idx) {
let ssa = dst.as_ssa().unwrap().clone();
let bar = *self.ssa_map.get(&ssa[0]).unwrap();
*dst = bar.into();
src/nouveau/compiler/nak/opt_dce.rs
+10 -10
View File
@@ -12,7 +12,7 @@ struct DeadCodePass {
any_dead: bool,
new_live: bool,
live_ssa: HashSet<SSAValue>,
live_phi: HashSet<u32>,
live_phi: HashSet<Phi>,
}
impl DeadCodePass {
@@ -35,8 +35,8 @@ impl DeadCodePass {
}
}
fn mark_phi_live(&mut self, id: u32) {
self.new_live |= self.live_phi.insert(id);
fn mark_phi_live(&mut self, phi: Phi) {
self.new_live |= self.live_phi.insert(phi);
}
fn is_dst_live(&self, dst: &Dst) -> bool {
@@ -54,8 +54,8 @@ impl DeadCodePass {
}
}
fn is_phi_live(&self, id: u32) -> bool {
self.live_phi.contains(&id)
fn is_phi_live(&self, phi: Phi) -> bool {
self.live_phi.contains(&phi)
}
fn is_instr_live(&self, instr: &Instr) -> bool {
@@ -80,8 +80,8 @@ impl DeadCodePass {
match &instr.op {
Op::PhiSrcs(phi) => {
assert!(instr.pred.is_true());
for (id, src) in phi.srcs.iter() {
if self.is_phi_live(*id) {
for (phi, src) in phi.srcs.iter() {
if self.is_phi_live(*phi) {
self.mark_src_live(src);
} else {
self.any_dead = true;
@@ -90,9 +90,9 @@ impl DeadCodePass {
}
Op::PhiDsts(phi) => {
assert!(instr.pred.is_true());
for (id, dst) in phi.dsts.iter() {
for (phi, dst) in phi.dsts.iter() {
if self.is_dst_live(dst) {
self.mark_phi_live(*id);
self.mark_phi_live(*phi);
} else {
self.any_dead = true;
}
@@ -127,7 +127,7 @@ impl DeadCodePass {
fn map_instr(&self, mut instr: Box<Instr>) -> MappedInstrs {
let is_live = match &mut instr.op {
Op::PhiSrcs(phi) => {
phi.srcs.retain(|id, _| self.is_phi_live(*id));
phi.srcs.retain(|phi, _| self.is_phi_live(*phi));
!phi.srcs.is_empty()
}
Op::PhiDsts(phi) => {
src/nouveau/compiler/nak/repair_ssa.rs
+5 -5
View File
@@ -11,7 +11,7 @@ use std::cmp::Reverse;
use std::collections::BinaryHeap;
struct PhiTracker {
idx: u32,
phi: Phi,
orig: SSAValue,
dst: SSAValue,
srcs: FxHashMap<usize, SSAValue>,
@@ -82,10 +82,10 @@ fn get_ssa_or_phi(
let b_ssa = if all_same {
pred_ssa.expect("Undefined value")
} else {
let phi_idx = phi_alloc.alloc();
let phi = phi_alloc.alloc();
let phi_ssa = ssa_alloc.alloc(ssa.file());
let mut pt = PhiTracker {
idx: phi_idx,
phi,
orig: ssa,
dst: phi_ssa,
srcs: Default::default(),
@@ -317,7 +317,7 @@ impl Function {
if !b_phis.is_empty() {
let phi_dst = get_or_insert_phi_dsts(bb);
for pt in b_phis.iter() {
phi_dst.dsts.push(pt.idx, pt.dst.into());
phi_dst.dsts.push(pt.phi, pt.dst.into());
}
}
@@ -337,7 +337,7 @@ impl Function {
for pt in s_phis.iter() {
let mut ssa = *pt.srcs.get(&b_idx).unwrap();
ssa = ssa_map.find(ssa);
phi_src.srcs.push(pt.idx, ssa.into());
phi_src.srcs.push(pt.phi, ssa.into());
}
}
}
src/nouveau/compiler/nak/spill_values.rs
+18 -18
View File
@@ -15,7 +15,7 @@ use std::cmp::{max, Ordering, Reverse};
use std::collections::BinaryHeap;
struct PhiDstMap {
ssa_phi: FxHashMap<SSAValue, u32>,
ssa_phi: FxHashMap<SSAValue, Phi>,
}
impl PhiDstMap {
@@ -25,10 +25,10 @@ impl PhiDstMap {
}
}
fn add_phi_dst(&mut self, phi_idx: u32, dst: &Dst) {
fn add_phi_dst(&mut self, phi: Phi, dst: &Dst) {
let vec = dst.as_ssa().expect("Not an SSA destination");
debug_assert!(vec.comps() == 1);
self.ssa_phi.insert(vec[0], phi_idx);
self.ssa_phi.insert(vec[0], phi);
}
pub fn from_block(block: &BasicBlock) -> PhiDstMap {
@@ -41,13 +41,13 @@ impl PhiDstMap {
map
}
fn get_phi_idx(&self, ssa: &SSAValue) -> Option<&u32> {
fn get_phi(&self, ssa: &SSAValue) -> Option<&Phi> {
self.ssa_phi.get(ssa)
}
}
struct PhiSrcMap {
phi_src: FxHashMap<u32, SSAValue>,
phi_src: FxHashMap<Phi, SSAValue>,
}
impl PhiSrcMap {
@@ -57,25 +57,25 @@ impl PhiSrcMap {
}
}
fn add_phi_src(&mut self, phi_idx: u32, src: &Src) {
fn add_phi_src(&mut self, phi: Phi, src: &Src) {
debug_assert!(src.is_unmodified());
let vec = src.src_ref.as_ssa().expect("Not an SSA source");
debug_assert!(vec.comps() == 1);
self.phi_src.insert(phi_idx, vec[0]);
self.phi_src.insert(phi, vec[0]);
}
pub fn from_block(block: &BasicBlock) -> PhiSrcMap {
let mut map = PhiSrcMap::new();
if let Some(op) = block.phi_srcs() {
for (idx, src) in op.srcs.iter() {
map.add_phi_src(*idx, src);
for (phi, src) in op.srcs.iter() {
map.add_phi_src(*phi, src);
}
}
map
}
pub fn get_src_ssa(&self, phi_idx: &u32) -> &SSAValue {
self.phi_src.get(phi_idx).expect("Phi source missing")
pub fn get_src_ssa(&self, phi: &Phi) -> &SSAValue {
self.phi_src.get(phi).expect("Phi source missing")
}
}
@@ -495,7 +495,7 @@ fn spill_values<S: Spill>(
}
let mut spill = SpillCache::new(&mut func.ssa_alloc, spill);
let mut spilled_phis: BitSet<usize> = BitSet::new();
let mut spilled_phis: BitSet<Phi> = BitSet::new();
let mut ssa_state_in: Vec<SSAState> = Vec::new();
let mut ssa_state_out: Vec<SSAState> = Vec::new();
@@ -599,7 +599,7 @@ fn spill_values<S: Spill>(
let phi_src_map = &phi_src_maps[*p_idx];
for mut ssa in ssa_state_out[*p_idx].w.iter().cloned() {
if let Some(phi) = phi_dst_map.get_phi_idx(&ssa) {
if let Some(phi) = phi_dst_map.get_phi(&ssa) {
ssa = *phi_src_map.get_src_ssa(phi);
}
@@ -708,13 +708,13 @@ fn spill_values<S: Spill>(
Op::PhiDsts(op) => {
// For phis, anything that is not in W needs to be spilled
// by setting the destination to some spill value.
for (idx, dst) in op.dsts.iter_mut() {
for (phi, dst) in op.dsts.iter_mut() {
let vec = dst.as_ssa().unwrap();
debug_assert!(vec.comps() == 1);
let ssa = &vec[0];
if ssa.file() == file && !b.w.contains(ssa) {
spilled_phis.insert((*idx).try_into().unwrap());
spilled_phis.insert(*phi);
b.s.insert(*ssa);
*dst = spill.get_spill(*ssa).into();
}
@@ -974,7 +974,7 @@ fn spill_values<S: Spill>(
let mut fills = Vec::new();
if let Some(op) = pb.phi_srcs_mut() {
for (idx, src) in op.srcs.iter_mut() {
for (phi, src) in op.srcs.iter_mut() {
debug_assert!(src.is_unmodified());
let vec = src.src_ref.as_ssa().unwrap();
debug_assert!(vec.comps() == 1);
@@ -984,7 +984,7 @@ fn spill_values<S: Spill>(
continue;
}
if spilled_phis.contains((*idx).try_into().unwrap()) {
if spilled_phis.contains(*phi) {
if !p_out.s.contains(ssa) {
spills.push(*ssa);
}
@@ -1005,7 +1005,7 @@ fn spill_values<S: Spill>(
}
for ssa in s_in.w.iter() {
if phi_dst_map.get_phi_idx(ssa).is_some() {
if phi_dst_map.get_phi(ssa).is_some() {
continue;
}
src/nouveau/compiler/nak/to_cssa.rs
+5 -5
View File
@@ -54,7 +54,7 @@ where
enum CoalesceItem {
SSA(SSAValue),
Phi(u32),
Phi(Phi),
}
struct CoalesceNode {
@@ -73,7 +73,7 @@ struct CoalesceGraph<'a> {
nodes: Vec<CoalesceNode>,
sets: Vec<CoalesceSet>,
ssa_node: FxHashMap<SSAValue, usize>,
phi_node_file: FxHashMap<u32, (usize, RegFile)>,
phi_node_file: FxHashMap<Phi, (usize, RegFile)>,
}
impl<'a> CoalesceGraph<'a> {
@@ -102,7 +102,7 @@ impl<'a> CoalesceGraph<'a> {
}
}
fn add_phi_dst(&mut self, phi: u32, file: RegFile, block: usize) {
fn add_phi_dst(&mut self, phi: Phi, file: RegFile, block: usize) {
debug_assert!(self.sets.is_empty());
// Record the register file now. We'll set the node later
@@ -117,7 +117,7 @@ impl<'a> CoalesceGraph<'a> {
});
}
fn add_phi_src(&mut self, phi: u32, block: usize) {
fn add_phi_src(&mut self, phi: Phi, block: usize) {
debug_assert!(self.sets.is_empty());
self.nodes.push(CoalesceNode {
@@ -250,7 +250,7 @@ impl<'a> CoalesceGraph<'a> {
self.nodes[*self.ssa_node.get(ssa).unwrap()].set
}
pub fn phi_set_file(&self, phi: &u32) -> (usize, RegFile) {
pub fn phi_set_file(&self, phi: &Phi) -> (usize, RegFile) {
let (n, file) = self.phi_node_file.get(phi).unwrap();
(self.nodes[*n].set, *file)
}