Skip to content

Commit

Permalink
refactor(wgsl-in): track diagnostic directives in func. analysis
Browse files Browse the repository at this point in the history
  • Loading branch information
ErichDonGubler committed Nov 9, 2024
1 parent 5f26bb3 commit f950c59
Show file tree
Hide file tree
Showing 40 changed files with 308 additions and 12 deletions.
74 changes: 73 additions & 1 deletion naga/src/diagnostic_filter.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,24 @@
//! [`DiagnosticFilter`]s and supporting functionality.
use crate::Handle;
#[cfg(feature = "wgsl-in")]
use crate::Span;
use crate::{Arena, Handle};
#[cfg(feature = "arbitrary")]
use arbitrary::Arbitrary;
#[cfg(feature = "wgsl-in")]
use indexmap::IndexMap;
#[cfg(feature = "deserialize")]
use serde::Deserialize;
#[cfg(feature = "serialize")]
use serde::Serialize;

/// A severity set on a [`DiagnosticFilter`].
///
/// <https://www.w3.org/TR/WGSL/#diagnostic-severity>
#[derive(Clone, Copy, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)]
#[cfg_attr(feature = "serialize", derive(Serialize))]
#[cfg_attr(feature = "deserialize", derive(Deserialize))]
#[cfg_attr(feature = "arbitrary", derive(Arbitrary))]
pub enum Severity {
Off,
Info,
Expand Down Expand Up @@ -63,6 +72,9 @@ impl Severity {
///
/// <https://www.w3.org/TR/WGSL/#filterable-triggering-rules>
#[derive(Clone, Copy, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)]
#[cfg_attr(feature = "serialize", derive(Serialize))]
#[cfg_attr(feature = "deserialize", derive(Deserialize))]
#[cfg_attr(feature = "arbitrary", derive(Arbitrary))]
pub enum FilterableTriggeringRule {
DerivativeUniformity,
}
Expand All @@ -84,12 +96,26 @@ impl FilterableTriggeringRule {
Self::DerivativeUniformity => Self::DERIVATIVE_UNIFORMITY,
}
}

/// The default severity associated with this triggering rule.
///
/// See <https://www.w3.org/TR/WGSL/#filterable-triggering-rules> for a table of default
/// severities.
#[allow(dead_code)]
pub(crate) const fn default_severity(self) -> Severity {
match self {
FilterableTriggeringRule::DerivativeUniformity => Severity::Error,
}
}
}

/// A filter that modifies how diagnostics are emitted for shaders.
///
/// <https://www.w3.org/TR/WGSL/#diagnostic-filter>
#[derive(Clone, Debug)]
#[cfg_attr(feature = "serialize", derive(Serialize))]
#[cfg_attr(feature = "deserialize", derive(Deserialize))]
#[cfg_attr(feature = "arbitrary", derive(Arbitrary))]
pub struct DiagnosticFilter {
pub new_severity: Severity,
pub triggering_rule: FilterableTriggeringRule,
Expand Down Expand Up @@ -140,6 +166,18 @@ impl DiagnosticFilterMap {
}
}

#[cfg(feature = "wgsl-in")]
impl IntoIterator for DiagnosticFilterMap {
type Item = (FilterableTriggeringRule, (Severity, Span));

type IntoIter = indexmap::map::IntoIter<FilterableTriggeringRule, (Severity, Span)>;

fn into_iter(self) -> Self::IntoIter {
let Self(this) = self;
this.into_iter()
}
}

