commit 202d97d846e61ff97f9bdce389e2b54bcee6d73c
author James Price <jrprice@google.com> Wed May 05 19:38:12 2021 +0000
committer Commit Bot service account <commit-bot@chromium.org> Wed May 05 19:38:12 2021 +0000
tree a046887deaa6b53e2ece3423f8ee3fd14f716e5b
parent 58f93c9e26634deccf41e436ad3b51b21ebc73a9

writer/spirv: Add type to constructor cache key

This prevents two initializers with different types from incorrectly
using the same OpConstantComposite instruction.

Fixed: tint:777
Change-Id: I2cdef9d5b0a83c26cbd4be318fed218e6f3e93b0
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/49981
Commit-Queue: James Price <jrprice@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Auto-Submit: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>

src/writer/spirv/builder.cc

diff --git a/src/writer/spirv/builder.cc b/src/writer/spirv/builder.cc
index defc9b0..27300ff 100644
--- a/src/writer/spirv/builder.cc
+++ b/src/writer/spirv/builder.cc
@@ -1302,7 +1302,7 @@
   }
 
   std::ostringstream out;
-  out << "__const";
+  out << "__const_" << init->type()->FriendlyName(builder_.Symbols()) << "_";
 
   result_type = result_type->UnwrapAll();
   bool constructor_is_const = is_constructor_const(init, is_global_init);

src/writer/spirv/builder_constructor_expression_test.cc

diff --git a/src/writer/spirv/builder_constructor_expression_test.cc b/src/writer/spirv/builder_constructor_expression_test.cc
index 463246f..6c2ea8f 100644
--- a/src/writer/spirv/builder_constructor_expression_test.cc
+++ b/src/writer/spirv/builder_constructor_expression_test.cc
@@ -975,6 +975,82 @@
 )");
 }
 
+TEST_F(SpvBuilderConstructorTest, CommonInitializer_TwoVectors) {
+  auto* v1 = vec3<f32>(2.0f, 2.0f, 2.0f);
+  auto* v2 = vec3<f32>(2.0f, 2.0f, 2.0f);
+  ast::StatementList stmts = {
+      WrapInStatement(v1),
+      WrapInStatement(v2),
+  };
+  WrapInFunction(stmts);
+
+  spirv::Builder& b = Build();
+
+  b.push_function(Function{});
+  EXPECT_EQ(b.GenerateExpression(v1), 4u);
+  EXPECT_EQ(b.GenerateExpression(v2), 4u);
+
+  EXPECT_EQ(DumpInstructions(b.types()), R"(%2 = OpTypeFloat 32
+%1 = OpTypeVector %2 3
+%3 = OpConstant %2 2
+%4 = OpConstantComposite %1 %3 %3 %3
+)");
+}
+
+TEST_F(SpvBuilderConstructorTest, CommonInitializer_TwoArrays) {
+  auto* a1 = array<f32, 3>(2.0f, 2.0f, 2.0f);
+  auto* a2 = array<f32, 3>(2.0f, 2.0f, 2.0f);
+  ast::StatementList stmts = {
+      WrapInStatement(a1),
+      WrapInStatement(a2),
+  };
+  WrapInFunction(stmts);
+
+  spirv::Builder& b = Build();
+
+  b.push_function(Function{});
+  EXPECT_EQ(b.GenerateExpression(a1), 6u);
+  EXPECT_EQ(b.GenerateExpression(a2), 6u);
+
+  EXPECT_EQ(DumpInstructions(b.types()), R"(%2 = OpTypeFloat 32
+%3 = OpTypeInt 32 0
+%4 = OpConstant %3 3
+%1 = OpTypeArray %2 %4
+%5 = OpConstant %2 2
+%6 = OpConstantComposite %1 %5 %5 %5
+)");
+}
+
+TEST_F(SpvBuilderConstructorTest, CommonInitializer_Array_VecArray) {
+  // Test that initializers of different types with the same values produce
+  // different OpConstantComposite instructions.
+  // crbug.com/tint/777
+  auto* a1 = array<f32, 2>(1.0f, 2.0f);
+  auto* a2 = array<f32, 2>(vec2<f32>(1.0f, 2.0f), vec2<f32>(1.0f, 2.0f));
+  ast::StatementList stmts = {
+      WrapInStatement(a1),
+      WrapInStatement(a2),
+  };
+  WrapInFunction(stmts);
+  spirv::Builder& b = Build();
+
+  b.push_function(Function{});
+  EXPECT_EQ(b.GenerateExpression(a1), 7u);
+  EXPECT_EQ(b.GenerateExpression(a2), 10u);
+
+  EXPECT_EQ(DumpInstructions(b.types()), R"(%2 = OpTypeFloat 32
+%3 = OpTypeInt 32 0
+%4 = OpConstant %3 2
+%1 = OpTypeArray %2 %4
+%5 = OpConstant %2 1
+%6 = OpConstant %2 2
+%7 = OpConstantComposite %1 %5 %6
+%8 = OpTypeVector %2 2
+%9 = OpConstantComposite %8 %5 %6
+%10 = OpConstantComposite %1 %9 %9
+)");
+}
+
 TEST_F(SpvBuilderConstructorTest, Type_Struct) {
   auto s = Structure("my_struct", {
                                       Member("a", ty.f32()),