[naga] Remove unused types and global expressions in a single pass.

In compaction, correctly identify unused types and global expressions,
rather than treating all global expressions referred to by
`PendingArraySize::Expression` array lengths as used even if the array
type itself is not.

This is tested by checking that going from expression to type and
back, along with the opposite, is correctly marked as used. It also
checks that adding an unused type using an expression gets compacted
out.

See the comments on `ModuleTracer::type_expression_tandem` for
details.

Fixes #6788.
This commit is contained in:
Kent Slaney
2025-01-16 10:02:18 -08:00
committed by Jim Blandy
parent 0282d61f91
commit 3cd2e7caf0
4 changed files with 381 additions and 221 deletions

View File

@@ -364,6 +364,7 @@ By @ErichDonGubler in [#6456](https://github.com/gfx-rs/wgpu/pull/6456), [#6148]
- In validation, forbid cycles between global expressions and types. By @jimblandy in [#6800](https://github.com/gfx-rs/wgpu-pull/6800)
- Allow abstract scalars in modf and frexp results. By @jimblandy in [#6821](https://github.com/gfx-rs/wgpu-pull/6821)
- In the WGSL front end, apply automatic conversions to values being assigned. By @jimblandy in [#6822](https://github.com/gfx-rs/wgpu-pull/6822)
- Fix a leak by ensuring that types that depend on expressions are correctly compacted. By @KentSlaney in [#6934](https://github.com/gfx-rs/wgpu/pull/6934).
#### Vulkan

View File

@@ -62,168 +62,171 @@ impl ExpressionTracer<'_> {
}
log::trace!("tracing new expression {:?}", expr);
self.trace_expression(expr);
}
}
use crate::Expression as Ex;
match *expr {
// Expressions that do not contain handles that need to be traced.
Ex::Literal(_)
| Ex::FunctionArgument(_)
| Ex::GlobalVariable(_)
| Ex::LocalVariable(_)
| Ex::CallResult(_)
| Ex::SubgroupBallotResult
| Ex::RayQueryProceedResult => {}
pub fn trace_expression(&mut self, expr: &crate::Expression) {
use crate::Expression as Ex;
match *expr {
// Expressions that do not contain handles that need to be traced.
Ex::Literal(_)
| Ex::FunctionArgument(_)
| Ex::GlobalVariable(_)
| Ex::LocalVariable(_)
| Ex::CallResult(_)
| Ex::SubgroupBallotResult
| Ex::RayQueryProceedResult => {}
Ex::Constant(handle) => {
self.constants_used.insert(handle);
// Constants and expressions are mutually recursive, which
// complicates our nice one-pass algorithm. However, since
// constants don't refer to each other, we can get around
// this by looking *through* each constant and marking its
// initializer as used. Since `expr` refers to the constant,
// and the constant refers to the initializer, it must
// precede `expr` in the arena.
let init = self.constants[handle].init;
match self.global_expressions_used {
Some(ref mut used) => used.insert(init),
None => self.expressions_used.insert(init),
};
Ex::Constant(handle) => {
self.constants_used.insert(handle);
// Constants and expressions are mutually recursive, which
// complicates our nice one-pass algorithm. However, since
// constants don't refer to each other, we can get around
// this by looking *through* each constant and marking its
// initializer as used. Since `expr` refers to the constant,
// and the constant refers to the initializer, it must
// precede `expr` in the arena.
let init = self.constants[handle].init;
match self.global_expressions_used {
Some(ref mut used) => used.insert(init),
None => self.expressions_used.insert(init),
};
}
Ex::Override(_) => {
// All overrides are considered used by definition. We mark
// their types and initialization expressions as used in
// `compact::compact`, so we have no more work to do here.
}
Ex::ZeroValue(ty) => {
self.types_used.insert(ty);
}
Ex::Compose { ty, ref components } => {
self.types_used.insert(ty);
self.expressions_used
.insert_iter(components.iter().cloned());
}
Ex::Access { base, index } => self.expressions_used.insert_iter([base, index]),
Ex::AccessIndex { base, index: _ } => {
self.expressions_used.insert(base);
}
Ex::Splat { size: _, value } => {
self.expressions_used.insert(value);
}
Ex::Swizzle {
size: _,
vector,
pattern: _,
} => {
self.expressions_used.insert(vector);
}
Ex::Load { pointer } => {
self.expressions_used.insert(pointer);
}
Ex::ImageSample {
image,
sampler,
gather: _,
coordinate,
array_index,
offset,
ref level,
depth_ref,
} => {
self.expressions_used
.insert_iter([image, sampler, coordinate]);
self.expressions_used.insert_iter(array_index);
match self.global_expressions_used {
Some(ref mut used) => used.insert_iter(offset),
None => self.expressions_used.insert_iter(offset),
}
Ex::Override(_) => {
// All overrides are considered used by definition. We mark
// their types and initialization expressions as used in
// `compact::compact`, so we have no more work to do here.
}
Ex::ZeroValue(ty) => {
self.types_used.insert(ty);
}
Ex::Compose { ty, ref components } => {
self.types_used.insert(ty);
self.expressions_used
.insert_iter(components.iter().cloned());
}
Ex::Access { base, index } => self.expressions_used.insert_iter([base, index]),
Ex::AccessIndex { base, index: _ } => {
self.expressions_used.insert(base);
}
Ex::Splat { size: _, value } => {
self.expressions_used.insert(value);
}
Ex::Swizzle {
size: _,
vector,
pattern: _,
} => {
self.expressions_used.insert(vector);
}
Ex::Load { pointer } => {
self.expressions_used.insert(pointer);
}
Ex::ImageSample {
image,
sampler,
gather: _,
coordinate,
array_index,
offset,
ref level,
depth_ref,
} => {
self.expressions_used
.insert_iter([image, sampler, coordinate]);
self.expressions_used.insert_iter(array_index);
match self.global_expressions_used {
Some(ref mut used) => used.insert_iter(offset),
None => self.expressions_used.insert_iter(offset),
use crate::SampleLevel as Sl;
match *level {
Sl::Auto | Sl::Zero => {}
Sl::Exact(expr) | Sl::Bias(expr) => {
self.expressions_used.insert(expr);
}
use crate::SampleLevel as Sl;
match *level {
Sl::Auto | Sl::Zero => {}
Sl::Exact(expr) | Sl::Bias(expr) => {
self.expressions_used.insert(expr);
}
Sl::Gradient { x, y } => self.expressions_used.insert_iter([x, y]),
}
self.expressions_used.insert_iter(depth_ref);
Sl::Gradient { x, y } => self.expressions_used.insert_iter([x, y]),
}
Ex::ImageLoad {
image,
coordinate,
array_index,
sample,
level,
} => {
self.expressions_used.insert(image);
self.expressions_used.insert(coordinate);
self.expressions_used.insert_iter(array_index);
self.expressions_used.insert_iter(sample);
self.expressions_used.insert_iter(level);
}
Ex::ImageQuery { image, ref query } => {
self.expressions_used.insert(image);
use crate::ImageQuery as Iq;
match *query {
Iq::Size { level } => self.expressions_used.insert_iter(level),
Iq::NumLevels | Iq::NumLayers | Iq::NumSamples => {}
}
}
Ex::Unary { op: _, expr } => {
self.expressions_used.insert(expr);
}
Ex::Binary { op: _, left, right } => {
self.expressions_used.insert_iter([left, right]);
}
Ex::Select {
condition,
accept,
reject,
} => self
.expressions_used
.insert_iter([condition, accept, reject]),
Ex::Derivative {
axis: _,
ctrl: _,
expr,
} => {
self.expressions_used.insert(expr);
}
Ex::Relational { fun: _, argument } => {
self.expressions_used.insert(argument);
}
Ex::Math {
fun: _,
arg,
arg1,
arg2,
arg3,
} => {
self.expressions_used.insert(arg);
self.expressions_used.insert_iter(arg1);
self.expressions_used.insert_iter(arg2);
self.expressions_used.insert_iter(arg3);
}
Ex::As {
expr,
kind: _,
convert: _,
} => {
self.expressions_used.insert(expr);
}
Ex::ArrayLength(expr) => {
self.expressions_used.insert(expr);
}
Ex::AtomicResult { ty, comparison: _ }
| Ex::WorkGroupUniformLoadResult { ty }
| Ex::SubgroupOperationResult { ty } => {
self.types_used.insert(ty);
}
Ex::RayQueryGetIntersection {
query,
committed: _,
} => {
self.expressions_used.insert(query);
self.expressions_used.insert_iter(depth_ref);
}
Ex::ImageLoad {
image,
coordinate,
array_index,
sample,
level,
} => {
self.expressions_used.insert(image);
self.expressions_used.insert(coordinate);
self.expressions_used.insert_iter(array_index);
self.expressions_used.insert_iter(sample);
self.expressions_used.insert_iter(level);
}
Ex::ImageQuery { image, ref query } => {
self.expressions_used.insert(image);
use crate::ImageQuery as Iq;
match *query {
Iq::Size { level } => self.expressions_used.insert_iter(level),
Iq::NumLevels | Iq::NumLayers | Iq::NumSamples => {}
}
}
Ex::Unary { op: _, expr } => {
self.expressions_used.insert(expr);
}
Ex::Binary { op: _, left, right } => {
self.expressions_used.insert_iter([left, right]);
}
Ex::Select {
condition,
accept,
reject,
} => self
.expressions_used
.insert_iter([condition, accept, reject]),
Ex::Derivative {
axis: _,
ctrl: _,
expr,
} => {
self.expressions_used.insert(expr);
}
Ex::Relational { fun: _, argument } => {
self.expressions_used.insert(argument);
}
Ex::Math {
fun: _,
arg,
arg1,
arg2,
arg3,
} => {
self.expressions_used.insert(arg);
self.expressions_used.insert_iter(arg1);
self.expressions_used.insert_iter(arg2);
self.expressions_used.insert_iter(arg3);
}
Ex::As {
expr,
kind: _,
convert: _,
} => {
self.expressions_used.insert(expr);
}
Ex::ArrayLength(expr) => {
self.expressions_used.insert(expr);
}
Ex::AtomicResult { ty, comparison: _ }
| Ex::WorkGroupUniformLoadResult { ty }
| Ex::SubgroupOperationResult { ty } => {
self.types_used.insert(ty);
}
Ex::RayQueryGetIntersection {
query,
committed: _,
} => {
self.expressions_used.insert(query);
}
}
}
}

View File

@@ -63,16 +63,6 @@ pub fn compact(module: &mut crate::Module) {
}
}
for (_, ty) in module.types.iter() {
if let crate::TypeInner::Array {
size: crate::ArraySize::Pending(crate::PendingArraySize::Expression(size_expr)),
..
} = ty.inner
{
module_tracer.global_expressions_used.insert(size_expr);
}
}
// We assume that all functions are used.
//
// Observe which types, constant expressions, constants, and
@@ -111,12 +101,6 @@ pub fn compact(module: &mut crate::Module) {
})
.collect();
// Given that the above steps have marked all the constant
// expressions used directly by globals, constants, functions, and
// entry points, walk the constant expression arena to find all
// constant expressions used, directly or indirectly.
module_tracer.as_const_expression().trace_expressions();
// Constants' initializers are taken care of already, because
// expression tracing sees through constants. But we still need to
// note type usage.
@@ -134,8 +118,7 @@ pub fn compact(module: &mut crate::Module) {
}
}
// Propagate usage through types.
module_tracer.as_type().trace_types();
module_tracer.type_expression_tandem();
// Now that we know what is used and what is never touched,
// produce maps from the `Handle`s that appear in `module` now to
@@ -271,10 +254,77 @@ impl<'module> ModuleTracer<'module> {
}
}
/// Traverse types and global expressions in tandem to determine which are used.
///
/// Assuming that all types and global expressions used by other parts of
/// the module have been added to [`types_used`] and
/// [`global_expressions_used`], expand those sets to include all types and
/// global expressions reachable from those.
///
/// [`types_used`]: ModuleTracer::types_used
/// [`global_expressions_used`]: ModuleTracer::global_expressions_used
fn type_expression_tandem(&mut self) {
// For each type T, compute the latest global expression E that T and
// its predecessors refer to. Given the ordering rules on types and
// global expressions in valid modules, we can do this with a single
// forward scan of the type arena. The rules further imply that T can
// only be referred to by expressions after E.
let mut max_dep = Vec::with_capacity(self.module.types.len());
let mut previous = None;
for (_handle, ty) in self.module.types.iter() {
previous = std::cmp::max(
previous,
match ty.inner {
crate::TypeInner::Array {
base: _,
size: crate::ArraySize::Pending(crate::PendingArraySize::Expression(expr)),
stride: _,
}
| crate::TypeInner::BindingArray {
base: _,
size: crate::ArraySize::Pending(crate::PendingArraySize::Expression(expr)),
} => Some(expr),
_ => None,
},
);
max_dep.push(previous);
}
// Visit types and global expressions from youngest to oldest.
//
// The outer loop visits types. Before visiting each type, the inner
// loop ensures that all global expressions that could possibly refer to
// it have been visited. And since the inner loop stop at the latest
// expression that the type could possibly refer to, we know that we
// have previously visited any types that might refer to each expression
// we visit.
//
// This lets us assume that any type or expression that is *not* marked
// as used by the time we visit it is genuinely unused, and can be
// ignored.
let mut exprs = self.module.global_expressions.iter().rev().peekable();
for ((ty_handle, ty), dep) in self.module.types.iter().rev().zip(max_dep.iter().rev()) {
while let Some((expr_handle, expr)) = exprs.next_if(|&(h, _)| Some(h) > *dep) {
if self.global_expressions_used.contains(expr_handle) {
self.as_const_expression().trace_expression(expr);
}
}
if self.types_used.contains(ty_handle) {
self.as_type().trace_type(ty);
}
}
// Visit any remaining expressions.
for (expr_handle, expr) in exprs {
if self.global_expressions_used.contains(expr_handle) {
self.as_const_expression().trace_expression(expr);
}
}
}
fn as_type(&mut self) -> types::TypeTracer {
types::TypeTracer {
types: &self.module.types,
types_used: &mut self.types_used,
expressions_used: &mut self.global_expressions_used,
}
}
@@ -352,3 +402,114 @@ impl From<FunctionTracer<'_>> for FunctionMap {
}
}
}
#[test]
fn type_expression_interdependence() {
let mut module: crate::Module = Default::default();
let u32 = module.types.insert(
crate::Type {
name: None,
inner: crate::TypeInner::Scalar(crate::Scalar {
kind: crate::ScalarKind::Uint,
width: 4,
}),
},
crate::Span::default(),
);
let expr = module.global_expressions.append(
crate::Expression::Literal(crate::Literal::U32(0)),
crate::Span::default(),
);
let type_needs_expression = |module: &mut crate::Module, handle| {
module.types.insert(
crate::Type {
name: None,
inner: crate::TypeInner::Array {
base: u32,
size: crate::ArraySize::Pending(crate::PendingArraySize::Expression(handle)),
stride: 4,
},
},
crate::Span::default(),
)
};
let expression_needs_type = |module: &mut crate::Module, handle| {
module
.global_expressions
.append(crate::Expression::ZeroValue(handle), crate::Span::default())
};
let expression_needs_expression = |module: &mut crate::Module, handle| {
module.global_expressions.append(
crate::Expression::Load { pointer: handle },
crate::Span::default(),
)
};
let type_needs_type = |module: &mut crate::Module, handle| {
module.types.insert(
crate::Type {
name: None,
inner: crate::TypeInner::Array {
base: handle,
size: crate::ArraySize::Dynamic,
stride: 0,
},
},
crate::Span::default(),
)
};
let mut type_name_counter = 0;
let mut type_needed = |module: &mut crate::Module, handle| {
let name = Some(format!("type{}", type_name_counter));
type_name_counter += 1;
module.types.insert(
crate::Type {
name,
inner: crate::TypeInner::Array {
base: handle,
size: crate::ArraySize::Dynamic,
stride: 0,
},
},
crate::Span::default(),
)
};
let mut override_name_counter = 0;
let mut expression_needed = |module: &mut crate::Module, handle| {
let name = Some(format!("override{}", override_name_counter));
override_name_counter += 1;
module.overrides.append(
crate::Override {
name,
id: None,
ty: u32,
init: Some(handle),
},
crate::Span::default(),
)
};
let cmp_modules = |mod0: &crate::Module, mod1: &crate::Module| {
(mod0.types.iter().collect::<Vec<_>>() == mod1.types.iter().collect::<Vec<_>>())
&& (mod0.global_expressions.iter().collect::<Vec<_>>()
== mod1.global_expressions.iter().collect::<Vec<_>>())
};
// borrow checker breaks without the tmp variables as of Rust 1.83.0
let expr_end = type_needs_expression(&mut module, expr);
let ty_trace = type_needs_type(&mut module, expr_end);
let expr_init = expression_needs_type(&mut module, ty_trace);
expression_needed(&mut module, expr_init);
let ty_end = expression_needs_type(&mut module, u32);
let expr_trace = expression_needs_expression(&mut module, ty_end);
let ty_init = type_needs_expression(&mut module, expr_trace);
type_needed(&mut module, ty_init);
let untouched = module.clone();
compact(&mut module);
assert!(cmp_modules(&module, &untouched));
let unused_expr = module.global_expressions.append(
crate::Expression::Literal(crate::Literal::U32(1)),
crate::Span::default(),
);
type_needs_expression(&mut module, unused_expr);
assert!(!cmp_modules(&module, &untouched));
compact(&mut module);
assert!(cmp_modules(&module, &untouched));
}