/// An error returned by [`DiagnosticFilterMap::add`] when it encounters conflicting rules.
#[cfg(feature = "wgsl-in")]
#[derive(Clone, Debug)]
Expand Down Expand Up @@ -175,7 +213,41 @@ pub(crate) struct ConflictingDiagnosticRuleError {
/// - `d` is the first leaf consulted by validation in `c_and_d_func`.
/// - `e` is the first leaf consulted by validation in `e_func`.
#[derive(Clone, Debug)]
#[cfg_attr(feature = "serialize", derive(Serialize))]
#[cfg_attr(feature = "deserialize", derive(Deserialize))]
#[cfg_attr(feature = "arbitrary", derive(Arbitrary))]
pub struct DiagnosticFilterNode {
pub inner: DiagnosticFilter,
pub parent: Option<Handle<DiagnosticFilterNode>>,
}

impl DiagnosticFilterNode {
/// Finds the most specific filter rule applicable to `triggering_rule` from the chain of
/// diagnostic filter rules in `arena`, starting with `node`, and returns its severity. If none
/// is found, return the value of [`FilterableTriggeringRule::default_severity`].
///
/// When `triggering_rule` is not applicable to this node, its parent is consulted recursively.
#[allow(dead_code)]
pub(crate) fn search(
node: Option<Handle<Self>>,
arena: &Arena<Self>,
triggering_rule: FilterableTriggeringRule,
) -> Severity {
let mut next = node;
while let Some(handle) = next {
let node = &arena[handle];
let &Self { ref inner, parent } = node;
let &DiagnosticFilter {
triggering_rule: rule,
new_severity,
} = inner;

if rule == triggering_rule {
return new_severity;
}

next = parent;
}
triggering_rule.default_severity()
}
}
6 changes: 5 additions & 1 deletion naga/src/front/wgsl/lower/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1013,7 +1013,11 @@ impl<'source, 'temp> Lowerer<'source, 'temp> {
&mut self,
tu: &'temp ast::TranslationUnit<'source>,
) -> Result<crate::Module, Error<'source>> {
let mut module = crate::Module::default();
let mut module = crate::Module {
diagnostic_filters: tu.diagnostic_filters.clone(),
diagnostic_filter_leaf: tu.diagnostic_filter_leaf,
..Default::default()
};

let mut ctx = GlobalContext {
ast_expressions: &tu.expressions,
Expand Down
12 changes: 12 additions & 0 deletions naga/src/front/wgsl/parse/ast.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use crate::diagnostic_filter::DiagnosticFilterNode;
use crate::front::wgsl::parse::directive::enable_extension::EnableExtensions;
use crate::front::wgsl::parse::number::Number;
use crate::front::wgsl::Scalar;
Expand Down Expand Up @@ -26,6 +27,17 @@ pub struct TranslationUnit<'a> {
/// These are referred to by `Handle<ast::Type<'a>>` values.
/// User-defined types are referred to by name until lowering.
pub types: Arena<Type<'a>>,

/// Arena for all diagnostic filter rules parsed in this module, including those in functions.
///
/// See [`DiagnosticFilterNode`] for details on how the tree is represented and used in
/// validation.
pub diagnostic_filters: Arena<DiagnosticFilterNode>,
/// The leaf of all `diagnostic(…)` directives in this module.
///
/// See [`DiagnosticFilterNode`] for details on how the tree is represented and used in
/// validation.
pub diagnostic_filter_leaf: Option<Handle<DiagnosticFilterNode>>,
}

#[derive(Debug, Clone, Copy)]
Expand Down
25 changes: 24 additions & 1 deletion naga/src/front/wgsl/parse/mod.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::diagnostic_filter::{
self, DiagnosticFilter, DiagnosticFilterMap, FilterableTriggeringRule,
self, DiagnosticFilter, DiagnosticFilterMap, DiagnosticFilterNode, FilterableTriggeringRule,
};
use crate::front::wgsl::error::{Error, ExpectedToken};
use crate::front::wgsl::parse::directive::enable_extension::{
Expand Down Expand Up @@ -2581,6 +2581,8 @@ impl Parser {

lexer.enable_extensions = enable_extensions.clone();
tu.enable_extensions = enable_extensions;
tu.diagnostic_filter_leaf =
Self::write_diagnostic_filters(&mut tu.diagnostic_filters, diagnostic_filters, None);

loop {
match self.global_decl(&mut lexer, &mut tu) {
Expand Down Expand Up @@ -2672,4 +2674,25 @@ impl Parser {

Ok(filter)
}

pub(crate) fn write_diagnostic_filters(
arena: &mut Arena<DiagnosticFilterNode>,
filters: DiagnosticFilterMap,
parent: Option<Handle<DiagnosticFilterNode>>,
) -> Option<Handle<DiagnosticFilterNode>> {
filters
.into_iter()
.fold(parent, |parent, (triggering_rule, (new_severity, span))| {
Some(arena.append(
DiagnosticFilterNode {
inner: DiagnosticFilter {
new_severity,
triggering_rule,
},
parent,
},
span,
))
})
}
}
14 changes: 14 additions & 0 deletions naga/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,7 @@ pub use crate::arena::{Arena, Handle, Range, UniqueArena};
pub use crate::span::{SourceLocation, Span, SpanContext, WithSpan};
#[cfg(feature = "arbitrary")]
use arbitrary::Arbitrary;
use diagnostic_filter::DiagnosticFilterNode;
#[cfg(feature = "deserialize")]
use serde::Deserialize;
#[cfg(feature = "serialize")]
Expand Down Expand Up @@ -2271,4 +2272,17 @@ pub struct Module {
pub functions: Arena<Function>,
/// Entry points.
pub entry_points: Vec<EntryPoint>,
/// Arena for all diagnostic filter rules parsed in this module, including those in functions
/// and statements.
///
/// This arena contains elements of a _tree_ of diagnostic filter rules. When nodes are built
/// by a front-end, they refer to a parent scope
pub diagnostic_filters: Arena<DiagnosticFilterNode>,
/// The leaf of all diagnostic filter rules tree parsed from directives in this module.
///
/// In WGSL, this corresponds to `diagnostic(…);` directives.
///
/// See [`DiagnosticFilterNode`] for details on how the tree is represented and used in
/// validation.
pub diagnostic_filter_leaf: Option<Handle<DiagnosticFilterNode>>,
}
53 changes: 44 additions & 9 deletions naga/src/valid/analyzer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
//! - expression reference counts
use super::{ExpressionError, FunctionError, ModuleInfo, ShaderStages, ValidationFlags};
use crate::diagnostic_filter::DiagnosticFilterNode;
use crate::span::{AddSpan as _, WithSpan};
use crate::{
arena::{Arena, Handle},
Expand Down Expand Up @@ -289,6 +290,13 @@ pub struct FunctionInfo {

/// Indicates that the function is using dual source blending.
pub dual_source_blending: bool,

/// The leaf of all module-wide diagnostic filter rules tree parsed from directives in this
/// module.
///
/// See [`DiagnosticFilterNode`] for details on how the tree is represented and used in
/// validation.
diagnostic_filter_leaf: Option<Handle<DiagnosticFilterNode>>,
}

impl FunctionInfo {
Expand Down Expand Up @@ -820,12 +828,14 @@ impl FunctionInfo {
/// Returns a `NonUniformControlFlow` error if any of the expressions in the block
/// require uniformity, but the current flow is non-uniform.
#[allow(clippy::or_fun_call)]
#[allow(clippy::only_used_in_recursion)]
fn process_block(
&mut self,
statements: &crate::Block,
other_functions: &[FunctionInfo],
mut disruptor: Option<UniformityDisruptor>,
expression_arena: &Arena<crate::Expression>,
diagnostic_filter_arena: &Arena<DiagnosticFilterNode>,
) -> Result<FunctionUniformity, WithSpan<FunctionError>> {
use crate::Statement as S;

Expand Down Expand Up @@ -901,9 +911,13 @@ impl FunctionInfo {
exit: ExitFlags::empty(),
}
}
S::Block(ref b) => {
self.process_block(b, other_functions, disruptor, expression_arena)?
}
S::Block(ref b) => self.process_block(
b,
other_functions,
disruptor,
expression_arena,
diagnostic_filter_arena,
)?,
S::If {
condition,
ref accept,
Expand All @@ -917,12 +931,14 @@ impl FunctionInfo {
other_functions,
branch_disruptor,
expression_arena,
diagnostic_filter_arena,
)?;
let reject_uniformity = self.process_block(
reject,
other_functions,
branch_disruptor,
expression_arena,
diagnostic_filter_arena,
)?;
accept_uniformity | reject_uniformity
}
Expand All @@ -941,6 +957,7 @@ impl FunctionInfo {
other_functions,
case_disruptor,
expression_arena,
diagnostic_filter_arena,
)?;
case_disruptor = if case.fall_through {
case_disruptor.or(case_uniformity.exit_disruptor())
Expand All @@ -956,14 +973,20 @@ impl FunctionInfo {
ref continuing,
break_if,
} => {
let body_uniformity =
self.process_block(body, other_functions, disruptor, expression_arena)?;
let body_uniformity = self.process_block(
body,
other_functions,
disruptor,
expression_arena,
diagnostic_filter_arena,
)?;
let continuing_disruptor = disruptor.or(body_uniformity.exit_disruptor());
let continuing_uniformity = self.process_block(
continuing,
other_functions,
continuing_disruptor,
expression_arena,
diagnostic_filter_arena,
)?;
if let Some(expr) = break_if {
let _ = self.add_ref(expr);
Expand Down Expand Up @@ -1117,6 +1140,7 @@ impl ModuleInfo {
expressions: vec![ExpressionInfo::new(); fun.expressions.len()].into_boxed_slice(),
sampling: crate::FastHashSet::default(),
dual_source_blending: false,
diagnostic_filter_leaf: module.diagnostic_filter_leaf,
};
let resolve_context =
ResolveContext::with_locals(module, &fun.local_variables, &fun.arguments);
Expand All @@ -1140,7 +1164,13 @@ impl ModuleInfo {
}
}

let uniformity = info.process_block(&fun.body, &self.functions, None, &fun.expressions)?;
let uniformity = info.process_block(
&fun.body,
&self.functions,
None,
&fun.expressions,
&module.diagnostic_filters,
)?;
info.uniformity = uniformity.result;
info.may_kill = uniformity.exit.contains(ExitFlags::MAY_KILL);

Expand Down Expand Up @@ -1230,6 +1260,7 @@ fn uniform_control_flow() {
expressions: vec![ExpressionInfo::new(); expressions.len()].into_boxed_slice(),
sampling: crate::FastHashSet::default(),
dual_source_blending: false,
diagnostic_filter_leaf: None,
};
let resolve_context = ResolveContext {
constants: &Arena::new(),
Expand Down Expand Up @@ -1276,7 +1307,8 @@ fn uniform_control_flow() {
&vec![stmt_emit1, stmt_if_uniform].into(),
&[],
None,
&expressions
&expressions,
&Arena::new(),
),
Ok(FunctionUniformity {
result: Uniformity {
Expand Down Expand Up @@ -1308,6 +1340,7 @@ fn uniform_control_flow() {
&[],
None,
&expressions,
&Arena::new(),
);
if DISABLE_UNIFORMITY_REQ_FOR_FRAGMENT_STAGE {
assert_eq!(info[derivative_expr].ref_count, 2);
Expand Down Expand Up @@ -1335,7 +1368,8 @@ fn uniform_control_flow() {
&vec![stmt_emit3, stmt_return_non_uniform].into(),
&[],
Some(UniformityDisruptor::Return),
&expressions
&expressions,
&Arena::new(),
),
Ok(FunctionUniformity {
result: Uniformity {
Expand All @@ -1362,7 +1396,8 @@ fn uniform_control_flow() {
&vec![stmt_emit4, stmt_assign, stmt_kill, stmt_return_pointer].into(),
&[],
Some(UniformityDisruptor::Discard),
&expressions
&expressions,
&Arena::new(),
),
Ok(FunctionUniformity {
result: Uniformity {
Expand Down
Loading

0 comments on commit f950c59

Please sign in to comment.