[spirv-writer] Generate loads for const constructors.

When building a const variable from a pointer we need to make sure to
load the value and use the loaded ID as the constant Id.

Bug: tint:310
Change-Id: Ia544fd69f3d2ae13e9ff9a983935ddc332d8d6ff
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/32800
Reviewed-by: Sarah Mashayekhi <sarahmashay@google.com>
Reviewed-by: David Neto <dneto@google.com>
Reviewed-by: Ryan Harrison <rharrison@chromium.org>
Commit-Queue: David Neto <dneto@google.com>
Auto-Submit: dan sinclair <dsinclair@chromium.org>

src/writer/spirv/builder.cc

@@ -607,6 +607,7 @@ bool Builder::GenerateFunctionVariable(ast::Variable* var) {
     if (init_id == 0) {
       return false;
     }
+    init_id = GenerateLoadIfNeeded(var->constructor()->result_type(), init_id);
   }
 
   if (var->is_const()) {
@@ -644,7 +645,6 @@ bool Builder::GenerateFunctionVariable(ast::Variable* var) {
                      Operand::Int(null_id)});
 
   if (var->has_constructor()) {
-    init_id = GenerateLoadIfNeeded(var->constructor()->result_type(), init_id);
     GenerateStore(var_id, init_id);
   }
 
@@ -1055,7 +1055,7 @@ uint32_t Builder::GenerateIdentifierExpression(
     return val;
   }
 
-  error_ = "unable to find name for identifier: " + expr->name();
+  error_ = "unable to find variable with identifier: " + expr->name();
   return 0;
 }
 

src/writer/spirv/builder_function_test.cc

@@ -239,9 +239,9 @@ OpName %3 "tint_44617461"
 OpMemberName %3 0 "tint_64"
 OpName %1 "tint_64617461"
 OpName %7 "tint_61"
-OpName %13 "tint_76"
+OpName %14 "tint_76"
 OpName %17 "tint_62"
-OpName %20 "tint_76"
+OpName %21 "tint_76"
 OpDecorate %3 Block
 OpMemberDecorate %3 0 Offset 0
 OpDecorate %1 Binding 0
@@ -255,22 +255,22 @@ OpDecorate %1 DescriptorSet 0
 %9 = OpTypeInt 32 0
 %10 = OpConstant %9 0
 %11 = OpTypePointer StorageBuffer %4
-%14 = OpTypePointer Function %4
+%15 = OpTypePointer Function %4
-%15 = OpConstantNull %4
+%16 = OpConstantNull %4
 %7 = OpFunction %6 None %5
 %8 = OpLabel
-%13 = OpVariable %14 Function %15
+%14 = OpVariable %15 Function %16
 %12 = OpAccessChain %11 %1 %10
-%16 = OpLoad %4 %12
+%13 = OpLoad %4 %12
-OpStore %13 %16
+OpStore %14 %13
 OpReturn
 OpFunctionEnd
 %17 = OpFunction %6 None %5
 %18 = OpLabel
-%20 = OpVariable %14 Function %15
+%21 = OpVariable %15 Function %16
 %19 = OpAccessChain %11 %1 %10
-%21 = OpLoad %4 %19
+%20 = OpLoad %4 %19
-OpStore %20 %21
+OpStore %21 %20
 OpReturn
 OpFunctionEnd
 )");

src/writer/spirv/builder_function_variable_test.cc

@@ -154,13 +154,92 @@ OpStore %7 %6
 )");
 }
 
