Dawn: Dump shader module source before checking cache if required
This CL make shader module sources always get dumped if DumpShaders
toggle enabled, regardless of in-memory or on-disk cache hit or not.
Without this CL, shader sources are dumped only if both caches missed
and ParseShaderModule get called.
This CL also fix a typo.
Bug: 42240459, 402772740
Change-Id: I23f87567e726d6055c08293d59b33ef67f549800
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/247674
Reviewed-by: Loko Kung <lokokung@google.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Zhaoming Jiang <zhaoming.jiang@microsoft.com>
diff --git a/src/dawn/native/Device.cpp b/src/dawn/native/Device.cpp
index 9c32973..ee22450 100644
--- a/src/dawn/native/Device.cpp
+++ b/src/dawn/native/Device.cpp
@@ -2151,7 +2151,7 @@
Branch<ShaderSourceSPIRV, DawnShaderModuleSPIRVOptionsDescriptor,
ShaderModuleCompilationOptions>>()));
- // Module type specific validation.
+ // Module type specific validation
switch (moduleType) {
case wgpu::SType::ShaderSourceSPIRV: {
DAWN_INVALID_IF(!TINT_BUILD_SPV_READER || IsToggleEnabled(Toggle::DisallowSpirv),
@@ -2169,6 +2169,11 @@
DAWN_UNREACHABLE();
}
+ // Dump shader source code if required.
+ if (IsToggleEnabled(Toggle::DumpShaders)) {
+ DumpShaderFromDescriptor(this, unpacked);
+ }
+
// Check the cache and do actual validation and parsing if cache missed.
ShaderModuleBase blueprint(this, unpacked, internalExtensions,
ApiObjectBase::kUntrackedByDevice);
diff --git a/src/dawn/native/ShaderModule.cpp b/src/dawn/native/ShaderModule.cpp
index 5f54066..c22a863 100644
--- a/src/dawn/native/ShaderModule.cpp
+++ b/src/dawn/native/ShaderModule.cpp
@@ -1370,17 +1370,38 @@
DAWN_ASSERT(HasError());
}
+void DumpShaderFromDescriptor(LogEmitter* logEmitter,
+ const UnpackedPtr<ShaderModuleDescriptor>& shaderModuleDesc) {
+#if TINT_BUILD_SPV_READER
+ if ([[maybe_unused]] const auto* spirvDesc = shaderModuleDesc.Get<ShaderSourceSPIRV>()) {
+ // Dump SPIR-V if enabled.
+#ifdef DAWN_ENABLE_SPIRV_VALIDATION
+ DumpSpirv(logEmitter, spirvDesc->code, spirvDesc->codeSize);
+#endif // DAWN_ENABLE_SPIRV_VALIDATION
+ return;
+ }
+#else // TINT_BUILD_SPV_READER
+ // SPIR-V is not enabled, so the descriptor should not contain it.
+ DAWN_ASSERT(shaderModuleDesc.Get<ShaderSourceSPIRV>() == nullptr);
+#endif // TINT_BUILD_SPV_READER
+
+ // Dump WGSL.
+ const ShaderSourceWGSL* wgslDesc = shaderModuleDesc.Get<ShaderSourceWGSL>();
+ DAWN_ASSERT(wgslDesc != nullptr);
+ std::ostringstream dumpedMsg;
+ dumpedMsg << "// Dumped WGSL:\n" << std::string_view(wgslDesc->code) << "\n";
+ logEmitter->EmitLog(wgpu::LoggingType::Info, dumpedMsg.str().c_str());
+}
+
ResultOrError<ShaderModuleParseResult> ParseShaderModule(ShaderModuleParseRequest req) {
ShaderModuleParseResult outputParseResult;
const ShaderModuleParseDeviceInfo& deviceInfo = req.deviceInfo;
- LogEmitter* logEmitter = req.logEmitter.UnsafeGetValue();
- bool dumpShaders = deviceInfo.toggles.Has(Toggle::DumpShaders);
#if TINT_BUILD_SPV_READER
// Handling SPIR-V if enabled.
if (std::holds_alternative<ShaderModuleParseSpirvDescription>(req.shaderDescription)) {
- // SpirV toggle should have been validated before chacking cache.
+ // SpirV toggle should have been validated before checking cache.
DAWN_ASSERT(!deviceInfo.toggles.Has(Toggle::DisallowSpirv));
ShaderModuleParseSpirvDescription& spirvDesc =
@@ -1389,7 +1410,7 @@
#ifdef DAWN_ENABLE_SPIRV_VALIDATION
MaybeError validationResult =
- ValidateSpirv(logEmitter, spirvCode.data(), spirvCode.size(), dumpShaders);
+ ValidateSpirv(req.logEmitter.UnsafeGetValue(), spirvCode.data(), spirvCode.size());
// If SpirV validation error occurs, store it into outputParseResult and return.
if (validationResult.IsError()) {
outputParseResult.SetValidationError(validationResult.AcquireError());
@@ -1410,18 +1431,11 @@
if (std::holds_alternative<ShaderModuleParseWGSLDescription>(req.shaderDescription)) {
ShaderModuleParseWGSLDescription wgslDesc =
std::get<ShaderModuleParseWGSLDescription>(req.shaderDescription);
- const StringView& wgsl = wgslDesc.wgsl.UnsafeGetValue();
const std::vector<tint::wgsl::Extension>& internalExtensions =
wgslDesc.internalExtensions.UnsafeGetValue();
-
+ const StringView& wgsl = wgslDesc.wgsl.UnsafeGetValue();
auto tintFile = std::make_unique<tint::Source::File>("", wgsl);
- if (dumpShaders) {
- std::ostringstream dumpedMsg;
- dumpedMsg << "// Dumped WGSL:\n" << std::string_view(wgsl) << "\n";
- logEmitter->EmitLog(wgpu::LoggingType::Info, dumpedMsg.str().c_str());
- }
-
DAWN_TRY(ParseWGSL(std::move(tintFile), deviceInfo.wgslAllowedFeatures, internalExtensions,
&outputParseResult));
}
diff --git a/src/dawn/native/ShaderModule.h b/src/dawn/native/ShaderModule.h
index fe1ad33..4009fb1 100644
--- a/src/dawn/native/ShaderModule.h
+++ b/src/dawn/native/ShaderModule.h
@@ -55,6 +55,7 @@
#include "dawn/native/Forward.h"
#include "dawn/native/IntegerTypes.h"
#include "dawn/native/Limits.h"
+#include "dawn/native/LogEmitter.h"
#include "dawn/native/ObjectBase.h"
#include "dawn/native/PerStage.h"
#include "dawn/native/Serializable.h"
@@ -153,6 +154,9 @@
std::string name;
};
+void DumpShaderFromDescriptor(LogEmitter* logEmitter,
+ const UnpackedPtr<ShaderModuleDescriptor>& shaderModuleDesc);
+
// Parse a shader module from a validated ShaderModuleDescriptor, and generate reflection
// information if required. Validation errors generated during parsing are also made cacheable and
// returned within ShaderModuleParseResult together with compilation messages, rather than as an
diff --git a/src/dawn/native/SpirvValidation.cpp b/src/dawn/native/SpirvValidation.cpp
index 61aac75..3c2d2ce0 100644
--- a/src/dawn/native/SpirvValidation.cpp
+++ b/src/dawn/native/SpirvValidation.cpp
@@ -29,15 +29,13 @@
#include <spirv-tools/libspirv.hpp>
+#include <memory>
#include <sstream>
#include <string>
namespace dawn::native {
-MaybeError ValidateSpirv(LogEmitter* logEmitter,
- const uint32_t* spirv,
- size_t wordCount,
- bool dumpSpirv) {
+MaybeError ValidateSpirv(LogEmitter* logEmitter, const uint32_t* spirv, size_t wordCount) {
spvtools::SpirvTools spirvTools(SPV_ENV_VULKAN_1_1);
spirvTools.SetMessageConsumer([logEmitter](spv_message_level_t level, const char*,
const spv_position_t& position,
@@ -71,17 +69,9 @@
val_opts.SetFriendlyNames(false);
const bool valid = spirvTools.Validate(spirv, wordCount, val_opts);
- if (dumpSpirv || !valid) {
- std::ostringstream dumpedMsg;
- std::string disassembly;
- if (spirvTools.Disassemble(
- spirv, wordCount, &disassembly,
- SPV_BINARY_TO_TEXT_OPTION_FRIENDLY_NAMES | SPV_BINARY_TO_TEXT_OPTION_INDENT)) {
- dumpedMsg << "/* Dumped generated SPIRV disassembly */\n" << disassembly;
- } else {
- dumpedMsg << "/* Failed to disassemble generated SPIRV */";
- }
- logEmitter->EmitLog(wgpu::LoggingType::Info, dumpedMsg.str().c_str());
+ // Dump the generated SPIRV if it is invalid.
+ if (!valid) {
+ DumpSpirv(logEmitter, spirv, wordCount, &spirvTools);
}
DAWN_INVALID_IF(!valid, "Produced invalid SPIRV. Please file a bug at https://crbug.com/tint.");
@@ -89,4 +79,26 @@
return {};
}
+void DumpSpirv(LogEmitter* logEmitter,
+ const uint32_t* spirv,
+ size_t wordCount,
+ spvtools::SpirvTools* spirvTools) {
+ std::unique_ptr<spvtools::SpirvTools> inplaceSpirvTools;
+ if (spirvTools == nullptr) {
+ inplaceSpirvTools = std::make_unique<spvtools::SpirvTools>(SPV_ENV_VULKAN_1_1);
+ spirvTools = inplaceSpirvTools.get();
+ }
+
+ std::ostringstream dumpedMsg;
+ std::string disassembly;
+ if (spirvTools->Disassemble(
+ spirv, wordCount, &disassembly,
+ SPV_BINARY_TO_TEXT_OPTION_FRIENDLY_NAMES | SPV_BINARY_TO_TEXT_OPTION_INDENT)) {
+ dumpedMsg << "/* Dumped SPIRV disassembly */\n" << disassembly;
+ } else {
+ dumpedMsg << "/* Failed to disassemble SPIRV */";
+ }
+ logEmitter->EmitLog(wgpu::LoggingType::Info, dumpedMsg.str().c_str());
+}
+
} // namespace dawn::native
diff --git a/src/dawn/native/SpirvValidation.h b/src/dawn/native/SpirvValidation.h
index a4c3595..9cb703c 100644
--- a/src/dawn/native/SpirvValidation.h
+++ b/src/dawn/native/SpirvValidation.h
@@ -31,12 +31,18 @@
#include "dawn/native/Error.h"
#include "dawn/native/LogEmitter.h"
+namespace spvtools {
+class SpirvTools;
+}
+
namespace dawn::native {
-MaybeError ValidateSpirv(LogEmitter* logEmitter,
- const uint32_t* spirv,
- size_t wordCount,
- bool dumpSpirv);
+MaybeError ValidateSpirv(LogEmitter* logEmitter, const uint32_t* spirv, size_t wordCount);
+
+void DumpSpirv(LogEmitter* logEmitter,
+ const uint32_t* spirv,
+ size_t wordCount,
+ spvtools::SpirvTools* spirvTools = nullptr);
} // namespace dawn::native
diff --git a/src/dawn/native/vulkan/ShaderModuleVk.cpp b/src/dawn/native/vulkan/ShaderModuleVk.cpp
index ba734ad..b2f44aa 100644
--- a/src/dawn/native/vulkan/ShaderModuleVk.cpp
+++ b/src/dawn/native/vulkan/ShaderModuleVk.cpp
@@ -382,8 +382,11 @@
"Vulkan.CompileShaderToSPIRV");
#ifdef DAWN_ENABLE_SPIRV_VALIDATION
- DAWN_TRY(ValidateSpirv(GetDevice(), compilation->spirv.data(), compilation->spirv.size(),
- GetDevice()->IsToggleEnabled(Toggle::DumpShaders)));
+ // Validate and if required dump the compiled SPIR-V code.
+ DAWN_TRY(ValidateSpirv(GetDevice(), compilation->spirv.data(), compilation->spirv.size()));
+ if (GetDevice()->IsToggleEnabled(Toggle::DumpShaders)) {
+ DumpSpirv(GetDevice(), compilation->spirv.data(), compilation->spirv.size());
+ }
#endif
VkShaderModuleCreateInfo createInfo;
