Skip to content

[naga hlsl-out] Handle additional cases of Cx2 matrices #7438

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: trunk
Choose a base branch
from
Open

[naga hlsl-out] Handle additional cases of Cx2 matrices #7438

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1e4eca2
[naga hlsl-out] Handle additional cases of Cx2 matrices
andyleiserson Mar 22, 2025
da45585
[naga hlsl-out] Factor out some repetitive code
andyleiserson Mar 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 16 additions & 9 deletions naga/src/back/hlsl/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,17 @@ type should be stored in `uniform` and `storage` buffers. The HLSL we
generate must access values in that form, even when it is not what
HLSL would use normally.

The rules described here only apply to WGSL `uniform` variables. WGSL
`storage` buffers are translated as HLSL `ByteAddressBuffers`, for
which we generate `Load` and `Store` method calls with explicit byte
offsets. WGSL pipeline inputs must be scalars or vectors; they cannot
be matrices, which is where the interesting problems arise.
Matching the WGSL memory layout is a concern only for `uniform`
variables. WGSL `storage` buffers are translated as HLSL
`ByteAddressBuffers`, for which we generate `Load` and `Store` method
calls with explicit byte offsets. WGSL pipeline inputs must be scalars
or vectors; they cannot be matrices, which is where the interesting
problems arise. However, when an affected type appears in a struct
definition, the transformations described here are applied without
consideration of where the struct is used.

Access to storage buffers is implemented in `storage.rs`. Access to
uniform buffers is implemented where applicable in `writer.rs`.

## Row- and column-major ordering for matrices

Expand Down Expand Up @@ -57,10 +63,9 @@ that the columns of a `matKx2<f32>` need only be [aligned as required
for `vec2<f32>`][ilov], which is [eight-byte alignment][8bb].

To compensate for this, any time a `matKx2<f32>` appears in a WGSL
`uniform` variable, whether directly as the variable's type or as part
of a struct/array, we actually emit `K` separate `float2` members, and
assemble/disassemble the matrix from its columns (in WGSL; rows in
HLSL) upon load and store.
`uniform` value or as part of a struct/array, we actually emit `K`
separate `float2` members, and assemble/disassemble the matrix from its
columns (in WGSL; rows in HLSL) upon load and store.

For example, the following WGSL struct type:

Expand Down Expand Up @@ -448,6 +453,8 @@ pub enum Error {
Override,
#[error(transparent)]
ResolveArraySizeError(#[from] proc::ResolveArraySizeError),
#[error("Internal error: reached unreachable code: {0}")]
Unreachable(String),
}

#[derive(PartialEq, Eq, Hash)]
Expand Down
146 changes: 115 additions & 31 deletions naga/src/back/hlsl/storage.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,13 @@ pub(super) enum StoreValue {
base: Handle<crate::Type>,
member_index: u32,
},
// Access to a single column of a Cx2 matrix within a struct
TempColumnAccess {
depth: usize,
base: Handle<crate::Type>,
member_index: u32,
column: u32,
},
}

