From 183b8eb19c87ca468edd7bbf6efe1dc62610b47c Mon Sep 17 00:00:00 2001 From: David Neto Date: Thu, 10 Nov 2022 20:52:31 +0000 Subject: [PATCH] spirv-reader: func decl with handle or ptr-to-handle Support function declarations where formal parameters are textures, samplers, or pointers to them. Still need to update call sites. Bug: tint:1039 Change-Id: I5bb3ca73190b2e27c28205e78aa433108efec252 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/109540 Commit-Queue: David Neto Reviewed-by: Dan Sinclair Kokoro: Kokoro --- src/tint/reader/spirv/function.cc | 26 +++++-- src/tint/reader/spirv/function_decl_test.cc | 78 +++++++++++++++++++++ src/tint/reader/spirv/parser_impl.cc | 76 ++++++++++++-------- src/tint/reader/spirv/parser_impl.h | 17 ++--- 4 files changed, 157 insertions(+), 40 deletions(-) diff --git a/src/tint/reader/spirv/function.cc b/src/tint/reader/spirv/function.cc index 0aafdf98d8..43247fc9eb 100644 --- a/src/tint/reader/spirv/function.cc +++ b/src/tint/reader/spirv/function.cc @@ -1514,7 +1514,24 @@ bool FunctionEmitter::ParseFunctionDeclaration(FunctionDeclaration* decl) { ParameterList ast_params; function_.ForEachParam([this, &ast_params](const spvtools::opt::Instruction* param) { - auto* type = parser_impl_.ConvertType(param->type_id()); + const Type* type = nullptr; + auto* spirv_type = type_mgr_->GetType(param->type_id()); + TINT_ASSERT(Reader, spirv_type); + if (spirv_type->AsImage() || spirv_type->AsSampler() || + (spirv_type->AsPointer() && + (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; + } else { + type = parser_impl_.ConvertType(param->type_id()); + } + if (type != nullptr) { auto* ast_param = parser_impl_.MakeParameter(param->result_id(), type, AttributeList{}); // Parameters are treated as const declarations. @@ -5407,7 +5424,7 @@ const spvtools::opt::Instruction* FunctionEmitter::GetImage( } const Texture* FunctionEmitter::GetImageType(const spvtools::opt::Instruction& image) { - const Pointer* ptr_type = parser_impl_.GetTypeForHandleVar(image); + const Pointer* ptr_type = parser_impl_.GetTypeForHandleMemObjDecl(image); if (!parser_impl_.success()) { Fail(); return {}; @@ -5471,7 +5488,7 @@ bool FunctionEmitter::EmitImageAccess(const spvtools::opt::Instruction& inst) { } // Find the texture type. - const Pointer* texture_ptr_type = parser_impl_.GetTypeForHandleVar(*image); + const Pointer* texture_ptr_type = parser_impl_.GetTypeForHandleMemObjDecl(*image); if (!texture_ptr_type) { return Fail(); } @@ -5736,7 +5753,8 @@ bool FunctionEmitter::EmitImageAccess(const spvtools::opt::Instruction& inst) { // If necessary, convert the result to the signedness of the instruction // result type. Compare the SPIR-V image's sampled component type with the // component of the result type of the SPIR-V instruction. - auto* spirv_image_type = parser_impl_.GetSpirvTypeForHandleMemoryObjectDeclaration(*image); + auto* spirv_image_type = + parser_impl_.GetSpirvTypeForHandleOrHandleMemoryObjectDeclaration(*image); if (!spirv_image_type || (opcode(spirv_image_type) != spv::Op::OpTypeImage)) { return Fail() << "invalid image type for image memory object declaration " << image->PrettyPrint(); diff --git a/src/tint/reader/spirv/function_decl_test.cc b/src/tint/reader/spirv/function_decl_test.cc index 8af9da2172..9413d5333b 100644 --- a/src/tint/reader/spirv/function_decl_test.cc +++ b/src/tint/reader/spirv/function_decl_test.cc @@ -52,6 +52,19 @@ std::string CommonTypes() { )"; } +std::string CommonHandleTypes() { + return CommonTypes() + R"( + %v2float = OpTypeVector %float 2 + %v4float = OpTypeVector %float 4 + %v2_0 = OpConstantNull %v2float + %sampler = OpTypeSampler + %tex2d_f32 = OpTypeImage %float 2D 0 0 0 1 Unknown + %sampled_image_2d_f32 = OpTypeSampledImage %tex2d_f32 + %ptr_sampler = OpTypePointer UniformConstant %sampler + %ptr_tex2d_f32 = OpTypePointer UniformConstant %tex2d_f32 + )"; +} + std::string MainBody() { return R"( %100 = OpFunction %void None %voidfn @@ -147,5 +160,70 @@ TEST_F(SpvParserTest, Emit_GenerateParamNames) { EXPECT_THAT(got, HasSubstr(expect)); } +// ;%s = OpVariable %ptr_sampler UniformConstant +// ;%t = OpVariable %ptr_tex2d_f32 UniformConstant + +TEST_F(SpvParserTest, Emit_FunctionDecl_ParamPtrTexture_ParamPtrSampler) { + auto p = parser(test::Assemble(Preamble() + CommonHandleTypes() + R"( + + ; This is how Glslang generates functions that take texture and sampler arguments. + ; It passes them by pointer. + %fn_ty = OpTypeFunction %void %ptr_tex2d_f32 %ptr_sampler + + %200 = OpFunction %void None %fn_ty + %14 = OpFunctionParameter %ptr_tex2d_f32 + %15 = OpFunctionParameter %ptr_sampler + %mixed_entry = OpLabel + ; access the texture, to give the handles usages. + %im = OpLoad %tex2d_f32 %14 + %sam = OpLoad %sampler %15 + %imsam = OpSampledImage %sampled_image_2d_f32 %im %sam + %20 = OpImageSampleImplicitLod %v4float %imsam %v2_0 + OpReturn + OpFunctionEnd + )" + MainBody())); + ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()); + auto fe = p->function_emitter(200); + EXPECT_TRUE(fe.Emit()); + + auto got = test::ToString(p->program()); + std::string expect = R"(fn x_200(x_14 : texture_2d, x_15 : sampler) { + let x_20 : vec4 = textureSample(x_14, x_15, vec2()); + return; +} +)"; + EXPECT_EQ(got, expect); +} + +TEST_F(SpvParserTest, Emit_FunctionDecl_ParamTexture_ParamSampler) { + auto assembly = Preamble() + CommonHandleTypes() + R"( + + ; It is valid in SPIR-V to pass textures and samplers by value. + %fn_ty = OpTypeFunction %void %tex2d_f32 %sampler + + %200 = OpFunction %void None %fn_ty + %14 = OpFunctionParameter %tex2d_f32 + %15 = OpFunctionParameter %sampler + %mixed_entry = OpLabel + ; access the texture, to give the handles usages. + %imsam = OpSampledImage %sampled_image_2d_f32 %14 %15 + %20 = OpImageSampleImplicitLod %v4float %imsam %v2_0 + OpReturn + OpFunctionEnd + )" + MainBody(); + auto p = parser(test::Assemble(assembly)); + ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions()) << p->error() << assembly; + auto fe = p->function_emitter(200); + EXPECT_TRUE(fe.Emit()); + + auto got = test::ToString(p->program()); + std::string expect = R"(fn x_200(x_14 : texture_2d, x_15 : sampler) { + let x_20 : vec4 = textureSample(x_14, x_15, vec2()); + return; +} +)"; + EXPECT_EQ(got, expect); +} + } // namespace } // namespace tint::reader::spirv diff --git a/src/tint/reader/spirv/parser_impl.cc b/src/tint/reader/spirv/parser_impl.cc index f055f4c8c1..aa213d196f 100644 --- a/src/tint/reader/spirv/parser_impl.cc +++ b/src/tint/reader/spirv/parser_impl.cc @@ -1538,7 +1538,7 @@ bool ParserImpl::EmitModuleScopeVariables() { const Type* ast_type = nullptr; if (spirv_storage_class == spv::StorageClass::UniformConstant) { // These are opaque handles: samplers or textures - ast_type = GetTypeForHandleVar(var); + ast_type = GetTypeForHandleMemObjDecl(var); if (!ast_type) { return false; } @@ -2355,8 +2355,8 @@ const spvtools::opt::Instruction* ParserImpl::GetMemoryObjectDeclarationForHandl } } -const spvtools::opt::Instruction* ParserImpl::GetSpirvTypeForHandleMemoryObjectDeclaration( - const spvtools::opt::Instruction& var) { +const spvtools::opt::Instruction* ParserImpl::GetSpirvTypeForHandleOrHandleMemoryObjectDeclaration( + const spvtools::opt::Instruction& mem_obj_decl) { if (!success()) { return nullptr; } @@ -2371,14 +2371,29 @@ const spvtools::opt::Instruction* ParserImpl::GetSpirvTypeForHandleMemoryObjectD // are the only SPIR-V handles supported by WGSL. // Get the SPIR-V handle type. - const auto* ptr_type = def_use_mgr_->GetDef(var.type_id()); - if (!ptr_type || (opcode(ptr_type) != spv::Op::OpTypePointer)) { - Fail() << "Invalid type for variable or function parameter " << var.PrettyPrint(); + const auto* type = def_use_mgr_->GetDef(mem_obj_decl.type_id()); + if (!type) { + Fail() << "Invalid type for variable or function parameter " << mem_obj_decl.PrettyPrint(); return nullptr; } - const auto* raw_handle_type = def_use_mgr_->GetDef(ptr_type->GetSingleWordInOperand(1)); + switch (opcode(type)) { + case spv::Op::OpTypeSampler: + case spv::Op::OpTypeImage: + return type; + case spv::Op::OpTypePointer: + // The remaining cases. + break; + default: + Fail() << "Invalid type for variable or function parameter " + << mem_obj_decl.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 " << var.PrettyPrint(); + Fail() << "Invalid pointer type for variable or function parameter " + << mem_obj_decl.PrettyPrint(); return nullptr; } switch (opcode(raw_handle_type)) { @@ -2390,38 +2405,41 @@ const spvtools::opt::Instruction* ParserImpl::GetSpirvTypeForHandleMemoryObjectD case spv::Op::OpTypeRuntimeArray: Fail() << "arrays of textures or samplers are not supported in WGSL; can't " "translate variable or function parameter: " - << var.PrettyPrint(); + << mem_obj_decl.PrettyPrint(); return nullptr; case spv::Op::OpTypeSampledImage: - Fail() << "WGSL does not support combined image-samplers: " << var.PrettyPrint(); + Fail() << "WGSL does not support combined image-samplers: " + << mem_obj_decl.PrettyPrint(); return nullptr; default: Fail() << "invalid type for image or sampler variable or function " "parameter: " - << var.PrettyPrint(); + << mem_obj_decl.PrettyPrint(); return nullptr; } return raw_handle_type; } -const Pointer* ParserImpl::GetTypeForHandleVar(const spvtools::opt::Instruction& var) { - auto where = handle_type_.find(&var); +const Pointer* ParserImpl::GetTypeForHandleMemObjDecl( + const spvtools::opt::Instruction& mem_obj_decl) { + auto where = handle_type_.find(&mem_obj_decl); if (where != handle_type_.end()) { return where->second; } const spvtools::opt::Instruction* raw_handle_type = - GetSpirvTypeForHandleMemoryObjectDeclaration(var); + GetSpirvTypeForHandleOrHandleMemoryObjectDeclaration(mem_obj_decl); if (!raw_handle_type) { return nullptr; } - // The variable could be a sampler or image. + // 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_[&var]; + Usage usage = handle_usage_[&mem_obj_decl]; if (!usage.IsValid()) { - Fail() << "Invalid sampler or texture usage for variable " << var.PrettyPrint() << "\n" + Fail() << "Invalid sampler or texture usage for variable or function parameter " + << mem_obj_decl.PrettyPrint() << "\n" << usage; return nullptr; } @@ -2447,7 +2465,7 @@ const Pointer* ParserImpl::GetTypeForHandleVar(const spvtools::opt::Instruction& // Get NonWritable and NonReadable attributes of the variable. bool is_nonwritable = false; bool is_nonreadable = false; - for (const auto& deco : GetDecorationsFor(var.result_id())) { + for (const auto& deco : GetDecorationsFor(mem_obj_decl.result_id())) { if (deco.size() != 1) { continue; } @@ -2460,11 +2478,11 @@ const Pointer* ParserImpl::GetTypeForHandleVar(const spvtools::opt::Instruction& } if (is_nonwritable && is_nonreadable) { Fail() << "storage image variable is both NonWritable and NonReadable" - << var.PrettyPrint(); + << mem_obj_decl.PrettyPrint(); } if (!is_nonwritable && !is_nonreadable) { Fail() << "storage image variable is neither NonWritable nor NonReadable" - << var.PrettyPrint(); + << mem_obj_decl.PrettyPrint(); } // Let's make it one of the storage textures. if (is_nonwritable) { @@ -2507,8 +2525,9 @@ const Pointer* ParserImpl::GetTypeForHandleVar(const spvtools::opt::Instruction& break; default: Fail() << "WGSL arrayed textures must be 2d_array or cube_array: " - "invalid multisampled texture variable " - << namer_.Name(var.result_id()) << ": " << var.PrettyPrint(); + "invalid multisampled texture variable or function parameter " + << namer_.Name(mem_obj_decl.result_id()) << ": " + << mem_obj_decl.PrettyPrint(); return nullptr; } } @@ -2540,8 +2559,9 @@ const Pointer* ParserImpl::GetTypeForHandleVar(const spvtools::opt::Instruction& } 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 " - << namer_.Name(var.result_id()) << ": " << var.PrettyPrint(); + "invalid multisampled texture variable or function parameter " + << namer_.Name(mem_obj_decl.result_id()) << ": " + << mem_obj_decl.PrettyPrint(); } // Multisampled textures are never depth textures. ast_store_type = ty_.MultisampledTexture(dim, ast_sampled_component_type); @@ -2557,16 +2577,16 @@ const Pointer* ParserImpl::GetTypeForHandleVar(const spvtools::opt::Instruction& ast_store_type = ty_.StorageTexture(dim, format, access); } } else { - Fail() << "unsupported: UniformConstant variable is not a recognized " - "sampler or texture" - << var.PrettyPrint(); + Fail() << "unsupported: UniformConstant variable or function parameter is not a recognized " + "sampler or texture " + << mem_obj_decl.PrettyPrint(); return nullptr; } // Form the pointer type. auto* result = ty_.Pointer(ast_store_type, ast::AddressSpace::kHandle); // Remember it for later. - handle_type_[&var] = result; + handle_type_[&mem_obj_decl] = result; return result; } diff --git a/src/tint/reader/spirv/parser_impl.h b/src/tint/reader/spirv/parser_impl.h index 6e79ecce30..389f113dd3 100644 --- a/src/tint/reader/spirv/parser_impl.h +++ b/src/tint/reader/spirv/parser_impl.h @@ -642,20 +642,21 @@ class ParserImpl : Reader { /// 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 . Returns null and emits an - /// error on failure. - /// @param var the OpVariable instruction or OpFunctionParameter + /// into the UniformConstant address space, or image or sampler type. + /// 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 - const spvtools::opt::Instruction* GetSpirvTypeForHandleMemoryObjectDeclaration( - const spvtools::opt::Instruction& var); + const spvtools::opt::Instruction* GetSpirvTypeForHandleOrHandleMemoryObjectDeclaration( + const spvtools::opt::Instruction& mem_obj_decl); /// Returns the AST type for the pointer-to-sampler or pointer-to-texture type - /// for the given variable in UniformConstant address space. Returns null and + /// 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 var the OpVariable instruction + /// @param mem_obj_decl the OpVariable instruction /// @returns the Tint AST type for the poiner-to-{sampler|texture} or null on /// error - const Pointer* GetTypeForHandleVar(const spvtools::opt::Instruction& var); + const Pointer* GetTypeForHandleMemObjDecl(const spvtools::opt::Instruction& mem_obj_decl); /// Returns the AST variable for the SPIR-V ID of a module-scope variable, /// or null if there isn't one.