[hlsl-out] Add more padding when necessary (#1814)

* [hlsl-out] add padding at the end of structs and after struct members of type matrix and array (when necessary)

* use wrapped constructor fn for constants

* add array as fn arg test

* fix glsl array fn arg

* add wrapped constructor for arrays

* [glsl-out] support multidimensional arrays

* address comments
This commit is contained in:
Teodor Tanasoaia
2022-04-12 05:34:06 +02:00
committed by GitHub
parent 0e77d26241
commit bd62887a13
24 changed files with 1088 additions and 450 deletions

View File

@@ -727,7 +727,11 @@ impl<'a, W: Write> Writer<'a, W> {
self.collect_reflection_info()
}
fn write_array_size(&mut self, size: crate::ArraySize) -> BackendResult {
fn write_array_size(
&mut self,
base: Handle<crate::Type>,
size: crate::ArraySize,
) -> BackendResult {
write!(self.out, "[")?;
// Write the array size
@@ -751,6 +755,16 @@ impl<'a, W: Write> Writer<'a, W> {
}
write!(self.out, "]")?;
if let TypeInner::Array {
base: next_base,
size: next_size,
..
} = self.module.types[base].inner
{
self.write_array_size(next_base, next_size)?;
}
Ok(())
}
@@ -807,7 +821,7 @@ impl<'a, W: Write> Writer<'a, W> {
// GLSL arrays are written as `type name[size]`
// Current code is written arrays only as `[size]`
// Base `type` and `name` should be written outside
TypeInner::Array { size, .. } => self.write_array_size(size)?,
TypeInner::Array { base, size, .. } => self.write_array_size(base, size)?,
// Panic if either Image, Sampler, Pointer, or a Struct is being written
//
// Write all variants instead of `_` so that if new variants are added a
@@ -994,8 +1008,8 @@ impl<'a, W: Write> Writer<'a, W> {
write!(self.out, " ")?;
self.write_global_name(handle, global)?;
if let TypeInner::Array { size, .. } = self.module.types[global.ty].inner {
self.write_array_size(size)?;
if let TypeInner::Array { base, size, .. } = self.module.types[global.ty].inner {
self.write_array_size(base, size)?;
}
if global.space.initializable() && is_value_init_supported(self.module, global.ty) {
@@ -1298,6 +1312,11 @@ impl<'a, W: Write> Writer<'a, W> {
// The leading space is important
write!(this.out, " {}", &this.names[&ctx.argument_key(i as u32)])?;
// Write array size
if let TypeInner::Array { base, size, .. } = this.module.types[arg.ty].inner {
this.write_array_size(base, size)?;
}
Ok(())
})?;
@@ -1356,8 +1375,8 @@ impl<'a, W: Write> Writer<'a, W> {
// The leading space is important
write!(self.out, " {}", self.names[&ctx.name_key(handle)])?;
// Write size for array type
if let TypeInner::Array { size, .. } = self.module.types[local.ty].inner {
self.write_array_size(size)?;
if let TypeInner::Array { base, size, .. } = self.module.types[local.ty].inner {
self.write_array_size(base, size)?;
}
// Write the local initializer if needed
if let Some(init) = local.init {
@@ -1448,8 +1467,8 @@ impl<'a, W: Write> Writer<'a, W> {
// `type(components)` where `components` is a comma separated list of constants
crate::ConstantInner::Composite { ty, ref components } => {
self.write_type(ty)?;
if let TypeInner::Array { size, .. } = self.module.types[ty].inner {
self.write_array_size(size)?;
if let TypeInner::Array { base, size, .. } = self.module.types[ty].inner {
self.write_array_size(base, size)?;
}
write!(self.out, "(")?;
@@ -1525,7 +1544,7 @@ impl<'a, W: Write> Writer<'a, W> {
&self.names[&NameKey::StructMember(handle, idx as u32)]
)?;
// Write [size]
self.write_array_size(size)?;
self.write_array_size(base, size)?;
// Newline is important
writeln!(self.out, ";")?;
}
@@ -2056,8 +2075,8 @@ impl<'a, W: Write> Writer<'a, W> {
self.write_type(ty)?;
let resolved = ctx.info[expr].ty.inner_with(&self.module.types);
if let TypeInner::Array { size, .. } = *resolved {
self.write_array_size(size)?;
if let TypeInner::Array { base, size, .. } = *resolved {
self.write_array_size(base, size)?;
}
write!(self.out, "(")?;
@@ -2909,8 +2928,8 @@ impl<'a, W: Write> Writer<'a, W> {
let resolved = base_ty_res.inner_with(&self.module.types);
write!(self.out, " {}", name)?;
if let TypeInner::Array { size, .. } = *resolved {
self.write_array_size(size)?;
if let TypeInner::Array { base, size, .. } = *resolved {
self.write_array_size(base, size)?;
}
write!(self.out, " = ")?;
self.write_expr(handle, ctx)?;
@@ -2948,7 +2967,7 @@ impl<'a, W: Write> Writer<'a, W> {
proc::IndexableLength::Dynamic => return Ok(()),
};
self.write_type(base)?;
self.write_array_size(size)?;
self.write_array_size(base, size)?;
write!(self.out, "(")?;
for _ in 1..count {
self.write_zero_init_value(base)?;

View File

@@ -1,3 +1,5 @@
use std::borrow::Cow;
use super::Error;
impl crate::ScalarKind {
@@ -35,6 +37,79 @@ impl crate::TypeInner {
_ => false,
}
}
pub(super) fn try_size_hlsl(
&self,
types: &crate::UniqueArena<crate::Type>,
constants: &crate::Arena<crate::Constant>,
) -> Result<u32, crate::arena::BadHandle> {
Ok(match *self {
Self::Matrix {
columns,
rows,
width,
} => {
let aligned_rows = if rows > crate::VectorSize::Bi { 4 } else { 2 };
let stride = aligned_rows * width as u32;
let last_row_size = rows as u32 * width as u32;
((columns as u32 - 1) * stride) + last_row_size
}
Self::Array { base, size, stride } => {
let count = match size {
crate::ArraySize::Constant(handle) => {
let constant = constants.try_get(handle)?;
constant.to_array_length().unwrap_or(1)
}
// A dynamically-sized array has to have at least one element
crate::ArraySize::Dynamic => 1,
};
let last_el_size = types[base].inner.try_size_hlsl(types, constants)?;
((count - 1) * stride) + last_el_size
}
_ => self.try_size(constants)?,
})
}
/// Used to generate the name of the wrapped type constructor
pub(super) fn hlsl_type_id<'a>(
&self,
base: crate::Handle<crate::Type>,
types: &crate::UniqueArena<crate::Type>,
constants: &crate::Arena<crate::Constant>,
names: &'a crate::FastHashMap<crate::proc::NameKey, String>,
) -> Result<Cow<'a, str>, Error> {
Ok(match types[base].inner {
crate::TypeInner::Scalar { kind, width } => Cow::Borrowed(kind.to_hlsl_str(width)?),
crate::TypeInner::Vector { size, kind, width } => Cow::Owned(format!(
"{}{}",
kind.to_hlsl_str(width)?,
crate::back::vector_size_str(size)
)),
crate::TypeInner::Matrix {
columns,
rows,
width,
} => Cow::Owned(format!(
"{}{}x{}",
crate::ScalarKind::Float.to_hlsl_str(width)?,
crate::back::vector_size_str(columns),
crate::back::vector_size_str(rows),
)),
crate::TypeInner::Array {
base,
size: crate::ArraySize::Constant(size),
..
} => Cow::Owned(format!(
"array{}_{}_",
constants[size].to_array_length().unwrap(),
self.hlsl_type_id(base, types, constants, names)?
)),
crate::TypeInner::Struct { .. } => {
Cow::Borrowed(&names[&crate::proc::NameKey::Type(base)])
}
_ => unreachable!(),
})
}
}
impl crate::StorageFormat {

View File

@@ -26,7 +26,7 @@ int dim_1d = NagaDimensions1D(image_1d);
```
*/
use super::{super::FunctionCtx, BackendResult, Error};
use super::{super::FunctionCtx, BackendResult};
use crate::{arena::Handle, proc::NameKey};
use std::fmt::Write;
@@ -350,9 +350,15 @@ impl<'a, W: Write> super::Writer<'a, W> {
pub(super) fn write_wrapped_constructor_function_name(
&mut self,
module: &crate::Module,
constructor: WrappedConstructor,
) -> BackendResult {
let name = &self.names[&NameKey::Type(constructor.ty)];
let name = module.types[constructor.ty].inner.hlsl_type_id(
constructor.ty,
&module.types,
&module.constants,
&self.names,
)?;
write!(self.out, "Construct{}", name)?;
Ok(())
}
@@ -369,69 +375,114 @@ impl<'a, W: Write> super::Writer<'a, W> {
const RETURN_VARIABLE_NAME: &str = "ret";
// Write function return type and name
let struct_name = &self.names[&NameKey::Type(constructor.ty)];
write!(self.out, "{} ", struct_name)?;
self.write_wrapped_constructor_function_name(constructor)?;
self.write_type(module, constructor.ty)?;
write!(self.out, " ")?;
self.write_wrapped_constructor_function_name(module, constructor)?;
// Write function parameters
write!(self.out, "(")?;
let members = match module.types[constructor.ty].inner {
crate::TypeInner::Struct { ref members, .. } => members,
_ => return Err(Error::Unimplemented("non-struct constructor".to_string())),
};
for (i, member) in members.iter().enumerate() {
let mut write_arg = |i, ty| -> BackendResult {
if i != 0 {
write!(self.out, ", ")?;
}
self.write_type(module, member.ty)?;
self.write_type(module, ty)?;
write!(self.out, " {}{}", ARGUMENT_VARIABLE_NAME, i)?;
if let crate::TypeInner::Array { size, .. } = module.types[member.ty].inner {
self.write_array_size(module, size)?;
if let crate::TypeInner::Array { base, size, .. } = module.types[ty].inner {
self.write_array_size(module, base, size)?;
}
Ok(())
};
match module.types[constructor.ty].inner {
crate::TypeInner::Struct { ref members, .. } => {
for (i, member) in members.iter().enumerate() {
write_arg(i, member.ty)?;
}
}
crate::TypeInner::Array {
base,
size: crate::ArraySize::Constant(size),
..
} => {
let count = module.constants[size].to_array_length().unwrap();
for i in 0..count as usize {
write_arg(i, base)?;
}
}
_ => unreachable!(),
};
write!(self.out, ")")?;
if let crate::TypeInner::Array { base, size, .. } = module.types[constructor.ty].inner {
self.write_array_size(module, base, size)?;
}
// Write function body
writeln!(self.out, ") {{")?;
writeln!(self.out, " {{")?;
let struct_name = &self.names[&NameKey::Type(constructor.ty)];
writeln!(
self.out,
"{}{} {};",
INDENT, struct_name, RETURN_VARIABLE_NAME
)?;
for i in 0..members.len() as u32 {
let member = &members[i as usize];
match module.types[constructor.ty].inner {
crate::TypeInner::Struct { ref members, .. } => {
let struct_name = &self.names[&NameKey::Type(constructor.ty)];
writeln!(
self.out,
"{}{} {};",
INDENT, struct_name, RETURN_VARIABLE_NAME
)?;
for (i, member) in members.iter().enumerate() {
let field_name = &self.names[&NameKey::StructMember(constructor.ty, i as u32)];
let field_name = &self.names[&NameKey::StructMember(constructor.ty, i)];
match module.types[member.ty].inner {
crate::TypeInner::Matrix {
columns,
rows: crate::VectorSize::Bi,
..
} if member.binding.is_none() => {
for j in 0..columns as u8 {
writeln!(
self.out,
"{}{}.{}_{} = {}{}[{}];",
INDENT,
RETURN_VARIABLE_NAME,
field_name,
j,
ARGUMENT_VARIABLE_NAME,
i,
j
)?;
match module.types[member.ty].inner {
crate::TypeInner::Matrix {
columns,
rows: crate::VectorSize::Bi,
..
} if member.binding.is_none() => {
for j in 0..columns as u8 {
writeln!(
self.out,
"{}{}.{}_{} = {}{}[{}];",
INDENT,
RETURN_VARIABLE_NAME,
field_name,
j,
ARGUMENT_VARIABLE_NAME,
i,
j
)?;
}
}
_ => {
writeln!(
self.out,
"{}{}.{} = {}{};",
INDENT, RETURN_VARIABLE_NAME, field_name, ARGUMENT_VARIABLE_NAME, i,
)?;
}
}
}
_ => {
//TODO: handle arrays?
writeln!(
self.out,
"{}{}.{} = {}{};",
INDENT, RETURN_VARIABLE_NAME, field_name, ARGUMENT_VARIABLE_NAME, i,
)?;
}
}
crate::TypeInner::Array {
base,
size: crate::ArraySize::Constant(size),
..
} => {
write!(self.out, "{}", INDENT)?;
self.write_type(module, base)?;
write!(self.out, " {}", RETURN_VARIABLE_NAME)?;
self.write_array_size(module, base, crate::ArraySize::Constant(size))?;
write!(self.out, " = {{ ")?;
let count = module.constants[size].to_array_length().unwrap();
for i in 0..count {
if i != 0 {
write!(self.out, ", ")?;
}
write!(self.out, "{}{}", ARGUMENT_VARIABLE_NAME, i)?;
}
writeln!(self.out, " }};",)?;
}
_ => unreachable!(),
}
// Write return value
@@ -831,7 +882,9 @@ impl<'a, W: Write> super::Writer<'a, W> {
}
crate::Expression::Compose { ty, components: _ } => {
let constructor = match module.types[ty].inner {
crate::TypeInner::Struct { .. } => WrappedConstructor { ty },
crate::TypeInner::Struct { .. } | crate::TypeInner::Array { .. } => {
WrappedConstructor { ty }
}
_ => continue,
};
if !self.wrapped.constructors.contains(&constructor) {
@@ -887,6 +940,33 @@ impl<'a, W: Write> super::Writer<'a, W> {
Ok(())
}
pub(super) fn write_wrapped_constructor_function_for_constant(
&mut self,
module: &crate::Module,
constant: &crate::Constant,
) -> BackendResult {
if let crate::ConstantInner::Composite { ty, ref components } = constant.inner {
match module.types[ty].inner {
crate::TypeInner::Struct { .. } | crate::TypeInner::Array { .. } => {
let constructor = WrappedConstructor { ty };
if !self.wrapped.constructors.contains(&constructor) {
self.write_wrapped_constructor_function(module, constructor)?;
self.wrapped.constructors.insert(constructor);
}
}
_ => {}
}
for constant in components {
self.write_wrapped_constructor_function_for_constant(
module,
&module.constants[*constant],
)?;
}
}
Ok(())
}
pub(super) fn write_texture_coordinates(
&mut self,
kind: &str,

View File

@@ -307,7 +307,7 @@ impl<W: fmt::Write> super::Writer<'_, W> {
self.write_value_type(module, &module.types[base].inner)?;
let depth = level.next().0;
write!(self.out, " {}{}", STORE_TEMP_NAME, depth)?;
self.write_array_size(module, crate::ArraySize::Constant(const_handle))?;
self.write_array_size(module, base, crate::ArraySize::Constant(const_handle))?;
write!(self.out, " = ")?;
self.write_store_value(module, &value, func_ctx)?;
writeln!(self.out, ";")?;

View File

@@ -152,7 +152,7 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {
// Write all structs
for (handle, ty) in module.types.iter() {
if let TypeInner::Struct { ref members, .. } = ty.inner {
if let TypeInner::Struct { ref members, span } = ty.inner {
if let Some(member) = members.last() {
if let TypeInner::Array {
size: crate::ArraySize::Dynamic,
@@ -176,12 +176,18 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {
module,
handle,
members,
span,
ep_result.map(|r| (r.0, Io::Output)),
)?;
writeln!(self.out)?;
}
}
// Write wrapped constructor functions used in constants
for (_, constant) in module.constants.iter() {
self.write_wrapped_constructor_function_for_constant(module, constant)?;
}
// Write all globals
for (ty, _) in module.global_variables.iter() {
self.write_global(module, ty)?;
@@ -525,8 +531,8 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {
let arg_name = &self.names[&NameKey::EntryPointArgument(ep_index, arg_index as u32)];
write!(self.out, " {}", arg_name)?;
match module.types[arg.ty].inner {
TypeInner::Array { size, .. } => {
self.write_array_size(module, size)?;
TypeInner::Array { base, size, .. } => {
self.write_array_size(module, base, size)?;
let fake_member = fake_iter.next().unwrap();
writeln!(self.out, " = {}.{};", ep_input.arg_name, fake_member.name)?;
}
@@ -631,8 +637,8 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {
write!(self.out, ")")?;
} else {
// need to write the array size if the type was emitted with `write_type`
if let TypeInner::Array { size, .. } = module.types[global.ty].inner {
self.write_array_size(module, size)?;
if let TypeInner::Array { base, size, .. } = module.types[global.ty].inner {
self.write_array_size(module, base, size)?;
}
if global.space == crate::AddressSpace::Private {
write!(self.out, " = ")?;
@@ -650,8 +656,8 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {
let sub_name = &self.names[&NameKey::GlobalVariable(handle)];
write!(self.out, " {}", sub_name)?;
// need to write the array size if the type was emitted with `write_type`
if let TypeInner::Array { size, .. } = module.types[global.ty].inner {
self.write_array_size(module, size)?;
if let TypeInner::Array { base, size, .. } = module.types[global.ty].inner {
self.write_array_size(module, base, size)?;
}
writeln!(self.out, "; }}")?;
} else {
@@ -713,6 +719,7 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {
pub(super) fn write_array_size(
&mut self,
module: &Module,
base: Handle<crate::Type>,
size: crate::ArraySize,
) -> BackendResult {
write!(self.out, "[")?;
@@ -729,6 +736,16 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {
}
write!(self.out, "]")?;
if let TypeInner::Array {
base: next_base,
size: next_size,
..
} = module.types[base].inner
{
self.write_array_size(module, next_base, next_size)?;
}
Ok(())
}
@@ -741,6 +758,7 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {
module: &Module,
handle: Handle<crate::Type>,
members: &[crate::StructMember],
span: u32,
shader_stage: Option<(ShaderStage, Io)>,
) -> BackendResult {
// Write struct name
@@ -759,7 +777,10 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {
}
}
let ty_inner = &module.types[member.ty].inner;
last_offset = member.offset + ty_inner.size(&module.constants);
last_offset = member.offset
+ ty_inner
.try_size_hlsl(&module.types, &module.constants)
.unwrap();
// The indentation is only for readability
write!(self.out, "{}", back::INDENT)?;
@@ -782,7 +803,7 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {
&self.names[&NameKey::StructMember(handle, index as u32)]
)?;
// Write [size]
self.write_array_size(module, size)?;
self.write_array_size(module, base, size)?;
}
// We treat matrices of the form `matCx2` as a sequence of C `vec2`s
// (see top level module docs for details).
@@ -832,6 +853,14 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {
writeln!(self.out, ";")?;
}
// add padding at the end since sizes of types don't get rounded up to their alignment in HLSL
if members.last().unwrap().binding.is_none() && span > last_offset {
let padding = (span - last_offset) / 4;
for i in 0..padding {
writeln!(self.out, "{}int _end_pad_{};", back::INDENT, i)?;
}
}
writeln!(self.out, "}};")?;
Ok(())
}
@@ -904,8 +933,8 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {
// HLSL arrays are written as `type name[size]`
// Current code is written arrays only as `[size]`
// Base `type` and `name` should be written outside
TypeInner::Array { size, .. } => {
self.write_array_size(module, size)?;
TypeInner::Array { base, size, .. } => {
self.write_array_size(module, base, size)?;
}
_ => {
return Err(Error::Unimplemented(format!(
@@ -989,8 +1018,8 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {
// Write argument name. Space is important.
write!(self.out, " {}", argument_name)?;
if let TypeInner::Array { size, .. } = module.types[arg.ty].inner {
self.write_array_size(module, size)?;
if let TypeInner::Array { base, size, .. } = module.types[arg.ty].inner {
self.write_array_size(module, base, size)?;
}
}
}
@@ -1009,8 +1038,8 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {
&self.names[&NameKey::EntryPointArgument(ep_index, index as u32)];
write!(self.out, " {}", argument_name)?;
if let TypeInner::Array { size, .. } = module.types[arg.ty].inner {
self.write_array_size(module, size)?;
if let TypeInner::Array { base, size, .. } = module.types[arg.ty].inner {
self.write_array_size(module, base, size)?;
}
if let Some(ref binding) = arg.binding {
@@ -1053,8 +1082,8 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {
self.write_type(module, local.ty)?;
write!(self.out, " {}", self.names[&func_ctx.name_key(handle)])?;
// Write size for array type
if let TypeInner::Array { size, .. } = module.types[local.ty].inner {
self.write_array_size(module, size)?;
if let TypeInner::Array { base, size, .. } = module.types[local.ty].inner {
self.write_array_size(module, base, size)?;
}
write!(self.out, " = ")?;
@@ -1706,19 +1735,19 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {
match *expression {
Expression::Constant(constant) => self.write_constant(module, constant)?,
Expression::Compose { ty, ref components } => {
let (brace_open, brace_close) = match module.types[ty].inner {
TypeInner::Struct { .. } => {
self.write_wrapped_constructor_function_name(WrappedConstructor { ty })?;
("(", ")")
match module.types[ty].inner {
TypeInner::Struct { .. } | TypeInner::Array { .. } => {
self.write_wrapped_constructor_function_name(
module,
WrappedConstructor { ty },
)?;
}
TypeInner::Array { .. } => ("{ ", " }"),
_ => {
self.write_type(module, ty)?;
("(", ")")
}
};
write!(self.out, "{}", brace_open)?;
write!(self.out, "(")?;
for (index, &component) in components.iter().enumerate() {
if index != 0 {
@@ -1728,7 +1757,7 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {
self.write_expr(module, component, func_ctx)?;
}
write!(self.out, "{}", brace_close)?;
write!(self.out, ")")?;
}
// All of the multiplication can be expressed as `mul`,
// except vector * vector, which needs to use the "*" operator.
@@ -2324,15 +2353,15 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {
ty: Handle<crate::Type>,
components: &[Handle<crate::Constant>],
) -> BackendResult {
let (open_b, close_b) = match module.types[ty].inner {
TypeInner::Array { .. } | TypeInner::Struct { .. } => ("{ ", " }"),
match module.types[ty].inner {
TypeInner::Struct { .. } | TypeInner::Array { .. } => {
self.write_wrapped_constructor_function_name(module, WrappedConstructor { ty })?;
}
_ => {
// We should write type only for non struct/array constants
self.write_type(module, ty)?;
("(", ")")
}
};
write!(self.out, "{}", open_b)?;
write!(self.out, "(")?;
for (index, constant) in components.iter().enumerate() {
self.write_constant(module, *constant)?;
// Only write a comma if isn't the last element
@@ -2341,7 +2370,7 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {
write!(self.out, ", ")?;
}
}
write!(self.out, "{}", close_b)?;
write!(self.out, ")")?;
Ok(())
}
@@ -2392,8 +2421,8 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {
write!(self.out, " {}", name)?;
// If rhs is a array type, we should write array size
if let TypeInner::Array { size, .. } = *resolved {
self.write_array_size(module, size)?;
if let TypeInner::Array { base, size, .. } = *resolved {
self.write_array_size(module, base, size)?;
}
write!(self.out, " = ")?;
self.write_expr(module, handle, ctx)?;