From 1461b032aa702c1909d39fa365cdcc18dda2adeb Mon Sep 17 00:00:00 2001 From: James Price Date: Thu, 9 Dec 2021 18:54:35 +0000 Subject: [PATCH] glsl: Don't emit structs with runtime-sized arrays The GLSL emitted for these was invalid, and we don't need these structs since they're only used as the store types of buffers, which are handled elsewhere. Change-Id: I17c15e408b5c36e9b895e5950528a6d02d1802a6 Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/72381 Reviewed-by: Stephen White Kokoro: Kokoro --- src/writer/glsl/generator_impl.cc | 14 +++- .../glsl/generator_impl_sanitizer_test.cc | 10 --- test/bug/tint/1113.wgsl.expected.glsl | 66 +++++++++++++++++++ test/bug/tint/1121.wgsl.expected.glsl | 20 ++++++ test/bug/tint/534.wgsl.expected.glsl | 6 ++ test/bug/tint/744.wgsl.expected.glsl | 5 ++ test/bug/tint/757.wgsl.expected.glsl | 3 + test/bug/tint/913.wgsl.expected.glsl | 7 ++ test/bug/tint/914.wgsl.expected.glsl | 5 ++ .../ignore/runtime_array.wgsl.expected.glsl | 2 +- .../ignore/storage_buffer.wgsl.expected.glsl | 4 +- .../ignore/uniform_buffer.wgsl.expected.glsl | 4 +- 12 files changed, 129 insertions(+), 17 deletions(-) diff --git a/src/writer/glsl/generator_impl.cc b/src/writer/glsl/generator_impl.cc index c021420263..8e2cc8e98f 100644 --- a/src/writer/glsl/generator_impl.cc +++ b/src/writer/glsl/generator_impl.cc @@ -123,8 +123,18 @@ bool GeneratorImpl::Generate() { return false; } } else if (auto* str = decl->As()) { - if (!EmitStructType(current_buffer_, builder_.Sem().Get(str))) { - return false; + // Skip emission if the struct contains a runtime-sized array, since its + // only use will be as the store-type of a buffer and we emit those + // elsewhere. + // TODO(crbug.com/tint/1339): We could also avoid emitting any other + // struct that is only used as a buffer store type. + TINT_ASSERT(Writer, str->members.size() > 0); + auto* last_member = str->members[str->members.size() - 1]; + auto* arr = last_member->type->As(); + if (!arr || !arr->IsRuntimeArray()) { + if (!EmitStructType(current_buffer_, builder_.Sem().Get(str))) { + return false; + } } } else if (auto* func = decl->As()) { if (func->IsEntryPoint()) { diff --git a/src/writer/glsl/generator_impl_sanitizer_test.cc b/src/writer/glsl/generator_impl_sanitizer_test.cc index c7be764c4f..8d5fdb2fad 100644 --- a/src/writer/glsl/generator_impl_sanitizer_test.cc +++ b/src/writer/glsl/generator_impl_sanitizer_test.cc @@ -51,9 +51,6 @@ TEST_F(GlslSanitizerTest, Call_ArrayLength) { auto* expect = R"(#version 310 es precision mediump float; -struct my_struct { - float a[]; -}; layout (binding = 1) buffer my_struct_1 { float a[]; @@ -105,10 +102,6 @@ TEST_F(GlslSanitizerTest, Call_ArrayLength_OtherMembersInStruct) { auto* expect = R"(#version 310 es precision mediump float; -struct my_struct { - float z; - float a[]; -}; layout (binding = 1) buffer my_struct_1 { float z; @@ -163,9 +156,6 @@ TEST_F(GlslSanitizerTest, Call_ArrayLength_ViaLets) { auto* expect = R"(#version 310 es precision mediump float; -struct my_struct { - float a[]; -}; layout (binding = 1) buffer my_struct_1 { float a[]; diff --git a/test/bug/tint/1113.wgsl.expected.glsl b/test/bug/tint/1113.wgsl.expected.glsl index 07877d9003..b27604e1b0 100644 --- a/test/bug/tint/1113.wgsl.expected.glsl +++ b/test/bug/tint/1113.wgsl.expected.glsl @@ -1,6 +1,28 @@ #version 310 es precision mediump float; +struct Uniforms { + uint numTriangles; + uint gridSize; + uint pad1; + uint pad2; + vec3 bbMin; + vec3 bbMax; +}; +struct Dbg { + uint offsetCounter; + uint pad0; + uint pad1; + uint pad2; + uint value0; + uint value1; + uint value2; + uint value3; + float value_f32_0; + float value_f32_1; + float value_f32_2; + float value_f32_3; +}; layout (binding = 0) uniform Uniforms_1 { uint numTriangles; @@ -118,6 +140,28 @@ void main() { #version 310 es precision mediump float; +struct Uniforms { + uint numTriangles; + uint gridSize; + uint pad1; + uint pad2; + vec3 bbMin; + vec3 bbMax; +}; +struct Dbg { + uint offsetCounter; + uint pad0; + uint pad1; + uint pad2; + uint value0; + uint value1; + uint value2; + uint value3; + float value_f32_0; + float value_f32_1; + float value_f32_2; + float value_f32_3; +}; layout (binding = 0) uniform Uniforms_1 { uint numTriangles; @@ -204,6 +248,28 @@ void main() { #version 310 es precision mediump float; +struct Uniforms { + uint numTriangles; + uint gridSize; + uint pad1; + uint pad2; + vec3 bbMin; + vec3 bbMax; +}; +struct Dbg { + uint offsetCounter; + uint pad0; + uint pad1; + uint pad2; + uint value0; + uint value1; + uint value2; + uint value3; + float value_f32_0; + float value_f32_1; + float value_f32_2; + float value_f32_3; +}; layout (binding = 0) uniform Uniforms_1 { uint numTriangles; diff --git a/test/bug/tint/1121.wgsl.expected.glsl b/test/bug/tint/1121.wgsl.expected.glsl index fbdcc648e4..880f6a2f56 100644 --- a/test/bug/tint/1121.wgsl.expected.glsl +++ b/test/bug/tint/1121.wgsl.expected.glsl @@ -15,11 +15,23 @@ struct TileLightIdData { uint count; uint lightId[64]; }; +struct Tiles { + TileLightIdData data[4]; +}; layout (binding = 0) buffer Tiles_1 { TileLightIdData data[4]; } tileLightId; +struct Config { + uint numLights; + uint numTiles; + uint tileCountX; + uint tileCountY; + uint numTileLightSlot; + uint tileSize; +}; + layout (binding = 0) uniform Config_1 { uint numLights; uint numTiles; @@ -29,6 +41,14 @@ layout (binding = 0) uniform Config_1 { uint tileSize; } config; +struct Uniforms { + vec4 tint_symbol; + vec4 tint_symbol_1; + mat4 viewMatrix; + mat4 projectionMatrix; + vec4 fullScreenSize; +}; + layout (binding = 0) uniform Uniforms_1 { vec4 tint_symbol; vec4 tint_symbol_1; diff --git a/test/bug/tint/534.wgsl.expected.glsl b/test/bug/tint/534.wgsl.expected.glsl index 6a80ad66da..44c97214b0 100644 --- a/test/bug/tint/534.wgsl.expected.glsl +++ b/test/bug/tint/534.wgsl.expected.glsl @@ -1,6 +1,12 @@ #version 310 es precision mediump float; +struct Uniforms { + uint dstTextureFlipY; + uint isFloat16; + uint isRGB10A2Unorm; + uint channelCount; +}; uniform highp sampler2D src; uniform highp sampler2D dst; diff --git a/test/bug/tint/744.wgsl.expected.glsl b/test/bug/tint/744.wgsl.expected.glsl index e8890c4ccb..d79301cc24 100644 --- a/test/bug/tint/744.wgsl.expected.glsl +++ b/test/bug/tint/744.wgsl.expected.glsl @@ -1,6 +1,11 @@ #version 310 es precision mediump float; +struct Uniforms { + uvec2 aShape; + uvec2 bShape; + uvec2 outShape; +}; layout (binding = 0) buffer Matrix_1 { uint numbers[]; diff --git a/test/bug/tint/757.wgsl.expected.glsl b/test/bug/tint/757.wgsl.expected.glsl index b1f584588d..33da240279 100644 --- a/test/bug/tint/757.wgsl.expected.glsl +++ b/test/bug/tint/757.wgsl.expected.glsl @@ -1,6 +1,9 @@ #version 310 es precision mediump float; +struct Constants { + int level; +}; uniform highp sampler2DArray myTexture; diff --git a/test/bug/tint/913.wgsl.expected.glsl b/test/bug/tint/913.wgsl.expected.glsl index 76e16aebcf..d70e652ef2 100644 --- a/test/bug/tint/913.wgsl.expected.glsl +++ b/test/bug/tint/913.wgsl.expected.glsl @@ -1,6 +1,13 @@ #version 310 es precision mediump float; +struct Uniforms { + uint dstTextureFlipY; + uint channelCount; + uvec2 srcCopyOrigin; + uvec2 dstCopyOrigin; + uvec2 copySize; +}; uniform highp sampler2D src; uniform highp sampler2D dst; diff --git a/test/bug/tint/914.wgsl.expected.glsl b/test/bug/tint/914.wgsl.expected.glsl index 951dea5dbf..4ca2f35fe2 100644 --- a/test/bug/tint/914.wgsl.expected.glsl +++ b/test/bug/tint/914.wgsl.expected.glsl @@ -1,6 +1,11 @@ #version 310 es precision mediump float; +struct Uniforms { + uint dimAOuter; + uint dimInner; + uint dimBOuter; +}; layout (binding = 0) buffer Matrix_1 { float numbers[]; diff --git a/test/intrinsics/ignore/runtime_array.wgsl.expected.glsl b/test/intrinsics/ignore/runtime_array.wgsl.expected.glsl index 18488609cb..0d0e581e87 100644 --- a/test/intrinsics/ignore/runtime_array.wgsl.expected.glsl +++ b/test/intrinsics/ignore/runtime_array.wgsl.expected.glsl @@ -1,4 +1,4 @@ -intrinsics/ignore/runtime_array.wgsl:10:5 warning: use of deprecated intrinsic +intrinsics/ignore/runtime_array.wgsl:9:5 warning: use of deprecated intrinsic ignore(s.arr); ^^^^^^ diff --git a/test/intrinsics/ignore/storage_buffer.wgsl.expected.glsl b/test/intrinsics/ignore/storage_buffer.wgsl.expected.glsl index 211b0a1560..3ad8ba86a9 100644 --- a/test/intrinsics/ignore/storage_buffer.wgsl.expected.glsl +++ b/test/intrinsics/ignore/storage_buffer.wgsl.expected.glsl @@ -1,8 +1,8 @@ -intrinsics/ignore/storage_buffer.wgsl:10:5 warning: use of deprecated intrinsic +intrinsics/ignore/storage_buffer.wgsl:9:5 warning: use of deprecated intrinsic ignore(s); ^^^^^^ -intrinsics/ignore/storage_buffer.wgsl:11:5 warning: use of deprecated intrinsic +intrinsics/ignore/storage_buffer.wgsl:10:5 warning: use of deprecated intrinsic ignore(s.i); ^^^^^^ diff --git a/test/intrinsics/ignore/uniform_buffer.wgsl.expected.glsl b/test/intrinsics/ignore/uniform_buffer.wgsl.expected.glsl index 987d1c2e57..fdd2c8107f 100644 --- a/test/intrinsics/ignore/uniform_buffer.wgsl.expected.glsl +++ b/test/intrinsics/ignore/uniform_buffer.wgsl.expected.glsl @@ -1,8 +1,8 @@ -intrinsics/ignore/uniform_buffer.wgsl:10:5 warning: use of deprecated intrinsic +intrinsics/ignore/uniform_buffer.wgsl:9:5 warning: use of deprecated intrinsic ignore(u); ^^^^^^ -intrinsics/ignore/uniform_buffer.wgsl:11:5 warning: use of deprecated intrinsic +intrinsics/ignore/uniform_buffer.wgsl:10:5 warning: use of deprecated intrinsic ignore(u.i); ^^^^^^