diff --git a/src/reader/spirv/function.cc b/src/reader/spirv/function.cc index a6270c1bb8..ea87c3d3bf 100644 --- a/src/reader/spirv/function.cc +++ b/src/reader/spirv/function.cc @@ -3450,17 +3450,12 @@ TypedExpression FunctionEmitter::MakeAccessChain( return {}; } - // A SPIR-V access chain is a single instruction with multiple indices - // walking down into composites. The Tint AST represents this as - // ever-deeper nested indexing expressions. Start off with an expression - // for the base, and then bury that inside nested indexing expressions. - TypedExpression current_expr(MakeOperand(inst, 0)); - const auto constants = constant_mgr_->GetOperandConstants(&inst); - auto ptr_ty_id = def_use_mgr_->GetDef(base_id)->type_id(); uint32_t first_index = 1; const auto num_in_operands = inst.NumInOperands(); + TypedExpression current_expr; + // 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 @@ -3526,6 +3521,15 @@ TypedExpression FunctionEmitter::MakeAccessChain( } } + // A SPIR-V access chain is a single instruction with multiple indices + // walking down into composites. The Tint AST represents this as + // ever-deeper nested indexing expressions. Start off with an expression + // for the base, and then bury that inside nested indexing expressions. + if (!current_expr.expr) { + current_expr = MakeOperand(inst, 0); + } + const auto constants = constant_mgr_->GetOperandConstants(&inst); + const auto* ptr_type_inst = def_use_mgr_->GetDef(ptr_ty_id); if (!ptr_type_inst || (ptr_type_inst->opcode() != SpvOpTypePointer)) { Fail() << "Access chain %" << inst.result_id() diff --git a/src/reader/spirv/parser_impl.cc b/src/reader/spirv/parser_impl.cc index 336e18bce4..cfcf98618c 100644 --- a/src/reader/spirv/parser_impl.cc +++ b/src/reader/spirv/parser_impl.cc @@ -845,15 +845,13 @@ sem::Type* ParserImpl::ConvertType( const spvtools::opt::analysis::Struct* struct_ty) { // Compute the struct decoration. auto struct_decorations = this->GetDecorationsFor(type_id); - ast::DecorationList ast_struct_decorations; + bool is_block_decorated = false; if (struct_decorations.size() == 1) { const auto decoration = struct_decorations[0][0]; if (decoration == SpvDecorationBlock) { - ast_struct_decorations.push_back( - create(Source{})); + is_block_decorated = true; } else if (decoration == SpvDecorationBufferBlock) { - ast_struct_decorations.push_back( - create(Source{})); + is_block_decorated = true; remap_buffer_block_type_.insert(type_id); } else { Fail() << "struct with ID " << type_id @@ -869,7 +867,6 @@ sem::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]); @@ -878,8 +875,10 @@ sem::Type* ParserImpl::ConvertType( // Already emitted diagnostics. return nullptr; } - ast::DecorationList ast_member_decorations; - bool is_non_writable = false; + + // Scan member for built-in decorations. Some vertex built-ins are handled + // specially, and should not generate a structure member. + bool create_ast_member = true; for (auto& decoration : GetDecorationsForMember(type_id, member_index)) { if (decoration.empty()) { Fail() << "malformed SPIR-V decoration: it's empty"; @@ -892,23 +891,31 @@ sem::Type* ParserImpl::ConvertType( builtin_position_.struct_type_id = 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. - is_per_vertex_struct = true; + create_ast_member = false; // Not part of the WGSL structure. break; case SpvBuiltInPointSize: // not supported in WGSL, but ignore builtin_position_.pointsize_member_index = member_index; - is_per_vertex_struct = true; + create_ast_member = false; // Not part of the WGSL structure. 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; + create_ast_member = false; // Not part of the WGSL structure. break; default: Fail() << "unrecognized builtin " << decoration[1]; return nullptr; } - } else if (decoration[0] == SpvDecorationNonWritable) { + } + } + if (!create_ast_member) { + // This member is decorated as a built-in, and is handled specially. + continue; + } + + bool is_non_writable = false; + ast::DecorationList ast_member_decorations; + for (auto& decoration : GetDecorationsForMember(type_id, member_index)) { + 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 // the members are non-writable. @@ -924,6 +931,7 @@ sem::Type* ParserImpl::ConvertType( } } } + if (is_non_writable) { // Count a member as non-writable only once, no matter how many // NonWritable decorations are applied to it. @@ -935,8 +943,9 @@ sem::Type* ParserImpl::ConvertType( 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. + + if (ast_members.empty()) { + // All members were likely built-ins. Don't generate an empty AST structure. return nullptr; } @@ -946,6 +955,11 @@ sem::Type* ParserImpl::ConvertType( // Now make the struct. auto sym = builder_.Symbols().Register(name); + ast::DecorationList ast_struct_decorations; + if (is_block_decorated) { + ast_struct_decorations.emplace_back( + create(Source{})); + } auto* ast_struct = create(Source{}, sym, std::move(ast_members), std::move(ast_struct_decorations)); auto* result = builder_.create(ast_struct); diff --git a/src/resolver/resolver.cc b/src/resolver/resolver.cc index 85501b0469..ebb18c07dc 100644 --- a/src/resolver/resolver.cc +++ b/src/resolver/resolver.cc @@ -272,7 +272,8 @@ bool Resolver::ResolveInternal() { } TINT_ICE(diagnostics_) << "AST node '" << node->TypeInfo().name << "' was not reached by the resolver\n" - << "At: " << node->source(); + << "At: " << node->source() << "\n" + << "Content: " << builder_->str(node); } }