Skip to content

Commit b5ebab5

Browse files
committed
[hlsl-out] Fix accesses on zero value expressions (gfx-rs#5587)
1 parent a16d790 commit b5ebab5

File tree

3 files changed

+109
-4
lines changed

3 files changed

+109
-4
lines changed

naga/src/back/hlsl/help.rs

+94-1
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,12 @@ pub(super) struct WrappedMatCx2 {
5959
pub(super) columns: crate::VectorSize,
6060
}
6161

62+
63+
#[derive(Clone, Copy, Debug, Hash, Eq, Ord, PartialEq, PartialOrd)]
64+
pub(super) struct WrappedZeroValue {
65+
pub(super) ty: Handle<crate::Type>,
66+
}
67+
6268
/// HLSL backend requires its own `ImageQuery` enum.
6369
///
6470
/// It is used inside `WrappedImageQuery` and should be unique per ImageQuery function.
@@ -348,7 +354,7 @@ impl<'a, W: Write> super::Writer<'a, W> {
348354
}
349355

350356
/// Helper function that write wrapped function for `Expression::Compose` for structures.
351-
pub(super) fn write_wrapped_constructor_function(
357+
fn write_wrapped_constructor_function(
352358
&mut self,
353359
module: &crate::Module,
354360
constructor: WrappedConstructor,
@@ -856,13 +862,33 @@ impl<'a, W: Write> super::Writer<'a, W> {
856862
Ok(())
857863
}
858864

865+
// TODO: we could merge this with iteration in write_wrapped_compose_functions...
866+
//
867+
/// Helper function that writes zero value wrapped functions
868+
pub(super) fn write_wrapped_zero_value_functions(
869+
&mut self,
870+
module: &crate::Module,
871+
expressions: &crate::Arena<crate::Expression>,
872+
) -> BackendResult {
873+
for (handle, _) in expressions.iter() {
874+
if let crate::Expression::ZeroValue(ty) = expressions[handle] {
875+
let zero_value = WrappedZeroValue { ty };
876+
if self.wrapped.zero_values.insert(zero_value) {
877+
self.write_wrapped_zero_value_function(module, zero_value)?;
878+
}
879+
}
880+
}
881+
Ok(())
882+
}
883+
859884
/// Helper function that writes various wrapped functions
860885
pub(super) fn write_wrapped_functions(
861886
&mut self,
862887
module: &crate::Module,
863888
func_ctx: &FunctionCtx,
864889
) -> BackendResult {
865890
self.write_wrapped_compose_functions(module, func_ctx.expressions)?;
891+
self.write_wrapped_zero_value_functions(module, func_ctx.expressions)?;
866892

867893
for (handle, _) in func_ctx.expressions.iter() {
868894
match func_ctx.expressions[handle] {
@@ -1140,4 +1166,71 @@ impl<'a, W: Write> super::Writer<'a, W> {
11401166

11411167
Ok(())
11421168
}
1169+
1170+
pub(super) fn write_wrapped_zero_value_function_name(
1171+
&mut self,
1172+
module: &crate::Module,
1173+
zero_value: WrappedZeroValue,
1174+
) -> BackendResult {
1175+
let name = crate::TypeInner::hlsl_type_id(zero_value.ty, module.to_ctx(), &self.names)?;
1176+
write!(self.out, "ZeroValue{name}")?;
1177+
Ok(())
1178+
}
1179+
1180+
/// Helper function that write wrapped function for `Expression::ZeroValue`
1181+
///
1182+
/// This is necessary since we might have a member access after the zero value expression, e.g.
1183+
/// `.y` (in practice this can come up when consuming SPIRV that's been produced by glslc).
1184+
///
1185+
/// So we can't just write `(float4)0` since `(float4)0.y` won't parse correctly.
1186+
///
1187+
/// Parenthesizing the expression like `((float4)0).y` would work... except DXC can't handle
1188+
/// cases like:
1189+
///
1190+
/// ```ignore
1191+
/// tests\out\hlsl\access.hlsl:183:41: error: cannot compile this l-value expression yet
1192+
/// t_1.am = (__mat4x2[2])((float4x2[2])0);
1193+
/// ^
1194+
/// ```
1195+
fn write_wrapped_zero_value_function(
1196+
&mut self,
1197+
module: &crate::Module,
1198+
zero_value: WrappedZeroValue,
1199+
) -> BackendResult {
1200+
use crate::back::INDENT;
1201+
1202+
const RETURN_VARIABLE_NAME: &str = "ret";
1203+
1204+
// Write function return type and name
1205+
if let crate::TypeInner::Array { base, size, .. } = module.types[zero_value.ty].inner {
1206+
write!(self.out, "typedef ")?;
1207+
self.write_type(module, zero_value.ty)?;
1208+
write!(self.out, " ret_")?;
1209+
self.write_wrapped_zero_value_function_name(module, zero_value)?;
1210+
self.write_array_size(module, base, size)?;
1211+
writeln!(self.out, ";")?;
1212+
1213+
write!(self.out, "ret_")?;
1214+
self.write_wrapped_zero_value_function_name(module, zero_value)?;
1215+
} else {
1216+
self.write_type(module, zero_value.ty)?;
1217+
}
1218+
write!(self.out, " ")?;
1219+
self.write_wrapped_zero_value_function_name(module, zero_value)?;
1220+
1221+
// Write function parameters (none) and start function body
1222+
writeln!(self.out, "() {{")?;
1223+
1224+
// Write `ZeroValue` function.
1225+
write!(self.out, "{INDENT}return ")?;
1226+
self.write_default_init(module, zero_value.ty)?;
1227+
writeln!(self.out, ";")?;
1228+
1229+
// End of function body
1230+
writeln!(self.out, "}}")?;
1231+
// Write extra new line
1232+
writeln!(self.out)?;
1233+
1234+
Ok(())
1235+
}
11431236
}

naga/src/back/hlsl/mod.rs

+1
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,7 @@ pub enum Error {
251251

252252
#[derive(Default)]
253253
struct Wrapped {
254+
zero_values: crate::FastHashSet<help::WrappedZeroValue>,
254255
array_lengths: crate::FastHashSet<help::WrappedArrayLength>,
255256
image_queries: crate::FastHashSet<help::WrappedImageQuery>,
256257
constructors: crate::FastHashSet<help::WrappedConstructor>,

naga/src/back/hlsl/writer.rs

+14-3
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
use super::{
2-
help::{WrappedArrayLength, WrappedConstructor, WrappedImageQuery, WrappedStructMatrixAccess},
2+
help::{
3+
WrappedArrayLength, WrappedConstructor, WrappedImageQuery, WrappedStructMatrixAccess,
4+
WrappedZeroValue,
5+
},
36
storage::StoreValue,
47
BackendResult, Error, FragmentEntryPoint, Options,
58
};
@@ -252,6 +255,7 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {
252255
self.write_special_functions(module)?;
253256

254257
self.write_wrapped_compose_functions(module, &module.const_expressions)?;
258+
self.write_wrapped_zero_value_functions(module, &module.const_expressions)?;
255259

256260
// Write all named constants
257261
let mut constants = module
@@ -2110,7 +2114,10 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {
21102114
self.write_const_expression(module, constant.init)?;
21112115
}
21122116
}
2113-
Expression::ZeroValue(ty) => self.write_default_init(module, ty)?,
2117+
Expression::ZeroValue(ty) => {
2118+
self.write_wrapped_zero_value_function_name(module, WrappedZeroValue { ty })?;
2119+
write!(self.out, "()")?;
2120+
}
21142121
Expression::Compose { ty, ref components } => {
21152122
match module.types[ty].inner {
21162123
TypeInner::Struct { .. } | TypeInner::Array { .. } => {
@@ -3260,7 +3267,11 @@ impl<'a, W: fmt::Write> super::Writer<'a, W> {
32603267
}
32613268

32623269
/// Helper function that write default zero initialization
3263-
fn write_default_init(&mut self, module: &Module, ty: Handle<crate::Type>) -> BackendResult {
3270+
pub(super) fn write_default_init(
3271+
&mut self,
3272+
module: &Module,
3273+
ty: Handle<crate::Type>,
3274+
) -> BackendResult {
32643275
write!(self.out, "(")?;
32653276
self.write_type(module, ty)?;
32663277
if let TypeInner::Array { base, size, .. } = module.types[ty].inner {

0 commit comments

Comments
 (0)