Skip to content

Commit

Permalink
resolve array sizes in backends (mostly via gfx-rs#6787)
Browse files Browse the repository at this point in the history
  • Loading branch information
kentslaney committed Feb 7, 2025
1 parent de9a2af commit 60c12b3
Show file tree
Hide file tree
Showing 12 changed files with 137 additions and 75 deletions.
21 changes: 9 additions & 12 deletions naga/src/back/glsl/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -503,6 +503,8 @@ pub enum Error {
/// [`crate::Sampling::First`] is unsupported.
#[error("`{:?}` sampling is unsupported", crate::Sampling::First)]
FirstSamplingNotSupported,
#[error(transparent)]
ResolveArraySizeError(#[from] proc::ResolveArraySizeError),
}

/// Binary operation with a different logic on the GLSL side.
Expand Down Expand Up @@ -973,13 +975,12 @@ impl<'a, W: Write> Writer<'a, W> {
write!(self.out, "[")?;

// Write the array size
// Writes nothing if `ArraySize::Dynamic`
match size {
crate::ArraySize::Constant(size) => {
// Writes nothing if `ResolvedSize::Dynamic`
match size.resolve(self.module.to_ctx())? {
proc::ResolvedSize::Constant(size) => {
write!(self.out, "{size}")?;
}
crate::ArraySize::Pending(_) => unreachable!(),
crate::ArraySize::Dynamic => (),
proc::ResolvedSize::Dynamic => (),
}

write!(self.out, "]")?;
Expand Down Expand Up @@ -4516,13 +4517,9 @@ impl<'a, W: Write> Writer<'a, W> {
write!(self.out, ")")?;
}
TypeInner::Array { base, size, .. } => {
let count = match size
.to_indexable_length(self.module)
.expect("Bad array size")
{
proc::IndexableLength::Known(count) => count,
proc::IndexableLength::Pending => unreachable!(),
proc::IndexableLength::Dynamic => return Ok(()),
let count = match size.resolve(self.module.to_ctx())? {
proc::ResolvedSize::Constant(count) => count,
proc::ResolvedSize::Dynamic => return Ok(()),
};
self.write_type(base)?;
self.write_array_size(base, size)?;
Expand Down
17 changes: 8 additions & 9 deletions naga/src/back/hlsl/conv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ impl crate::TypeInner {
}
}

pub(super) fn size_hlsl(&self, gctx: crate::proc::GlobalCtx) -> u32 {
pub(super) fn size_hlsl(&self, gctx: crate::proc::GlobalCtx) -> Result<u32, Error> {
match *self {
Self::Matrix {
columns,
Expand All @@ -62,19 +62,18 @@ impl crate::TypeInner {
} => {
let stride = Alignment::from(rows) * scalar.width as u32;
let last_row_size = rows as u32 * scalar.width as u32;
((columns as u32 - 1) * stride) + last_row_size
Ok(((columns as u32 - 1) * stride) + last_row_size)
}
Self::Array { base, size, stride } => {
let count = match size {
crate::ArraySize::Constant(size) => size.get(),
let count = match size.resolve(gctx)? {
crate::proc::ResolvedSize::Constant(size) => size,
// A dynamically-sized array has to have at least one element
crate::ArraySize::Pending(_) => unreachable!(),
crate::ArraySize::Dynamic => 1,
crate::proc::ResolvedSize::Dynamic => 1,
};
let last_el_size = gctx.types[base].inner.size_hlsl(gctx);
((count - 1) * stride) + last_el_size
let last_el_size = gctx.types[base].inner.size_hlsl(gctx)?;
Ok(((count - 1) * stride) + last_el_size)
}
_ => self.size(gctx),
_ => Ok(self.size(gctx)),
}
}

Expand Down
2 changes: 2 additions & 0 deletions naga/src/back/hlsl/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -353,6 +353,8 @@ pub enum Error {
Custom(String),
#[error("overrides should not be present at this stage")]
Override,
#[error(transparent)]
ResolveArraySizeError(#[from] proc::ResolveArraySizeError),
}

#[derive(Default)]
Expand Down
10 changes: 4 additions & 6 deletions naga/src/back/hlsl/writer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1113,12 +1113,11 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {
) -> BackendResult {
write!(self.out, "[")?;

match size {
crate::ArraySize::Constant(size) => {
match size.resolve(module.to_ctx())? {
proc::ResolvedSize::Constant(size) => {
write!(self.out, "{size}")?;
}
crate::ArraySize::Pending(_) => unreachable!(),
crate::ArraySize::Dynamic => unreachable!(),
proc::ResolvedSize::Dynamic => unreachable!(),
}

write!(self.out, "]")?;
Expand Down Expand Up @@ -1163,7 +1162,7 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {
}
}
let ty_inner = &module.types[member.ty].inner;
last_offset = member.offset + ty_inner.size_hlsl(module.to_ctx());
last_offset = member.offset + ty_inner.size_hlsl(module.to_ctx())?;

// The indentation is only for readability
write!(self.out, "{}", back::INDENT)?;
Expand Down Expand Up @@ -2885,7 +2884,6 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {
index::IndexableLength::Known(limit) => {
write!(self.out, "{}u", limit - 1)?;
}
index::IndexableLength::Pending => unreachable!(),
index::IndexableLength::Dynamic => unreachable!(),
}
write!(self.out, ")")?;
Expand Down
2 changes: 2 additions & 0 deletions naga/src/back/msl/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,8 @@ pub enum Error {
Override,
#[error("bitcasting to {0:?} is not supported")]
UnsupportedBitCast(crate::TypeInner),
#[error(transparent)]
ResolveArraySizeError(#[from] crate::proc::ResolveArraySizeError),
}

#[derive(Clone, Debug, PartialEq, thiserror::Error)]
Expand Down
24 changes: 8 additions & 16 deletions naga/src/back/msl/writer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@ use super::{sampler as sm, Error, LocationMode, Options, PipelineOptions, Transl
use crate::{
arena::{Handle, HandleSet},
back::{self, Baked},
proc::index,
proc::{self, NameKey, TypeResolution},
proc::{self, index, NameKey, TypeResolution},
valid, FastHashMap, FastHashSet,
};
#[cfg(test)]
Expand Down Expand Up @@ -2554,7 +2553,6 @@ impl<W: Write> Writer<W> {
self.out.write_str(") < ")?;
match length {
index::IndexableLength::Known(value) => write!(self.out, "{value}")?,
index::IndexableLength::Pending => unreachable!(),
index::IndexableLength::Dynamic => {
let global =
context.function.originating_global(base).ok_or_else(|| {
Expand Down Expand Up @@ -2691,7 +2689,7 @@ impl<W: Write> Writer<W> {
) -> BackendResult {
let accessing_wrapped_array = match *base_ty {
crate::TypeInner::Array {
size: crate::ArraySize::Constant(_),
size: crate::ArraySize::Constant(_) | crate::ArraySize::Pending(_),
..
} => true,
_ => false,
Expand Down Expand Up @@ -2719,7 +2717,6 @@ impl<W: Write> Writer<W> {
index::IndexableLength::Known(limit) => {
write!(self.out, "{}u", limit - 1)?;
}
index::IndexableLength::Pending => unreachable!(),
index::IndexableLength::Dynamic => {
let global = context.function.originating_global(base).ok_or_else(|| {
Error::GenericValidation("Could not find originating global".into())
Expand Down Expand Up @@ -3908,8 +3905,8 @@ impl<W: Write> Writer<W> {
first_time: false,
};

match size {
crate::ArraySize::Constant(size) => {
match size.resolve(module.to_ctx())? {
proc::ResolvedSize::Constant(size) => {
writeln!(self.out, "struct {name} {{")?;
writeln!(
self.out,
Expand All @@ -3921,10 +3918,7 @@ impl<W: Write> Writer<W> {
)?;
writeln!(self.out, "}};")?;
}
crate::ArraySize::Pending(_) => {
unreachable!()
}
crate::ArraySize::Dynamic => {
proc::ResolvedSize::Dynamic => {
writeln!(self.out, "typedef {base_name} {name}[1];")?;
}
}
Expand Down Expand Up @@ -6318,11 +6312,9 @@ mod workgroup_mem_init {
writeln!(self.out, ", 0, {NAMESPACE}::memory_order_relaxed);")?;
}
crate::TypeInner::Array { base, size, .. } => {
let count = match size.to_indexable_length(module).expect("Bad array size")
{
proc::IndexableLength::Known(count) => count,
proc::IndexableLength::Pending => unreachable!(),
proc::IndexableLength::Dynamic => unreachable!(),
let count = match size.resolve(module.to_ctx())? {
proc::ResolvedSize::Constant(count) => count,
proc::ResolvedSize::Dynamic => unreachable!(),
};

access_stack.enter_array(|access_stack, array_depth| {
Expand Down
5 changes: 1 addition & 4 deletions naga/src/back/spv/index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -267,13 +267,10 @@ impl BlockContext<'_> {
block: &mut Block,
) -> Result<MaybeKnown<u32>, Error> {
let sequence_ty = self.fun_info[sequence].ty.inner_with(&self.ir_module.types);
match sequence_ty.indexable_length(self.ir_module) {
match sequence_ty.indexable_length_resolved(self.ir_module) {
Ok(crate::proc::IndexableLength::Known(known_length)) => {
Ok(MaybeKnown::Known(known_length))
}
Ok(crate::proc::IndexableLength::Pending) => {
unreachable!()
}
Ok(crate::proc::IndexableLength::Dynamic) => {
let length_id = self.write_runtime_array_length(sequence, block)?;
Ok(MaybeKnown::Computed(length_id))
Expand Down
2 changes: 2 additions & 0 deletions naga/src/back/spv/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ pub enum Error {
Validation(&'static str),
#[error("overrides should not be present at this stage")]
Override,
#[error(transparent)]
ResolveArraySizeError(#[from] crate::proc::ResolveArraySizeError),
}

#[derive(Default)]
Expand Down
32 changes: 17 additions & 15 deletions naga/src/back/spv/writer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -946,10 +946,10 @@ impl Writer {

fn write_type_declaration_arena(
&mut self,
arena: &UniqueArena<crate::Type>,
module: &crate::Module,
handle: Handle<crate::Type>,
) -> Result<Word, Error> {
let ty = &arena[handle];
let ty = &module.types[handle];
// If it's a type that needs SPIR-V capabilities, request them now.
// This needs to happen regardless of the LocalType lookup succeeding,
// because some types which map to the same LocalType have different
Expand Down Expand Up @@ -982,24 +982,26 @@ impl Writer {
self.decorate(id, Decoration::ArrayStride, &[stride]);

let type_id = self.get_type_id(LookupType::Handle(base));
match size {
crate::ArraySize::Constant(length) => {
let length_id = self.get_index_constant(length.get());
match size.resolve(module.to_ctx())? {
crate::proc::ResolvedSize::Constant(length) => {
let length_id = self.get_index_constant(length);
Instruction::type_array(id, type_id, length_id)
}
crate::ArraySize::Pending(_) => unreachable!(),
crate::ArraySize::Dynamic => Instruction::type_runtime_array(id, type_id),
crate::proc::ResolvedSize::Dynamic => {
Instruction::type_runtime_array(id, type_id)
}
}
}
crate::TypeInner::BindingArray { base, size } => {
let type_id = self.get_type_id(LookupType::Handle(base));
match size {
crate::ArraySize::Constant(length) => {
let length_id = self.get_index_constant(length.get());
match size.resolve(module.to_ctx())? {
crate::proc::ResolvedSize::Constant(length) => {
let length_id = self.get_index_constant(length);
Instruction::type_array(id, type_id, length_id)
}
crate::ArraySize::Pending(_) => unreachable!(),
crate::ArraySize::Dynamic => Instruction::type_runtime_array(id, type_id),
crate::proc::ResolvedSize::Dynamic => {
Instruction::type_runtime_array(id, type_id)
}
}
}
crate::TypeInner::Struct {
Expand All @@ -1009,7 +1011,7 @@ impl Writer {
let mut has_runtime_array = false;
let mut member_ids = Vec::with_capacity(members.len());
for (index, member) in members.iter().enumerate() {
let member_ty = &arena[member.ty];
let member_ty = &module.types[member.ty];
match member_ty.inner {
crate::TypeInner::Array {
base: _,
Expand All @@ -1020,7 +1022,7 @@ impl Writer {
}
_ => (),
}
self.decorate_struct_member(id, index, member, arena)?;
self.decorate_struct_member(id, index, member, &module.types)?;
let member_id = self.get_type_id(LookupType::Handle(member.ty));
member_ids.push(member_id);
}
Expand Down Expand Up @@ -1958,7 +1960,7 @@ impl Writer {

// write all types
for (handle, _) in ir_module.types.iter() {
self.write_type_declaration_arena(&ir_module.types, handle)?;
self.write_type_declaration_arena(ir_module, handle)?;
}

// write all const-expressions as constants
Expand Down
44 changes: 34 additions & 10 deletions naga/src/proc/index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ pub fn access_needs_check(
// Unwrap safety: `Err` here indicates unindexable base types and invalid
// length constants, but `access_needs_check` is only used by back ends, so
// validation should have caught those problems.
let length = base_inner.indexable_length(module).unwrap();
let length = base_inner.indexable_length_resolved(module).unwrap();
index.try_resolve_to_constant(expressions, module);
if let (&GuardedIndex::Known(index), &IndexableLength::Known(length)) = (&index, &length) {
if index < length {
Expand Down Expand Up @@ -357,8 +357,10 @@ impl GuardedIndex {
pub enum IndexableLengthError {
#[error("Type is not indexable, and has no length (validation error)")]
TypeNotIndexable,
#[error("Array length constant {0:?} is invalid")]
InvalidArrayLength(Handle<crate::Expression>),
#[error(transparent)]
ResolveArraySizeError(#[from] super::ResolveArraySizeError),
#[error("Array size is still pending")]
Pending(crate::ArraySize),
}

impl crate::TypeInner {
Expand Down Expand Up @@ -405,6 +407,30 @@ impl crate::TypeInner {
};
Ok(IndexableLength::Known(known_length))
}

pub fn indexable_length_pending(
&self,
module: &crate::Module,
) -> Result<IndexableLength, IndexableLengthError> {
let length = self.indexable_length(module);
if let Err(IndexableLengthError::Pending(_)) = length {
return Ok(IndexableLength::Dynamic);
}
length
}

pub fn indexable_length_resolved(
&self,
module: &crate::Module,
) -> Result<IndexableLength, IndexableLengthError> {
let length = self.indexable_length(module);
if let Err(IndexableLengthError::Pending(size)) = length {
if let super::ResolvedSize::Constant(computed) = size.resolve(module.to_ctx())? {
return Ok(IndexableLength::Known(computed));
}
}
length
}
}

/// The number of elements in an indexable type.
Expand All @@ -416,8 +442,6 @@ pub enum IndexableLength {
/// Values of this type always have the given number of elements.
Known(u32),

Pending,

/// The number of elements is determined at runtime.
Dynamic,
}
Expand All @@ -427,10 +451,10 @@ impl crate::ArraySize {
self,
_module: &crate::Module,
) -> Result<IndexableLength, IndexableLengthError> {
Ok(match self {
Self::Constant(length) => IndexableLength::Known(length.get()),
Self::Pending(_) => IndexableLength::Pending,
Self::Dynamic => IndexableLength::Dynamic,
})
match self {
Self::Constant(length) => Ok(IndexableLength::Known(length.get())),
Self::Pending(_) => Err(IndexableLengthError::Pending(self)),
Self::Dynamic => Ok(IndexableLength::Dynamic),
}
}
}
Loading

0 comments on commit 60c12b3

Please sign in to comment.