Move tint writer outputs into the SPIRV request.
This CL moves the options which are passed to the tint writer for the
SPIR-V backend out of the compilation callback and passes them directly
through the `SpirvCompilationRequest`. This should incur less copying of
the data in the request.
Change-Id: I1a91ec348001fc8d463fc424d51db60d4723d939
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/154100
Commit-Queue: dan sinclair <dsinclair@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Ben Clayton <bclayton@google.com>
diff --git a/src/dawn/native/StreamImplTint.cpp b/src/dawn/native/StreamImplTint.cpp
index 3cc052c..d1c4f9e 100644
--- a/src/dawn/native/StreamImplTint.cpp
+++ b/src/dawn/native/StreamImplTint.cpp
@@ -60,6 +60,14 @@
#if TINT_BUILD_SPV_WRITER
// static
template <>
+void stream::Stream<tint::spirv::writer::Options>::Write(
+ stream::Sink* sink,
+ const tint::spirv::writer::Options& options) {
+ StreamInTintObject(options, sink);
+}
+
+// static
+template <>
void stream::Stream<tint::spirv::writer::Bindings>::Write(
stream::Sink* sink,
const tint::spirv::writer::Bindings& bindings) {
diff --git a/src/dawn/native/vulkan/ShaderModuleVk.cpp b/src/dawn/native/vulkan/ShaderModuleVk.cpp
index 3698afc..d425e93 100644
--- a/src/dawn/native/vulkan/ShaderModuleVk.cpp
+++ b/src/dawn/native/vulkan/ShaderModuleVk.cpp
@@ -172,20 +172,11 @@
#define SPIRV_COMPILATION_REQUEST_MEMBERS(X) \
X(SingleShaderStage, stage) \
X(const tint::Program*, inputProgram) \
- X(tint::spirv::writer::Bindings, bindings) \
X(std::optional<tint::ast::transform::SubstituteOverride::Config>, substituteOverrideConfig) \
X(LimitsForCompilationRequest, limits) \
X(std::string_view, entryPointName) \
- X(bool, isRobustnessEnabled) \
- X(bool, disableWorkgroupInit) \
X(bool, disableSymbolRenaming) \
- X(bool, useZeroInitializeWorkgroupMemoryExtension) \
- X(bool, clampFragDepth) \
- X(bool, disableImageRobustness) \
- X(bool, disableRuntimeSizedArrayIndexClamping) \
- X(bool, experimentalRequireSubgroupUniformControlFlow) \
- X(bool, passMatrixByPointer) \
- X(bool, useTintIR) \
+ X(tint::spirv::writer::Options, tintOptions) \
X(CacheKey::UnsafeUnkeyedValue<dawn::platform::Platform*>, platform)
DAWN_MAKE_CACHE_REQUEST(SpirvCompilationRequest, SPIRV_COMPILATION_REQUEST_MEMBERS);
@@ -301,31 +292,38 @@
SpirvCompilationRequest req = {};
req.stage = stage;
req.inputProgram = GetTintProgram();
- req.bindings = std::move(bindings);
req.entryPointName = programmableStage.entryPoint;
- req.isRobustnessEnabled = GetDevice()->IsRobustnessEnabled();
- req.disableWorkgroupInit = GetDevice()->IsToggleEnabled(Toggle::DisableWorkgroupInit);
req.disableSymbolRenaming = GetDevice()->IsToggleEnabled(Toggle::DisableSymbolRenaming);
- req.useZeroInitializeWorkgroupMemoryExtension =
- GetDevice()->IsToggleEnabled(Toggle::VulkanUseZeroInitializeWorkgroupMemoryExtension);
- req.clampFragDepth = clampFragDepth;
- req.disableImageRobustness = GetDevice()->IsToggleEnabled(Toggle::VulkanUseImageRobustAccess2);
- // Currently we can disable index clamping on all runtime-sized arrays in Tint robustness
- // transform as unsized arrays can only be declared on storage address space.
- req.disableRuntimeSizedArrayIndexClamping =
- GetDevice()->IsToggleEnabled(Toggle::VulkanUseBufferRobustAccess2);
req.platform = UnsafeUnkeyedValue(GetDevice()->GetPlatform());
req.substituteOverrideConfig = std::move(substituteOverrideConfig);
- req.useTintIR = GetDevice()->IsToggleEnabled(Toggle::UseTintIR);
+
+ req.tintOptions.clamp_frag_depth = clampFragDepth;
+ req.tintOptions.disable_robustness = !GetDevice()->IsRobustnessEnabled();
+ req.tintOptions.emit_vertex_point_size = true;
+ req.tintOptions.disable_workgroup_init =
+ GetDevice()->IsToggleEnabled(Toggle::DisableWorkgroupInit);
+ req.tintOptions.use_zero_initialize_workgroup_memory_extension =
+ GetDevice()->IsToggleEnabled(Toggle::VulkanUseZeroInitializeWorkgroupMemoryExtension);
+ req.tintOptions.bindings = std::move(bindings);
+ req.tintOptions.disable_image_robustness =
+ GetDevice()->IsToggleEnabled(Toggle::VulkanUseImageRobustAccess2);
+ // Currently we can disable index clamping on all runtime-sized arrays in Tint robustness
+ // transform as unsized arrays can only be declared on storage address space.
+ req.tintOptions.disable_runtime_sized_array_index_clamping =
+ GetDevice()->IsToggleEnabled(Toggle::VulkanUseBufferRobustAccess2);
+ req.tintOptions.use_tint_ir = GetDevice()->IsToggleEnabled(Toggle::UseTintIR);
+
// Set subgroup uniform control flow flag for subgroup experiment, if device has
// Chromium-experimental-subgroup-uniform-control-flow feature. (dawn:464)
if (GetDevice()->HasFeature(Feature::ChromiumExperimentalSubgroupUniformControlFlow)) {
- req.experimentalRequireSubgroupUniformControlFlow = true;
+ req.tintOptions.experimental_require_subgroup_uniform_control_flow = true;
+ } else {
+ req.tintOptions.experimental_require_subgroup_uniform_control_flow = false;
}
// Pass matrices to user functions by pointer on Qualcomm devices to workaround a known bug.
// See crbug.com/tint/2045.
if (ToBackend(GetDevice()->GetPhysicalDevice())->IsAndroidQualcomm()) {
- req.passMatrixByPointer = true;
+ req.tintOptions.pass_matrix_by_pointer = true;
}
const CombinedLimits& limits = GetDevice()->GetLimits();
@@ -387,24 +385,8 @@
program, remappedEntryPoint.c_str(), r.limits));
}
- tint::spirv::writer::Options options;
- options.clamp_frag_depth = r.clampFragDepth;
- options.disable_robustness = !r.isRobustnessEnabled;
- options.emit_vertex_point_size = true;
- options.disable_workgroup_init = r.disableWorkgroupInit;
- options.use_zero_initialize_workgroup_memory_extension =
- r.useZeroInitializeWorkgroupMemoryExtension;
- options.bindings = r.bindings;
- options.disable_image_robustness = r.disableImageRobustness;
- options.disable_runtime_sized_array_index_clamping =
- r.disableRuntimeSizedArrayIndexClamping;
- options.experimental_require_subgroup_uniform_control_flow =
- r.experimentalRequireSubgroupUniformControlFlow;
- options.use_tint_ir = r.useTintIR;
- options.pass_matrix_by_pointer = r.passMatrixByPointer;
-
TRACE_EVENT0(r.platform.UnsafeGetValue(), General, "tint::spirv::writer::Generate()");
- auto tintResult = tint::spirv::writer::Generate(program, options);
+ auto tintResult = tint::spirv::writer::Generate(program, r.tintOptions);
DAWN_INVALID_IF(!tintResult, "An error occurred while generating SPIR-V\n%s",
tintResult.Failure().reason.str());
diff --git a/src/tint/lang/spirv/writer/common/option_builder.cc b/src/tint/lang/spirv/writer/common/option_builder.cc
index d68baa6d..66df1e4 100644
--- a/src/tint/lang/spirv/writer/common/option_builder.cc
+++ b/src/tint/lang/spirv/writer/common/option_builder.cc
@@ -25,7 +25,7 @@
auto wgsl_seen = [&diagnostics, &seen_wgsl_bindings](const tint::BindingPoint& info) -> bool {
if (seen_wgsl_bindings.Contains(info)) {
std::stringstream str;
- str << "Found duplicate WGSL binding point: " << info;
+ str << "found duplicate WGSL binding point: " << info;
diagnostics.add_error(diag::System::Writer, str.str());
return true;
@@ -38,7 +38,7 @@
&seen_spirv_bindings](const binding::BindingInfo& info) -> bool {
if (seen_spirv_bindings.Contains(info)) {
std::stringstream str;
- str << "Found duplicate SPIR-V binding point: [group: " << info.group
+ str << "found duplicate SPIR-V binding point: [group: " << info.group
<< ", binding: " << info.binding << "]";
diagnostics.add_error(diag::System::Writer, str.str());
return true;
@@ -64,23 +64,23 @@
};
if (!valid(options.bindings.uniform)) {
- diagnostics.add_note(diag::System::Writer, "When processing uniform", {});
+ diagnostics.add_note(diag::System::Writer, "when processing uniform", {});
return false;
}
if (!valid(options.bindings.storage)) {
- diagnostics.add_note(diag::System::Writer, "When processing storage", {});
+ diagnostics.add_note(diag::System::Writer, "when processing storage", {});
return false;
}
if (!valid(options.bindings.texture)) {
- diagnostics.add_note(diag::System::Writer, "When processing texture", {});
+ diagnostics.add_note(diag::System::Writer, "when processing texture", {});
return false;
}
if (!valid(options.bindings.storage_texture)) {
- diagnostics.add_note(diag::System::Writer, "When processing storage_texture", {});
+ diagnostics.add_note(diag::System::Writer, "when processing storage_texture", {});
return false;
}
if (!valid(options.bindings.sampler)) {
- diagnostics.add_note(diag::System::Writer, "When processing sampler", {});
+ diagnostics.add_note(diag::System::Writer, "when processing sampler", {});
return false;
}
@@ -92,20 +92,20 @@
// Validate with the actual source regardless of what the remapper will do
if (wgsl_seen(src_binding)) {
- diagnostics.add_note(diag::System::Writer, "When processing external_texture", {});
+ diagnostics.add_note(diag::System::Writer, "when processing external_texture", {});
return false;
}
if (spirv_seen(plane0)) {
- diagnostics.add_note(diag::System::Writer, "When processing external_texture", {});
+ diagnostics.add_note(diag::System::Writer, "when processing external_texture", {});
return false;
}
if (spirv_seen(plane1)) {
- diagnostics.add_note(diag::System::Writer, "When processing external_texture", {});
+ diagnostics.add_note(diag::System::Writer, "when processing external_texture", {});
return false;
}
if (spirv_seen(metadata)) {
- diagnostics.add_note(diag::System::Writer, "When processing external_texture", {});
+ diagnostics.add_note(diag::System::Writer, "when processing external_texture", {});
return false;
}
}
diff --git a/src/tint/lang/spirv/writer/helpers/generate_bindings.cc b/src/tint/lang/spirv/writer/helpers/generate_bindings.cc
index 0285b0b..5ed371c 100644
--- a/src/tint/lang/spirv/writer/helpers/generate_bindings.cc
+++ b/src/tint/lang/spirv/writer/helpers/generate_bindings.cc
@@ -25,6 +25,8 @@
#include "src/tint/lang/wgsl/ast/module.h"
#include "src/tint/lang/wgsl/program/program.h"
#include "src/tint/lang/wgsl/sem/variable.h"
+#include "src/tint/utils/containers/hashmap.h"
+#include "src/tint/utils/containers/vector.h"
#include "src/tint/utils/rtti/switch.h"
namespace tint::spirv::writer {
@@ -38,31 +40,34 @@
std::unordered_set<tint::BindingPoint> seen_binding_points;
// Collect next valid binding number per group
- std::unordered_map<uint32_t, uint32_t> group_to_next_binding_number;
- std::vector<tint::BindingPoint> ext_tex_bps;
+ Hashmap<uint32_t, uint32_t, 4> group_to_next_binding_number;
+ Vector<tint::BindingPoint, 4> ext_tex_bps;
for (auto* var : program.AST().GlobalVariables()) {
if (auto* sem_var = program.Sem().Get(var)->As<sem::GlobalVariable>()) {
if (auto bp = sem_var->BindingPoint()) {
// This is a bit of a hack. The binding points must be unique over all the `binding`
// entries. But, this is looking at _all_ entry points where bindings can overlap.
// In the case where both entry points used the same type (uniform, sampler, etc)
- // then it woudl be fine as it just overwrites with itself. But, if one entry point
+ // then it would be fine as it just overwrites with itself. But, if one entry point
// has a uniform and the other a sampler at the same (group,binding) pair then we'll
// get a validation error due to duplicate WGSL bindings.
//
- // For generate bindings we don't really care as we always map to itself, so if it
+ // For generating bindings we don't really care as we always map to itself, so if it
// exists anywhere, we just pretend that's the only one.
if (seen_binding_points.find(*bp) != seen_binding_points.end()) {
continue;
}
seen_binding_points.emplace(*bp);
- auto& n = group_to_next_binding_number[bp->group];
- n = std::max(n, bp->binding + 1);
+ if (auto val = group_to_next_binding_number.Find(bp->group)) {
+ *val = std::max(*val, bp->binding + 1);
+ } else {
+ group_to_next_binding_number.Add(bp->group, bp->binding + 1);
+ }
// Store up the external textures, we'll add them in the next step
if (sem_var->Type()->UnwrapRef()->Is<core::type::ExternalTexture>()) {
- ext_tex_bps.emplace_back(*bp);
+ ext_tex_bps.Push(*bp);
continue;
}
@@ -104,12 +109,14 @@
for (auto bp : ext_tex_bps) {
uint32_t g = bp.group;
- uint32_t& next_num = group_to_next_binding_number[g];
+ uint32_t next_num = *(group_to_next_binding_number.GetOrZero(g));
binding::BindingInfo plane0{bp.group, bp.binding};
binding::BindingInfo plane1{g, next_num++};
binding::BindingInfo metadata{g, next_num++};
+ group_to_next_binding_number.Replace(g, next_num);
+
bindings.external_texture.emplace(bp, binding::ExternalTexture{metadata, plane0, plane1});
}
