From 15838b98af3151dc27c191ca55754656e3c976cd Mon Sep 17 00:00:00 2001 From: James Price Date: Thu, 8 Jul 2021 22:35:27 +0000 Subject: [PATCH] Switch to new Tint generator API Tint now has a single-function API for code generation that automatically runs the backend-specific sanitizer transforms. This API allows for backend-specific configuration options such as the fixed sample mask for MSL (and, in the future, information about which compiler/version is being targeted), and returns any backend-specific metadata such as whether a UBO of buffer sizes is required by the shader. This change prevents the post-sanitizer program from being exposed to Dawn, which is a potential foot-gun (e.g. the Inspector is not expected to be run after the sanitizer transforms, and Dawn is currently doing this). The old Generator class API will be removed from Tint shortly after landing this change. Bug: tint:697 Change-Id: I9b988d55514f810d3091ec6471731e6eb41dc27f Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/57103 Auto-Submit: James Price Reviewed-by: Austin Eng Commit-Queue: Austin Eng --- src/dawn_native/ShaderModule.cpp | 47 ++++++++------------- src/dawn_native/d3d12/ShaderModuleD3D12.cpp | 11 +++-- src/dawn_native/metal/ShaderModuleMTL.mm | 23 ++++------ src/dawn_native/opengl/ShaderModuleGL.cpp | 19 +++------ src/dawn_native/vulkan/ShaderModuleVk.cpp | 25 ++++++----- 5 files changed, 48 insertions(+), 77 deletions(-) diff --git a/src/dawn_native/ShaderModule.cpp b/src/dawn_native/ShaderModule.cpp index 5f8336a61a..5db2d22929 100644 --- a/src/dawn_native/ShaderModule.cpp +++ b/src/dawn_native/ShaderModule.cpp @@ -406,14 +406,15 @@ namespace dawn_native { std::ostringstream errorStream; errorStream << "Tint SPIR-V writer failure:" << std::endl; - tint::writer::spirv::Generator generator(program); - if (!generator.Generate()) { - errorStream << "Generator: " << generator.error() << std::endl; + tint::writer::spirv::Options options; + options.emit_vertex_point_size = true; + auto result = tint::writer::spirv::Generate(program, options); + if (!result.success) { + errorStream << "Generator: " << result.error << std::endl; return DAWN_VALIDATION_ERROR(errorStream.str().c_str()); } - std::vector spirv = generator.result(); - return std::move(spirv); + return std::move(result.spirv); } std::vector GetBindGroupMinBufferSizes( @@ -1113,15 +1114,16 @@ namespace dawn_native { tint::Program program; DAWN_TRY_ASSIGN(program, ParseSPIRV(spirv, outMessages)); - tint::writer::wgsl::Generator generator(&program); - if (!generator.Generate()) { + tint::writer::wgsl::Options options; + auto result = tint::writer::wgsl::Generate(&program, options); + if (!result.success) { std::ostringstream errorStream; errorStream << "Tint WGSL failure:" << std::endl; - errorStream << "Generator: " << generator.error() << std::endl; + errorStream << "Generator: " << result.error << std::endl; return DAWN_VALIDATION_ERROR(errorStream.str().c_str()); } - newWgslCode = generator.result(); + newWgslCode = std::move(result.wgsl); newWgslDesc.source = newWgslCode.c_str(); if (device->IsToggleEnabled(Toggle::DumpTranslatedShaders)) { @@ -1160,18 +1162,6 @@ namespace dawn_native { parseResult->tintProgram = std::make_unique(std::move(program)); parseResult->tintSource = std::move(tintSource); } else { - tint::transform::Manager transformManager; - transformManager.Add(); - - tint::transform::DataMap transformInputs; - - tint::transform::Spirv::Config spirv_cfg; - spirv_cfg.emit_vertex_point_size = true; - transformInputs.Add(spirv_cfg); - - DAWN_TRY_ASSIGN(program, RunTransforms(&transformManager, &program, transformInputs, - nullptr, outMessages)); - std::vector spirv; DAWN_TRY_ASSIGN(spirv, ModuleToSPIRV(&program)); DAWN_TRY(ValidateSpirv(spirv.data(), spirv.size())); @@ -1433,17 +1423,12 @@ namespace dawn_native { tint::transform::Manager transformManager; transformManager.Add(); - transformManager.Add(); if (GetDevice()->IsRobustnessEnabled()) { transformManager.Add(); } tint::transform::DataMap transformInputs; - tint::transform::Spirv::Config spirv_cfg; - spirv_cfg.emit_vertex_point_size = true; - transformInputs.Add(spirv_cfg); - AddVertexPullingTransformConfig(vertexState, entryPoint, pullingBufferBindingSet, &transformInputs); @@ -1454,13 +1439,15 @@ namespace dawn_native { DAWN_TRY_ASSIGN(program, RunTransforms(&transformManager, programIn, transformInputs, nullptr, nullptr)); - tint::writer::spirv::Generator generator(&program); - if (!generator.Generate()) { - errorStream << "Generator: " << generator.error() << std::endl; + tint::writer::spirv::Options options; + options.emit_vertex_point_size = true; + auto result = tint::writer::spirv::Generate(&program, options); + if (!result.success) { + errorStream << "Generator: " << result.error << std::endl; return DAWN_VALIDATION_ERROR(errorStream.str().c_str()); } - std::vector spirv = generator.result(); + std::vector spirv = std::move(result.spirv); DAWN_TRY(ValidateSpirv(spirv.data(), spirv.size())); return std::move(spirv); } diff --git a/src/dawn_native/d3d12/ShaderModuleD3D12.cpp b/src/dawn_native/d3d12/ShaderModuleD3D12.cpp index 7e9b3ed7c4..0ea9acbca3 100644 --- a/src/dawn_native/d3d12/ShaderModuleD3D12.cpp +++ b/src/dawn_native/d3d12/ShaderModuleD3D12.cpp @@ -258,7 +258,6 @@ namespace dawn_native { namespace d3d12 { layout->GetFirstIndexOffsetRegisterSpace()); } transformManager.Add(); - transformManager.Add(); if (!GetDevice()->IsToggleEnabled(Toggle::DumpTranslatedShaders)) { transformManager.Add(); @@ -300,14 +299,14 @@ namespace dawn_native { namespace d3d12 { } } - tint::writer::hlsl::Generator generator(&program); - // TODO: Switch to GenerateEntryPoint once HLSL writer supports it. - if (!generator.Generate()) { - errorStream << "Generator: " << generator.error() << std::endl; + tint::writer::hlsl::Options options; + auto result = tint::writer::hlsl::Generate(&program, options); + if (!result.success) { + errorStream << "Generator: " << result.error << std::endl; return DAWN_VALIDATION_ERROR(errorStream.str().c_str()); } - return generator.result(); + return std::move(result.hlsl); } ResultOrError ShaderModule::TranslateToHLSLWithSPIRVCross( diff --git a/src/dawn_native/metal/ShaderModuleMTL.mm b/src/dawn_native/metal/ShaderModuleMTL.mm index eddc904bc8..d2475e057c 100644 --- a/src/dawn_native/metal/ShaderModuleMTL.mm +++ b/src/dawn_native/metal/ShaderModuleMTL.mm @@ -122,7 +122,6 @@ namespace dawn_native { namespace metal { transformManager.Add(); } transformManager.Add(); - transformManager.Add(); if (!GetDevice()->IsToggleEnabled(Toggle::DumpTranslatedShaders)) { transformManager.Add(); @@ -132,8 +131,6 @@ namespace dawn_native { namespace metal { std::move(accessControls), /* mayCollide */ true); - transformInputs.Add(kBufferLengthBufferSlot, sampleMask); - tint::Program program; tint::transform::DataMap transformOutputs; DAWN_TRY_ASSIGN(program, RunTransforms(&transformManager, GetTintProgram(), transformInputs, @@ -153,20 +150,18 @@ namespace dawn_native { namespace metal { } } - if (auto* data = transformOutputs.Get()) { - *needsStorageBufferLength = data->needs_storage_buffer_sizes; - } else { - return DAWN_VALIDATION_ERROR("Transform output missing MSL data."); - } - - tint::writer::msl::Generator generator(&program); - if (!generator.Generate()) { - errorStream << "Generator: " << generator.error() << std::endl; + tint::writer::msl::Options options; + options.buffer_size_ubo_index = kBufferLengthBufferSlot; + options.fixed_sample_mask = sampleMask; + auto result = tint::writer::msl::Generate(&program, options); + if (!result.success) { + errorStream << "Generator: " << result.error << std::endl; return DAWN_VALIDATION_ERROR(errorStream.str().c_str()); } - std::string msl = generator.result(); - return std::move(msl); + *needsStorageBufferLength = result.needs_storage_buffer_sizes; + + return std::move(result.msl); } ResultOrError ShaderModule::TranslateToMSLWithSPIRVCross( diff --git a/src/dawn_native/opengl/ShaderModuleGL.cpp b/src/dawn_native/opengl/ShaderModuleGL.cpp index b147626cbf..de8d986cbe 100644 --- a/src/dawn_native/opengl/ShaderModuleGL.cpp +++ b/src/dawn_native/opengl/ShaderModuleGL.cpp @@ -84,24 +84,15 @@ namespace dawn_native { namespace opengl { // Tint currently does not support emitting GLSL, so when provided a Tint program need to // generate SPIRV and SPIRV-Cross reflection data to be used in this backend. if (GetDevice()->IsToggleEnabled(Toggle::UseTintGenerator)) { - tint::transform::Manager transformManager; - transformManager.append(std::make_unique()); - - tint::transform::DataMap transformInputs; - - tint::Program program; - DAWN_TRY_ASSIGN(program, - RunTransforms(&transformManager, GetTintProgram(), transformInputs, - nullptr, GetCompilationMessages())); - - tint::writer::spirv::Generator generator(&program); - if (!generator.Generate()) { + tint::writer::spirv::Options options; + auto result = tint::writer::spirv::Generate(GetTintProgram(), options); + if (!result.success) { std::ostringstream errorStream; - errorStream << "Generator: " << generator.error() << std::endl; + errorStream << "Generator: " << result.error << std::endl; return DAWN_VALIDATION_ERROR(errorStream.str().c_str()); } - mGLSpirv = generator.result(); + mGLSpirv = std::move(result.spirv); DAWN_TRY_ASSIGN(mGLEntryPoints, ReflectShaderUsingSPIRVCross(GetDevice(), mGLSpirv)); } diff --git a/src/dawn_native/vulkan/ShaderModuleVk.cpp b/src/dawn_native/vulkan/ShaderModuleVk.cpp index ea1a74f019..82bb0de864 100644 --- a/src/dawn_native/vulkan/ShaderModuleVk.cpp +++ b/src/dawn_native/vulkan/ShaderModuleVk.cpp @@ -97,27 +97,24 @@ namespace dawn_native { namespace vulkan { if (GetDevice()->IsRobustnessEnabled()) { transformManager.Add(); } - transformManager.Add(); tint::transform::DataMap transformInputs; - tint::transform::Spirv::Config spirv_cfg; - spirv_cfg.emit_vertex_point_size = true; - transformInputs.Add(spirv_cfg); - tint::Program program; DAWN_TRY_ASSIGN(program, RunTransforms(&transformManager, parseResult->tintProgram.get(), transformInputs, nullptr, nullptr)); // We will miss the messages generated in this RunTransforms. - tint::writer::spirv::Generator generator(&program); - if (!generator.Generate()) { - errorStream << "Generator: " << generator.error() << std::endl; + tint::writer::spirv::Options options; + options.emit_vertex_point_size = true; + auto result = tint::writer::spirv::Generate(&program, options); + if (!result.success) { + errorStream << "Generator: " << result.error << std::endl; return DAWN_VALIDATION_ERROR(errorStream.str().c_str()); } - spirv = generator.result(); + spirv = std::move(result.spirv); spirvPtr = &spirv; // Rather than use a new ParseResult object, we just reuse the original parseResult @@ -214,13 +211,15 @@ namespace dawn_native { namespace vulkan { DAWN_TRY_ASSIGN(program, RunTransforms(&transformManager, GetTintProgram(), transformInputs, nullptr, nullptr)); - tint::writer::spirv::Generator generator(&program); - if (!generator.Generate()) { - errorStream << "Generator: " << generator.error() << std::endl; + tint::writer::spirv::Options options; + options.emit_vertex_point_size = true; + auto result = tint::writer::spirv::Generate(&program, options); + if (!result.success) { + errorStream << "Generator: " << result.error << std::endl; return DAWN_VALIDATION_ERROR(errorStream.str().c_str()); } - std::vector spirv = generator.result(); + std::vector spirv = result.spirv; // Don't save the transformedParseResult but just create a VkShaderModule VkShaderModuleCreateInfo createInfo;