Skip to content

Commit

Permalink
removes PendingArraySize in favor of Handle<Override> and keeps overr…
Browse files Browse the repository at this point in the history
…ides after pipeline_constants
  • Loading branch information
kentslaney committed Feb 7, 2025
1 parent 410ab29 commit a22ff5e
Show file tree
Hide file tree
Showing 13 changed files with 99 additions and 149 deletions.
10 changes: 10 additions & 0 deletions naga/src/arena/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,16 @@ impl<T> Arena<T> {
.map(|(i, v)| unsafe { (Handle::from_usize_unchecked(i), v) })
}

/// Returns an iterator over the items stored in this arena, returning both
/// the item's handle and a reference to it.
pub fn iter_mut_span(&mut self) -> impl DoubleEndedIterator<Item = (Handle<T>, &mut T, &Span)> + ExactSizeIterator {
self.data
.iter_mut()
.zip(self.span_info.iter())
.enumerate()
.map(|(i, (v, span))| unsafe { (Handle::from_usize_unchecked(i), v, span) })
}

/// Drains the arena, returning an iterator over the items stored.
pub fn drain(&mut self) -> impl DoubleEndedIterator<Item = (Handle<T>, T, Span)> {
let arena = std::mem::take(self);
Expand Down
4 changes: 0 additions & 4 deletions naga/src/back/glsl/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -576,10 +576,6 @@ impl<'a, W: Write> Writer<'a, W> {
pipeline_options: &'a PipelineOptions,
policies: proc::BoundsCheckPolicies,
) -> Result<Self, Error> {
if !module.overrides.is_empty() {
return Err(Error::Override);
}

// Check if the requested version is supported
if !options.version.is_supported() {
log::error!("Version {}", options.version);
Expand Down
4 changes: 0 additions & 4 deletions naga/src/back/hlsl/writer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -256,10 +256,6 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {
module_info: &valid::ModuleInfo,
fragment_entry_point: Option<&FragmentEntryPoint<'_>>,
) -> Result<super::ReflectionInfo, Error> {
if !module.overrides.is_empty() {
return Err(Error::Override);
}

self.reset(module);

// Write special constants, if needed
Expand Down
4 changes: 0 additions & 4 deletions naga/src/back/msl/writer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3713,10 +3713,6 @@ impl<W: Write> Writer<W> {
options: &Options,
pipeline_options: &PipelineOptions,
) -> Result<TranslationInfo, Error> {
if !module.overrides.is_empty() {
return Err(Error::Override);
}

self.names.clear();
self.namer.reset(
module,
Expand Down
95 changes: 21 additions & 74 deletions naga/src/back/pipeline_constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,8 @@ pub fn process_overrides<'a>(

// An iterator through the original overrides table, consumed in
// approximate tandem with the global expressions.
let mut override_iter = module.overrides.drain();
let mut overrides = mem::take(&mut module.overrides);
let mut override_iter = overrides.iter_mut_span();

// Do two things in tandem:
//
Expand Down Expand Up @@ -156,15 +157,20 @@ pub fn process_overrides<'a>(

// Finish processing any overrides we didn't visit in the loop above.
for entry in override_iter {
process_override(
entry,
pipeline_constants,
&mut module,
&mut override_map,
&adjusted_global_expressions,
&mut adjusted_constant_initializers,
&mut global_expression_kind_tracker,
)?;
let (_, Override { ref name, ref id, .. }, _) = entry;
if name.is_some() || id.is_some() {
process_override(
entry,
pipeline_constants,
&mut module,
&mut override_map,
&adjusted_global_expressions,
&mut adjusted_constant_initializers,
&mut global_expression_kind_tracker,
)?;
} else if let (_, Override { init: Some(ref mut init), .. }, _) = entry {
*init = adjusted_global_expressions[*init];
}
}

// Update the initialization expression handles of all `Constant`s
Expand Down Expand Up @@ -196,76 +202,17 @@ pub fn process_overrides<'a>(
process_workgroup_size_override(&mut module, &adjusted_global_expressions, ep)?;
}
module.entry_points = entry_points;

process_pending(&mut module, &override_map, &adjusted_global_expressions)?;
module.overrides = overrides;

// Now that we've rewritten all the expressions, we need to
// recompute their types and other metadata. For the time being,
// do a full re-validation.
let mut validator = Validator::new(ValidationFlags::all(), Capabilities::all());
let module_info = validator.validate_no_overrides(&module)?;
let module_info = validator.validate(&module)?;

Ok((Cow::Owned(module), Cow::Owned(module_info)))
}

fn process_pending(
module: &mut Module,
override_map: &HandleVec<Override, Handle<Constant>>,
adjusted_global_expressions: &HandleVec<Expression, Handle<Expression>>,
) -> Result<(), PipelineConstantError> {
for (handle, ty) in module.types.clone().iter() {
if let TypeInner::Array {
base,
size: crate::ArraySize::Pending(size),
stride,
} = ty.inner
{
let expr = match size {
crate::PendingArraySize::Expression(size_expr) => {
adjusted_global_expressions[size_expr]
}
crate::PendingArraySize::Override(size_override) => {
module.constants[override_map[size_override]].init
}
};
let value = module
.to_ctx()
.eval_expr_to_u32(expr)
.map(|n| {
if n == 0 {
Err(PipelineConstantError::ValidationError(
WithSpan::new(ValidationError::ArraySizeError { handle: expr })
.with_span(
module.global_expressions.get_span(expr),
"evaluated to zero",
),
))
} else {
Ok(std::num::NonZeroU32::new(n).unwrap())
}
})
.map_err(|_| {
PipelineConstantError::ValidationError(
WithSpan::new(ValidationError::ArraySizeError { handle: expr })
.with_span(module.global_expressions.get_span(expr), "negative"),
)
})??;
module.types.replace(
handle,
crate::Type {
name: None,
inner: TypeInner::Array {
base,
size: crate::ArraySize::Constant(value),
stride,
},
},
);
}
}
Ok(())
}

fn process_workgroup_size_override(
module: &mut Module,
adjusted_global_expressions: &HandleVec<Expression, Handle<Expression>>,
Expand Down Expand Up @@ -305,7 +252,7 @@ fn process_workgroup_size_override(
///
/// Add the new `Constant` to `override_map` and `adjusted_constant_initializers`.
fn process_override(
(old_h, r#override, span): (Handle<Override>, Override, Span),
(old_h, r#override, span): (Handle<Override>, &mut Override, &Span),
pipeline_constants: &PipelineConstants,
module: &mut Module,
override_map: &mut HandleVec<Override, Handle<Constant>>,
Expand Down Expand Up @@ -343,11 +290,11 @@ fn process_override(

// Generate a new `Constant` to represent the override's value.
let constant = Constant {
name: r#override.name,
name: r#override.name.clone(),
ty: r#override.ty,
init,
};
let h = module.constants.append(constant, span);
let h = module.constants.append(constant, *span);
override_map.insert(old_h, h);
adjusted_constant_initializers.insert(h);
Ok(h)
Expand Down
4 changes: 0 additions & 4 deletions naga/src/back/spv/writer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2078,10 +2078,6 @@ impl Writer {
debug_info: &Option<DebugInfo>,
words: &mut Vec<Word>,
) -> Result<(), Error> {
if !ir_module.overrides.is_empty() {
return Err(Error::Override);
}

self.reset();

// Try to find the entry point and corresponding index
Expand Down
27 changes: 18 additions & 9 deletions naga/src/compact/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -301,11 +301,8 @@ impl<'module> ModuleTracer<'module> {
crate::TypeInner::Array { size, .. }
| crate::TypeInner::BindingArray { size, .. } => match size {
crate::ArraySize::Constant(_) | crate::ArraySize::Dynamic => None,
crate::ArraySize::Pending(pending) => match pending {
crate::PendingArraySize::Expression(handle) => Some(handle),
crate::PendingArraySize::Override(handle) => {
self.module.overrides[handle].init
}
crate::ArraySize::Pending(handle) => {
self.module.overrides[handle].init
},
},
_ => None,
Expand Down Expand Up @@ -454,12 +451,18 @@ fn type_expression_interdependence() {
crate::Span::default(),
);
let type_needs_expression = |module: &mut crate::Module, handle| {
let ty_handle = module.overrides.append(crate::Override {
name: None,
id: None,
ty: u32,
init: Some(handle),
}, crate::Span::default());
module.types.insert(
crate::Type {
name: None,
inner: crate::TypeInner::Array {
base: u32,
size: crate::ArraySize::Pending(crate::PendingArraySize::Expression(handle)),
size: crate::ArraySize::Pending(ty_handle),
stride: 4,
},
},
Expand Down Expand Up @@ -591,7 +594,7 @@ fn array_length_override() {
name: Some("array<bool, o>".to_string()),
inner: crate::TypeInner::Array {
base: ty_bool,
size: crate::ArraySize::Pending(crate::PendingArraySize::Override(o)),
size: crate::ArraySize::Pending(o),
stride: 4,
},
},
Expand Down Expand Up @@ -697,7 +700,7 @@ fn array_length_override_mutual() {
name: Some("delicious_array".to_string()),
inner: Ti::Array {
base: ty_u32,
size: crate::ArraySize::Pending(crate::PendingArraySize::Override(second_override)),
size: crate::ArraySize::Pending(second_override),
stride: 4,
},
},
Expand Down Expand Up @@ -732,12 +735,18 @@ fn array_length_expression() {
crate::Expression::Literal(crate::Literal::U32(1)),
crate::Span::default(),
);
let ty_handle = module.overrides.append(crate::Override {
name: None,
id: None,
ty: ty_u32,
init: Some(one),
}, crate::Span::default());
let _ty_array = module.types.insert(
crate::Type {
name: Some("array<u32, 1>".to_string()),
inner: crate::TypeInner::Array {
base: ty_u32,
size: crate::ArraySize::Pending(crate::PendingArraySize::Expression(one)),
size: crate::ArraySize::Pending(ty_handle),
stride: 4,
},
},
Expand Down
24 changes: 6 additions & 18 deletions naga/src/compact/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,18 +32,13 @@ impl TypeTracer<'_> {
| Ti::BindingArray { base, size } => {
self.types_used.insert(base);
match size {
crate::ArraySize::Pending(pending) => match pending {
crate::PendingArraySize::Expression(expr) => {
crate::ArraySize::Pending(handle) => {
self.overrides_used.insert(handle);
let r#override = &self.overrides[handle];
self.types_used.insert(r#override.ty);
if let Some(expr) = r#override.init {
self.expressions_used.insert(expr);
}
crate::PendingArraySize::Override(handle) => {
self.overrides_used.insert(handle);
let r#override = &self.overrides[handle];
self.types_used.insert(r#override.ty);
if let Some(expr) = r#override.init {
self.expressions_used.insert(expr);
}
}
},
crate::ArraySize::Constant(_) | crate::ArraySize::Dynamic => {}
}
Expand Down Expand Up @@ -94,14 +89,7 @@ impl ModuleMap {
} => {
adjust(base);
match *size {
crate::ArraySize::Pending(crate::PendingArraySize::Expression(
ref mut size_expr,
)) => {
self.global_expressions.adjust(size_expr);
}
crate::ArraySize::Pending(crate::PendingArraySize::Override(
ref mut r#override,
)) => {
crate::ArraySize::Pending(ref mut r#override) => {
self.overrides.adjust(r#override);
}
crate::ArraySize::Constant(_) | crate::ArraySize::Dynamic => {}
Expand Down
12 changes: 9 additions & 3 deletions naga/src/front/wgsl/lower/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3175,14 +3175,20 @@ impl<'source, 'temp> Lowerer<'source, 'temp> {
size_expr: Handle<ast::Expression<'source>>,
ctx: &mut ExpressionContext<'source, '_, '_>,
span: Span,
) -> Result<crate::PendingArraySize, Error<'source>> {
) -> Result<Handle<crate::Override>, Error<'source>> {
let expr = self.expression(size_expr, ctx)?;
match resolve_inner!(ctx, expr).scalar_kind().ok_or(0) {
Ok(crate::ScalarKind::Sint) | Ok(crate::ScalarKind::Uint) => Ok({
if let crate::Expression::Override(handle) = ctx.module.global_expressions[expr] {
crate::PendingArraySize::Override(handle)
handle
} else {
crate::PendingArraySize::Expression(expr)
let ty = ctx.register_type(expr)?;
ctx.module.overrides.append(crate::Override {
name: None,
id: None,
ty,
init: Some(expr),
}, span)
}
}),
_ => Err(Error::ExpectedConstExprConcreteIntegerScalar(span)),
Expand Down
11 changes: 1 addition & 10 deletions naga/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -494,15 +494,6 @@ pub struct Scalar {
pub width: Bytes,
}

#[derive(Clone, Copy, Debug, Hash, Eq, Ord, PartialEq, PartialOrd)]
#[cfg_attr(feature = "serialize", derive(Serialize))]
#[cfg_attr(feature = "deserialize", derive(Deserialize))]
#[cfg_attr(feature = "arbitrary", derive(Arbitrary))]
pub enum PendingArraySize {
Expression(Handle<Expression>),
Override(Handle<Override>),
}

/// Size of an array.
#[repr(u8)]
#[derive(Clone, Copy, Debug, Hash, Eq, Ord, PartialEq, PartialOrd)]
Expand All @@ -513,7 +504,7 @@ pub enum ArraySize {
/// The array size is constant.
Constant(std::num::NonZeroU32),
/// The array size is an override-expression.
Pending(PendingArraySize),
Pending(Handle<Override>),
/// The array size can change at runtime.
Dynamic,
}
Expand Down
Loading

0 comments on commit a22ff5e

Please sign in to comment.