From 05cc833ae89408f3e8f2ae147327a73c554fb800 Mon Sep 17 00:00:00 2001 From: David Neto Date: Tue, 15 Jun 2021 22:47:17 +0000 Subject: [PATCH] spirv-reader: pipeline IO: handle position builtin Bug: tint:508 Change-Id: I9a4dea29bddf0d511fb62f353dda24de72cf85f4 Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/54486 Commit-Queue: David Neto Auto-Submit: David Neto Kokoro: Kokoro Reviewed-by: James Price --- src/reader/spirv/function.cc | 43 ++++++++----- src/reader/spirv/parser_impl.cc | 14 +++-- .../spirv/parser_impl_module_var_test.cc | 62 +++++++++++++++++++ 3 files changed, 101 insertions(+), 18 deletions(-) diff --git a/src/reader/spirv/function.cc b/src/reader/spirv/function.cc index 2024a2fb26..a0de6f829a 100644 --- a/src/reader/spirv/function.cc +++ b/src/reader/spirv/function.cc @@ -20,6 +20,8 @@ #include "src/ast/assignment_statement.h" #include "src/ast/bitcast_expression.h" #include "src/ast/break_statement.h" +#include "src/ast/builtin.h" +#include "src/ast/builtin_decoration.h" #include "src/ast/call_statement.h" #include "src/ast/continue_statement.h" #include "src/ast/discard_statement.h" @@ -1026,24 +1028,37 @@ bool FunctionEmitter::EmitEntryPointAsWrapper() { const auto return_struct_sym = builder_.Symbols().Register(return_struct_name); + const auto& builtin_position_info = parser_impl_.GetBuiltInPositionInfo(); + // Define the structure. ast::ExpressionList return_exprs; std::vector return_members; for (uint32_t var_id : ep_info_->outputs) { - const auto* var = def_use_mgr_->GetDef(var_id); - TINT_ASSERT(var != nullptr); - TINT_ASSERT(var->opcode() == SpvOpVariable); - const auto* store_type = GetVariableStoreType(*var); - const auto* forced_store_type = store_type; + const Type* param_type = nullptr; + const Type* store_type = nullptr; ast::DecorationList out_decos; - if (!parser_impl_.ConvertDecorationsForVariable( - var_id, &forced_store_type, &out_decos)) { - // This occurs, and is not an error, for the PointSize builtin. - if (!success()) { - // But exit early if an error was logged. - return false; + if (var_id == builtin_position_info.per_vertex_var_id) { + // The SPIR-V gl_PerVertex variable has already been remapped to + // a gl_Position variable. + // Substitute the type. + param_type = ty_.Vector(ty_.F32(), 4); + out_decos.emplace_back( + create(source, ast::Builtin::kPosition)); + } else { + const auto* var = def_use_mgr_->GetDef(var_id); + TINT_ASSERT(var != nullptr); + TINT_ASSERT(var->opcode() == SpvOpVariable); + store_type = GetVariableStoreType(*var); + param_type = store_type; + if (!parser_impl_.ConvertDecorationsForVariable(var_id, ¶m_type, + &out_decos)) { + // This occurs, and is not an error, for the PointSize builtin. + if (!success()) { + // But exit early if an error was logged. + return false; + } + continue; } - continue; } // TODO(dneto): flatten structs and arrays to vectors or scalars. @@ -1057,7 +1072,7 @@ bool FunctionEmitter::EmitEntryPointAsWrapper() { // Form the member type. // Reuse the var name for the member name. They can't clash. ast::StructMember* return_member = create( - Source{}, var_sym, forced_store_type->Build(builder_), out_decos); + Source{}, var_sym, param_type->Build(builder_), out_decos); return_members.push_back(return_member); ast::Expression* return_member_value = @@ -1070,7 +1085,7 @@ bool FunctionEmitter::EmitEntryPointAsWrapper() { if (store_type->As()->type->IsSignedScalarOrVector()) { // sample_mask is unsigned in WGSL. Bitcast it. return_member_value = create( - source, forced_store_type->Build(builder_), return_member_value); + source, param_type->Build(builder_), return_member_value); } } else { // No other builtin outputs need signedness conversion. diff --git a/src/reader/spirv/parser_impl.cc b/src/reader/spirv/parser_impl.cc index 01df33cd53..04e2e5d363 100644 --- a/src/reader/spirv/parser_impl.cc +++ b/src/reader/spirv/parser_impl.cc @@ -1053,7 +1053,8 @@ const Type* ParserImpl::ConvertType(uint32_t type_id, if (pointee_type_id == builtin_position_.struct_type_id) { builtin_position_.pointer_type_id = type_id; - builtin_position_.storage_class = storage_class; + builtin_position_.storage_class = + hlsl_style_pipeline_io_ ? SpvStorageClassPrivate : storage_class; return nullptr; } auto* ast_elem_ty = ConvertType(pointee_type_id, PtrAs::Ptr); @@ -1314,13 +1315,18 @@ bool ParserImpl::EmitModuleScopeVariables() { // Make sure the variable has a name. namer_.SuggestSanitizedName(builtin_position_.per_vertex_var_id, "gl_Position"); + ast::DecorationList decos; + if (!hlsl_style_pipeline_io_) { + // When doing HLSL-style pipeline IO, the decoration goes on the + // parameter, not the variable. + decos.push_back( + create(Source{}, ast::Builtin::kPosition)); + } auto* var = MakeVariable( builtin_position_.per_vertex_var_id, enum_converter_.ToStorageClass(builtin_position_.storage_class), ConvertType(builtin_position_.position_member_type_id), false, nullptr, - ast::DecorationList{ - create(Source{}, ast::Builtin::kPosition), - }); + std::move(decos)); builder_.AST().AddGlobalVariable(var); } diff --git a/src/reader/spirv/parser_impl_module_var_test.cc b/src/reader/spirv/parser_impl_module_var_test.cc index 729d3293f6..11f2328619 100644 --- a/src/reader/spirv/parser_impl_module_var_test.cc +++ b/src/reader/spirv/parser_impl_module_var_test.cc @@ -4841,6 +4841,68 @@ TEST_F(SpvModuleScopeVarParserTest, EXPECT_EQ(got, expected) << got; } +TEST_F(SpvModuleScopeVarParserTest, + BuiltinPosition_BuiltIn_Position_MapsToVec4) { + // In Vulkan SPIR-V, Position is the first member of gl_PerVertex + const std::string assembly = PerVertexPreamble() + R"( + %main = OpFunction %void None %voidfn + %entry = OpLabel + OpReturn + OpFunctionEnd +)"; + auto p = parser(test::Assemble(assembly)); + + // TODO(crbug.com/tint/508): Remove this when everything is converted + // to HLSL style pipeline IO. + p->SetHLSLStylePipelineIO(); + + ASSERT_TRUE(p->Parse()) << p->error() << assembly; + EXPECT_TRUE(p->error().empty()); + + const auto got = p->program().to_str(); + const std::string expected = R"(Module{ + Struct main_out { + StructMember{[[ BuiltinDecoration{position} + ]] gl_Position: __vec_4__f32} + } + Variable{ + gl_Position + private + undefined + __vec_4__f32 + } + Function main_1 -> __void + () + { + Return{} + } + Function main -> __type_name_main_out + StageDecoration{vertex} + () + { + Call[not set]{ + Identifier[not set]{main_1} + ( + ) + } + Return{ + { + TypeConstructor[not set]{ + __type_name_main_out + Identifier[not set]{gl_Position} + } + } + } + } +} +)"; + EXPECT_EQ(got, expected) << got; +} + +// TODO(dneto): pipeline IO: gl_Position, with initializer +// TODO(dneto): pipeline IO: read PointSize +// TODO(dneto): pipeline IO: write PointSize + // TODO(dneto): pipeline IO: flatten structures, and distribute locations } // namespace