diff --git a/src/dawn_native/CompilationMessages.cpp b/src/dawn_native/CompilationMessages.cpp index 310b794405..f3196541d6 100644 --- a/src/dawn_native/CompilationMessages.cpp +++ b/src/dawn_native/CompilationMessages.cpp @@ -57,7 +57,11 @@ namespace dawn_native { // Cannot add messages after GetCompilationInfo has been called. ASSERT(mCompilationInfo.messages == nullptr); - mMessageStrings.push_back(diagnostic.message); + if (diagnostic.code) { + mMessageStrings.push_back(std::string(diagnostic.code) + ": " + diagnostic.message); + } else { + mMessageStrings.push_back(diagnostic.message); + } mMessages.push_back({nullptr, tintSeverityToMessageType(diagnostic.severity), diagnostic.source.range.begin.line, diagnostic.source.range.begin.column}); diff --git a/src/dawn_native/Device.cpp b/src/dawn_native/Device.cpp index 259020b9a4..91c905f5f7 100644 --- a/src/dawn_native/Device.cpp +++ b/src/dawn_native/Device.cpp @@ -22,6 +22,7 @@ #include "dawn_native/Buffer.h" #include "dawn_native/CommandBuffer.h" #include "dawn_native/CommandEncoder.h" +#include "dawn_native/CompilationMessages.h" #include "dawn_native/ComputePipeline.h" #include "dawn_native/CreatePipelineAsyncTracker.h" #include "dawn_native/DynamicUploader.h" @@ -599,6 +600,8 @@ namespace dawn_native { ResultOrError> DeviceBase::GetOrCreateShaderModule( const ShaderModuleDescriptor* descriptor, ShaderModuleParseResult* parseResult) { + ASSERT(parseResult != nullptr); + ShaderModuleBase blueprint(this, descriptor); const size_t blueprintHash = blueprint.ComputeContentHash(); @@ -609,18 +612,15 @@ namespace dawn_native { if (iter != mCaches->shaderModules.end()) { result = *iter; } else { - if (parseResult == nullptr) { + if (!parseResult->HasParsedShader()) { // We skip the parse on creation if validation isn't enabled which let's us quickly // lookup in the cache without validating and parsing. We need the parsed module // now, so call validate. Most of |ValidateShaderModuleDescriptor| is parsing, but // we can consider splitting it if additional validation is added. ASSERT(!IsValidationEnabled()); - ShaderModuleParseResult localParseResult = - ValidateShaderModuleDescriptor(this, descriptor).AcquireSuccess(); - DAWN_TRY_ASSIGN(result, CreateShaderModuleImpl(descriptor, &localParseResult)); - } else { - DAWN_TRY_ASSIGN(result, CreateShaderModuleImpl(descriptor, parseResult)); + DAWN_TRY(ValidateShaderModuleDescriptor(this, descriptor, parseResult)); } + DAWN_TRY_ASSIGN(result, CreateShaderModuleImpl(descriptor, parseResult)); result->SetIsCachedReference(); result->SetContentHash(blueprintHash); mCaches->shaderModules.insert(result.Get()); @@ -888,8 +888,9 @@ namespace dawn_native { } ShaderModuleBase* DeviceBase::APICreateShaderModule(const ShaderModuleDescriptor* descriptor) { Ref result; - if (ConsumedError(CreateShaderModuleInternal(descriptor), &result)) { - return ShaderModuleBase::MakeError(this); + ShaderModuleParseResult parseResult = {}; + if (ConsumedError(CreateShaderModuleInternal(descriptor, &parseResult), &result)) { + return ShaderModuleBase::MakeError(this, std::move(parseResult.compilationMessages)); } return result.Detach(); } @@ -1242,17 +1243,15 @@ namespace dawn_native { } ResultOrError> DeviceBase::CreateShaderModuleInternal( - const ShaderModuleDescriptor* descriptor) { + const ShaderModuleDescriptor* descriptor, + ShaderModuleParseResult* parseResult) { DAWN_TRY(ValidateIsAlive()); - ShaderModuleParseResult parseResult = {}; - ShaderModuleParseResult* parseResultPtr = nullptr; if (IsValidationEnabled()) { - DAWN_TRY_ASSIGN(parseResult, ValidateShaderModuleDescriptor(this, descriptor)); - parseResultPtr = &parseResult; + DAWN_TRY(ValidateShaderModuleDescriptor(this, descriptor, parseResult)); } - return GetOrCreateShaderModule(descriptor, parseResultPtr); + return GetOrCreateShaderModule(descriptor, parseResult); } ResultOrError> DeviceBase::CreateSwapChainInternal( diff --git a/src/dawn_native/Device.h b/src/dawn_native/Device.h index 5bb6f6b8bc..c5e7ad81d1 100644 --- a/src/dawn_native/Device.h +++ b/src/dawn_native/Device.h @@ -38,6 +38,7 @@ namespace dawn_native { class DynamicUploader; class ErrorScopeStack; class ExternalTextureBase; + class OwnedCompilationMessages; class PersistentCache; class StagingBufferBase; struct InternalPipelineStore; @@ -321,7 +322,8 @@ namespace dawn_native { const RenderPipelineDescriptor2* descriptor); ResultOrError> CreateSamplerInternal(const SamplerDescriptor* descriptor); ResultOrError> CreateShaderModuleInternal( - const ShaderModuleDescriptor* descriptor); + const ShaderModuleDescriptor* descriptor, + ShaderModuleParseResult* parseResult); ResultOrError> CreateSwapChainInternal( Surface* surface, const SwapChainDescriptor* descriptor); diff --git a/src/dawn_native/ShaderModule.cpp b/src/dawn_native/ShaderModule.cpp index f8b2980d27..af6b2d7b1e 100644 --- a/src/dawn_native/ShaderModule.cpp +++ b/src/dawn_native/ShaderModule.cpp @@ -390,11 +390,15 @@ namespace dawn_native { return {}; } - ResultOrError ParseWGSL(const tint::Source::File* file) { + ResultOrError ParseWGSL(const tint::Source::File* file, + OwnedCompilationMessages* outMessages) { std::ostringstream errorStream; errorStream << "Tint WGSL reader failure:" << std::endl; tint::Program program = tint::reader::wgsl::Parse(file); + if (outMessages != nullptr) { + outMessages->AddMessages(program.Diagnostics()); + } if (!program.IsValid()) { auto err = program.Diagnostics().str(); errorStream << "Parser: " << err << std::endl @@ -406,11 +410,15 @@ namespace dawn_native { return std::move(program); } - ResultOrError ParseSPIRV(const std::vector& spirv) { + ResultOrError ParseSPIRV(const std::vector& spirv, + OwnedCompilationMessages* outMessages) { std::ostringstream errorStream; errorStream << "Tint SPIRV reader failure:" << std::endl; tint::Program program = tint::reader::spirv::Parse(spirv); + if (outMessages != nullptr) { + outMessages->AddMessages(program.Diagnostics()); + } if (!program.IsValid()) { auto err = program.Diagnostics().str(); errorStream << "Parser: " << err << std::endl; @@ -420,12 +428,17 @@ namespace dawn_native { return std::move(program); } - MaybeError ValidateModule(const tint::Program* program) { + MaybeError ValidateModule(const tint::Program* program, + OwnedCompilationMessages* outMessages) { std::ostringstream errorStream; errorStream << "Tint program validation" << std::endl; tint::Validator validator; - if (!validator.Validate(program)) { + bool isValid = validator.Validate(program); + if (outMessages != nullptr) { + outMessages->AddMessages(validator.diagnostics()); + } + if (!isValid) { auto err = validator.diagnostics().str(); errorStream << "Validation: " << err << std::endl; return DAWN_VALIDATION_ERROR(errorStream.str().c_str()); @@ -1039,7 +1052,9 @@ namespace dawn_native { } } // anonymous namespace - ShaderModuleParseResult::ShaderModuleParseResult() = default; + ShaderModuleParseResult::ShaderModuleParseResult() + : compilationMessages(new OwnedCompilationMessages()) { + } ShaderModuleParseResult::~ShaderModuleParseResult() = default; ShaderModuleParseResult::ShaderModuleParseResult(ShaderModuleParseResult&& rhs) = default; @@ -1047,9 +1062,15 @@ namespace dawn_native { ShaderModuleParseResult& ShaderModuleParseResult::operator=(ShaderModuleParseResult&& rhs) = default; - ResultOrError ValidateShaderModuleDescriptor( - DeviceBase* device, - const ShaderModuleDescriptor* descriptor) { + bool ShaderModuleParseResult::HasParsedShader() const { + return tintProgram != nullptr || spirv.size() > 0; + } + + MaybeError ValidateShaderModuleDescriptor(DeviceBase* device, + const ShaderModuleDescriptor* descriptor, + ShaderModuleParseResult* parseResult) { + ASSERT(parseResult != nullptr); + const ChainedStruct* chainedDescriptor = descriptor->nextInChain; if (chainedDescriptor == nullptr) { return DAWN_VALIDATION_ERROR("Shader module descriptor missing chained descriptor"); @@ -1060,9 +1081,10 @@ namespace dawn_native { "Shader module descriptor chained nextInChain must be nullptr"); } + OwnedCompilationMessages* outMessages = parseResult->compilationMessages.get(); + ScopedTintICEHandler scopedICEHandler(device); - ShaderModuleParseResult parseResult = {}; switch (chainedDescriptor->sType) { case wgpu::SType::ShaderModuleSPIRVDescriptor: { const auto* spirvDesc = @@ -1070,16 +1092,16 @@ namespace dawn_native { std::vector spirv(spirvDesc->code, spirvDesc->code + spirvDesc->codeSize); if (device->IsToggleEnabled(Toggle::UseTintGenerator)) { tint::Program program; - DAWN_TRY_ASSIGN(program, ParseSPIRV(spirv)); + DAWN_TRY_ASSIGN(program, ParseSPIRV(spirv, outMessages)); if (device->IsValidationEnabled()) { - DAWN_TRY(ValidateModule(&program)); + DAWN_TRY(ValidateModule(&program, outMessages)); } - parseResult.tintProgram = std::make_unique(std::move(program)); + parseResult->tintProgram = std::make_unique(std::move(program)); } else { if (device->IsValidationEnabled()) { DAWN_TRY(ValidateSpirv(spirv.data(), spirv.size())); } - parseResult.spirv = std::move(spirv); + parseResult->spirv = std::move(spirv); } break; } @@ -1091,29 +1113,30 @@ namespace dawn_native { tint::Source::File file("", wgslDesc->source); tint::Program program; - DAWN_TRY_ASSIGN(program, ParseWGSL(&file)); + DAWN_TRY_ASSIGN(program, ParseWGSL(&file, outMessages)); if (device->IsToggleEnabled(Toggle::UseTintGenerator)) { if (device->IsValidationEnabled()) { - DAWN_TRY(ValidateModule(&program)); + DAWN_TRY(ValidateModule(&program, outMessages)); } - parseResult.tintProgram = std::make_unique(std::move(program)); + parseResult->tintProgram = std::make_unique(std::move(program)); } else { tint::transform::Manager transformManager; transformManager.append( std::make_unique()); transformManager.append(std::make_unique()); - DAWN_TRY_ASSIGN(program, RunTransforms(&transformManager, &program)); + DAWN_TRY_ASSIGN(program, + RunTransforms(&transformManager, &program, outMessages)); if (device->IsValidationEnabled()) { - DAWN_TRY(ValidateModule(&program)); + DAWN_TRY(ValidateModule(&program, outMessages)); } std::vector spirv; DAWN_TRY_ASSIGN(spirv, ModuleToSPIRV(&program)); DAWN_TRY(ValidateSpirv(spirv.data(), spirv.size())); - parseResult.spirv = std::move(spirv); + parseResult->spirv = std::move(spirv); } break; } @@ -1121,7 +1144,7 @@ namespace dawn_native { return DAWN_VALIDATION_ERROR("Unsupported sType"); } - return std::move(parseResult); + return {}; } RequiredBufferSizes ComputeRequiredBufferSizesForLayout(const EntryPointMetadata& entryPoint, @@ -1136,8 +1159,12 @@ namespace dawn_native { } ResultOrError RunTransforms(tint::transform::Transform* transform, - const tint::Program* program) { + const tint::Program* program, + OwnedCompilationMessages* outMessages) { tint::transform::Transform::Output output = transform->Run(program); + if (outMessages != nullptr) { + outMessages->AddMessages(output.program.Diagnostics()); + } if (!output.program.IsValid()) { std::string err = "Tint program failure: " + output.program.Diagnostics().str(); return DAWN_VALIDATION_ERROR(err.c_str()); @@ -1196,9 +1223,7 @@ namespace dawn_native { // ShaderModuleBase ShaderModuleBase::ShaderModuleBase(DeviceBase* device, const ShaderModuleDescriptor* descriptor) - : CachedObject(device), - mType(Type::Undefined), - mCompilationMessages(std::make_unique()) { + : CachedObject(device), mType(Type::Undefined) { ASSERT(descriptor->nextInChain != nullptr); switch (descriptor->nextInChain->sType) { case wgpu::SType::ShaderModuleSPIRVDescriptor: { @@ -1220,10 +1245,13 @@ namespace dawn_native { } } - ShaderModuleBase::ShaderModuleBase(DeviceBase* device, ObjectBase::ErrorTag tag) + ShaderModuleBase::ShaderModuleBase( + DeviceBase* device, + ObjectBase::ErrorTag tag, + std::unique_ptr compilationMessages) : CachedObject(device, tag), mType(Type::Undefined), - mCompilationMessages(std::make_unique()) { + mCompilationMessages(std::move(compilationMessages)) { } ShaderModuleBase::~ShaderModuleBase() { @@ -1233,8 +1261,10 @@ namespace dawn_native { } // static - ShaderModuleBase* ShaderModuleBase::MakeError(DeviceBase* device) { - return new ShaderModuleBase(device, ObjectBase::kError); + ShaderModuleBase* ShaderModuleBase::MakeError( + DeviceBase* device, + std::unique_ptr compilationMessages) { + return new ShaderModuleBase(device, ObjectBase::kError, std::move(compilationMessages)); } bool ShaderModuleBase::HasEntryPoint(const std::string& entryPoint) const { @@ -1286,7 +1316,7 @@ namespace dawn_native { const std::string& entryPoint, BindGroupIndex pullingBufferBindingSet) const { tint::Program program; - DAWN_TRY_ASSIGN(program, ParseSPIRV(spirv)); + DAWN_TRY_ASSIGN(program, ParseSPIRV(spirv, nullptr)); return GeneratePullingSpirv(&program, vertexState, entryPoint, pullingBufferBindingSet); } @@ -1308,8 +1338,11 @@ namespace dawn_native { transformManager.append(std::make_unique()); } + // A nullptr is passed in for the CompilationMessages here since this method is called + // during RenderPipeline creation, by which point the shader module's CompilationInfo may + // have already been queried. tint::Program program; - DAWN_TRY_ASSIGN(program, RunTransforms(&transformManager, programIn)); + DAWN_TRY_ASSIGN(program, RunTransforms(&transformManager, programIn, nullptr)); tint::writer::spirv::Generator generator(&program); if (!generator.Generate()) { @@ -1325,6 +1358,7 @@ namespace dawn_native { MaybeError ShaderModuleBase::InitializeBase(ShaderModuleParseResult* parseResult) { mTintProgram = std::move(parseResult->tintProgram); mSpirv = std::move(parseResult->spirv); + mCompilationMessages = std::move(parseResult->compilationMessages); if (GetDevice()->IsToggleEnabled(Toggle::UseTintGenerator)) { DAWN_TRY_ASSIGN(mEntryPoints, ReflectShaderUsingTint(GetDevice(), mTintProgram.get())); diff --git a/src/dawn_native/ShaderModule.h b/src/dawn_native/ShaderModule.h index 059dee96fc..2e8c3af949 100644 --- a/src/dawn_native/ShaderModule.h +++ b/src/dawn_native/ShaderModule.h @@ -19,6 +19,7 @@ #include "common/ityp_array.h" #include "dawn_native/BindingInfo.h" #include "dawn_native/CachedObject.h" +#include "dawn_native/CompilationMessages.h" #include "dawn_native/Error.h" #include "dawn_native/Format.h" #include "dawn_native/Forward.h" @@ -48,7 +49,6 @@ namespace spirv_cross { namespace dawn_native { - class OwnedCompilationMessages; struct EntryPointMetadata; // A map from name to EntryPointMetadata. @@ -61,13 +61,16 @@ namespace dawn_native { ShaderModuleParseResult(ShaderModuleParseResult&& rhs); ShaderModuleParseResult& operator=(ShaderModuleParseResult&& rhs); + bool HasParsedShader() const; + std::unique_ptr tintProgram; std::vector spirv; + std::unique_ptr compilationMessages; }; - ResultOrError ValidateShaderModuleDescriptor( - DeviceBase* device, - const ShaderModuleDescriptor* descriptor); + MaybeError ValidateShaderModuleDescriptor(DeviceBase* device, + const ShaderModuleDescriptor* descriptor, + ShaderModuleParseResult* parseResult); MaybeError ValidateCompatibilityWithPipelineLayout(DeviceBase* device, const EntryPointMetadata& entryPoint, const PipelineLayoutBase* layout); @@ -75,7 +78,8 @@ namespace dawn_native { RequiredBufferSizes ComputeRequiredBufferSizesForLayout(const EntryPointMetadata& entryPoint, const PipelineLayoutBase* layout); ResultOrError RunTransforms(tint::transform::Transform* transform, - const tint::Program* program); + const tint::Program* program, + OwnedCompilationMessages* messages); std::unique_ptr MakeVertexPullingTransform( const VertexState& vertexState, @@ -125,7 +129,9 @@ namespace dawn_native { ShaderModuleBase(DeviceBase* device, const ShaderModuleDescriptor* descriptor); ~ShaderModuleBase() override; - static ShaderModuleBase* MakeError(DeviceBase* device); + static ShaderModuleBase* MakeError( + DeviceBase* device, + std::unique_ptr compilationMessages); // Return true iff the program has an entrypoint called `entryPoint`. bool HasEntryPoint(const std::string& entryPoint) const; @@ -169,7 +175,9 @@ namespace dawn_native { const std::vector& spirv); private: - ShaderModuleBase(DeviceBase* device, ObjectBase::ErrorTag tag); + ShaderModuleBase(DeviceBase* device, + ObjectBase::ErrorTag tag, + std::unique_ptr compilationMessages); // The original data in the descriptor for caching. enum class Type { Undefined, Spirv, Wgsl }; diff --git a/src/dawn_native/vulkan/ShaderModuleVk.cpp b/src/dawn_native/vulkan/ShaderModuleVk.cpp index 01ed4bf072..0fb4c610b4 100644 --- a/src/dawn_native/vulkan/ShaderModuleVk.cpp +++ b/src/dawn_native/vulkan/ShaderModuleVk.cpp @@ -59,7 +59,8 @@ namespace dawn_native { namespace vulkan { tint::Program program; DAWN_TRY_ASSIGN(program, - RunTransforms(&transformManager, parseResult->tintProgram.get())); + RunTransforms(&transformManager, parseResult->tintProgram.get(), + CompilationMessages())); tint::writer::spirv::Generator generator(&program); if (!generator.Generate()) { diff --git a/src/tests/unittests/validation/ShaderModuleValidationTests.cpp b/src/tests/unittests/validation/ShaderModuleValidationTests.cpp index ceb6e183cf..73d32a56a5 100644 --- a/src/tests/unittests/validation/ShaderModuleValidationTests.cpp +++ b/src/tests/unittests/validation/ShaderModuleValidationTests.cpp @@ -14,7 +14,6 @@ #include "common/Constants.h" -#include "dawn_native/CompilationMessages.h" #include "dawn_native/ShaderModule.h" #include "tests/unittests/validation/ValidationTest.h"