From 99688918e167ff6990885ec89e89d9d9eb45aaeb Mon Sep 17 00:00:00 2001 From: David Neto Date: Wed, 16 Dec 2020 20:40:53 +0000 Subject: [PATCH] spirv-writer: sampled type must be f32,i32,or u32 Fix emission of the sampled type for write-only storage images. Fixed: tint:415 Change-Id: I83b74272630f16258295a354f952ce19c2eae57a Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/35863 Auto-Submit: David Neto Commit-Queue: dan sinclair Reviewed-by: dan sinclair --- src/writer/spirv/builder.cc | 7 +- .../spirv/builder_global_variable_test.cc | 2 +- .../spirv/builder_intrinsic_texture_test.cc | 148 +++++++++--------- src/writer/spirv/builder_type_test.cc | 52 +++++- 4 files changed, 123 insertions(+), 86 deletions(-) diff --git a/src/writer/spirv/builder.cc b/src/writer/spirv/builder.cc index 283043e627..917cbdc8a8 100644 --- a/src/writer/spirv/builder.cc +++ b/src/writer/spirv/builder.cc @@ -2696,12 +2696,7 @@ bool Builder::GenerateTextureType(ast::type::Texture* texture, } else if (auto* ms = texture->As()) { type_id = GenerateTypeIfNeeded(ms->type()); } else if (auto* st = texture->As()) { - if (st->access() == ast::AccessControl::kWriteOnly) { - ast::type::Void void_type; - type_id = GenerateTypeIfNeeded(&void_type); - } else { - type_id = GenerateTypeIfNeeded(st->type()); - } + type_id = GenerateTypeIfNeeded(st->type()); } if (type_id == 0u) { return false; diff --git a/src/writer/spirv/builder_global_variable_test.cc b/src/writer/spirv/builder_global_variable_test.cc index cd5537c927..f8b3fed5f6 100644 --- a/src/writer/spirv/builder_global_variable_test.cc +++ b/src/writer/spirv/builder_global_variable_test.cc @@ -693,7 +693,7 @@ TEST_F(BuilderTest, GlobalVar_TextureStorageWriteOnly) { EXPECT_EQ(DumpInstructions(b.annots()), R"(OpDecorate %1 NonReadable )"); - EXPECT_EQ(DumpInstructions(b.types()), R"(%4 = OpTypeVoid + EXPECT_EQ(DumpInstructions(b.types()), R"(%4 = OpTypeInt 32 0 %3 = OpTypeImage %4 2D 0 0 0 2 R32ui %2 = OpTypePointer UniformConstant %3 %1 = OpVariable %2 UniformConstant diff --git a/src/writer/spirv/builder_intrinsic_texture_test.cc b/src/writer/spirv/builder_intrinsic_texture_test.cc index 88b9cb84c8..f73ff56ea7 100644 --- a/src/writer/spirv/builder_intrinsic_texture_test.cc +++ b/src/writer/spirv/builder_intrinsic_texture_test.cc @@ -2567,132 +2567,132 @@ expected_texture_overload_spirv expected_texture_overload( )"}; case ValidTextureOverload::kStoreWO1dRgba32float: return {R"( -%4 = OpTypeVoid +%4 = OpTypeFloat 32 %3 = OpTypeImage %4 1D 0 0 0 2 Rgba32f %2 = OpTypePointer Private %3 %1 = OpVariable %2 Private %7 = OpTypeSampler %6 = OpTypePointer Private %7 %5 = OpVariable %6 Private -%10 = OpTypeInt 32 1 -%11 = OpConstant %10 1 -%13 = OpTypeFloat 32 -%12 = OpTypeVector %13 4 -%14 = OpConstant %13 2 -%15 = OpConstant %13 3 -%16 = OpConstant %13 4 -%17 = OpConstant %13 5 -%18 = OpConstantComposite %12 %14 %15 %16 %17 +%9 = OpTypeVoid +%11 = OpTypeInt 32 1 +%12 = OpConstant %11 1 +%13 = OpTypeVector %4 4 +%14 = OpConstant %4 2 +%15 = OpConstant %4 3 +%16 = OpConstant %4 4 +%17 = OpConstant %4 5 +%18 = OpConstantComposite %13 %14 %15 %16 %17 )", R"( -%9 = OpLoad %3 %1 -OpImageWrite %9 %11 %18 +%10 = OpLoad %3 %1 +OpImageWrite %10 %12 %18 )"}; case ValidTextureOverload::kStoreWO1dArrayRgba32float: return {R"( -%4 = OpTypeVoid +%4 = OpTypeFloat 32 %3 = OpTypeImage %4 1D 0 1 0 2 Rgba32f %2 = OpTypePointer Private %3 %1 = OpVariable %2 Private %7 = OpTypeSampler %6 = OpTypePointer Private %7 %5 = OpVariable %6 Private -%11 = OpTypeInt 32 1 -%10 = OpTypeVector %11 2 -%12 = OpConstant %11 1 -%13 = OpConstant %11 2 -%14 = OpConstantComposite %10 %12 %13 -%16 = OpTypeFloat 32 -%15 = OpTypeVector %16 4 -%17 = OpConstant %16 3 -%18 = OpConstant %16 4 -%19 = OpConstant %16 5 -%20 = OpConstant %16 6 -%21 = OpConstantComposite %15 %17 %18 %19 %20 +%9 = OpTypeVoid +%12 = OpTypeInt 32 1 +%11 = OpTypeVector %12 2 +%13 = OpConstant %12 1 +%14 = OpConstant %12 2 +%15 = OpConstantComposite %11 %13 %14 +%16 = OpTypeVector %4 4 +%17 = OpConstant %4 3 +%18 = OpConstant %4 4 +%19 = OpConstant %4 5 +%20 = OpConstant %4 6 +%21 = OpConstantComposite %16 %17 %18 %19 %20 )", R"( -%9 = OpLoad %3 %1 -OpImageWrite %9 %14 %21 +%10 = OpLoad %3 %1 +OpImageWrite %10 %15 %21 )"}; case ValidTextureOverload::kStoreWO2dRgba32float: return {R"( -%4 = OpTypeVoid +%4 = OpTypeFloat 32 %3 = OpTypeImage %4 2D 0 0 0 2 Rgba32f %2 = OpTypePointer Private %3 %1 = OpVariable %2 Private %7 = OpTypeSampler %6 = OpTypePointer Private %7 %5 = OpVariable %6 Private -%11 = OpTypeInt 32 1 -%10 = OpTypeVector %11 2 -%12 = OpConstant %11 1 -%13 = OpConstant %11 2 -%14 = OpConstantComposite %10 %12 %13 -%16 = OpTypeFloat 32 -%15 = OpTypeVector %16 4 -%17 = OpConstant %16 3 -%18 = OpConstant %16 4 -%19 = OpConstant %16 5 -%20 = OpConstant %16 6 -%21 = OpConstantComposite %15 %17 %18 %19 %20 +%9 = OpTypeVoid +%12 = OpTypeInt 32 1 +%11 = OpTypeVector %12 2 +%13 = OpConstant %12 1 +%14 = OpConstant %12 2 +%15 = OpConstantComposite %11 %13 %14 +%16 = OpTypeVector %4 4 +%17 = OpConstant %4 3 +%18 = OpConstant %4 4 +%19 = OpConstant %4 5 +%20 = OpConstant %4 6 +%21 = OpConstantComposite %16 %17 %18 %19 %20 )", R"( -%9 = OpLoad %3 %1 -OpImageWrite %9 %14 %21 +%10 = OpLoad %3 %1 +OpImageWrite %10 %15 %21 )"}; case ValidTextureOverload::kStoreWO2dArrayRgba32float: return {R"( -%4 = OpTypeVoid +%4 = OpTypeFloat 32 %3 = OpTypeImage %4 2D 0 1 0 2 Rgba32f %2 = OpTypePointer Private %3 %1 = OpVariable %2 Private %7 = OpTypeSampler %6 = OpTypePointer Private %7 %5 = OpVariable %6 Private -%11 = OpTypeInt 32 1 -%10 = OpTypeVector %11 3 -%12 = OpConstant %11 1 -%13 = OpConstant %11 2 -%14 = OpConstant %11 3 -%15 = OpConstantComposite %10 %12 %13 %14 -%17 = OpTypeFloat 32 -%16 = OpTypeVector %17 4 -%18 = OpConstant %17 4 -%19 = OpConstant %17 5 -%20 = OpConstant %17 6 -%21 = OpConstant %17 7 -%22 = OpConstantComposite %16 %18 %19 %20 %21 +%9 = OpTypeVoid +%12 = OpTypeInt 32 1 +%11 = OpTypeVector %12 3 +%13 = OpConstant %12 1 +%14 = OpConstant %12 2 +%15 = OpConstant %12 3 +%16 = OpConstantComposite %11 %13 %14 %15 +%17 = OpTypeVector %4 4 +%18 = OpConstant %4 4 +%19 = OpConstant %4 5 +%20 = OpConstant %4 6 +%21 = OpConstant %4 7 +%22 = OpConstantComposite %17 %18 %19 %20 %21 )", R"( -%9 = OpLoad %3 %1 -OpImageWrite %9 %15 %22 +%10 = OpLoad %3 %1 +OpImageWrite %10 %16 %22 )"}; case ValidTextureOverload::kStoreWO3dRgba32float: return {R"( -%4 = OpTypeVoid +%4 = OpTypeFloat 32 %3 = OpTypeImage %4 3D 0 0 0 2 Rgba32f %2 = OpTypePointer Private %3 %1 = OpVariable %2 Private %7 = OpTypeSampler %6 = OpTypePointer Private %7 %5 = OpVariable %6 Private -%11 = OpTypeInt 32 1 -%10 = OpTypeVector %11 3 -%12 = OpConstant %11 1 -%13 = OpConstant %11 2 -%14 = OpConstant %11 3 -%15 = OpConstantComposite %10 %12 %13 %14 -%17 = OpTypeFloat 32 -%16 = OpTypeVector %17 4 -%18 = OpConstant %17 4 -%19 = OpConstant %17 5 -%20 = OpConstant %17 6 -%21 = OpConstant %17 7 -%22 = OpConstantComposite %16 %18 %19 %20 %21 +%9 = OpTypeVoid +%12 = OpTypeInt 32 1 +%11 = OpTypeVector %12 3 +%13 = OpConstant %12 1 +%14 = OpConstant %12 2 +%15 = OpConstant %12 3 +%16 = OpConstantComposite %11 %13 %14 %15 +%17 = OpTypeVector %4 4 +%18 = OpConstant %4 4 +%19 = OpConstant %4 5 +%20 = OpConstant %4 6 +%21 = OpConstant %4 7 +%22 = OpConstantComposite %17 %18 %19 %20 %21 )", R"( -%9 = OpLoad %3 %1 -OpImageWrite %9 %15 %22 +%10 = OpLoad %3 %1 +OpImageWrite %10 %16 %22 )"}; } diff --git a/src/writer/spirv/builder_type_test.cc b/src/writer/spirv/builder_type_test.cc index 34d185c7a2..76b1da4eba 100644 --- a/src/writer/spirv/builder_type_test.cc +++ b/src/writer/spirv/builder_type_test.cc @@ -944,7 +944,7 @@ TEST_F(BuilderTest_Type, StorageTexture_GenerateWriteonly_1d) { ASSERT_TRUE(td.DetermineStorageTextureSubtype(&s)) << td.error(); EXPECT_EQ(b.GenerateTypeIfNeeded(&s), 1u); ASSERT_FALSE(b.has_error()) << b.error(); - EXPECT_EQ(DumpInstructions(b.types()), R"(%2 = OpTypeVoid + EXPECT_EQ(DumpInstructions(b.types()), R"(%2 = OpTypeFloat 32 %1 = OpTypeImage %2 1D 0 0 0 2 R16f )"); @@ -962,7 +962,7 @@ TEST_F(BuilderTest_Type, StorageTexture_GenerateWriteonly_1dArray) { ASSERT_TRUE(td.DetermineStorageTextureSubtype(&s)) << td.error(); EXPECT_EQ(b.GenerateTypeIfNeeded(&s), 1u); ASSERT_FALSE(b.has_error()) << b.error(); - EXPECT_EQ(DumpInstructions(b.types()), R"(%2 = OpTypeVoid + EXPECT_EQ(DumpInstructions(b.types()), R"(%2 = OpTypeFloat 32 %1 = OpTypeImage %2 1D 0 1 0 2 R16f )"); @@ -980,7 +980,7 @@ TEST_F(BuilderTest_Type, StorageTexture_GenerateWriteonly_2d) { ASSERT_TRUE(td.DetermineStorageTextureSubtype(&s)) << td.error(); EXPECT_EQ(b.GenerateTypeIfNeeded(&s), 1u); ASSERT_FALSE(b.has_error()) << b.error(); - EXPECT_EQ(DumpInstructions(b.types()), R"(%2 = OpTypeVoid + EXPECT_EQ(DumpInstructions(b.types()), R"(%2 = OpTypeFloat 32 %1 = OpTypeImage %2 2D 0 0 0 2 R16f )"); } @@ -993,7 +993,7 @@ TEST_F(BuilderTest_Type, StorageTexture_GenerateWriteonly_2dArray) { ASSERT_TRUE(td.DetermineStorageTextureSubtype(&s)) << td.error(); EXPECT_EQ(b.GenerateTypeIfNeeded(&s), 1u); ASSERT_FALSE(b.has_error()) << b.error(); - EXPECT_EQ(DumpInstructions(b.types()), R"(%2 = OpTypeVoid + EXPECT_EQ(DumpInstructions(b.types()), R"(%2 = OpTypeFloat 32 %1 = OpTypeImage %2 2D 0 1 0 2 R16f )"); } @@ -1006,11 +1006,53 @@ TEST_F(BuilderTest_Type, StorageTexture_GenerateWriteonly_3d) { ASSERT_TRUE(td.DetermineStorageTextureSubtype(&s)) << td.error(); EXPECT_EQ(b.GenerateTypeIfNeeded(&s), 1u); ASSERT_FALSE(b.has_error()) << b.error(); - EXPECT_EQ(DumpInstructions(b.types()), R"(%2 = OpTypeVoid + EXPECT_EQ(DumpInstructions(b.types()), R"(%2 = OpTypeFloat 32 %1 = OpTypeImage %2 3D 0 0 0 2 R16f )"); } +TEST_F(BuilderTest_Type, + StorageTexture_GenerateWriteonly_SampledTypeFloat_Format_r32float) { + ast::type::StorageTexture s(ast::type::TextureDimension::k2d, + ast::AccessControl::kWriteOnly, + ast::type::ImageFormat::kR32Float); + + ASSERT_TRUE(td.DetermineStorageTextureSubtype(&s)) << td.error(); + EXPECT_EQ(b.GenerateTypeIfNeeded(&s), 1u); + ASSERT_FALSE(b.has_error()) << b.error(); + EXPECT_EQ(DumpInstructions(b.types()), R"(%2 = OpTypeFloat 32 +%1 = OpTypeImage %2 2D 0 0 0 2 R32f +)"); +} + +TEST_F(BuilderTest_Type, + StorageTexture_GenerateWriteonly_SampledTypeSint_Format_r32sint) { + ast::type::StorageTexture s(ast::type::TextureDimension::k2d, + ast::AccessControl::kWriteOnly, + ast::type::ImageFormat::kR32Sint); + + ASSERT_TRUE(td.DetermineStorageTextureSubtype(&s)) << td.error(); + EXPECT_EQ(b.GenerateTypeIfNeeded(&s), 1u); + ASSERT_FALSE(b.has_error()) << b.error(); + EXPECT_EQ(DumpInstructions(b.types()), R"(%2 = OpTypeInt 32 1 +%1 = OpTypeImage %2 2D 0 0 0 2 R32i +)"); +} + +TEST_F(BuilderTest_Type, + StorageTexture_GenerateWriteonly_SampledTypeUint_Format_r32uint) { + ast::type::StorageTexture s(ast::type::TextureDimension::k2d, + ast::AccessControl::kWriteOnly, + ast::type::ImageFormat::kR32Uint); + + ASSERT_TRUE(td.DetermineStorageTextureSubtype(&s)) << td.error(); + EXPECT_EQ(b.GenerateTypeIfNeeded(&s), 1u); + ASSERT_FALSE(b.has_error()) << b.error(); + EXPECT_EQ(DumpInstructions(b.types()), R"(%2 = OpTypeInt 32 0 +%1 = OpTypeImage %2 2D 0 0 0 2 R32ui +)"); +} + TEST_F(BuilderTest_Type, Sampler) { ast::type::Sampler sampler(ast::type::SamplerKind::kSampler); EXPECT_EQ(b.GenerateTypeIfNeeded(&sampler), 1u);