[spv-out] Always give structs with runtime arrays a Block decoration (#…

…2455)
gfx-rs · Sep 12, 2023 · 1281c11 · 1281c11
1 parent 0f36a74
commit 1281c11
Show file tree

Hide file tree

Showing 11 changed files with 67 additions and 15 deletions.
diff --git a/src/back/spv/writer.rs b/src/back/spv/writer.rs
@@ -1016,12 +1016,27 @@ impl Writer {
                     ref members,
                     span: _,
                 } => {
+                    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];
+                        match member_ty.inner {
+                            crate::TypeInner::Array {
+                                base: _,
+                                size: crate::ArraySize::Dynamic,
+                                stride: _,
+                            } => {
+                                has_runtime_array = true;
+                            }
+                            _ => (),
+                        }
                         self.decorate_struct_member(id, index, member, arena)?;
                         let member_id = self.get_type_id(LookupType::Handle(member.ty));
                         member_ids.push(member_id);
                     }
+                    if has_runtime_array {
+                        self.decorate(id, Decoration::Block, &[]);
+                    }
                     Instruction::type_struct(id, member_ids.as_slice())
                 }
 
@@ -1656,16 +1671,17 @@ impl Writer {
         } else {
             // This is a global variable in the Storage address space. The only
             // way it could have `global_needs_wrapper() == false` is if it has
-            // a runtime-sized array. In this case, we need to decorate it with
-            // Block.
+            // a runtime-sized or binding array.
+            // Runtime-sized arrays were decorated when iterating through struct content.
+            // Now binding arrays require Block decorating.
             if let crate::AddressSpace::Storage { .. } = global_variable.space {
-                let decorated_id = match ir_module.types[global_variable.ty].inner {
+                match ir_module.types[global_variable.ty].inner {
                     crate::TypeInner::BindingArray { base, .. } => {
-                        self.get_type_id(LookupType::Handle(base))
+                        let decorated_id = self.get_type_id(LookupType::Handle(base));
+                        self.decorate(decorated_id, Decoration::Block, &[]);
                     }
-                    _ => inner_type_id,
+                    _ => (),
                 };
-                self.decorate(decorated_id, Decoration::Block, &[]);
             }
             if substitute_inner_type_lookup.is_some() {
                 inner_type_id

diff --git a/tests/in/runtime-array-in-unused-struct.wgsl b/tests/in/runtime-array-in-unused-struct.wgsl
@@ -0,0 +1,12 @@
+struct DataStruct {
+    data: f32,
+    data_vec: vec4<f32>,
+}
+
+struct Struct {
+    data: array<DataStruct>,
+};
+
+struct PrimitiveStruct {
+    data: array<f32>,
+};
diff --git a/tests/out/spv/access.spvasm b/tests/out/spv/access.spvasm
@@ -73,6 +73,7 @@ OpMemberDecorate %20 2 Offset 96
 OpMemberDecorate %20 3 Offset 100
 OpMemberDecorate %20 4 Offset 144
 OpMemberDecorate %20 5 Offset 160
+OpDecorate %20 Block
 OpMemberDecorate %22 0 Offset 0
 OpMemberDecorate %22 0 ColMajor
 OpMemberDecorate %22 0 MatrixStride 8
@@ -86,7 +87,6 @@ OpDecorate %33 ArrayStride 4
 OpDecorate %36 ArrayStride 16
 OpDecorate %47 DescriptorSet 0
 OpDecorate %47 Binding 0
-OpDecorate %20 Block
 OpDecorate %49 DescriptorSet 0
 OpDecorate %49 Binding 1
 OpDecorate %50 Block

diff --git a/tests/out/spv/boids.spvasm b/tests/out/spv/boids.spvasm
@@ -48,17 +48,16 @@ OpMemberDecorate %7 5 Offset 20
 OpMemberDecorate %7 6 Offset 24
 OpDecorate %8 ArrayStride 16
 OpMemberDecorate %9 0 Offset 0
+OpDecorate %9 Block
 OpDecorate %13 DescriptorSet 0
 OpDecorate %13 Binding 0
 OpDecorate %14 Block
 OpMemberDecorate %14 0 Offset 0
 OpDecorate %16 NonWritable
 OpDecorate %16 DescriptorSet 0
 OpDecorate %16 Binding 1
-OpDecorate %9 Block
 OpDecorate %18 DescriptorSet 0
 OpDecorate %18 Binding 2
-OpDecorate %9 Block
 OpDecorate %36 BuiltIn GlobalInvocationId
 %2 = OpTypeVoid
 %3 = OpTypeInt 32 0

diff --git a/tests/out/spv/bounds-check-restrict.spvasm b/tests/out/spv/bounds-check-restrict.spvasm
@@ -15,9 +15,9 @@ OpMemberDecorate %10 2 Offset 64
 OpMemberDecorate %10 2 ColMajor
 OpMemberDecorate %10 2 MatrixStride 16
 OpMemberDecorate %10 3 Offset 112
+OpDecorate %10 Block
 OpDecorate %13 DescriptorSet 0
 OpDecorate %13 Binding 0
-OpDecorate %10 Block
 %2 = OpTypeVoid
 %3 = OpTypeFloat 32
 %6 = OpTypeInt 32 0

diff --git a/tests/out/spv/bounds-check-zero.spvasm b/tests/out/spv/bounds-check-zero.spvasm
@@ -15,9 +15,9 @@ OpMemberDecorate %10 2 Offset 64
 OpMemberDecorate %10 2 ColMajor
 OpMemberDecorate %10 2 MatrixStride 16
 OpMemberDecorate %10 3 Offset 112
+OpDecorate %10 Block
 OpDecorate %13 DescriptorSet 0
 OpDecorate %13 Binding 0
-OpDecorate %10 Block
 %2 = OpTypeVoid
 %3 = OpTypeFloat 32
 %6 = OpTypeInt 32 0

diff --git a/tests/out/spv/collatz.spvasm b/tests/out/spv/collatz.spvasm
@@ -19,9 +19,9 @@ OpName %48 "global_id"
 OpName %51 "main"
 OpDecorate %4 ArrayStride 4
 OpMemberDecorate %5 0 Offset 0
+OpDecorate %5 Block
 OpDecorate %7 DescriptorSet 0
 OpDecorate %7 Binding 0
-OpDecorate %5 Block
 OpDecorate %48 BuiltIn GlobalInvocationId
 %2 = OpTypeVoid
 %3 = OpTypeInt 32 0

diff --git a/tests/out/spv/debug-symbol-terrain.spvasm b/tests/out/spv/debug-symbol-terrain.spvasm
@@ -415,8 +415,10 @@ OpMemberDecorate %14 0 Offset 0
 OpMemberDecorate %14 1 Offset 16
 OpDecorate %15 ArrayStride 32
 OpMemberDecorate %16 0 Offset 0
+OpDecorate %16 Block
 OpDecorate %17 ArrayStride 4
 OpMemberDecorate %18 0 Offset 0
+OpDecorate %18 Block
 OpMemberDecorate %20 0 Offset 0
 OpMemberDecorate %20 1 Offset 8
 OpMemberDecorate %20 2 Offset 16
@@ -442,10 +444,8 @@ OpDecorate %30 Block
 OpMemberDecorate %30 0 Offset 0
 OpDecorate %32 DescriptorSet 0
 OpDecorate %32 Binding 1
-OpDecorate %16 Block
 OpDecorate %34 DescriptorSet 0
 OpDecorate %34 Binding 2
-OpDecorate %18 Block
 OpDecorate %36 DescriptorSet 0
 OpDecorate %36 Binding 0
 OpDecorate %37 Block

diff --git a/tests/out/spv/pointers.spvasm b/tests/out/spv/pointers.spvasm
@@ -20,9 +20,9 @@ OpName %35 "v"
 OpName %36 "index_dynamic_array"
 OpDecorate %6 ArrayStride 4
 OpMemberDecorate %7 0 Offset 0
+OpDecorate %7 Block
 OpDecorate %10 DescriptorSet 0
 OpDecorate %10 Binding 0
-OpDecorate %7 Block
 %2 = OpTypeVoid
 %4 = OpTypeInt 32 1
 %3 = OpTypeVector %4 2

diff --git a/tests/out/spv/runtime-array-in-unused-struct.spvasm b/tests/out/spv/runtime-array-in-unused-struct.spvasm
@@ -0,0 +1,24 @@
+; SPIR-V
+; Version: 1.1
+; Generator: rspirv
+; Bound: 10
+OpCapability Shader
+OpCapability Linkage
+%1 = OpExtInstImport "GLSL.std.450"
+OpMemoryModel Logical GLSL450
+OpMemberDecorate %5 0 Offset 0
+OpMemberDecorate %5 1 Offset 16
+OpDecorate %6 ArrayStride 32
+OpMemberDecorate %7 0 Offset 0
+OpDecorate %7 Block
+OpDecorate %8 ArrayStride 4
+OpMemberDecorate %9 0 Offset 0
+OpDecorate %9 Block
+%2 = OpTypeVoid
+%3 = OpTypeFloat 32
+%4 = OpTypeVector %3 4
+%5 = OpTypeStruct %3 %4
+%6 = OpTypeRuntimeArray %5
+%7 = OpTypeStruct %6
+%8 = OpTypeRuntimeArray %3
+%9 = OpTypeStruct %8
diff --git a/tests/snapshots.rs b/tests/snapshots.rs
@@ -606,6 +606,7 @@ fn convert_wgsl() {
             "workgroup-uniform-load",
             Targets::WGSL | Targets::GLSL | Targets::SPIRV | Targets::HLSL | Targets::METAL,
         ),
+        ("runtime-array-in-unused-struct", Targets::SPIRV),
         ("sprite", Targets::SPIRV),
         ("force_point_size_vertex_shader_webgl", Targets::GLSL),
         ("invariant", Targets::GLSL),