From 00000b57e8a50c6ee128fbfde618dab3604e19b7 Mon Sep 17 00:00:00 2001 From: dan sinclair Date: Wed, 30 Sep 2020 18:44:40 +0000 Subject: [PATCH] [spirv-writer] Emit spec constants for non-constructor scalars. In WGSL you can provide a constant_id variable without a constructor. In SPIR-V we must synthesize a constant to attach the SpecId too. This CL adds that variable creation. Bug: tint:254 Change-Id: I2f25fdc3cb7e2c9c0f9e2129885865bd24298416 Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/29200 Commit-Queue: Sarah Mashayekhi Reviewed-by: Sarah Mashayekhi --- src/writer/spirv/builder.cc | 39 ++++++-- .../spirv/builder_global_variable_test.cc | 98 +++++++++++++++++++ 2 files changed, 131 insertions(+), 6 deletions(-) diff --git a/src/writer/spirv/builder.cc b/src/writer/spirv/builder.cc index c84338642b..2b8ee2c4f8 100644 --- a/src/writer/spirv/builder.cc +++ b/src/writer/spirv/builder.cc @@ -683,12 +683,39 @@ bool Builder::GenerateGlobalVariable(ast::Variable* var) { if (var->has_constructor()) { ops.push_back(Operand::Int(init_id)); } else if (!var->type()->IsTexture() && !var->type()->IsSampler()) { - // If we don't have a constructor and we're an Output or Private variable - // then WGSL requires an initializer. - if (var->storage_class() == ast::StorageClass::kPrivate || - var->storage_class() == ast::StorageClass::kNone || - var->storage_class() == ast::StorageClass::kOutput) { - ast::NullLiteral nl(var->type()->UnwrapPtrIfNeeded()); + // Certain cases require us to generate a constructor value. + // + // 1- ConstantId's must be attached to the OpConstant, if we have a + // variable with a constant_id that doesn't have a constructor we make + // one + // 2- If we don't have a constructor and we're an Output or Private variable + // then WGSL requires an initializer. + auto* type = var->type()->UnwrapPtrIfNeeded(); + if (var->IsDecorated() && var->AsDecorated()->HasConstantIdDecoration()) { + if (type->IsF32()) { + ast::FloatLiteral l(type, 0.0f); + init_id = GenerateLiteralIfNeeded(var, &l); + } else if (type->IsU32()) { + ast::UintLiteral l(type, 0); + init_id = GenerateLiteralIfNeeded(var, &l); + } else if (type->IsI32()) { + ast::SintLiteral l(type, 0); + init_id = GenerateLiteralIfNeeded(var, &l); + } else if (type->IsBool()) { + ast::BoolLiteral l(type, false); + init_id = GenerateLiteralIfNeeded(var, &l); + } else { + error_ = "invalid type for constant_id, must be scalar"; + return false; + } + if (init_id == 0) { + return 0; + } + ops.push_back(Operand::Int(init_id)); + } else if (var->storage_class() == ast::StorageClass::kPrivate || + var->storage_class() == ast::StorageClass::kNone || + var->storage_class() == ast::StorageClass::kOutput) { + ast::NullLiteral nl(type); init_id = GenerateLiteralIfNeeded(var, &nl); if (init_id == 0) { return 0; diff --git a/src/writer/spirv/builder_global_variable_test.cc b/src/writer/spirv/builder_global_variable_test.cc index 73ae3035a0..a6ebce8569 100644 --- a/src/writer/spirv/builder_global_variable_test.cc +++ b/src/writer/spirv/builder_global_variable_test.cc @@ -29,6 +29,8 @@ #include "src/ast/storage_class.h" #include "src/ast/type/bool_type.h" #include "src/ast/type/f32_type.h" +#include "src/ast/type/i32_type.h" +#include "src/ast/type/u32_type.h" #include "src/ast/type/vector_type.h" #include "src/ast/type_constructor_expression.h" #include "src/ast/variable.h" @@ -389,6 +391,30 @@ TEST_F(BuilderTest, GlobalVar_ConstantId_Bool) { )"); } +TEST_F(BuilderTest, GlobalVar_ConstantId_Bool_NoConstructor) { + ast::type::BoolType bool_type; + + ast::VariableDecorationList decos; + decos.push_back(std::make_unique(1200)); + + ast::DecoratedVariable v(std::make_unique( + "var", ast::StorageClass::kNone, &bool_type)); + v.set_decorations(std::move(decos)); + + ast::Module mod; + Builder b(&mod); + EXPECT_TRUE(b.GenerateGlobalVariable(&v)) << b.error(); + EXPECT_EQ(DumpInstructions(b.debug()), R"(OpName %1 "var" +)"); + EXPECT_EQ(DumpInstructions(b.annots()), R"(OpDecorate %4 SpecId 1200 +)"); + EXPECT_EQ(DumpInstructions(b.types()), R"(%3 = OpTypeBool +%2 = OpTypePointer Private %3 +%4 = OpSpecConstantFalse %3 +%1 = OpVariable %2 Private %4 +)"); +} + TEST_F(BuilderTest, GlobalVar_ConstantId_Scalar) { ast::type::F32Type f32; @@ -415,6 +441,78 @@ TEST_F(BuilderTest, GlobalVar_ConstantId_Scalar) { )"); } +TEST_F(BuilderTest, GlobalVar_ConstantId_Scalar_F32_NoConstructor) { + ast::type::F32Type f32; + + ast::VariableDecorationList decos; + decos.push_back(std::make_unique(0)); + + ast::DecoratedVariable v( + std::make_unique("var", ast::StorageClass::kNone, &f32)); + v.set_decorations(std::move(decos)); + + ast::Module mod; + Builder b(&mod); + EXPECT_TRUE(b.GenerateGlobalVariable(&v)) << b.error(); + EXPECT_EQ(DumpInstructions(b.debug()), R"(OpName %1 "var" +)"); + EXPECT_EQ(DumpInstructions(b.annots()), R"(OpDecorate %4 SpecId 0 +)"); + EXPECT_EQ(DumpInstructions(b.types()), R"(%3 = OpTypeFloat 32 +%2 = OpTypePointer Private %3 +%4 = OpSpecConstant %3 0 +%1 = OpVariable %2 Private %4 +)"); +} + +TEST_F(BuilderTest, GlobalVar_ConstantId_Scalar_I32_NoConstructor) { + ast::type::I32Type i32; + + ast::VariableDecorationList decos; + decos.push_back(std::make_unique(0)); + + ast::DecoratedVariable v( + std::make_unique("var", ast::StorageClass::kNone, &i32)); + v.set_decorations(std::move(decos)); + + ast::Module mod; + Builder b(&mod); + EXPECT_TRUE(b.GenerateGlobalVariable(&v)) << b.error(); + EXPECT_EQ(DumpInstructions(b.debug()), R"(OpName %1 "var" +)"); + EXPECT_EQ(DumpInstructions(b.annots()), R"(OpDecorate %4 SpecId 0 +)"); + EXPECT_EQ(DumpInstructions(b.types()), R"(%3 = OpTypeInt 32 1 +%2 = OpTypePointer Private %3 +%4 = OpSpecConstant %3 0 +%1 = OpVariable %2 Private %4 +)"); +} + +TEST_F(BuilderTest, GlobalVar_ConstantId_Scalar_U32_NoConstructor) { + ast::type::U32Type u32; + + ast::VariableDecorationList decos; + decos.push_back(std::make_unique(0)); + + ast::DecoratedVariable v( + std::make_unique("var", ast::StorageClass::kNone, &u32)); + v.set_decorations(std::move(decos)); + + ast::Module mod; + Builder b(&mod); + EXPECT_TRUE(b.GenerateGlobalVariable(&v)) << b.error(); + EXPECT_EQ(DumpInstructions(b.debug()), R"(OpName %1 "var" +)"); + EXPECT_EQ(DumpInstructions(b.annots()), R"(OpDecorate %4 SpecId 0 +)"); + EXPECT_EQ(DumpInstructions(b.types()), R"(%3 = OpTypeInt 32 0 +%2 = OpTypePointer Private %3 +%4 = OpSpecConstant %3 0 +%1 = OpVariable %2 Private %4 +)"); +} + struct BuiltinData { ast::Builtin builtin; SpvBuiltIn result;