Skip to content

Commit

Permalink
insert order check
Browse files Browse the repository at this point in the history
  • Loading branch information
@kentslaney
kentslaney committed Jan 3, 2025
1 parent c9c38b2 commit babbb86
Show file tree
Hide file tree
Showing 3 changed files with 185 additions and 159 deletions.
313 changes: 158 additions & 155 deletions naga/src/compact/expressions.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,168 +84,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::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);
}
Sl::Gradient { x, y } => self.expressions_used.insert_iter([x, y]),
}
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::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::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 => {}
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]),
}
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);
}
}
}
}
Expand Down
5 changes: 2 additions & 3 deletions naga/src/compact/mod.rs
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
mod expressions;
pub mod expressions;
mod functions;
mod handle_set_map;
mod statements;
Expand Down Expand Up @@ -259,8 +259,7 @@ impl<'module> ModuleTracer<'module> {
// assume there are no cycles in the type/expression graph (guaranteed by validator)
// assume that the expressions are well ordered since they're not merged like types are
// ie. expression A referring to a type referring to expression B has A > B.

// TODO: check dependency ordering in validator
// (also guaranteed by validator)

// 1. iterate over types, skipping unused ones
// a. if the type references an expression, mark it used
Expand Down
26 changes: 25 additions & 1 deletion naga/src/valid/handles.rs
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
//! Implementation of `Validator::validate_module_handles`.
use crate::{
arena::{BadHandle, BadRangeError},
arena::{HandleSet, BadHandle, BadRangeError},
diagnostic_filter::DiagnosticFilterNode,
compact::expressions::ExpressionTracer,
Handle,
};

Expand Down Expand Up @@ -74,6 +75,7 @@ impl super::Validator {

for handle_and_expr in global_expressions.iter() {
Self::validate_const_expression_handles(handle_and_expr, constants, overrides, types)?;
Self::well_ordered_deps(handle_and_expr, constants, global_expressions, types)?;
}

let validate_type = |handle| Self::validate_type_handle(handle, types);
Expand Down Expand Up @@ -629,6 +631,28 @@ impl super::Validator {
| crate::Statement::Barrier(_) => Ok(()),
})
}

pub fn well_ordered_deps(
(handle, expression): (Handle<crate::Expression>, &crate::Expression),
constants: &Arena<crate::Constant>,
global_expressions: &Arena<crate::Expression>,
types: &UniqueArena<crate::Type>,
) -> Result<(), InvalidHandleError> {
let mut exprs = HandleSet::for_arena(global_expressions);
ExpressionTracer {
types: Some(types),
expressions: global_expressions,
constants: constants,
types_used: &mut HandleSet::for_arena(types),
constants_used: &mut HandleSet::for_arena(constants),
expressions_used: &mut exprs,
global_expressions_used: None,
}.trace_expression(expression);
if let Err(error) = handle.check_dep_iter(exprs.iter()) {
return Err(InvalidHandleError::ForwardDependency(error));
}
Ok(())
}
}

impl From<BadHandle> for ValidationError {
Expand Down

0 comments on commit babbb86

Please sign in to comment.