-TEST_F(BuilderTest,
+TEST_F(BuilderTest, FunctionVar_WithNonConstantConstructorLoadedFromVar) {
-       DISABLED_FunctionVar_WithNonConstantConstructorLoadedFromVar) {
+  // var v : f32 = 1.0;
-  // fn main() -> void {
+  // var v2 : f32 = v; // Should generate the load and store automatically.
-  //   var v : f32 = 1.0;
+
-  //   var v2 : f32 = v; // Should generate the load automatically.
+  ast::type::F32Type f32;
-  // }
+
-  FAIL();
+  auto init = create<ast::ScalarConstructorExpression>(
+      create<ast::FloatLiteral>(&f32, 1.0f));
+
+  ASSERT_TRUE(td.DetermineResultType(init.get())) << td.error();
+
+  ast::Variable v("v", ast::StorageClass::kFunction, &f32);
+  v.set_constructor(std::move(init));
+  td.RegisterVariableForTesting(&v);
+
+  ast::Variable v2("v2", ast::StorageClass::kFunction, &f32);
+  v2.set_constructor(create<ast::IdentifierExpression>("v"));
+  td.RegisterVariableForTesting(&v2);
+
+  ASSERT_TRUE(td.DetermineResultType(v2.constructor())) << td.error();
+
+  b.push_function(Function{});
+  EXPECT_TRUE(b.GenerateFunctionVariable(&v)) << b.error();
+  EXPECT_TRUE(b.GenerateFunctionVariable(&v2)) << b.error();
+  ASSERT_FALSE(b.has_error()) << b.error();
+
+  EXPECT_EQ(DumpInstructions(b.debug()), R"(OpName %3 "tint_76"
+OpName %7 "tint_7632"
+)");
+  EXPECT_EQ(DumpInstructions(b.types()), R"(%1 = OpTypeFloat 32
+%2 = OpConstant %1 1
+%4 = OpTypePointer Function %1
+%5 = OpConstantNull %1
+)");
+  EXPECT_EQ(DumpInstructions(b.functions()[0].variables()),
+            R"(%3 = OpVariable %4 Function %5
+%7 = OpVariable %4 Function %5
+)");
+  EXPECT_EQ(DumpInstructions(b.functions()[0].instructions()),
+            R"(OpStore %3 %2
+%6 = OpLoad %1 %3
+OpStore %7 %6
+)");
+}
+
+TEST_F(BuilderTest, FunctionVar_ConstWithVarInitializer) {
+  // var v : f32 = 1.0;
+  // const v2 : f32 = v; // Should generate the load
+
+  ast::type::F32Type f32;
+
+  auto init = create<ast::ScalarConstructorExpression>(
+      create<ast::FloatLiteral>(&f32, 1.0f));
+
+  EXPECT_TRUE(td.DetermineResultType(init.get())) << td.error();
+
+  ast::Variable v("v", ast::StorageClass::kFunction, &f32);
+  v.set_constructor(std::move(init));
+  td.RegisterVariableForTesting(&v);
+
+  ast::Variable v2("v2", ast::StorageClass::kFunction, &f32);
+  v2.set_is_const(true);
+  v2.set_constructor(create<ast::IdentifierExpression>("v"));
+  td.RegisterVariableForTesting(&v2);
+
+  ASSERT_TRUE(td.DetermineResultType(v2.constructor())) << td.error();
+
+  b.push_function(Function{});
+  EXPECT_TRUE(b.GenerateFunctionVariable(&v)) << b.error();
+  EXPECT_TRUE(b.GenerateFunctionVariable(&v2)) << b.error();
+  ASSERT_FALSE(b.has_error()) << b.error();
+
+  EXPECT_EQ(DumpInstructions(b.debug()), R"(OpName %3 "tint_76"
+)");
+  EXPECT_EQ(DumpInstructions(b.types()), R"(%1 = OpTypeFloat 32
+%2 = OpConstant %1 1
+%4 = OpTypePointer Function %1
+%5 = OpConstantNull %1
+)");
+  EXPECT_EQ(DumpInstructions(b.functions()[0].variables()),
+            R"(%3 = OpVariable %4 Function %5
+)");
+  EXPECT_EQ(DumpInstructions(b.functions()[0].instructions()),
+            R"(OpStore %3 %2
+%6 = OpLoad %1 %3
+)");
 }
 
 TEST_F(BuilderTest, FunctionVar_Const) {