Skip to content

Commit

Permalink
[WGSL] Packed struct should not assume every nested struct is also pa…
Browse files Browse the repository at this point in the history
…cked

https://bugs.webkit.org/show_bug.cgi?id=264429
rdar://118129804

Reviewed by Mike Wyrzykowski.

A struct `S` might contain a field `x` whose type is another struct `T`. Currently,
if we pack `S`, during code generation we'll emit the type of `x` as `T::Packed`,
i.e. we just assume that all the structs referenced by `S` will also be packed.
Instead, we need to check before emitting code whether `T` was packed or not.

* Source/WebGPU/WGSL/Metal/MetalFunctionWriter.cpp:
(WGSL::Metal::FunctionDefinitionWriter::visit):
* Source/WebGPU/WGSL/tests/valid/packing.wgsl:

Canonical link: https://commits.webkit.org/270438@main
  • Loading branch information
tadeuzagallo committed Nov 9, 2023
1 parent 291703d commit 9c5b697
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 1 deletion.
2 changes: 1 addition & 1 deletion Source/WebGPU/WGSL/Metal/MetalFunctionWriter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -764,7 +764,7 @@ void FunctionDefinitionWriter::visit(const Type* type)
},
[&](const Struct& structure) {
m_stringBuilder.append(structure.structure.name());
if (m_structRole.has_value() && *m_structRole == AST::StructureRole::PackedResource)
if (m_structRole.has_value() && *m_structRole == AST::StructureRole::PackedResource && structure.structure.role() == AST::StructureRole::UserDefinedResource)
m_stringBuilder.append("::PackedType");
},
[&](const PrimitiveStruct& structure) {
Expand Down
5 changes: 5 additions & 0 deletions Source/WebGPU/WGSL/tests/valid/packing.wgsl
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
// RUN: %metal-compile main
// RUN: %metal main 2>&1 | %check

struct Unpacked {
x: i32,
}

struct T {
v2f: vec2<f32>,
v3f: vec3<f32>,
Expand All @@ -10,6 +14,7 @@ struct T {
v4u: vec4<u32>,
f: f32,
u: u32,
unpacked: Unpacked,
}

struct U {
Expand Down

0 comments on commit 9c5b697

Please sign in to comment.