From e5d288be5efef2d5515f3f3950899688426695af Mon Sep 17 00:00:00 2001 From: David Neto Date: Wed, 16 Dec 2020 20:50:10 +0000 Subject: [PATCH] spirv-reader: ignore storing 1.0 to PointSize builtin If you try to load it, return 1.0f instead. Some cases of copy-object of intermediates are unhandled, and will error out. This is being done as an aid to porting GLSL Vulkan shaders that do store 1 to gl_PointSize. Fixed: tint:412 Change-Id: Ia33dc70bca630dccfbf11644f71d6be4b3f43f1a Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/35861 Commit-Queue: dan sinclair Reviewed-by: dan sinclair Auto-Submit: David Neto --- src/reader/spirv/function.cc | 104 +++++++- src/reader/spirv/function.h | 33 ++- src/reader/spirv/parser_impl.cc | 40 +-- src/reader/spirv/parser_impl.h | 8 +- .../spirv/parser_impl_module_var_test.cc | 236 ++++++++++++++++-- 5 files changed, 370 insertions(+), 51 deletions(-) diff --git a/src/reader/spirv/function.cc b/src/reader/spirv/function.cc index 0e7db13f31..83ac79738a 100644 --- a/src/reader/spirv/function.cc +++ b/src/reader/spirv/function.cc @@ -41,6 +41,7 @@ #include "src/ast/discard_statement.h" #include "src/ast/else_statement.h" #include "src/ast/fallthrough_statement.h" +#include "src/ast/float_literal.h" #include "src/ast/identifier_expression.h" #include "src/ast/if_statement.h" #include "src/ast/intrinsic.h" @@ -1971,6 +1972,24 @@ TypedExpression FunctionEmitter::MakeExpression(uint32_t id) { if (failed()) { return {}; } + switch (GetSkipReason(id)) { + case SkipReason::kDontSkip: + break; + case SkipReason::kOpaqueObject: + Fail() << "internal error: unhandled use of opaque object with ID: " + << id; + return {}; + case SkipReason::kPointSizeBuiltinValue: { + auto* f32 = create(); + return {f32, + create( + Source{}, create(Source{}, f32, 1.0f))}; + } + case SkipReason::kPointSizeBuiltinPointer: + Fail() << "unhandled use of a pointer to the PointSize builtin, with ID: " + << id; + return {}; + } if (identifier_values_.count(id) || parser_impl_.IsScalarSpecConstant(id)) { auto name = namer_.Name(id); return TypedExpression{ @@ -2848,21 +2867,31 @@ bool FunctionEmitter::EmitStatement(const spvtools::opt::Instruction& inst) { if (type_id != 0) { const auto& builtin_position_info = parser_impl_.GetBuiltInPositionInfo(); - if ((type_id == builtin_position_info.struct_type_id) || - (type_id == builtin_position_info.pointer_type_id)) { + if (type_id == builtin_position_info.struct_type_id) { return Fail() << "operations producing a per-vertex structure are not " "supported: " << inst.PrettyPrint(); } + if (type_id == builtin_position_info.pointer_type_id) { + return Fail() << "operations producing a pointer to a per-vertex " + "structure are not " + "supported: " + << inst.PrettyPrint(); + } } // Handle combinatorial instructions. const auto* def_info = GetDefInfo(result_id); if (def_info) { + TypedExpression combinatorial_expr; + if (def_info->skip == SkipReason::kDontSkip) { + combinatorial_expr = MaybeEmitCombinatorialValue(inst); + } + // An access chain or OpCopyObject can generate a skip. if (def_info->skip != SkipReason::kDontSkip) { return true; } - auto combinatorial_expr = MaybeEmitCombinatorialValue(inst); + if (combinatorial_expr.expr != nullptr) { if (def_info->requires_hoisted_def || def_info->requires_named_const_def || def_info->num_uses != 1) { @@ -2892,6 +2921,23 @@ bool FunctionEmitter::EmitStatement(const spvtools::opt::Instruction& inst) { case SpvOpStore: { const auto ptr_id = inst.GetSingleWordInOperand(0); const auto value_id = inst.GetSingleWordInOperand(1); + + // Handle exceptional cases + if (GetSkipReason(ptr_id) == SkipReason::kPointSizeBuiltinPointer) { + if (const auto* c = constant_mgr_->FindDeclaredConstant(value_id)) { + // If we're writing a constant 1.0, then skip the write. That's all + // that WebGPU handles. + auto* ct = c->type(); + if (ct->AsFloat() && (ct->AsFloat()->width() == 32) && + (c->GetFloat() == 1.0f)) { + // Don't store to PointSize + return true; + } + } + return Fail() << "cannot store a value other than constant 1.0 to " + "PointSize builtin: " + << inst.PrettyPrint(); + } const auto ptr_type_id = def_use_mgr_->GetDef(ptr_id)->type_id(); const auto& builtin_position_info = parser_impl_.GetBuiltInPositionInfo(); if (ptr_type_id == builtin_position_info.pointer_type_id) { @@ -2900,6 +2946,7 @@ bool FunctionEmitter::EmitStatement(const spvtools::opt::Instruction& inst) { << inst.PrettyPrint(); } + // Handle an ordinary store as an assignment. // TODO(dneto): Order of evaluation? auto lhs = MakeExpression(ptr_id); auto rhs = MakeExpression(value_id); @@ -2907,23 +2954,42 @@ bool FunctionEmitter::EmitStatement(const spvtools::opt::Instruction& inst) { create(Source{}, lhs.expr, rhs.expr)); return success(); } + case SpvOpLoad: { // Memory accesses must be issued in SPIR-V program order. // So represent a load by a new const definition. - auto expr = MakeExpression(inst.GetSingleWordInOperand(0)); + const auto ptr_id = inst.GetSingleWordInOperand(0); + if (GetSkipReason(ptr_id) == SkipReason::kPointSizeBuiltinPointer) { + GetDefInfo(inst.result_id())->skip = SkipReason::kPointSizeBuiltinValue; + return true; + } + auto expr = MakeExpression(ptr_id); // The load result type is the pointee type of its operand. assert(expr.type->Is()); expr.type = expr.type->As()->type(); return EmitConstDefOrWriteToHoistedVar(inst, expr); } + case SpvOpCopyObject: { // Arguably, OpCopyObject is purely combinatorial. On the other hand, // it exists to make a new name for something. So we choose to make // a new named constant definition. - auto expr = MakeExpression(inst.GetSingleWordInOperand(0)); + auto value_id = inst.GetSingleWordInOperand(0); + const auto skip = GetSkipReason(value_id); + switch (skip) { + case SkipReason::kDontSkip: + break; + case SkipReason::kOpaqueObject: + case SkipReason::kPointSizeBuiltinPointer: + case SkipReason::kPointSizeBuiltinValue: + GetDefInfo(inst.result_id())->skip = skip; + return true; + } + auto expr = MakeExpression(value_id); expr.type = RemapStorageClass(expr.type, result_id); return EmitConstDefOrWriteToHoistedVar(inst, expr); } + case SpvOpPhi: { // Emit a read from the associated state variable. TypedExpression expr{ @@ -2933,8 +2999,10 @@ bool FunctionEmitter::EmitStatement(const spvtools::opt::Instruction& inst) { def_info->phi_var)}; return EmitConstDefOrWriteToHoistedVar(inst, expr); } + case SpvOpFunctionCall: return EmitFunctionCall(inst); + default: break; } @@ -3159,6 +3227,12 @@ TypedExpression FunctionEmitter::MakeAccessChain( // If the variable was originally gl_PerVertex, then in the AST we // have instead emitted a gl_Position variable. + // If computing the pointer to the Position builtin, then emit the + // pointer to the generated gl_Position variable. + // If computing the pointer to the PointSize builtin, then mark the + // result as skippable due to being the point-size pointer. + // If computing the pointer to the ClipDistance or CullDistance builtins, + // then error out. { const auto& builtin_position_info = parser_impl_.GetBuiltInPositionInfo(); if (base_id == builtin_position_info.per_vertex_var_id) { @@ -3188,16 +3262,26 @@ TypedExpression FunctionEmitter::MakeAccessChain( } const auto member_index_value = member_index_const_int->GetZeroExtendedValue(); - if (member_index_value != builtin_position_info.member_index) { - Fail() << "accessing per-vertex member " << member_index_value - << " is not supported. Only Position is supported"; - return {}; + if (member_index_value != builtin_position_info.position_member_index) { + if (member_index_value == + builtin_position_info.pointsize_member_index) { + if (auto* def_info = GetDefInfo(inst.result_id())) { + def_info->skip = SkipReason::kPointSizeBuiltinPointer; + return {}; + } + } else { + // TODO(dneto): Handle ClipDistance and CullDistance + Fail() << "accessing per-vertex member " << member_index_value + << " is not supported. Only Position is supported, and " + "PointSize is ignored"; + return {}; + } } // Skip past the member index that gets us to Position. first_index = first_index + 1; // Replace the gl_PerVertex reference with the gl_Position reference - ptr_ty_id = builtin_position_info.member_pointer_type_id; + ptr_ty_id = builtin_position_info.position_member_pointer_type_id; auto name = namer_.Name(base_id); current_expr.expr = create( diff --git a/src/reader/spirv/function.h b/src/reader/spirv/function.h index 29f407618b..c82fa91e9e 100644 --- a/src/reader/spirv/function.h +++ b/src/reader/spirv/function.h @@ -213,11 +213,14 @@ enum class SkipReason { /// function parameter). kOpaqueObject, - /// `kPointSizeBuiltin`: the value is a pointer to the Position builtin - /// variable. Don't generate its address. Avoid generating stores to - /// this pointer. When loading from the pointer, yield the value 1, - /// the only supported value for PointSize. - kPointSizeBuiltin + /// `kPointSizeBuiltinPointer`: the value is a pointer to the Position builtin + /// variable. Don't generate its address. Avoid generating stores to this + /// pointer. + kPointSizeBuiltinPointer, + /// `kPointSizeBuiltinValue`: the value is the value loaded from the + /// PointSize builtin. Use 1.0f instead, because that's the only value + /// supported by WebGPU. + kPointSizeBuiltinValue, }; /// Bookkeeping info for a SPIR-V ID defined in the function. @@ -287,6 +290,10 @@ struct DefInfo { /// This is kNone for non-pointers. ast::StorageClass storage_class = ast::StorageClass::kNone; + /// The reason, if any, that this value should not be generated. + /// Normally all values are generated. This field can be updated while + /// generating code because sometimes we only discover necessary facts + /// in the middle of generating code. SkipReason skip = SkipReason::kDontSkip; }; @@ -307,8 +314,11 @@ inline std::ostream& operator<<(std::ostream& o, const DefInfo& di) { case SkipReason::kOpaqueObject: o << " skip:opaque"; break; - case SkipReason::kPointSizeBuiltin: - o << " skip:pointsize"; + case SkipReason::kPointSizeBuiltinPointer: + o << " skip:pointsize_pointer"; + break; + case SkipReason::kPointSizeBuiltinValue: + o << " skip:pointsize_value"; break; } o << "}"; @@ -696,6 +706,15 @@ class FunctionEmitter { } return where->second.get(); } + /// Returns the skip reason for a result ID. + /// @param id SPIR-V result ID + /// @returns the skip reason for the given ID, or SkipReason::kDontSkip + SkipReason GetSkipReason(uint32_t id) const { + if (auto* def_info = GetDefInfo(id)) { + return def_info->skip; + } + return SkipReason::kDontSkip; + } /// Returns the most deeply nested structured construct which encloses the /// WGSL scopes of names declared in both block positions. Each position must diff --git a/src/reader/spirv/parser_impl.cc b/src/reader/spirv/parser_impl.cc index b9ee3c6a54..4ebc6423c7 100644 --- a/src/reader/spirv/parser_impl.cc +++ b/src/reader/spirv/parser_impl.cc @@ -886,6 +886,7 @@ ast::type::Type* ParserImpl::ConvertType( ast::StructMemberList ast_members; const auto members = struct_ty->element_types(); unsigned num_non_writable_members = 0; + bool is_per_vertex_struct = false; for (uint32_t member_index = 0; member_index < members.size(); ++member_index) { const auto member_type_id = type_mgr_->GetId(members[member_index]); @@ -906,18 +907,24 @@ ast::type::Type* ParserImpl::ConvertType( case SpvBuiltInPosition: // Record this built-in variable specially. builtin_position_.struct_type_id = type_id; - builtin_position_.member_index = member_index; - builtin_position_.member_type_id = member_type_id; + builtin_position_.position_member_index = member_index; + builtin_position_.position_member_type_id = member_type_id; // Don't map the struct type. But this is not an error either. - return nullptr; - case SpvBuiltInPointSize: // not supported in WGSL - case SpvBuiltInCullDistance: // not supported in WGSL - case SpvBuiltInClipDistance: // not supported in WGSL - default: + is_per_vertex_struct = true; break; + case SpvBuiltInPointSize: // not supported in WGSL, but ignore + builtin_position_.pointsize_member_index = member_index; + is_per_vertex_struct = true; + break; + case SpvBuiltInClipDistance: // not supported in WGSL + case SpvBuiltInCullDistance: // not supported in WGSL + // Silently ignore, so we can detect Position and PointSize + is_per_vertex_struct = true; + break; + default: + Fail() << "unrecognized builtin " << decoration[1]; + return nullptr; } - Fail() << "unrecognized builtin " << decoration[1]; - return nullptr; } else if (decoration[0] == SpvDecorationNonWritable) { // WGSL doesn't represent individual members as non-writable. Instead, // apply the ReadOnly access control to the containing struct if all @@ -945,6 +952,10 @@ ast::type::Type* ParserImpl::ConvertType( ast_member_ty, std::move(ast_member_decorations)); ast_members.push_back(ast_struct_member); } + if (is_per_vertex_struct) { + // We're replacing it by the Position builtin alone. + return nullptr; + } // Now make the struct. auto* ast_struct = create(Source{}, std::move(ast_members), @@ -1010,10 +1021,11 @@ bool ParserImpl::RegisterTypes() { } // Manufacture a type for the gl_Position varible if we have to. if ((builtin_position_.struct_type_id != 0) && - (builtin_position_.member_pointer_type_id == 0)) { - builtin_position_.member_pointer_type_id = type_mgr_->FindPointerToType( - builtin_position_.member_type_id, builtin_position_.storage_class); - ConvertType(builtin_position_.member_pointer_type_id); + (builtin_position_.position_member_pointer_type_id == 0)) { + builtin_position_.position_member_pointer_type_id = + type_mgr_->FindPointerToType(builtin_position_.position_member_type_id, + builtin_position_.storage_class); + ConvertType(builtin_position_.position_member_pointer_type_id); } return success_; } @@ -1208,7 +1220,7 @@ bool ParserImpl::EmitModuleScopeVariables() { auto* var = MakeVariable( builtin_position_.per_vertex_var_id, enum_converter_.ToStorageClass(builtin_position_.storage_class), - ConvertType(builtin_position_.member_type_id), false, nullptr, + ConvertType(builtin_position_.position_member_type_id), false, nullptr, ast::VariableDecorationList{ create(Source{}, ast::Builtin::kPosition), }); diff --git a/src/reader/spirv/parser_impl.h b/src/reader/spirv/parser_impl.h index 4e2741d08b..b34177c776 100644 --- a/src/reader/spirv/parser_impl.h +++ b/src/reader/spirv/parser_impl.h @@ -373,16 +373,18 @@ class ParserImpl : Reader { /// The ID for the gl_PerVertex struct containing the Position builtin. uint32_t struct_type_id = 0; /// The member index for the Position builtin within the struct. - uint32_t member_index = 0; + uint32_t position_member_index = 0; + /// The member index for the PointSize builtin within the struct. + uint32_t pointsize_member_index = 0; /// The ID for the member type, which should map to vec4. - uint32_t member_type_id = 0; + uint32_t position_member_type_id = 0; /// The ID of the type of a pointer to the struct in the Output storage /// class class. uint32_t pointer_type_id = 0; /// The SPIR-V storage class. SpvStorageClass storage_class = SpvStorageClassOutput; /// The ID of the type of a pointer to the Position member. - uint32_t member_pointer_type_id = 0; + uint32_t position_member_pointer_type_id = 0; /// The ID of the gl_PerVertex variable, if it was declared. /// We'll use this for the gl_Position variable instead. uint32_t per_vertex_var_id = 0; diff --git a/src/reader/spirv/parser_impl_module_var_test.cc b/src/reader/spirv/parser_impl_module_var_test.cc index 7ab05d1d83..e38bba3f18 100644 --- a/src/reader/spirv/parser_impl_module_var_test.cc +++ b/src/reader/spirv/parser_impl_module_var_test.cc @@ -243,8 +243,8 @@ TEST_F(SpvParserTest, ModuleScopeVar_BuiltinPosition_MapsToModuleScopeVec4Var) { EXPECT_TRUE(p->error().empty()) << p->error(); const auto& position_info = p->GetBuiltInPositionInfo(); EXPECT_EQ(position_info.struct_type_id, 10u); - EXPECT_EQ(position_info.member_index, 0u); - EXPECT_EQ(position_info.member_type_id, 12u); + EXPECT_EQ(position_info.position_member_index, 0u); + EXPECT_EQ(position_info.position_member_type_id, 12u); EXPECT_EQ(position_info.pointer_type_id, 11u); EXPECT_EQ(position_info.storage_class, SpvStorageClassOutput); EXPECT_EQ(position_info.per_vertex_var_id, 1u); @@ -307,8 +307,9 @@ TEST_F(SpvParserTest, )"; auto p = parser(test::Assemble(assembly)); EXPECT_FALSE(p->BuildAndParseInternalModule()); - EXPECT_THAT(p->error(), Eq("operations producing a per-vertex structure are " - "not supported: %1000 = OpUndef %11")) + EXPECT_THAT(p->error(), + Eq("operations producing a pointer to a per-vertex structure are " + "not supported: %1000 = OpUndef %11")) << p->error(); } @@ -343,6 +344,61 @@ TEST_F(SpvParserTest, ModuleScopeVar_BuiltinPosition_StorePosition) { << module_str; } +TEST_F( + SpvParserTest, + ModuleScopeVar_BuiltinPosition_StorePosition_PerVertexStructOutOfOrderDecl) { + const std::string assembly = R"( + OpCapability Shader + OpCapability Linkage ; so we don't have to declare an entry point + OpMemoryModel Logical Simple + + ; scramble the member indices + OpMemberDecorate %10 0 BuiltIn ClipDistance + OpMemberDecorate %10 1 BuiltIn CullDistance + OpMemberDecorate %10 2 BuiltIn Position + OpMemberDecorate %10 3 BuiltIn PointSize + %void = OpTypeVoid + %voidfn = OpTypeFunction %void + %float = OpTypeFloat 32 + %12 = OpTypeVector %float 4 + %uint = OpTypeInt 32 0 + %uint_0 = OpConstant %uint 0 + %uint_1 = OpConstant %uint 1 + %uint_2 = OpConstant %uint 2 + %arr = OpTypeArray %float %uint_1 + %10 = OpTypeStruct %arr %arr %12 %float + %11 = OpTypePointer Output %10 + %1 = OpVariable %11 Output + + %ptr_v4float = OpTypePointer Output %12 + %nil = OpConstantNull %12 + + %main = OpFunction %void None %voidfn + %entry = OpLabel + %100 = OpAccessChain %ptr_v4float %1 %uint_2 ; address of the Position member + OpStore %100 %nil + OpReturn + OpFunctionEnd + )"; + auto p = parser(test::Assemble(assembly)); + EXPECT_TRUE(p->BuildAndParseInternalModule()); + EXPECT_TRUE(p->error().empty()); + const auto module_str = + Demangler().Demangle(p->get_module(), p->get_module().to_str()); + EXPECT_THAT(module_str, HasSubstr(R"( + Assignment{ + Identifier[not set]{gl_Position} + TypeConstructor[not set]{ + __vec_4__f32 + ScalarConstructor[not set]{0.000000} + ScalarConstructor[not set]{0.000000} + ScalarConstructor[not set]{0.000000} + ScalarConstructor[not set]{0.000000} + } + })")) + << module_str; +} + TEST_F(SpvParserTest, ModuleScopeVar_BuiltinPosition_StorePositionMember_OneAccessChain) { const std::string assembly = PerVertexPreamble() + R"( @@ -376,13 +432,13 @@ TEST_F(SpvParserTest, ModuleScopeVar_BuiltinPosition_StorePositionMember_TwoAccessChain) { // The algorithm is smart enough to collapse it down. const std::string assembly = PerVertexPreamble() + R"( - %ptr_v4float = OpTypePointer Output %12 + %ptr = OpTypePointer Output %12 %ptr_float = OpTypePointer Output %float %nil = OpConstantNull %float %main = OpFunction %void None %voidfn %entry = OpLabel - %100 = OpAccessChain %ptr_v4float %1 %uint_0 ; address of the Position member + %100 = OpAccessChain %ptr %1 %uint_0 ; address of the Position member %101 = OpAccessChain %ptr_float %100 %uint_1 ; address of the Position.y member OpStore %101 %nil OpReturn @@ -407,22 +463,166 @@ TEST_F(SpvParserTest, << module_str; } -TEST_F(SpvParserTest, ModuleScopeVar_BuiltinPointSize_NotSupported) { +TEST_F(SpvParserTest, ModuleScopeVar_BuiltinPointSize_Write1_IsErased) { const std::string assembly = PerVertexPreamble() + R"( - %ptr_v4float = OpTypePointer Output %12 - %nil = OpConstantNull %12 + %ptr = OpTypePointer Output %float + %one = OpConstant %float 1.0 %main = OpFunction %void None %voidfn %entry = OpLabel - %100 = OpAccessChain %ptr_v4float %1 %uint_1 ; address of the PointSize member - OpStore %100 %nil + %100 = OpAccessChain %ptr %1 %uint_1 ; address of the PointSize member + OpStore %100 %one + OpReturn + OpFunctionEnd + )"; + auto p = parser(test::Assemble(assembly)); + EXPECT_TRUE(p->BuildAndParseInternalModule()); + EXPECT_TRUE(p->error().empty()); + const auto module_str = + Demangler().Demangle(p->get_module(), p->get_module().to_str()); + EXPECT_EQ(module_str, R"(Module{ + Variable{ + Decorations{ + BuiltinDecoration{position} + } + gl_Position + out + __vec_4__f32 + } + Function x_14 -> __void + () + { + Return{} + } +} +)") << module_str; +} + +TEST_F(SpvParserTest, ModuleScopeVar_BuiltinPointSize_WriteNon1_IsError) { + const std::string assembly = PerVertexPreamble() + R"( + %ptr = OpTypePointer Output %float + %999 = OpConstant %float 2.0 + + %main = OpFunction %void None %voidfn + %entry = OpLabel + %100 = OpAccessChain %ptr %1 %uint_1 ; address of the PointSize member + OpStore %100 %999 OpReturn OpFunctionEnd )"; auto p = parser(test::Assemble(assembly)); EXPECT_FALSE(p->BuildAndParseInternalModule()); - EXPECT_THAT(p->error(), Eq("accessing per-vertex member 1 is not supported. " - "Only Position is supported")); + EXPECT_THAT(p->error(), + HasSubstr("cannot store a value other than constant 1.0 to " + "PointSize builtin: OpStore %100 %999")); +} + +TEST_F(SpvParserTest, ModuleScopeVar_BuiltinPointSize_ReadReplaced) { + const std::string assembly = PerVertexPreamble() + R"( + %ptr = OpTypePointer Output %float + %nil = OpConstantNull %12 + %private_ptr = OpTypePointer Private %float + %900 = OpVariable %private_ptr Private + + %main = OpFunction %void None %voidfn + %entry = OpLabel + %100 = OpAccessChain %ptr %1 %uint_1 ; address of the PointSize member + %99 = OpLoad %float %100 + OpStore %900 %99 + OpReturn + OpFunctionEnd + )"; + auto p = parser(test::Assemble(assembly)); + EXPECT_TRUE(p->BuildAndParseInternalModule()); + EXPECT_TRUE(p->error().empty()); + const auto module_str = + Demangler().Demangle(p->get_module(), p->get_module().to_str()); + EXPECT_EQ(module_str, R"(Module{ + Variable{ + x_900 + private + __f32 + } + Variable{ + Decorations{ + BuiltinDecoration{position} + } + gl_Position + out + __vec_4__f32 + } + Function x_15 -> __void + () + { + Assignment{ + Identifier[not set]{x_900} + ScalarConstructor[not set]{1.000000} + } + Return{} + } +} +)") << module_str; +} + +TEST_F( + SpvParserTest, + ModuleScopeVar_BuiltinPointSize_WriteViaCopyObjectPriorAccess_Unsupported) { + const std::string assembly = PerVertexPreamble() + R"( + %ptr = OpTypePointer Output %float + %nil = OpConstantNull %12 + + %main = OpFunction %void None %voidfn + %entry = OpLabel + %20 = OpCopyObject %11 %1 + %100 = OpAccessChain %20 %1 %uint_1 ; address of the PointSize member + OpStore %100 %nil + OpReturn + OpFunctionEnd + )"; + auto p = parser(test::Assemble(assembly)); + EXPECT_FALSE(p->BuildAndParseInternalModule()) << p->error(); + EXPECT_THAT( + p->error(), + HasSubstr("operations producing a pointer to a per-vertex structure are " + "not supported: %20 = OpCopyObject %11 %1")); +} + +TEST_F( + SpvParserTest, + ModuleScopeVar_BuiltinPointSize_WriteViaCopyObjectPostAccessChainErased) { + const std::string assembly = PerVertexPreamble() + R"( + %ptr = OpTypePointer Output %12 + %one = OpConstant %float 1.0 + + %main = OpFunction %void None %voidfn + %entry = OpLabel + %100 = OpAccessChain %ptr %1 %uint_1 ; address of the PointSize member + %101 = OpCopyObject %ptr %100 + OpStore %101 %one + OpReturn + OpFunctionEnd + )"; + auto p = parser(test::Assemble(assembly)); + EXPECT_TRUE(p->BuildAndParseInternalModule()) << p->error(); + EXPECT_TRUE(p->error().empty()); + const auto module_str = + Demangler().Demangle(p->get_module(), p->get_module().to_str()); + EXPECT_EQ(module_str, R"(Module{ + Variable{ + Decorations{ + BuiltinDecoration{position} + } + gl_Position + out + __vec_4__f32 + } + Function x_14 -> __void + () + { + Return{} + } +} +)") << module_str; } TEST_F(SpvParserTest, ModuleScopeVar_BuiltinClipDistance_NotSupported) { @@ -441,8 +641,9 @@ TEST_F(SpvParserTest, ModuleScopeVar_BuiltinClipDistance_NotSupported) { )"; auto p = parser(test::Assemble(assembly)); EXPECT_FALSE(p->BuildAndParseInternalModule()); - EXPECT_THAT(p->error(), Eq("accessing per-vertex member 2 is not supported. " - "Only Position is supported")); + EXPECT_EQ(p->error(), + "accessing per-vertex member 2 is not supported. Only Position is " + "supported, and PointSize is ignored"); } TEST_F(SpvParserTest, ModuleScopeVar_BuiltinCullDistance_NotSupported) { @@ -461,8 +662,9 @@ TEST_F(SpvParserTest, ModuleScopeVar_BuiltinCullDistance_NotSupported) { )"; auto p = parser(test::Assemble(assembly)); EXPECT_FALSE(p->BuildAndParseInternalModule()); - EXPECT_THAT(p->error(), Eq("accessing per-vertex member 3 is not supported. " - "Only Position is supported")); + EXPECT_EQ(p->error(), + "accessing per-vertex member 3 is not supported. Only Position is " + "supported, and PointSize is ignored"); } TEST_F(SpvParserTest, ModuleScopeVar_BuiltinPerVertex_MemberIndex_NotConstant) {