writer/spirv: Fix constant initialization

The code to auto-generate initializers for overridable pipeline
constants was in the non-const code path, so move it to the right
place and fix the tests that were wrongly testing non-const variables.

Bug: tint:254
Change-Id: Ifc96681492768ecf844f67e114377cc643ef7609
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/50040
Auto-Submit: James Price <jrprice@google.com>
Commit-Queue: Ben Clayton <bclayton@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
This commit is contained in:
James Price 2021-05-06 09:30:42 +00:00 committed by Commit Bot service account
parent 0aeb77c40f
commit a67f8a44bb
2 changed files with 79 additions and 93 deletions

View File

@ -688,8 +688,33 @@ bool Builder::GenerateGlobalVariable(ast::Variable* var) {
if (var->is_const()) { if (var->is_const()) {
if (!var->has_constructor()) { if (!var->has_constructor()) {
error_ = "missing constructor for constant"; // Constants must have an initializer unless they have an override
return false; // decoration.
if (!ast::HasDecoration<ast::OverrideDecoration>(var->decorations())) {
error_ = "missing constructor for constant";
return false;
}
// SPIR-V requires specialization constants to have initializers.
if (sem->Type()->Is<sem::F32>()) {
ast::FloatLiteral l(ProgramID(), Source{}, 0.0f);
init_id = GenerateLiteralIfNeeded(var, &l);
} else if (sem->Type()->Is<sem::U32>()) {
ast::UintLiteral l(ProgramID(), Source{}, 0);
init_id = GenerateLiteralIfNeeded(var, &l);
} else if (sem->Type()->Is<sem::I32>()) {
ast::SintLiteral l(ProgramID(), Source{}, 0);
init_id = GenerateLiteralIfNeeded(var, &l);
} else if (sem->Type()->Is<sem::Bool>()) {
ast::BoolLiteral l(ProgramID(), Source{}, false);
init_id = GenerateLiteralIfNeeded(var, &l);
} else {
error_ = "invalid type for pipeline constant ID, must be scalar";
return false;
}
if (init_id == 0) {
return 0;
}
} }
push_debug(spv::Op::OpName, push_debug(spv::Op::OpName,
{Operand::Int(init_id), {Operand::Int(init_id),
@ -743,37 +768,10 @@ bool Builder::GenerateGlobalVariable(ast::Variable* var) {
} }
} }
} else if (!type_no_ac->Is<sem::Sampler>()) { } else if (!type_no_ac->Is<sem::Sampler>()) {
// Certain cases require us to generate a constructor value. // If we don't have a constructor and we're an Output or Private variable,
// // then WGSL requires that we zero-initialize.
// 1- Pipeline constant IDs must be attached to the OpConstant, if we have a if (sem->StorageClass() == ast::StorageClass::kPrivate ||
// variable with an override attribute that doesn't have a constructor we sem->StorageClass() == ast::StorageClass::kOutput) {
// make one
// 2- If we don't have a constructor and we're an Output or Private variable
// then WGSL requires an initializer.
if (ast::HasDecoration<ast::OverrideDecoration>(var->decorations())) {
if (type_no_ac->Is<sem::F32>()) {
ast::FloatLiteral l(ProgramID(), Source{}, 0.0f);
init_id = GenerateLiteralIfNeeded(var, &l);
} else if (type_no_ac->Is<sem::U32>()) {
ast::UintLiteral l(ProgramID(), Source{}, 0);
init_id = GenerateLiteralIfNeeded(var, &l);
} else if (type_no_ac->Is<sem::I32>()) {
ast::SintLiteral l(ProgramID(), Source{}, 0);
init_id = GenerateLiteralIfNeeded(var, &l);
} else if (type_no_ac->Is<sem::Bool>()) {
ast::BoolLiteral l(ProgramID(), Source{}, false);
init_id = GenerateLiteralIfNeeded(var, &l);
} else {
error_ = "invalid type for pipeline constant ID, must be scalar";
return false;
}
if (init_id == 0) {
return 0;
}
ops.push_back(Operand::Int(init_id));
} else if (sem->StorageClass() == ast::StorageClass::kPrivate ||
sem->StorageClass() == ast::StorageClass::kNone ||
sem->StorageClass() == ast::StorageClass::kOutput) {
init_id = GenerateConstantNullIfNeeded(type_no_ac); init_id = GenerateConstantNullIfNeeded(type_no_ac);
if (init_id == 0) { if (init_id == 0) {
return 0; return 0;

View File

@ -203,123 +203,111 @@ TEST_F(BuilderTest, GlobalVar_WithBuiltin) {
)"); )");
} }
TEST_F(BuilderTest, GlobalVar_ConstantId_Bool) { TEST_F(BuilderTest, GlobalVar_Override_Bool) {
auto* v = Global("var", ty.bool_(), ast::StorageClass::kInput, Expr(true), auto* v = GlobalConst("var", ty.bool_(), Expr(true),
ast::DecorationList{ ast::DecorationList{
create<ast::OverrideDecoration>(1200), create<ast::OverrideDecoration>(1200),
}); });
spirv::Builder& b = Build(); spirv::Builder& b = Build();
EXPECT_TRUE(b.GenerateGlobalVariable(v)) << b.error(); EXPECT_TRUE(b.GenerateGlobalVariable(v)) << b.error();
EXPECT_EQ(DumpInstructions(b.debug()), R"(OpName %3 "var" EXPECT_EQ(DumpInstructions(b.debug()), R"(OpName %2 "var"
)"); )");
EXPECT_EQ(DumpInstructions(b.annots()), R"(OpDecorate %2 SpecId 1200 EXPECT_EQ(DumpInstructions(b.annots()), R"(OpDecorate %2 SpecId 1200
)"); )");
EXPECT_EQ(DumpInstructions(b.types()), R"(%1 = OpTypeBool EXPECT_EQ(DumpInstructions(b.types()), R"(%1 = OpTypeBool
%2 = OpSpecConstantTrue %1 %2 = OpSpecConstantTrue %1
%4 = OpTypePointer Input %1
%3 = OpVariable %4 Input %2
)"); )");
} }
TEST_F(BuilderTest, GlobalVar_ConstantId_Bool_NoConstructor) { TEST_F(BuilderTest, GlobalVar_Override_Bool_NoConstructor) {
auto* v = Global("var", ty.bool_(), ast::StorageClass::kInput, nullptr, auto* v = GlobalConst("var", ty.bool_(), nullptr,
ast::DecorationList{ ast::DecorationList{
create<ast::OverrideDecoration>(1200), create<ast::OverrideDecoration>(1200),
}); });
spirv::Builder& b = Build(); spirv::Builder& b = Build();
EXPECT_TRUE(b.GenerateGlobalVariable(v)) << b.error(); EXPECT_TRUE(b.GenerateGlobalVariable(v)) << b.error();
EXPECT_EQ(DumpInstructions(b.debug()), R"(OpName %1 "var" EXPECT_EQ(DumpInstructions(b.debug()), R"(OpName %2 "var"
)"); )");
EXPECT_EQ(DumpInstructions(b.annots()), R"(OpDecorate %4 SpecId 1200 EXPECT_EQ(DumpInstructions(b.annots()), R"(OpDecorate %2 SpecId 1200
)"); )");
EXPECT_EQ(DumpInstructions(b.types()), R"(%3 = OpTypeBool EXPECT_EQ(DumpInstructions(b.types()), R"(%1 = OpTypeBool
%2 = OpTypePointer Input %3 %2 = OpSpecConstantFalse %1
%4 = OpSpecConstantFalse %3
%1 = OpVariable %2 Input %4
)"); )");
} }
TEST_F(BuilderTest, GlobalVar_ConstantId_Scalar) { TEST_F(BuilderTest, GlobalVar_Override_Scalar) {
auto* v = Global("var", ty.f32(), ast::StorageClass::kInput, Expr(2.f), auto* v = GlobalConst("var", ty.f32(), Expr(2.f),
ast::DecorationList{ ast::DecorationList{
create<ast::OverrideDecoration>(0), create<ast::OverrideDecoration>(0),
}); });
spirv::Builder& b = Build(); spirv::Builder& b = Build();
EXPECT_TRUE(b.GenerateGlobalVariable(v)) << b.error(); EXPECT_TRUE(b.GenerateGlobalVariable(v)) << b.error();
EXPECT_EQ(DumpInstructions(b.debug()), R"(OpName %3 "var" EXPECT_EQ(DumpInstructions(b.debug()), R"(OpName %2 "var"
)"); )");
EXPECT_EQ(DumpInstructions(b.annots()), R"(OpDecorate %2 SpecId 0 EXPECT_EQ(DumpInstructions(b.annots()), R"(OpDecorate %2 SpecId 0
)"); )");
EXPECT_EQ(DumpInstructions(b.types()), R"(%1 = OpTypeFloat 32 EXPECT_EQ(DumpInstructions(b.types()), R"(%1 = OpTypeFloat 32
%2 = OpSpecConstant %1 2 %2 = OpSpecConstant %1 2
%4 = OpTypePointer Input %1
%3 = OpVariable %4 Input %2
)"); )");
} }
TEST_F(BuilderTest, GlobalVar_ConstantId_Scalar_F32_NoConstructor) { TEST_F(BuilderTest, GlobalVar_Override_Scalar_F32_NoConstructor) {
auto* v = Global("var", ty.f32(), ast::StorageClass::kInput, nullptr, auto* v = GlobalConst("var", ty.f32(), nullptr,
ast::DecorationList{ ast::DecorationList{
create<ast::OverrideDecoration>(0), create<ast::OverrideDecoration>(0),
}); });
spirv::Builder& b = Build(); spirv::Builder& b = Build();
EXPECT_TRUE(b.GenerateGlobalVariable(v)) << b.error(); EXPECT_TRUE(b.GenerateGlobalVariable(v)) << b.error();
EXPECT_EQ(DumpInstructions(b.debug()), R"(OpName %1 "var" EXPECT_EQ(DumpInstructions(b.debug()), R"(OpName %2 "var"
)"); )");
EXPECT_EQ(DumpInstructions(b.annots()), R"(OpDecorate %4 SpecId 0 EXPECT_EQ(DumpInstructions(b.annots()), R"(OpDecorate %2 SpecId 0
)"); )");
EXPECT_EQ(DumpInstructions(b.types()), R"(%3 = OpTypeFloat 32 EXPECT_EQ(DumpInstructions(b.types()), R"(%1 = OpTypeFloat 32
%2 = OpTypePointer Input %3 %2 = OpSpecConstant %1 0
%4 = OpSpecConstant %3 0
%1 = OpVariable %2 Input %4
)"); )");
} }
TEST_F(BuilderTest, GlobalVar_ConstantId_Scalar_I32_NoConstructor) { TEST_F(BuilderTest, GlobalVar_Override_Scalar_I32_NoConstructor) {
auto* v = Global("var", ty.i32(), ast::StorageClass::kInput, nullptr, auto* v = GlobalConst("var", ty.i32(), nullptr,
ast::DecorationList{ ast::DecorationList{
create<ast::OverrideDecoration>(0), create<ast::OverrideDecoration>(0),
}); });
spirv::Builder& b = Build(); spirv::Builder& b = Build();
EXPECT_TRUE(b.GenerateGlobalVariable(v)) << b.error(); EXPECT_TRUE(b.GenerateGlobalVariable(v)) << b.error();
EXPECT_EQ(DumpInstructions(b.debug()), R"(OpName %1 "var" EXPECT_EQ(DumpInstructions(b.debug()), R"(OpName %2 "var"
)"); )");
EXPECT_EQ(DumpInstructions(b.annots()), R"(OpDecorate %4 SpecId 0 EXPECT_EQ(DumpInstructions(b.annots()), R"(OpDecorate %2 SpecId 0
)"); )");
EXPECT_EQ(DumpInstructions(b.types()), R"(%3 = OpTypeInt 32 1 EXPECT_EQ(DumpInstructions(b.types()), R"(%1 = OpTypeInt 32 1
%2 = OpTypePointer Input %3 %2 = OpSpecConstant %1 0
%4 = OpSpecConstant %3 0
%1 = OpVariable %2 Input %4
)"); )");
} }
TEST_F(BuilderTest, GlobalVar_ConstantId_Scalar_U32_NoConstructor) { TEST_F(BuilderTest, GlobalVar_Override_Scalar_U32_NoConstructor) {
auto* v = Global("var", ty.u32(), ast::StorageClass::kInput, nullptr, auto* v = GlobalConst("var", ty.u32(), nullptr,
ast::DecorationList{ ast::DecorationList{
create<ast::OverrideDecoration>(0), create<ast::OverrideDecoration>(0),
}); });
spirv::Builder& b = Build(); spirv::Builder& b = Build();
EXPECT_TRUE(b.GenerateGlobalVariable(v)) << b.error(); EXPECT_TRUE(b.GenerateGlobalVariable(v)) << b.error();
EXPECT_EQ(DumpInstructions(b.debug()), R"(OpName %1 "var" EXPECT_EQ(DumpInstructions(b.debug()), R"(OpName %2 "var"
)"); )");
EXPECT_EQ(DumpInstructions(b.annots()), R"(OpDecorate %4 SpecId 0 EXPECT_EQ(DumpInstructions(b.annots()), R"(OpDecorate %2 SpecId 0
)"); )");
EXPECT_EQ(DumpInstructions(b.types()), R"(%3 = OpTypeInt 32 0 EXPECT_EQ(DumpInstructions(b.types()), R"(%1 = OpTypeInt 32 0
%2 = OpTypePointer Input %3 %2 = OpSpecConstant %1 0
%4 = OpSpecConstant %3 0
%1 = OpVariable %2 Input %4
)"); )");
} }