Output more details when pipeline layout mismatches shader declaration
This patch makes Dawn provide more detailed error messages when the
pipeline layout is not compatible with shader module, which is helpful
to debug such errors in WebGPU applications.
BUG=dawn:456
Change-Id: Ib5a870d8e66645481434c4d3dc6fdc1a585aac36
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/22881
Commit-Queue: Kai Ninomiya <kainino@chromium.org>
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
diff --git a/src/dawn_native/Pipeline.cpp b/src/dawn_native/Pipeline.cpp
index 6ca811f..c0c48b6 100644
--- a/src/dawn_native/Pipeline.cpp
+++ b/src/dawn_native/Pipeline.cpp
@@ -33,8 +33,8 @@
if (descriptor->module->GetExecutionModel() != stage) {
return DAWN_VALIDATION_ERROR("Setting module with wrong stages");
}
- if (layout != nullptr && !descriptor->module->IsCompatibleWithPipelineLayout(layout)) {
- return DAWN_VALIDATION_ERROR("Stage not compatible with layout");
+ if (layout != nullptr) {
+ DAWN_TRY(descriptor->module->ValidateCompatibilityWithPipelineLayout(layout));
}
return {};
}
diff --git a/src/dawn_native/PipelineLayout.cpp b/src/dawn_native/PipelineLayout.cpp
index 86c09e1..66a4df9 100644
--- a/src/dawn_native/PipelineLayout.cpp
+++ b/src/dawn_native/PipelineLayout.cpp
@@ -221,7 +221,9 @@
}
for (uint32_t moduleIndex = 0; moduleIndex < count; ++moduleIndex) {
- ASSERT(modules[moduleIndex]->IsCompatibleWithPipelineLayout(pipelineLayout));
+ ASSERT(modules[moduleIndex]
+ ->ValidateCompatibilityWithPipelineLayout(pipelineLayout)
+ .IsSuccess());
}
return pipelineLayout;
diff --git a/src/dawn_native/ShaderModule.cpp b/src/dawn_native/ShaderModule.cpp
index 6536729..5c8dc33 100644
--- a/src/dawn_native/ShaderModule.cpp
+++ b/src/dawn_native/ShaderModule.cpp
@@ -292,6 +292,12 @@
return wgpu::TextureFormat::Undefined;
}
}
+
+ std::string GetShaderDeclarationString(size_t group, uint32_t binding) {
+ std::ostringstream ostream;
+ ostream << "the shader module declaration at set " << group << " binding " << binding;
+ return ostream.str();
+ }
} // anonymous namespace
MaybeError ValidateSpirv(DeviceBase*, const uint32_t* code, uint32_t codeSize) {
@@ -864,25 +870,28 @@
return mExecutionModel;
}
- bool ShaderModuleBase::IsCompatibleWithPipelineLayout(const PipelineLayoutBase* layout) const {
+ MaybeError ShaderModuleBase::ValidateCompatibilityWithPipelineLayout(
+ const PipelineLayoutBase* layout) const {
ASSERT(!IsError());
for (uint32_t group : IterateBitSet(layout->GetBindGroupLayoutsMask())) {
- if (!IsCompatibleWithBindGroupLayout(group, layout->GetBindGroupLayout(group))) {
- return false;
- }
+ DAWN_TRY(
+ ValidateCompatibilityWithBindGroupLayout(group, layout->GetBindGroupLayout(group)));
}
for (uint32_t group : IterateBitSet(~layout->GetBindGroupLayoutsMask())) {
if (mBindingInfo[group].size() > 0) {
- return false;
+ std::ostringstream ostream;
+ ostream << "No bind group layout entry matches the declaration set " << group
+ << " in the shader module";
+ return DAWN_VALIDATION_ERROR(ostream.str());
}
}
- return true;
+ return {};
}
- bool ShaderModuleBase::IsCompatibleWithBindGroupLayout(
+ MaybeError ShaderModuleBase::ValidateCompatibilityWithBindGroupLayout(
size_t group,
const BindGroupLayoutBase* layout) const {
ASSERT(!IsError());
@@ -897,7 +906,8 @@
const auto& bindingIt = bindingMap.find(bindingNumber);
if (bindingIt == bindingMap.end()) {
- return false;
+ return DAWN_VALIDATION_ERROR("Missing bind group layout entry for " +
+ GetShaderDeclarationString(group, bindingNumber));
}
BindingIndex bindingIndex(bindingIt->second);
@@ -922,22 +932,32 @@
moduleInfo.type == wgpu::BindingType::Sampler);
if (!validBindingConversion) {
- return false;
+ return DAWN_VALIDATION_ERROR(
+ "The binding type of the bind group layout entry conflicts " +
+ GetShaderDeclarationString(group, bindingNumber));
}
}
if ((bindingInfo.visibility & StageBit(mExecutionModel)) == 0) {
- return false;
+ return DAWN_VALIDATION_ERROR("The bind group layout entry for " +
+ GetShaderDeclarationString(group, bindingNumber) +
+ " is not visible for the shader stage");
}
switch (bindingInfo.type) {
case wgpu::BindingType::SampledTexture: {
if (bindingInfo.textureComponentType != moduleInfo.textureComponentType) {
- return false;
+ return DAWN_VALIDATION_ERROR(
+ "The textureComponentType of the bind group layout entry is different "
+ "from " +
+ GetShaderDeclarationString(group, bindingNumber));
}
if (bindingInfo.viewDimension != moduleInfo.viewDimension) {
- return false;
+ return DAWN_VALIDATION_ERROR(
+ "The viewDimension of the bind group layout entry is different "
+ "from " +
+ GetShaderDeclarationString(group, bindingNumber));
}
break;
}
@@ -947,10 +967,16 @@
ASSERT(bindingInfo.storageTextureFormat != wgpu::TextureFormat::Undefined);
ASSERT(moduleInfo.storageTextureFormat != wgpu::TextureFormat::Undefined);
if (bindingInfo.storageTextureFormat != moduleInfo.storageTextureFormat) {
- return false;
+ return DAWN_VALIDATION_ERROR(
+ "The storageTextureFormat of the bind group layout entry is different "
+ "from " +
+ GetShaderDeclarationString(group, bindingNumber));
}
if (bindingInfo.viewDimension != moduleInfo.viewDimension) {
- return false;
+ return DAWN_VALIDATION_ERROR(
+ "The viewDimension of the bind group layout entry is different "
+ "from " +
+ GetShaderDeclarationString(group, bindingNumber));
}
break;
}
@@ -965,11 +991,11 @@
case wgpu::BindingType::StorageTexture:
default:
UNREACHABLE();
- return false;
+ return DAWN_VALIDATION_ERROR("Unsupported binding type");
}
}
- return true;
+ return {};
}
size_t ShaderModuleBase::HashFunc::operator()(const ShaderModuleBase* module) const {
diff --git a/src/dawn_native/ShaderModule.h b/src/dawn_native/ShaderModule.h
index 6162d4e..4cd9656 100644
--- a/src/dawn_native/ShaderModule.h
+++ b/src/dawn_native/ShaderModule.h
@@ -75,7 +75,7 @@
using FragmentOutputBaseTypes = std::array<Format::Type, kMaxColorAttachments>;
const FragmentOutputBaseTypes& GetFragmentOutputBaseTypes() const;
- bool IsCompatibleWithPipelineLayout(const PipelineLayoutBase* layout) const;
+ MaybeError ValidateCompatibilityWithPipelineLayout(const PipelineLayoutBase* layout) const;
// Functors necessary for the unordered_set<ShaderModuleBase*>-based cache.
struct HashFunc {
@@ -98,7 +98,9 @@
private:
ShaderModuleBase(DeviceBase* device, ObjectBase::ErrorTag tag);
- bool IsCompatibleWithBindGroupLayout(size_t group, const BindGroupLayoutBase* layout) const;
+ MaybeError ValidateCompatibilityWithBindGroupLayout(
+ size_t group,
+ const BindGroupLayoutBase* layout) const;
// Different implementations reflection into the shader depending on
// whether using spvc, or directly accessing spirv-cross.