View File

@@ -1,58 +1,53 @@
use super::{HandleSet, ModuleMap};
use crate::{Handle, UniqueArena};
use crate::Handle;
pub struct TypeTracer<'a> {
pub types: &'a UniqueArena<crate::Type>,
pub types_used: &'a mut HandleSet<crate::Type>,
pub expressions_used: &'a mut HandleSet<crate::Expression>,
}
impl TypeTracer<'_> {
/// Propagate usage through `self.types`, starting with `self.types_used`.
///
/// Treat `self.types_used` as the initial set of "known
/// live" types, and follow through to identify all
/// transitively used types.
pub fn trace_types(&mut self) {
// We don't need recursion or a work list. Because an
// expression may only refer to other expressions that precede
// it in the arena, it suffices to make a single pass over the
// arena from back to front, marking the referents of used
// expressions as used themselves.
for (handle, ty) in self.types.iter().rev() {
// If this type isn't used, it doesn't matter what it uses.
if !self.types_used.contains(handle) {
continue;
pub fn trace_type(&mut self, ty: &crate::Type) {
use crate::TypeInner as Ti;
match ty.inner {
// Types that do not contain handles.
Ti::Scalar { .. }
| Ti::Vector { .. }
| Ti::Matrix { .. }
| Ti::Atomic { .. }
| Ti::ValuePointer { .. }
| Ti::Image { .. }
| Ti::Sampler { .. }
| Ti::AccelerationStructure
| Ti::RayQuery => {}
// Types that do contain handles.
Ti::Array {
base,
size: crate::ArraySize::Pending(crate::PendingArraySize::Expression(expr)),
stride: _,
}
use crate::TypeInner as Ti;
match ty.inner {
// Types that do not contain handles.
Ti::Scalar { .. }
| Ti::Vector { .. }
| Ti::Matrix { .. }
| Ti::Atomic { .. }
| Ti::ValuePointer { .. }
| Ti::Image { .. }
| Ti::Sampler { .. }
| Ti::AccelerationStructure
| Ti::RayQuery => {}
// Types that do contain handles.
Ti::Pointer { base, space: _ }
| Ti::Array {
base,
size: _,
stride: _,
}
| Ti::BindingArray { base, size: _ } => {
self.types_used.insert(base);
}
Ti::Struct {
ref members,
span: _,
} => {
self.types_used.insert_iter(members.iter().map(|m| m.ty));
}
| Ti::BindingArray {
base,
size: crate::ArraySize::Pending(crate::PendingArraySize::Expression(expr)),
} => {
self.expressions_used.insert(expr);
self.types_used.insert(base);
}
Ti::Pointer { base, space: _ }
| Ti::Array {
base,
size: _,
stride: _,
}
| Ti::BindingArray { base, size: _ } => {
self.types_used.insert(base);
}
Ti::Struct {
ref members,
span: _,
} => {
self.types_used.insert_iter(members.iter().map(|m| m.ty));
}
}
}