From 0f02a6de00906d8594055bb2bf792f3e49ee939b Mon Sep 17 00:00:00 2001 From: Ben Clayton Date: Tue, 30 Mar 2021 20:18:28 +0000 Subject: [PATCH] writer/hlsl: Don't emit structs for storage buffer usage Use the semantic StorageClassUsage() to determine whether the structure is used only for storage buffer usage. If it is, don't emit a struct definition for it. This fixes issues with attempting to generate runtime arrays - they're only legal for storage buffer usage. Storage buffers use ByteAddressBuffer instead of structured loads / stores. Bug: tint:185 Fixed: tint:682 Change-Id: I5b58a133eee2fe036a84e028fa85b611e4895b1a Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/46382 Commit-Queue: Ben Clayton Reviewed-by: James Price --- src/writer/hlsl/generator_impl.cc | 14 +++++++++++--- .../hlsl/generator_impl_alias_type_test.cc | 14 ++++++-------- .../hlsl/generator_impl_function_test.cc | 5 +---- src/writer/hlsl/generator_impl_type_test.cc | 18 ++++++++++++++++++ 4 files changed, 36 insertions(+), 15 deletions(-) diff --git a/src/writer/hlsl/generator_impl.cc b/src/writer/hlsl/generator_impl.cc index f7a7e9692f..bd5a00c808 100644 --- a/src/writer/hlsl/generator_impl.cc +++ b/src/writer/hlsl/generator_impl.cc @@ -2590,9 +2590,17 @@ bool GeneratorImpl::EmitType(std::ostream& out, bool GeneratorImpl::EmitStructType(std::ostream& out, const type::Struct* str, const std::string& name) { - // TODO(dsinclair): Block decoration? - // if (str->impl()->decoration() != ast::Decoration::kNone) { - // } + auto* sem_str = builder_.Sem().Get(str); + + auto storage_class_uses = sem_str->StorageClassUsage(); + if (storage_class_uses.size() == + storage_class_uses.count(ast::StorageClass::kStorage)) { + // The only use of the structure is as a storage buffer. + // Structures used as storage buffer are read and written to via a + // ByteAddressBuffer instead of true structure. + return true; + } + out << "struct " << name << " {" << std::endl; increment_indent(); diff --git a/src/writer/hlsl/generator_impl_alias_type_test.cc b/src/writer/hlsl/generator_impl_alias_type_test.cc index ca1007bdfe..8a2470197a 100644 --- a/src/writer/hlsl/generator_impl_alias_type_test.cc +++ b/src/writer/hlsl/generator_impl_alias_type_test.cc @@ -42,15 +42,13 @@ TEST_F(HlslGeneratorImplTest_Alias, EmitAlias_NameCollision) { } TEST_F(HlslGeneratorImplTest_Alias, EmitAlias_Struct) { - auto* str = create( - ast::StructMemberList{ - Member("a", ty.f32()), - Member("b", ty.i32()), - }, - ast::DecorationList{}); - - auto* s = ty.struct_("A", str); + auto* s = Structure("A", { + Member("a", ty.f32()), + Member("b", ty.i32()), + }); auto* alias = ty.alias("B", s); + AST().AddConstructedType(alias); + Global("g", alias, ast::StorageClass::kPrivate); GeneratorImpl& gen = Build(); diff --git a/src/writer/hlsl/generator_impl_function_test.cc b/src/writer/hlsl/generator_impl_function_test.cc index cc4da752fc..35dadb942c 100644 --- a/src/writer/hlsl/generator_impl_function_test.cc +++ b/src/writer/hlsl/generator_impl_function_test.cc @@ -916,10 +916,7 @@ TEST_F(HlslGeneratorImplTest_Function, GeneratorImpl& gen = Build(); ASSERT_TRUE(gen.Generate(out)) << gen.error(); - EXPECT_EQ(result(), R"(struct Data { - float d; -}; - + EXPECT_EQ(result(), R"( RWByteAddressBuffer data : register(u0, space0); [numthreads(1, 1, 1)] diff --git a/src/writer/hlsl/generator_impl_type_test.cc b/src/writer/hlsl/generator_impl_type_test.cc index 5353b5c4bf..5f6f63cde8 100644 --- a/src/writer/hlsl/generator_impl_type_test.cc +++ b/src/writer/hlsl/generator_impl_type_test.cc @@ -171,6 +171,7 @@ TEST_F(HlslGeneratorImplTest_Type, EmitType_StructDecl) { Member("a", ty.i32()), Member("b", ty.f32()), }); + Global("g", s, ast::StorageClass::kPrivate); GeneratorImpl& gen = Build(); @@ -182,11 +183,25 @@ TEST_F(HlslGeneratorImplTest_Type, EmitType_StructDecl) { )"); } +TEST_F(HlslGeneratorImplTest_Type, EmitType_StructDecl_OmittedIfStorageBuffer) { + auto* s = Structure("S", { + Member("a", ty.i32()), + Member("b", ty.f32()), + }); + Global("g", s, ast::StorageClass::kStorage); + + GeneratorImpl& gen = Build(); + + ASSERT_TRUE(gen.EmitStructType(out, s, "S")) << gen.error(); + EXPECT_EQ(result(), ""); +} + TEST_F(HlslGeneratorImplTest_Type, EmitType_Struct) { auto* s = Structure("S", { Member("a", ty.i32()), Member("b", ty.f32()), }); + Global("g", s, ast::StorageClass::kPrivate); GeneratorImpl& gen = Build(); @@ -203,6 +218,7 @@ TEST_F(HlslGeneratorImplTest_Type, DISABLED_EmitType_Struct_InjectPadding) { Member("b", ty.f32()), Member("c", ty.f32(), {MemberAlign(128), MemberSize(128)}), }); + Global("g", s, ast::StorageClass::kPrivate); GeneratorImpl& gen = Build(); @@ -223,6 +239,7 @@ TEST_F(HlslGeneratorImplTest_Type, EmitType_Struct_NameCollision) { Member("double", ty.i32()), Member("float", ty.f32()), }); + Global("g", s, ast::StorageClass::kPrivate); GeneratorImpl& gen = Build(); @@ -242,6 +259,7 @@ TEST_F(HlslGeneratorImplTest_Type, DISABLED_EmitType_Struct_WithDecoration) { Member("b", ty.f32()), }, {create()}); + Global("g", s, ast::StorageClass::kPrivate); GeneratorImpl& gen = Build();