Enable running robust buffer access pass on shaders
In addition to adding the pass running logic, how shaders are stored
in the ShaderModule is changed, so that cached shader that was used to
create the ShaderModule is differentiated from the
modified/intermediate shader that is actually being passed to
SPIRV-Cross.
Also rolling SPIRV-Tools to pick up a fix for a bug I discovered in
the RBA pass implementation.
Roll third_party/SPIRV-Tools/ fd05605be..34ef0c3fd (25 commits)
https://chromium.googlesource.com/external/github.com/KhronosGroup/SPIRV-Tools/+log/fd05605bef1c..34ef0c3fdc8e
$ git log fd05605be..34ef0c3fd --date=short --no-merges --format='%ad %ae %s'
2020-09-16 rharrison Fix missed modification flagging (#3814)
2020-09-16 andreperezmaselco.developer spirv-fuzz: Use an irrelevant id for the unused components (#3810)
2020-09-16 stefanomil spirv-fuzz: Improvements to random number generation (#3809)
2020-09-16 greg Add buffer oob check to bindless instrumentation (#3800)
2020-09-16 vasniktel spirv-fuzz: Remove CanFindOrCreateZeroConstant (#3807)
2020-09-15 andreperezmaselco.developer spirv-fuzz: Add bit instruction synonym transformation (#3775)
2020-09-16 vasniktel spirv-fuzz: Skip unreachable blocks (#3729)
2020-09-15 afdx Fix build errors (#3804)
2020-09-15 vasniktel spirv-fuzz: Handle invalid ids in fact manager (#3742)
2020-09-15 vasniktel spirv-fuzz: Support memory instructions MoveInstructionDown (#3700)
2020-09-15 stefanomil spirv-fuzz: Pass submanagers to other submanagers when necessary (#3796)
2020-09-15 stefanomil spirv-fuzz: Transformation to flatten conditional branch (#3667)
2020-09-14 46493288+sfricke-samsung spirv-val: Add BaseInstance, BaseVertex, DrawIndex, and ViewIndex (#3782)
2020-09-14 dnovillo Properly mark IR changed if instruction folder creates more than one constant. (#3799)
2020-09-11 afdx Add missing file to BUILD.gn (#3798)
2020-09-11 antonikarp spirv-fuzz: Add TransformationDuplicateRegionWithSelection (#3773)
2020-09-11 afdx spirv-reduce: Support reducing a specific function (#3774)
2020-09-10 afdx spirv-reduce: Refactoring (#3793)
2020-09-10 afdx Favour 'integrity' over 'coherence' as a replacement for 'sanity'. (#3619)
2020-09-10 antonikarp spirv-fuzz: Fix header guards in transformations/fuzzer passes (#3784)
2020-09-10 paulthomson spirv-fuzz: Add SPIRV_FUZZ_PROTOC_COMMAND (#3789)
2020-09-10 paulthomson Add missing include (#3788)
2020-09-09 paulthomson Improve spirv-fuzz CMake code (#3781)
2020-09-08 stevenperron Allow SPV_KHR_8bit_storage extension. (#3780)
2020-09-08 stefanomil spirv-opt: Add function to compute nesting depth of a block (#3771)
Created with:
roll-dep third_party/SPIRV-Tools
BUG=dawn:523,dawn:480
Change-Id: I1f424f5fe6d67999412f286e831ea2ea26372b9e
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/28540
Reviewed-by: Kai Ninomiya <kainino@chromium.org>
Commit-Queue: Ryan Harrison <rharrison@chromium.org>
diff --git a/DEPS b/DEPS
index a04cc80..634799b 100644
--- a/DEPS
+++ b/DEPS
@@ -72,7 +72,7 @@
# SPIRV compiler dependencies: SPIRV-Tools, SPIRV-headers, glslang and shaderc
'third_party/SPIRV-Tools': {
- 'url': '{chromium_git}/external/github.com/KhronosGroup/SPIRV-Tools@fd05605bef1c63c6f249c19ed90c7ba24f8a8150',
+ 'url': '{chromium_git}/external/github.com/KhronosGroup/SPIRV-Tools@34ef0c3fdc8e235fd6217a64c824b6ceac93be60',
'condition': 'dawn_standalone',
},
'third_party/spirv-headers': {
diff --git a/src/dawn_native/BUILD.gn b/src/dawn_native/BUILD.gn
index 51a6761..23bc55a 100644
--- a/src/dawn_native/BUILD.gn
+++ b/src/dawn_native/BUILD.gn
@@ -130,6 +130,7 @@
":dawn_native_utils_gen",
"${dawn_root}/src/common",
"${dawn_root}/third_party/gn/spirv_cross:spirv_cross",
+ "${dawn_spirv_tools_dir}:spvtools_opt",
"${dawn_spirv_tools_dir}:spvtools_val",
]
defines = []
diff --git a/src/dawn_native/ShaderModule.cpp b/src/dawn_native/ShaderModule.cpp
index 6b24b05..6513826 100644
--- a/src/dawn_native/ShaderModule.cpp
+++ b/src/dawn_native/ShaderModule.cpp
@@ -22,6 +22,7 @@
#include "dawn_native/SpirvUtils.h"
#include <spirv-tools/libspirv.hpp>
+#include <spirv-tools/optimizer.hpp>
#include <spirv_cross.hpp>
#ifdef DAWN_ENABLE_WGSL
@@ -327,6 +328,44 @@
return requiredBufferSizes;
}
+ ResultOrError<std::vector<uint32_t>> RunRobustBufferAccessPass(
+ const std::vector<uint32_t>& spirv) {
+ spvtools::Optimizer opt(SPV_ENV_VULKAN_1_1);
+
+ std::ostringstream errorStream;
+ errorStream << "SPIRV Optimizer failure:" << std::endl;
+ opt.SetMessageConsumer([&errorStream](spv_message_level_t level, const char*,
+ const spv_position_t& position,
+ const char* message) {
+ switch (level) {
+ case SPV_MSG_FATAL:
+ case SPV_MSG_INTERNAL_ERROR:
+ case SPV_MSG_ERROR:
+ errorStream << "error: line " << position.index << ": " << message
+ << std::endl;
+ break;
+ case SPV_MSG_WARNING:
+ errorStream << "warning: line " << position.index << ": " << message
+ << std::endl;
+ break;
+ case SPV_MSG_INFO:
+ errorStream << "info: line " << position.index << ": " << message
+ << std::endl;
+ break;
+ default:
+ break;
+ }
+ });
+ opt.RegisterPass(spvtools::CreateGraphicsRobustAccessPass());
+
+ std::vector<uint32_t> result;
+ if (!opt.Run(spirv.data(), spirv.size(), &result, spvtools::ValidatorOptions(),
+ false)) {
+ return DAWN_VALIDATION_ERROR(errorStream.str().c_str());
+ }
+ return std::move(result);
+ }
+
MaybeError ValidateCompatibilityWithBindGroupLayout(BindGroupIndex group,
const EntryPointMetadata& entryPoint,
const BindGroupLayoutBase* layout) {
@@ -741,7 +780,7 @@
mType = Type::Spirv;
const auto* spirvDesc =
static_cast<const ShaderModuleSPIRVDescriptor*>(descriptor->nextInChain);
- mSpirv.assign(spirvDesc->code, spirvDesc->code + spirvDesc->codeSize);
+ mOriginalSpirv.assign(spirvDesc->code, spirvDesc->code + spirvDesc->codeSize);
break;
}
case wgpu::SType::ShaderModuleWGSLDescriptor: {
@@ -789,16 +828,23 @@
size_t ShaderModuleBase::HashFunc::operator()(const ShaderModuleBase* module) const {
size_t hash = 0;
- for (uint32_t word : module->mSpirv) {
+ HashCombine(&hash, module->mType);
+
+ for (uint32_t word : module->mOriginalSpirv) {
HashCombine(&hash, word);
}
+ for (char c : module->mWgsl) {
+ HashCombine(&hash, c);
+ }
+
return hash;
}
bool ShaderModuleBase::EqualityFunc::operator()(const ShaderModuleBase* a,
const ShaderModuleBase* b) const {
- return a->mSpirv == b->mSpirv;
+ return a->mType == b->mType && a->mOriginalSpirv == b->mOriginalSpirv &&
+ a->mWgsl == b->mWgsl;
}
const std::vector<uint32_t>& ShaderModuleBase::GetSpirv() const {
@@ -810,20 +856,35 @@
const VertexStateDescriptor& vertexState,
const std::string& entryPoint,
uint32_t pullingBufferBindingSet) const {
- return ConvertWGSLToSPIRVWithPulling(mWgsl.c_str(), vertexState, entryPoint,
- pullingBufferBindingSet);
+ std::vector<uint32_t> spirv;
+ DAWN_TRY_ASSIGN(spirv, ConvertWGSLToSPIRVWithPulling(mWgsl.c_str(), vertexState, entryPoint,
+ pullingBufferBindingSet));
+ if (GetDevice()->IsRobustnessEnabled()) {
+ DAWN_TRY_ASSIGN(spirv, RunRobustBufferAccessPass(spirv));
+ }
+
+ return std::move(spirv);
}
#endif
MaybeError ShaderModuleBase::InitializeBase() {
+ std::vector<uint32_t> spirv;
if (mType == Type::Wgsl) {
#ifdef DAWN_ENABLE_WGSL
- DAWN_TRY_ASSIGN(mSpirv, ConvertWGSLToSPIRV(mWgsl.c_str()));
+ DAWN_TRY_ASSIGN(spirv, ConvertWGSLToSPIRV(mWgsl.c_str()));
#else
return DAWN_VALIDATION_ERROR("WGSL not supported (yet)");
#endif // DAWN_ENABLE_WGSL
+ } else {
+ spirv = mOriginalSpirv;
}
+ if (GetDevice()->IsRobustnessEnabled()) {
+ DAWN_TRY_ASSIGN(spirv, RunRobustBufferAccessPass(spirv));
+ }
+
+ mSpirv = std::move(spirv);
+
spirv_cross::Compiler compiler(mSpirv);
for (const spirv_cross::EntryPoint& entryPoint : compiler.get_entry_points_and_stages()) {
SingleShaderStage stage = ExecutionModelToShaderStage(entryPoint.execution_model);
diff --git a/src/dawn_native/ShaderModule.h b/src/dawn_native/ShaderModule.h
index 6f9f2a3..922b2bc 100644
--- a/src/dawn_native/ShaderModule.h
+++ b/src/dawn_native/ShaderModule.h
@@ -129,6 +129,7 @@
enum class Type { Undefined, Spirv, Wgsl };
Type mType;
+ std::vector<uint32_t> mOriginalSpirv;
std::vector<uint32_t> mSpirv;
std::string mWgsl;
diff --git a/src/tests/end2end/VertexBufferRobustnessTests.cpp b/src/tests/end2end/VertexBufferRobustnessTests.cpp
index 0605297..b2ae2c4 100644
--- a/src/tests/end2end/VertexBufferRobustnessTests.cpp
+++ b/src/tests/end2end/VertexBufferRobustnessTests.cpp
@@ -19,15 +19,12 @@
#include "utils/ComboRenderPipelineDescriptor.h"
#include "utils/WGPUHelpers.h"
-// TODO(rharrison): Re-enable as part of https://bugs.chromium.org/p/dawn/issues/detail?id=523
-
// Vertex buffer robustness tests that clamping is applied on vertex attributes. This would happen
// on backends where vertex pulling is enabled, such as Metal.
class VertexBufferRobustnessTest : public DawnTest {
protected:
void SetUp() override {
DawnTest::SetUp();
- GTEST_SKIP();
}
// Creates a vertex module that tests an expression with given attributes. If successful, the
