[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>
diff --git a/src/writer/spirv/builder.cc b/src/writer/spirv/builder.cc index 39ad3d3..7a55ac7 100644 --- a/src/writer/spirv/builder.cc +++ b/src/writer/spirv/builder.cc
@@ -607,6 +607,7 @@ if (init_id == 0) { return false; } + init_id = GenerateLoadIfNeeded(var->constructor()->result_type(), init_id); } if (var->is_const()) { @@ -644,7 +645,6 @@ 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 @@ return val; } - error_ = "unable to find name for identifier: " + expr->name(); + error_ = "unable to find variable with identifier: " + expr->name(); return 0; }
diff --git a/src/writer/spirv/builder_function_test.cc b/src/writer/spirv/builder_function_test.cc index 4855fef..62d3c92 100644 --- a/src/writer/spirv/builder_function_test.cc +++ b/src/writer/spirv/builder_function_test.cc
@@ -239,9 +239,9 @@ 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 @@ %9 = OpTypeInt 32 0 %10 = OpConstant %9 0 %11 = OpTypePointer StorageBuffer %4 -%14 = OpTypePointer Function %4 -%15 = OpConstantNull %4 +%15 = OpTypePointer Function %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 -OpStore %13 %16 +%13 = OpLoad %4 %12 +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 -OpStore %20 %21 +%20 = OpLoad %4 %19 +OpStore %21 %20 OpReturn OpFunctionEnd )");
diff --git a/src/writer/spirv/builder_function_variable_test.cc b/src/writer/spirv/builder_function_variable_test.cc index 7702627..af4f055 100644 --- a/src/writer/spirv/builder_function_variable_test.cc +++ b/src/writer/spirv/builder_function_variable_test.cc
@@ -154,13 +154,92 @@ )"); } -TEST_F(BuilderTest, - DISABLED_FunctionVar_WithNonConstantConstructorLoadedFromVar) { - // fn main() -> void { - // var v : f32 = 1.0; - // var v2 : f32 = v; // Should generate the load automatically. - // } - FAIL(); +TEST_F(BuilderTest, FunctionVar_WithNonConstantConstructorLoadedFromVar) { + // var v : f32 = 1.0; + // var v2 : f32 = v; // Should generate the load and store automatically. + + ast::type::F32Type f32; + + 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) {