From 37ad44eef201503fea5cb85b96e97dc61fd3d433 Mon Sep 17 00:00:00 2001 From: Brandon Jones Date: Thu, 25 Mar 2021 15:37:44 +0000 Subject: [PATCH] Allow Dawn to surface Tint internal compiler errors as uncaptured validation errors Raises any ICE errors reported by Tint during shader validation as uncaptured validation errors. This allows them to show up in Chrome's dev tools console, for example. BUG: dawn:718 Change-Id: I85741787103e6c1174b7c73be6b9860b988d6130 Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/45840 Reviewed-by: Ben Clayton Reviewed-by: Corentin Wallez Reviewed-by: Ben Clayton Commit-Queue: Brandon Jones --- src/dawn_native/BUILD.gn | 2 + src/dawn_native/CMakeLists.txt | 2 + src/dawn_native/ShaderModule.cpp | 3 ++ src/dawn_native/TintUtils.cpp | 55 +++++++++++++++++++++ src/dawn_native/TintUtils.h | 37 ++++++++++++++ src/dawn_native/d3d12/ShaderModuleD3D12.cpp | 4 ++ src/dawn_native/metal/ShaderModuleMTL.mm | 4 ++ src/dawn_native/opengl/ShaderModuleGL.cpp | 3 ++ src/dawn_native/vulkan/ShaderModuleVk.cpp | 3 ++ 9 files changed, 113 insertions(+) create mode 100644 src/dawn_native/TintUtils.cpp create mode 100644 src/dawn_native/TintUtils.h diff --git a/src/dawn_native/BUILD.gn b/src/dawn_native/BUILD.gn index d1eb49b5ab..b3bd28ce47 100644 --- a/src/dawn_native/BUILD.gn +++ b/src/dawn_native/BUILD.gn @@ -287,6 +287,8 @@ source_set("dawn_native_sources") { "SwapChain.h", "Texture.cpp", "Texture.h", + "TintUtils.cpp", + "TintUtils.h", "ToBackend.h", "Toggles.cpp", "Toggles.h", diff --git a/src/dawn_native/CMakeLists.txt b/src/dawn_native/CMakeLists.txt index 215be38800..915ffd87b0 100644 --- a/src/dawn_native/CMakeLists.txt +++ b/src/dawn_native/CMakeLists.txt @@ -155,6 +155,8 @@ target_sources(dawn_native PRIVATE "SwapChain.h" "Texture.cpp" "Texture.h" + "TintUtils.cpp" + "TintUtils.h" "ToBackend.h" "Toggles.cpp" "Toggles.h" diff --git a/src/dawn_native/ShaderModule.cpp b/src/dawn_native/ShaderModule.cpp index ba55707bb8..f1ff68bbf1 100644 --- a/src/dawn_native/ShaderModule.cpp +++ b/src/dawn_native/ShaderModule.cpp @@ -22,6 +22,7 @@ #include "dawn_native/PipelineLayout.h" #include "dawn_native/RenderPipeline.h" #include "dawn_native/SpirvUtils.h" +#include "dawn_native/TintUtils.h" #include #include @@ -947,6 +948,8 @@ namespace dawn_native { "Shader module descriptor chained nextInChain must be nullptr"); } + ScopedTintICEHandler scopedICEHandler(device); + ShaderModuleParseResult parseResult = {}; switch (chainedDescriptor->sType) { case wgpu::SType::ShaderModuleSPIRVDescriptor: { diff --git a/src/dawn_native/TintUtils.cpp b/src/dawn_native/TintUtils.cpp new file mode 100644 index 0000000000..7315904fef --- /dev/null +++ b/src/dawn_native/TintUtils.cpp @@ -0,0 +1,55 @@ +// Copyright 2021 The Dawn Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "dawn_native/TintUtils.h" +#include "dawn_native/Device.h" + +#include + +namespace dawn_native { + + namespace { + + thread_local DeviceBase* tlDevice = nullptr; + + void TintICEReporter(const tint::diag::List& diagnostics) { + if (tlDevice) { + tlDevice->HandleError(InternalErrorType::Validation, diagnostics.str().c_str()); + } + } + + bool InitializeTintErrorReporter() { + tint::SetInternalCompilerErrorReporter(&TintICEReporter); + return true; + } + + } // namespace + + ScopedTintICEHandler::ScopedTintICEHandler(DeviceBase* device) { + // Call tint::SetInternalCompilerErrorReporter() the first time + // this constructor is called. Static initialization is + // guaranteed to be thread-safe, and only occur once. + static bool init_once_tint_error_reporter = InitializeTintErrorReporter(); + (void)init_once_tint_error_reporter; + + // Shouldn't have overlapping instances of this handler. + ASSERT(tlDevice == nullptr); + tlDevice = device; + } + + ScopedTintICEHandler::~ScopedTintICEHandler() { + tlDevice = nullptr; + } + +} // namespace dawn_native \ No newline at end of file diff --git a/src/dawn_native/TintUtils.h b/src/dawn_native/TintUtils.h new file mode 100644 index 0000000000..c3761f69ff --- /dev/null +++ b/src/dawn_native/TintUtils.h @@ -0,0 +1,37 @@ +// Copyright 2021 The Dawn Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef DAWNNATIVE_TINTUTILS_H_ +#define DAWNNATIVE_TINTUTILS_H_ + +#include "common/NonCopyable.h" + +namespace dawn_native { + + class DeviceBase; + + // Indicates that for the lifetime of this object tint internal compiler errors should be + // reported to the given device. + class ScopedTintICEHandler : public NonCopyable { + public: + ScopedTintICEHandler(DeviceBase* device); + ~ScopedTintICEHandler(); + + private: + ScopedTintICEHandler(ScopedTintICEHandler&&) = delete; + }; + +} // namespace dawn_native + +#endif // DAWNNATIVE_TEXTURE_H_ \ No newline at end of file diff --git a/src/dawn_native/d3d12/ShaderModuleD3D12.cpp b/src/dawn_native/d3d12/ShaderModuleD3D12.cpp index b800d835bf..0609111a46 100644 --- a/src/dawn_native/d3d12/ShaderModuleD3D12.cpp +++ b/src/dawn_native/d3d12/ShaderModuleD3D12.cpp @@ -18,6 +18,7 @@ #include "common/BitSetIterator.h" #include "common/Log.h" #include "dawn_native/SpirvUtils.h" +#include "dawn_native/TintUtils.h" #include "dawn_native/d3d12/BindGroupLayoutD3D12.h" #include "dawn_native/d3d12/D3D12Error.h" #include "dawn_native/d3d12/DeviceD3D12.h" @@ -184,6 +185,7 @@ namespace dawn_native { namespace d3d12 { } MaybeError ShaderModule::Initialize(ShaderModuleParseResult* parseResult) { + ScopedTintICEHandler scopedICEHandler(GetDevice()); return InitializeBase(parseResult); } @@ -195,6 +197,8 @@ namespace dawn_native { namespace d3d12 { FirstOffsetInfo* firstOffsetInfo) const { ASSERT(!IsError()); + ScopedTintICEHandler scopedICEHandler(GetDevice()); + std::ostringstream errorStream; errorStream << "Tint HLSL failure:" << std::endl; diff --git a/src/dawn_native/metal/ShaderModuleMTL.mm b/src/dawn_native/metal/ShaderModuleMTL.mm index 9cc493d78b..fce4300864 100644 --- a/src/dawn_native/metal/ShaderModuleMTL.mm +++ b/src/dawn_native/metal/ShaderModuleMTL.mm @@ -16,6 +16,7 @@ #include "dawn_native/BindGroupLayout.h" #include "dawn_native/SpirvUtils.h" +#include "dawn_native/TintUtils.h" #include "dawn_native/metal/DeviceMTL.h" #include "dawn_native/metal/PipelineLayoutMTL.h" #include "dawn_native/metal/RenderPipelineMTL.h" @@ -46,6 +47,7 @@ namespace dawn_native { namespace metal { } MaybeError ShaderModule::Initialize(ShaderModuleParseResult* parseResult) { + ScopedTintICEHandler scopedICEHandler(GetDevice()); return InitializeBase(parseResult); } @@ -62,6 +64,8 @@ namespace dawn_native { namespace metal { // TODO(crbug.com/tint/256): Set this accordingly if arrayLength(..) is used. *needsStorageBufferLength = false; + ScopedTintICEHandler scopedICEHandler(GetDevice()); + std::ostringstream errorStream; errorStream << "Tint MSL failure:" << std::endl; diff --git a/src/dawn_native/opengl/ShaderModuleGL.cpp b/src/dawn_native/opengl/ShaderModuleGL.cpp index 67d9197c51..c4d060a696 100644 --- a/src/dawn_native/opengl/ShaderModuleGL.cpp +++ b/src/dawn_native/opengl/ShaderModuleGL.cpp @@ -18,6 +18,7 @@ #include "common/Platform.h" #include "dawn_native/BindGroupLayout.h" #include "dawn_native/SpirvUtils.h" +#include "dawn_native/TintUtils.h" #include "dawn_native/opengl/DeviceGL.h" #include "dawn_native/opengl/PipelineLayoutGL.h" @@ -77,6 +78,8 @@ namespace dawn_native { namespace opengl { } MaybeError ShaderModule::Initialize(ShaderModuleParseResult* parseResult) { + ScopedTintICEHandler scopedICEHandler(GetDevice()); + if (GetDevice()->IsToggleEnabled(Toggle::UseTintGenerator)) { std::ostringstream errorStream; errorStream << "Tint SPIR-V (for GLSL) writer failure:" << std::endl; diff --git a/src/dawn_native/vulkan/ShaderModuleVk.cpp b/src/dawn_native/vulkan/ShaderModuleVk.cpp index f905fee51a..5fddba2a84 100644 --- a/src/dawn_native/vulkan/ShaderModuleVk.cpp +++ b/src/dawn_native/vulkan/ShaderModuleVk.cpp @@ -14,6 +14,7 @@ #include "dawn_native/vulkan/ShaderModuleVk.h" +#include "dawn_native/TintUtils.h" #include "dawn_native/vulkan/DeviceVk.h" #include "dawn_native/vulkan/FencedDeleter.h" #include "dawn_native/vulkan/VulkanError.h" @@ -48,6 +49,8 @@ namespace dawn_native { namespace vulkan { std::vector spirv; const std::vector* spirvPtr; + ScopedTintICEHandler scopedICEHandler(GetDevice()); + if (GetDevice()->IsToggleEnabled(Toggle::UseTintGenerator)) { std::ostringstream errorStream; errorStream << "Tint SPIR-V writer failure:" << std::endl;