From 208fc35471c35da87b6cb0118fb43c52f629ac8a Mon Sep 17 00:00:00 2001 From: David Neto Date: Mon, 14 Nov 2022 21:13:30 +0000 Subject: [PATCH] spirv-reader: refactor getting handle type Rename ParserImpl::GetTypeForHandleMemObjDecl to ParserImpl::GetHandleTypeForSpirvHandle More importantly, it now returns the texture or sampler type rather than the pointer type to the texture or sampler. Most usages only wanted the store type. Change-Id: I875e11d97e6d3ecb10fdb3317b860c05fc5fe406 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/109760 Auto-Submit: David Neto Kokoro: Kokoro Reviewed-by: Dan Sinclair Commit-Queue: David Neto --- src/tint/reader/spirv/function.cc | 31 +++------- src/tint/reader/spirv/parser_impl.cc | 87 ++++++++++++++-------------- src/tint/reader/spirv/parser_impl.h | 32 +++++----- 3 files changed, 71 insertions(+), 79 deletions(-) diff --git a/src/tint/reader/spirv/function.cc b/src/tint/reader/spirv/function.cc index 43247fc9eb..e53238f732 100644 --- a/src/tint/reader/spirv/function.cc +++ b/src/tint/reader/spirv/function.cc @@ -1522,12 +1522,8 @@ bool FunctionEmitter::ParseFunctionDeclaration(FunctionDeclaration* decl) { (static_cast(spirv_type->AsPointer()->storage_class()) == spv::StorageClass::UniformConstant))) { // When we see image, sampler, pointer-to-image, or pointer-to-sampler, use the - // handle type deduced according to usage. Handle types are automatically generated as - // pointer-to-handle. Extract the handle type itself. - const auto* ptr_type = parser_impl_.GetTypeForHandleMemObjDecl(*param); - TINT_ASSERT(Reader, ptr_type); - // In WGSL, pass handles instead of pointers to them. - type = ptr_type->type; + // handle type deduced according to usage. + type = parser_impl_.GetHandleTypeForSpirvHandle(*param); } else { type = parser_impl_.ConvertType(param->type_id()); } @@ -5424,21 +5420,17 @@ const spvtools::opt::Instruction* FunctionEmitter::GetImage( } const Texture* FunctionEmitter::GetImageType(const spvtools::opt::Instruction& image) { - const Pointer* ptr_type = parser_impl_.GetTypeForHandleMemObjDecl(image); + const Type* type = parser_impl_.GetHandleTypeForSpirvHandle(image); if (!parser_impl_.success()) { Fail(); return {}; } - if (!ptr_type) { - Fail() << "invalid texture type for " << image.PrettyPrint(); - return {}; + TINT_ASSERT(Reader, type != nullptr); + if (auto* result = type->UnwrapAll()->As()) { + return result; } - auto* result = ptr_type->type->UnwrapAll()->As(); - if (!result) { - Fail() << "invalid texture type for " << image.PrettyPrint(); - return {}; - } - return result; + Fail() << "invalid texture type for " << image.PrettyPrint(); + return {}; } const ast::Expression* FunctionEmitter::GetImageExpression(const spvtools::opt::Instruction& inst) { @@ -5488,12 +5480,7 @@ bool FunctionEmitter::EmitImageAccess(const spvtools::opt::Instruction& inst) { } // Find the texture type. - const Pointer* texture_ptr_type = parser_impl_.GetTypeForHandleMemObjDecl(*image); - if (!texture_ptr_type) { - return Fail(); - } - const Texture* texture_type = texture_ptr_type->type->UnwrapAll()->As(); - + const auto* texture_type = parser_impl_.GetHandleTypeForSpirvHandle(*image)->As(); if (!texture_type) { return Fail(); } diff --git a/src/tint/reader/spirv/parser_impl.cc b/src/tint/reader/spirv/parser_impl.cc index aa213d196f..4f78802107 100644 --- a/src/tint/reader/spirv/parser_impl.cc +++ b/src/tint/reader/spirv/parser_impl.cc @@ -1535,28 +1535,33 @@ bool ParserImpl::EmitModuleScopeVariables() { if (!success_) { return false; } - const Type* ast_type = nullptr; + const Type* ast_store_type = nullptr; + ast::AddressSpace ast_address_space = ast::AddressSpace::kNone; if (spirv_storage_class == spv::StorageClass::UniformConstant) { // These are opaque handles: samplers or textures - ast_type = GetTypeForHandleMemObjDecl(var); - if (!ast_type) { + ast_store_type = GetHandleTypeForSpirvHandle(var); + if (!ast_store_type) { return false; } + // ast_storage_class should remain kNone because handle variables + // are never declared with an explicit address space. } else { - ast_type = ConvertType(type_id); + const Type* ast_type = ConvertType(type_id); if (ast_type == nullptr) { return Fail() << "internal error: failed to register Tint AST type for " "SPIR-V type with ID: " << var.type_id(); } - if (!ast_type->Is()) { + if (auto* ast_ptr_type = ast_type->As()) { + ast_store_type = ast_ptr_type->type; + ast_address_space = ast_ptr_type->address_space; + } else { return Fail() << "variable with ID " << var.result_id() << " has non-pointer type " << var.type_id(); } } + TINT_ASSERT(Reader, ast_store_type != nullptr); - auto* ast_store_type = ast_type->As()->type; - auto ast_address_space = ast_type->As()->address_space; const ast::Expression* ast_initializer = nullptr; if (var.NumInOperands() > 1) { // SPIR-V initializers are always constants. @@ -2356,7 +2361,7 @@ const spvtools::opt::Instruction* ParserImpl::GetMemoryObjectDeclarationForHandl } const spvtools::opt::Instruction* ParserImpl::GetSpirvTypeForHandleOrHandleMemoryObjectDeclaration( - const spvtools::opt::Instruction& mem_obj_decl) { + const spvtools::opt::Instruction& obj) { if (!success()) { return nullptr; } @@ -2371,9 +2376,11 @@ const spvtools::opt::Instruction* ParserImpl::GetSpirvTypeForHandleOrHandleMemor // are the only SPIR-V handles supported by WGSL. // Get the SPIR-V handle type. - const auto* type = def_use_mgr_->GetDef(mem_obj_decl.type_id()); + const auto* type = def_use_mgr_->GetDef(obj.type_id()); if (!type) { - Fail() << "Invalid type for variable or function parameter " << mem_obj_decl.PrettyPrint(); + Fail() << "Invalid type for image, sampler, variable or function parameter to image or " + "sampler " + << obj.PrettyPrint(); return nullptr; } switch (opcode(type)) { @@ -2384,16 +2391,16 @@ const spvtools::opt::Instruction* ParserImpl::GetSpirvTypeForHandleOrHandleMemor // The remaining cases. break; default: - Fail() << "Invalid type for variable or function parameter " - << mem_obj_decl.PrettyPrint(); + Fail() << "Invalid type for image, sampler, variable or function parameter to image or " + "sampler " + << obj.PrettyPrint(); return nullptr; } // Look at the pointee type instead. const auto* raw_handle_type = def_use_mgr_->GetDef(type->GetSingleWordInOperand(1)); if (!raw_handle_type) { - Fail() << "Invalid pointer type for variable or function parameter " - << mem_obj_decl.PrettyPrint(); + Fail() << "Invalid pointer type for variable or function parameter " << obj.PrettyPrint(); return nullptr; } switch (opcode(raw_handle_type)) { @@ -2405,30 +2412,28 @@ const spvtools::opt::Instruction* ParserImpl::GetSpirvTypeForHandleOrHandleMemor case spv::Op::OpTypeRuntimeArray: Fail() << "arrays of textures or samplers are not supported in WGSL; can't " "translate variable or function parameter: " - << mem_obj_decl.PrettyPrint(); + << obj.PrettyPrint(); return nullptr; case spv::Op::OpTypeSampledImage: - Fail() << "WGSL does not support combined image-samplers: " - << mem_obj_decl.PrettyPrint(); + Fail() << "WGSL does not support combined image-samplers: " << obj.PrettyPrint(); return nullptr; default: Fail() << "invalid type for image or sampler variable or function " "parameter: " - << mem_obj_decl.PrettyPrint(); + << obj.PrettyPrint(); return nullptr; } return raw_handle_type; } -const Pointer* ParserImpl::GetTypeForHandleMemObjDecl( - const spvtools::opt::Instruction& mem_obj_decl) { - auto where = handle_type_.find(&mem_obj_decl); +const Type* ParserImpl::GetHandleTypeForSpirvHandle(const spvtools::opt::Instruction& obj) { + auto where = handle_type_.find(&obj); if (where != handle_type_.end()) { return where->second; } const spvtools::opt::Instruction* raw_handle_type = - GetSpirvTypeForHandleOrHandleMemoryObjectDeclaration(mem_obj_decl); + GetSpirvTypeForHandleOrHandleMemoryObjectDeclaration(obj); if (!raw_handle_type) { return nullptr; } @@ -2436,10 +2441,10 @@ const Pointer* ParserImpl::GetTypeForHandleMemObjDecl( // The memory object declaration could be a sampler or image. // Where possible, determine which one it is from the usage inferred // for the variable. - Usage usage = handle_usage_[&mem_obj_decl]; + Usage usage = handle_usage_[&obj]; if (!usage.IsValid()) { Fail() << "Invalid sampler or texture usage for variable or function parameter " - << mem_obj_decl.PrettyPrint() << "\n" + << obj.PrettyPrint() << "\n" << usage; return nullptr; } @@ -2465,7 +2470,7 @@ const Pointer* ParserImpl::GetTypeForHandleMemObjDecl( // Get NonWritable and NonReadable attributes of the variable. bool is_nonwritable = false; bool is_nonreadable = false; - for (const auto& deco : GetDecorationsFor(mem_obj_decl.result_id())) { + for (const auto& deco : GetDecorationsFor(obj.result_id())) { if (deco.size() != 1) { continue; } @@ -2478,11 +2483,11 @@ const Pointer* ParserImpl::GetTypeForHandleMemObjDecl( } if (is_nonwritable && is_nonreadable) { Fail() << "storage image variable is both NonWritable and NonReadable" - << mem_obj_decl.PrettyPrint(); + << obj.PrettyPrint(); } if (!is_nonwritable && !is_nonreadable) { Fail() << "storage image variable is neither NonWritable nor NonReadable" - << mem_obj_decl.PrettyPrint(); + << obj.PrettyPrint(); } // Let's make it one of the storage textures. if (is_nonwritable) { @@ -2502,9 +2507,9 @@ const Pointer* ParserImpl::GetTypeForHandleMemObjDecl( } // Construct the Tint handle type. - const Type* ast_store_type = nullptr; + const Type* ast_handle_type = nullptr; if (usage.IsSampler()) { - ast_store_type = + ast_handle_type = ty_.Sampler(usage.IsComparisonSampler() ? ast::SamplerKind::kComparisonSampler : ast::SamplerKind::kSampler); } else if (usage.IsTexture()) { @@ -2526,8 +2531,7 @@ const Pointer* ParserImpl::GetTypeForHandleMemObjDecl( default: Fail() << "WGSL arrayed textures must be 2d_array or cube_array: " "invalid multisampled texture variable or function parameter " - << namer_.Name(mem_obj_decl.result_id()) << ": " - << mem_obj_decl.PrettyPrint(); + << namer_.Name(obj.result_id()) << ": " << obj.PrettyPrint(); return nullptr; } } @@ -2552,21 +2556,20 @@ const Pointer* ParserImpl::GetTypeForHandleMemObjDecl( // treat that as a depth texture. if (image_type->depth() || usage.IsDepthTexture()) { if (image_type->is_multisampled()) { - ast_store_type = ty_.DepthMultisampledTexture(dim); + ast_handle_type = ty_.DepthMultisampledTexture(dim); } else { - ast_store_type = ty_.DepthTexture(dim); + ast_handle_type = ty_.DepthTexture(dim); } } else if (image_type->is_multisampled()) { if (dim != ast::TextureDimension::k2d) { Fail() << "WGSL multisampled textures must be 2d and non-arrayed: " "invalid multisampled texture variable or function parameter " - << namer_.Name(mem_obj_decl.result_id()) << ": " - << mem_obj_decl.PrettyPrint(); + << namer_.Name(obj.result_id()) << ": " << obj.PrettyPrint(); } // Multisampled textures are never depth textures. - ast_store_type = ty_.MultisampledTexture(dim, ast_sampled_component_type); + ast_handle_type = ty_.MultisampledTexture(dim, ast_sampled_component_type); } else { - ast_store_type = ty_.SampledTexture(dim, ast_sampled_component_type); + ast_handle_type = ty_.SampledTexture(dim, ast_sampled_component_type); } } else { const auto access = ast::Access::kWrite; @@ -2574,20 +2577,18 @@ const Pointer* ParserImpl::GetTypeForHandleMemObjDecl( if (format == ast::TexelFormat::kUndefined) { return nullptr; } - ast_store_type = ty_.StorageTexture(dim, format, access); + ast_handle_type = ty_.StorageTexture(dim, format, access); } } else { Fail() << "unsupported: UniformConstant variable or function parameter is not a recognized " "sampler or texture " - << mem_obj_decl.PrettyPrint(); + << obj.PrettyPrint(); return nullptr; } - // Form the pointer type. - auto* result = ty_.Pointer(ast_store_type, ast::AddressSpace::kHandle); // Remember it for later. - handle_type_[&mem_obj_decl] = result; - return result; + handle_type_[&obj] = ast_handle_type; + return ast_handle_type; } const Type* ParserImpl::GetComponentTypeForFormat(ast::TexelFormat format) { diff --git a/src/tint/reader/spirv/parser_impl.h b/src/tint/reader/spirv/parser_impl.h index 389f113dd3..1780ca3b9b 100644 --- a/src/tint/reader/spirv/parser_impl.h +++ b/src/tint/reader/spirv/parser_impl.h @@ -640,23 +640,26 @@ class ParserImpl : Reader { /// @returns the handle usage, or an empty usage object. Usage GetHandleUsage(uint32_t id) const; - /// Returns the SPIR-V type for the sampler or image type for the given - /// variable in UniformConstant address space, or function parameter pointing - /// into the UniformConstant address space, or image or sampler type. + /// Returns the SPIR-V OpTypeImage or OpTypeSampler for the given: + /// image object, + /// sampler object, + /// memory object declaration image or sampler (i.e. a variable or + /// function parameter with type being a pointer to UniformConstant) /// Returns null and emits an error on failure. - /// @param mem_obj_decl the OpVariable instruction or OpFunctionParameter - /// @returns the Tint AST type for the sampler or texture, or null on error + /// @param obj the given image, sampler, or memory object declaration for an + /// image or sampler + /// @returns the SPIR-V instruction declaring the corresponding OpTypeImage + /// or OpTypeSampler const spvtools::opt::Instruction* GetSpirvTypeForHandleOrHandleMemoryObjectDeclaration( - const spvtools::opt::Instruction& mem_obj_decl); + const spvtools::opt::Instruction& obj); - /// Returns the AST type for the pointer-to-sampler or pointer-to-texture type - /// for the given variable in UniformConstant address space or function - /// parameter of type pointer-to-UniformConstant. Returns null and - /// emits an error on failure. - /// @param mem_obj_decl the OpVariable instruction + /// Returns the AST type for the texture or sampler type for the given + /// SPIR-V image, sampler, or memory object declaration for an image or + /// sampler. Returns null and emits an error on failure. + /// @param obj the OpVariable instruction /// @returns the Tint AST type for the poiner-to-{sampler|texture} or null on /// error - const Pointer* GetTypeForHandleMemObjDecl(const spvtools::opt::Instruction& mem_obj_decl); + const Type* GetHandleTypeForSpirvHandle(const spvtools::opt::Instruction& obj); /// Returns the AST variable for the SPIR-V ID of a module-scope variable, /// or null if there isn't one. @@ -879,8 +882,9 @@ class ParserImpl : Reader { // Maps a memory-object-declaration instruction to any sampler or texture // usages implied by usages of the memory-object-declaration. std::unordered_map handle_usage_; - // The inferred pointer type for the given handle variable. - std::unordered_map handle_type_; + // The inferred WGSL handle type for the given SPIR-V image, sampler, or + // memory object declaration for an image or sampler. + std::unordered_map handle_type_; /// Maps the SPIR-V ID of a module-scope variable to its AST variable. utils::Hashmap module_variable_;