impl<W: fmt::Write> super::Writer<'_, W> {
Expand Down Expand Up @@ -290,6 +297,15 @@ impl<W: fmt::Write> super::Writer<'_, W> {
let name = &self.names[&NameKey::StructMember(base, member_index)];
write!(self.out, "{STORE_TEMP_NAME}{depth}.{name}")?
}
StoreValue::TempColumnAccess {
depth,
base,
member_index,
column,
} => {
let name = &self.names[&NameKey::StructMember(base, member_index)];
write!(self.out, "{STORE_TEMP_NAME}{depth}.{name}_{column}")?
}
}
Ok(())
}
Expand All @@ -302,6 +318,7 @@ impl<W: fmt::Write> super::Writer<'_, W> {
value: StoreValue,
func_ctx: &FunctionCtx,
level: crate::back::Level,
within_struct: Option<Handle<crate::Type>>,
) -> BackendResult {
let temp_resolution;
let ty_resolution = match value {
Expand All @@ -325,6 +342,11 @@ impl<W: fmt::Write> super::Writer<'_, W> {
temp_resolution = TypeResolution::Handle(ty_handle);
&temp_resolution
}
StoreValue::TempColumnAccess { .. } => {
return Err(Error::Unreachable(
"attempting write_storage_store for TempColumnAccess".into(),
));
}
};
match *ty_resolution.inner_with(&module.types) {
crate::TypeInner::Scalar(scalar) => {
Expand Down Expand Up @@ -372,37 +394,92 @@ impl<W: fmt::Write> super::Writer<'_, W> {
rows,
scalar,
} => {
// first, assign the value to a temporary
writeln!(self.out, "{level}{{")?;
let depth = level.0 + 1;
write!(
self.out,
"{}{}{}x{} {}{} = ",
level.next(),
scalar.to_hlsl_str()?,
columns as u8,
rows as u8,
STORE_TEMP_NAME,
depth,
)?;
self.write_store_value(module, &value, func_ctx)?;
writeln!(self.out, ";")?;

// Note: Matrices containing vec3s, due to padding, act like they contain vec4s.
let row_stride = Alignment::from(rows) * scalar.width as u32;

// then iterate the stores
for i in 0..columns as u32 {
self.temp_access_chain
.push(SubAccess::Offset(i * row_stride));
let ty_inner = crate::TypeInner::Vector { size: rows, scalar };
let sv = StoreValue::TempIndex {
depth,
index: i,
ty: TypeResolution::Value(ty_inner),
};
self.write_storage_store(module, var_handle, sv, func_ctx, level.next())?;
self.temp_access_chain.pop();
writeln!(self.out, "{level}{{")?;

match within_struct {
Some(containing_struct) if rows == crate::VectorSize::Bi => {
// If we are within a struct, then the struct was already assigned to
// a temporary, we don't need to make another.
let mut chain = mem::take(&mut self.temp_access_chain);
for i in 0..columns as u32 {
chain.push(SubAccess::Offset(i * row_stride));
// working around the borrow checker in `self.write_expr`
let var_name = &self.names[&NameKey::GlobalVariable(var_handle)];
let StoreValue::TempAccess { member_index, .. } = value else {
return Err(Error::Unreachable(
"write_storage_store within_struct but not TempAccess".into(),
));
};
let column_value = StoreValue::TempColumnAccess {
depth: level.0, // note not incrementing, b/c no temp
base: containing_struct,
member_index,
column: i,
};
// See note about DXC and Load/Store in the module's documentation.
if scalar.width == 4 {
write!(
self.out,
"{}{}.Store{}(",
level.next(),
var_name,
rows as u8
)?;
self.write_storage_address(module, &chain, func_ctx)?;
write!(self.out, ", asuint(")?;
self.write_store_value(module, &column_value, func_ctx)?;
writeln!(self.out, "));")?;
} else {
write!(self.out, "{}{var_name}.Store(", level.next())?;
self.write_storage_address(module, &chain, func_ctx)?;
write!(self.out, ", ")?;
self.write_store_value(module, &column_value, func_ctx)?;
writeln!(self.out, ");")?;
}
chain.pop();
}
self.temp_access_chain = chain;
}
_ => {
// first, assign the value to a temporary
let depth = level.0 + 1;
write!(
self.out,
"{}{}{}x{} {}{} = ",
level.next(),
scalar.to_hlsl_str()?,
columns as u8,
rows as u8,
STORE_TEMP_NAME,
depth,
)?;
self.write_store_value(module, &value, func_ctx)?;
writeln!(self.out, ";")?;

// then iterate the stores
for i in 0..columns as u32 {
self.temp_access_chain
.push(SubAccess::Offset(i * row_stride));
let ty_inner = crate::TypeInner::Vector { size: rows, scalar };
let sv = StoreValue::TempIndex {
depth,
index: i,
ty: TypeResolution::Value(ty_inner),
};
self.write_storage_store(
module,
var_handle,
sv,
func_ctx,
level.next(),
None,
)?;
self.temp_access_chain.pop();
}
}
}
// done
writeln!(self.out, "{level}}}")?;
Expand All @@ -415,7 +492,7 @@ impl<W: fmt::Write> super::Writer<'_, W> {
// first, assign the value to a temporary
writeln!(self.out, "{level}{{")?;
write!(self.out, "{}", level.next())?;
self.write_value_type(module, &module.types[base].inner)?;
self.write_type(module, base)?;
let depth = level.next().0;
write!(self.out, " {STORE_TEMP_NAME}{depth}")?;
self.write_array_size(module, base, crate::ArraySize::Constant(size))?;
Expand All @@ -430,7 +507,7 @@ impl<W: fmt::Write> super::Writer<'_, W> {
index: i,
ty: TypeResolution::Handle(base),
};
self.write_storage_store(module, var_handle, sv, func_ctx, level.next())?;
self.write_storage_store(module, var_handle, sv, func_ctx, level.next(), None)?;
self.temp_access_chain.pop();
}
// done
Expand Down Expand Up @@ -461,7 +538,14 @@ impl<W: fmt::Write> super::Writer<'_, W> {
base: struct_ty,
member_index: i as u32,
};
self.write_storage_store(module, var_handle, sv, func_ctx, level.next())?;
self.write_storage_store(
module,
var_handle,
sv,
func_ctx,
level.next(),
Some(struct_ty),
)?;
self.temp_access_chain.pop();
}
// done
Expand Down
Loading