Remove the use_tint_generator toggle
We now always use Tint.
Bug: dawn:571
Change-Id: Ic65669a9e00493292ed60cde00af6fa3f6e55ec2
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/65665
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Ben Clayton <bclayton@google.com>
diff --git a/src/dawn_native/Device.cpp b/src/dawn_native/Device.cpp
index b30a2f8..cf33c02 100644
--- a/src/dawn_native/Device.cpp
+++ b/src/dawn_native/Device.cpp
@@ -186,15 +186,6 @@
mFormatTable = BuildFormatTable(this);
SetDefaultToggles();
- if ((adapter->GetBackendType() == wgpu::BackendType::Metal ||
- adapter->GetBackendType() == wgpu::BackendType::Vulkan ||
- adapter->GetBackendType() == wgpu::BackendType::D3D12) &&
- !IsToggleEnabled(Toggle::UseTintGenerator)) {
- EmitLog(
- WGPULoggingType_Warning,
- "Non-tint generator is not available on this backend; toggle disable ignored.\n");
- ForceSetToggle(Toggle::UseTintGenerator, true);
- }
}
DeviceBase::DeviceBase() : mState(State::Alive) {
@@ -1537,7 +1528,6 @@
void DeviceBase::SetDefaultToggles() {
SetToggle(Toggle::LazyClearResourceOnFirstUse, true);
SetToggle(Toggle::DisallowUnsafeAPIs, true);
- SetToggle(Toggle::UseTintGenerator, true);
}
void DeviceBase::ApplyToggleOverrides(const DeviceDescriptor* deviceDescriptor) {
diff --git a/src/dawn_native/Toggles.cpp b/src/dawn_native/Toggles.cpp
index e7f844d..8c7f33f 100644
--- a/src/dawn_native/Toggles.cpp
+++ b/src/dawn_native/Toggles.cpp
@@ -160,9 +160,6 @@
"Produces validation errors on API entry points or parameter combinations that "
"aren't considered secure yet.",
"http://crbug.com/1138528"}},
- {Toggle::UseTintGenerator,
- {"use_tint_generator", "Use Tint instead of SPRIV-cross to generate shaders.",
- "https://crbug.com/dawn/571"}},
{Toggle::FlushBeforeClientWaitSync,
{"flush_before_client_wait_sync",
"Call glFlush before glClientWaitSync to work around bugs in the latter",
diff --git a/src/dawn_native/Toggles.h b/src/dawn_native/Toggles.h
index 0988598..88a9d6d 100644
--- a/src/dawn_native/Toggles.h
+++ b/src/dawn_native/Toggles.h
@@ -48,7 +48,6 @@
DisableRobustness,
MetalEnableVertexPulling,
DisallowUnsafeAPIs,
- UseTintGenerator,
FlushBeforeClientWaitSync,
UseTempBufferInSmallFormatTextureToTextureCopyFromGreaterToLessMipLevel,
EmitHLSLDebugSymbols,
diff --git a/src/dawn_native/opengl/ShaderModuleGL.cpp b/src/dawn_native/opengl/ShaderModuleGL.cpp
index 414fb79..dba7b33 100644
--- a/src/dawn_native/opengl/ShaderModuleGL.cpp
+++ b/src/dawn_native/opengl/ShaderModuleGL.cpp
@@ -352,13 +352,13 @@
// Modify the decoration of variables so that SPIRV-Cross outputs only
// layout(binding=<index>) for interface variables.
//
- // When the use_tint_generator toggle is on, Tint is used for the reflection of bindings
- // for the implicit pipeline layout and pipeline/layout validation, but bindingInfo is set
- // to mGLEntryPoints which is the SPIRV-Cross reflection. Tint reflects bindings used more
- // precisely than SPIRV-Cross so some bindings in bindingInfo might not exist in the layout
- // and querying the layout for them would cause an ASSERT. That's why we defensively check
- // that bindings are in the layout before modifying them. This slight hack is ok because in
- // the long term we will use Tint to produce GLSL.
+ // Tint is used for the reflection of bindings for the implicit pipeline layout and
+ // pipeline/layout validation, but bindingInfo is set to mGLEntryPoints which is the
+ // SPIRV-Cross reflection. Tint reflects bindings used more precisely than SPIRV-Cross so
+ // some bindings in bindingInfo might not exist in the layout and querying the layout for
+ // them would cause an ASSERT. That's why we defensively check that bindings are in the
+ // layout before modifying them. This slight hack is ok because in the long term we will use
+ // Tint to produce GLSL.
for (BindGroupIndex group : IterateBitSet(layout->GetBindGroupLayoutsMask())) {
for (const auto& it : bindingInfo[group]) {
const BindGroupLayoutBase* bgl = layout->GetBindGroupLayout(group);
diff --git a/src/tests/DawnTest.cpp b/src/tests/DawnTest.cpp
index cb59775..ff07cb1 100644
--- a/src/tests/DawnTest.cpp
+++ b/src/tests/DawnTest.cpp
@@ -385,7 +385,7 @@
" enable all available backend validation with less performance overhead.\n"
" Set to 'disabled' to run with no validation (same as no flag).\n"
" --enable-toggles: Comma-delimited list of Dawn toggles to enable.\n"
- " ex.) skip_validation,use_tint_generator,disable_robustness,turn_off_vsync\n"
+ " ex.) skip_validation,disable_robustness,turn_off_vsync\n"
" --disable-toggles: Comma-delimited list of Dawn toggles to disable\n"
" --adapter-vendor-id: Select adapter by vendor id to run end2end tests"
"on multi-GPU systems \n"
diff --git a/src/tests/end2end/BindGroupTests.cpp b/src/tests/end2end/BindGroupTests.cpp
index 7460ba1..f6693b8 100644
--- a/src/tests/end2end/BindGroupTests.cpp
+++ b/src/tests/end2end/BindGroupTests.cpp
@@ -986,9 +986,6 @@
// Regression test for crbug.com/dawn/408 where dynamic offsets were applied in the wrong order.
// Dynamic offsets should be applied in increasing order of binding number.
TEST_P(BindGroupTests, DynamicOffsetOrder) {
- // Does not work with SPIRV-Cross. crbug.com/dawn/975
- DAWN_SUPPRESS_TEST_IF(IsD3D12() && !HasToggleEnabled("use_tint_generator"));
-
// We will put the following values and the respective offsets into a buffer.
// The test will ensure that the correct dynamic offset is applied to each buffer by reading the
// value from an offset binding.
diff --git a/src/tests/end2end/ComputeLayoutMemoryBufferTests.cpp b/src/tests/end2end/ComputeLayoutMemoryBufferTests.cpp
index 0f4e46b..ddc093e 100644
--- a/src/tests/end2end/ComputeLayoutMemoryBufferTests.cpp
+++ b/src/tests/end2end/ComputeLayoutMemoryBufferTests.cpp
@@ -135,8 +135,6 @@
: public DawnTestWithParams<ComputeLayoutMemoryBufferTestParams> {
void SetUp() override {
DawnTestBase::SetUp();
- DAWN_TEST_UNSUPPORTED_IF((IsD3D12() || IsMetal()) &&
- !HasToggleEnabled("use_tint_generator"));
}
};
diff --git a/src/tests/end2end/ComputeStorageBufferBarrierTests.cpp b/src/tests/end2end/ComputeStorageBufferBarrierTests.cpp
index fd7e4ee..ccf65fd 100644
--- a/src/tests/end2end/ComputeStorageBufferBarrierTests.cpp
+++ b/src/tests/end2end/ComputeStorageBufferBarrierTests.cpp
@@ -339,10 +339,6 @@
// 2 - Write ones into it with a compute shader.
// 3 - Use the indirect buffer in a Dispatch while also reading its data.
TEST_P(ComputeStorageBufferBarrierTests, IndirectBufferCorrectBarrier) {
- // For some reason SPIRV-Cross crashes when translating the step3 shader to HLSL. Suppress the
- // failure since we'll remove SPIRV-Cross at some point.
- DAWN_SUPPRESS_TEST_IF(IsD3D12() && !HasToggleEnabled("use_tint_generator"));
-
wgpu::ComputePipelineDescriptor step2PipelineDesc;
step2PipelineDesc.compute.entryPoint = "main";
step2PipelineDesc.compute.module = utils::CreateShaderModule(device, R"(
diff --git a/src/tests/end2end/CopyTextureForBrowserTests.cpp b/src/tests/end2end/CopyTextureForBrowserTests.cpp
index efeb3ba..3d25cdb 100644
--- a/src/tests/end2end/CopyTextureForBrowserTests.cpp
+++ b/src/tests/end2end/CopyTextureForBrowserTests.cpp
@@ -140,10 +140,6 @@
void SetUp() override {
DawnTest::SetUp();
- // crbug.com/dawn/948: Tint required for multiple entrypoints in a module.
- // CopyTextureForBrowser uses and internal pipeline with a multi-entrypoint
- // shader module.
- DAWN_TEST_UNSUPPORTED_IF(!HasToggleEnabled("use_tint_generator"));
testPipeline = MakeTestPipeline();
diff --git a/src/tests/end2end/DepthStencilSamplingTests.cpp b/src/tests/end2end/DepthStencilSamplingTests.cpp
index e65d265..de1732b 100644
--- a/src/tests/end2end/DepthStencilSamplingTests.cpp
+++ b/src/tests/end2end/DepthStencilSamplingTests.cpp
@@ -741,9 +741,6 @@
// Initialization via renderPass loadOp doesn't work on Mac Intel.
DAWN_SUPPRESS_TEST_IF(IsMetal() && IsIntel());
- // Depends on Tint's shader reflection
- DAWN_TEST_UNSUPPORTED_IF(!HasToggleEnabled("use_tint_generator"));
-
wgpu::RenderPipeline pipeline = CreateComparisonRenderPipeline();
for (wgpu::TextureFormat format : kDepthFormats) {
diff --git a/src/tests/end2end/ExternalTextureTests.cpp b/src/tests/end2end/ExternalTextureTests.cpp
index a29c9aa..15ea03d 100644
--- a/src/tests/end2end/ExternalTextureTests.cpp
+++ b/src/tests/end2end/ExternalTextureTests.cpp
@@ -62,10 +62,6 @@
}
TEST_P(ExternalTextureTests, SampleExternalTexture) {
- // SPIRV-Cross is unable to reflect texture_external correctly, which causes errors during
- // validation.
- DAWN_SUPPRESS_TEST_IF(!HasToggleEnabled("use_tint_generator"));
-
wgpu::ShaderModule vsModule = utils::CreateShaderModule(device, R"(
[[stage(vertex)]] fn main([[builtin(vertex_index)]] VertexIndex : u32) -> [[builtin(position)]] vec4<f32> {
var positions : array<vec4<f32>, 3> = array<vec4<f32>, 3>(
diff --git a/src/tests/end2end/FirstIndexOffsetTests.cpp b/src/tests/end2end/FirstIndexOffsetTests.cpp
index c716f2a..8d12bd2 100644
--- a/src/tests/end2end/FirstIndexOffsetTests.cpp
+++ b/src/tests/end2end/FirstIndexOffsetTests.cpp
@@ -50,8 +50,6 @@
protected:
void SetUp() override {
DawnTest::SetUp();
- DAWN_TEST_UNSUPPORTED_IF(IsD3D12() && !HasToggleEnabled("use_tint_generator"));
-
// WGSL doesn't have the ability to tag attributes as "flat". "flat" is required on u32
// attributes for correct runtime behavior under Vulkan and codegen under OpenGL(ES).
// TODO(tint:451): Remove once resolved by spec/tint
diff --git a/src/tests/end2end/MultisampledRenderingTests.cpp b/src/tests/end2end/MultisampledRenderingTests.cpp
index 8d16b6c..e97efe8 100644
--- a/src/tests/end2end/MultisampledRenderingTests.cpp
+++ b/src/tests/end2end/MultisampledRenderingTests.cpp
@@ -756,7 +756,7 @@
TEST_P(MultisampledRenderingTest, ResolveInto2DTextureWithSampleMaskAndShaderOutputMask) {
// TODO(github.com/KhronosGroup/SPIRV-Cross/issues/1626): SPIRV-Cross produces bad GLSL for
// unsigned SampleMask builtins
- DAWN_SUPPRESS_TEST_IF(HasToggleEnabled("use_tint_generator") && (IsOpenGL() || IsOpenGLES()));
+ DAWN_SUPPRESS_TEST_IF(IsOpenGL() || IsOpenGLES());
// TODO(crbug.com/dawn/673): Work around or enforce via validation that sample variables are not
// supported on some platforms.
@@ -818,7 +818,7 @@
TEST_P(MultisampledRenderingTest, ResolveIntoMultipleResolveTargetsWithShaderOutputMask) {
// TODO(github.com/KhronosGroup/SPIRV-Cross/issues/1626): SPIRV-Cross produces bad GLSL for
// unsigned SampleMask builtins
- DAWN_SUPPRESS_TEST_IF(HasToggleEnabled("use_tint_generator") && (IsOpenGL() || IsOpenGLES()));
+ DAWN_SUPPRESS_TEST_IF(IsOpenGL() || IsOpenGLES());
// TODO(crbug.com/dawn/673): Work around or enforce via validation that sample variables are not
// supported on some platforms.
diff --git a/src/tests/end2end/MultisampledSamplingTests.cpp b/src/tests/end2end/MultisampledSamplingTests.cpp
index 5a3acd5..160555d 100644
--- a/src/tests/end2end/MultisampledSamplingTests.cpp
+++ b/src/tests/end2end/MultisampledSamplingTests.cpp
@@ -126,8 +126,6 @@
// must cover both the X and Y coordinates of the sample position (no false positives if
// it covers the X position but not the Y, or vice versa).
TEST_P(MultisampledSamplingTest, SamplePositions) {
- DAWN_TEST_UNSUPPORTED_IF(!HasToggleEnabled("use_tint_generator"));
-
static constexpr wgpu::Extent3D kTextureSize = {1, 1, 1};
wgpu::Texture colorTexture;
diff --git a/src/tests/end2end/ShaderFloat16Tests.cpp b/src/tests/end2end/ShaderFloat16Tests.cpp
index f675b29..f284070 100644
--- a/src/tests/end2end/ShaderFloat16Tests.cpp
+++ b/src/tests/end2end/ShaderFloat16Tests.cpp
@@ -36,11 +36,10 @@
};
// Test basic 16bit float arithmetic and 16bit storage features.
-TEST_P(ShaderFloat16Tests, Basic16BitFloatFeaturesTest) {
+// TODO(crbug.com/tint/404): Implement float16 in Tint.
+TEST_P(ShaderFloat16Tests, DISABLED_Basic16BitFloatFeaturesTest) {
DAWN_TEST_UNSUPPORTED_IF(!IsShaderFloat16Supported());
DAWN_SUPPRESS_TEST_IF(IsD3D12() && IsIntel()); // Flaky crashes. crbug.com/dawn/586
- // TODO(crbug.com/tint/404): Implement float16 in Tint.
- DAWN_TEST_UNSUPPORTED_IF(HasToggleEnabled("use_tint_generator"));
uint16_t uniformData[] = {Float32ToFloat16(1.23), Float32ToFloat16(0.0)}; // 0.0 is a padding.
wgpu::Buffer uniformBuffer = utils::CreateBufferFromData(
diff --git a/src/tests/end2end/ShaderTests.cpp b/src/tests/end2end/ShaderTests.cpp
index aa40fe8..4650d7b 100644
--- a/src/tests/end2end/ShaderTests.cpp
+++ b/src/tests/end2end/ShaderTests.cpp
@@ -278,9 +278,6 @@
// Tests that shaders I/O structs can be shared between vertex and fragment shaders.
TEST_P(ShaderTests, WGSLSharedStructIO) {
- // crbug.com/dawn/948: Tint required for multiple entrypoints in a module.
- DAWN_TEST_UNSUPPORTED_IF(!HasToggleEnabled("use_tint_generator"));
-
std::string shader = R"(
struct VertexIn {
[[location(0)]] position : vec3<f32>;
diff --git a/src/tests/perf_tests/ShaderRobustnessPerf.cpp b/src/tests/perf_tests/ShaderRobustnessPerf.cpp
index a1fcf11..f883b86 100644
--- a/src/tests/perf_tests/ShaderRobustnessPerf.cpp
+++ b/src/tests/perf_tests/ShaderRobustnessPerf.cpp
@@ -409,7 +409,7 @@
DAWN_SUPPRESS_TEST_IF(IsD3D12() && IsWARP());
// TODO(crbug.com/dawn/945): Generation via SPIRV-Cross fails
- DAWN_SUPPRESS_TEST_IF(IsOpenGL() || !HasToggleEnabled("use_tint_generator"));
+ DAWN_SUPPRESS_TEST_IF(IsOpenGL());
const size_t dataASize = mDimAOuter * mDimInner;
std::vector<float> dataA(dataASize);
diff --git a/src/tests/unittests/validation/BindGroupValidationTests.cpp b/src/tests/unittests/validation/BindGroupValidationTests.cpp
index a3adb9f..c2af0a8 100644
--- a/src/tests/unittests/validation/BindGroupValidationTests.cpp
+++ b/src/tests/unittests/validation/BindGroupValidationTests.cpp
@@ -2426,7 +2426,7 @@
}
// Test that filtering sampler binding does not work with comparison sampler in the shader.
- if (HasToggleEnabled("use_tint_generator")) {
+ {
wgpu::BindGroupLayout bindGroupLayout = utils::MakeBindGroupLayout(
device, {{0, wgpu::ShaderStage::Fragment, wgpu::SamplerBindingType::Filtering}});
@@ -2438,7 +2438,7 @@
}
// Test that non-filtering sampler binding does not work with comparison sampler in the shader.
- if (HasToggleEnabled("use_tint_generator")) {
+ {
wgpu::BindGroupLayout bindGroupLayout = utils::MakeBindGroupLayout(
device, {{0, wgpu::ShaderStage::Fragment, wgpu::SamplerBindingType::NonFiltering}});
@@ -2450,7 +2450,7 @@
}
// Test that comparison sampler binding does not work with normal sampler in the shader.
- if (HasToggleEnabled("use_tint_generator")) {
+ {
wgpu::BindGroupLayout bindGroupLayout = utils::MakeBindGroupLayout(
device, {{0, wgpu::ShaderStage::Fragment, wgpu::SamplerBindingType::Comparison}});
@@ -2532,7 +2532,7 @@
}
// Test that a filtering sampler cannot be used to sample an unfilterable-float texture.
- if (HasToggleEnabled("use_tint_generator")) {
+ {
wgpu::BindGroupLayout bindGroupLayout = utils::MakeBindGroupLayout(
device, {{0, wgpu::ShaderStage::Fragment, wgpu::SamplerBindingType::Filtering},
{1, wgpu::ShaderStage::Fragment, wgpu::TextureSampleType::UnfilterableFloat}});
diff --git a/src/tests/unittests/validation/GetBindGroupLayoutValidationTests.cpp b/src/tests/unittests/validation/GetBindGroupLayoutValidationTests.cpp
index c332439..23d01ab 100644
--- a/src/tests/unittests/validation/GetBindGroupLayoutValidationTests.cpp
+++ b/src/tests/unittests/validation/GetBindGroupLayoutValidationTests.cpp
@@ -162,8 +162,6 @@
// Getting the same pointer to equivalent bind group layouts is an implementation detail of Dawn
// Native.
DAWN_SKIP_TEST_IF(UsesWire());
- // Relies on Tint shader reflection.
- DAWN_SKIP_TEST_IF(!HasToggleEnabled("use_tint_generator"));
wgpu::BindGroupLayout filteringBGL = utils::MakeBindGroupLayout(
device, {{0, wgpu::ShaderStage::Vertex | wgpu::ShaderStage::Fragment,
@@ -1082,10 +1080,7 @@
}
// Test that fragment output validation is for the correct entryPoint
-// TODO(dawn:216): Re-enable when we correctly reflect which bindings are used for an entryPoint.
TEST_F(GetBindGroupLayoutTests, FromCorrectEntryPoint) {
- DAWN_SKIP_TEST_IF(!HasToggleEnabled("use_tint_generator"));
-
wgpu::ShaderModule module = utils::CreateShaderModule(device, R"(
[[block]] struct Data {
data : f32;
diff --git a/src/tests/unittests/validation/ShaderModuleValidationTests.cpp b/src/tests/unittests/validation/ShaderModuleValidationTests.cpp
index eebc5d2..5ea0128 100644
--- a/src/tests/unittests/validation/ShaderModuleValidationTests.cpp
+++ b/src/tests/unittests/validation/ShaderModuleValidationTests.cpp
@@ -435,8 +435,6 @@
// Tests that we validate workgroup size limits.
TEST_F(ShaderModuleValidationTest, ComputeWorkgroupSizeLimits) {
- DAWN_SKIP_TEST_IF(!HasToggleEnabled("use_tint_generator"));
-
auto MakeShaderWithWorkgroupSize = [this](uint32_t x, uint32_t y, uint32_t z) {
std::ostringstream ss;
ss << "[[stage(compute), workgroup_size(" << x << "," << y << "," << z
@@ -467,8 +465,6 @@
// Tests that we validate workgroup storage size limits.
TEST_F(ShaderModuleValidationTest, ComputeWorkgroupStorageSizeLimits) {
- DAWN_SKIP_TEST_IF(!HasToggleEnabled("use_tint_generator"));
-
wgpu::Limits supportedLimits = GetSupportedLimits().limits;
constexpr uint32_t kVec4Size = 16;
diff --git a/src/tests/unittests/validation/ValidationTest.cpp b/src/tests/unittests/validation/ValidationTest.cpp
index 1cf834d..1a2ec0b 100644
--- a/src/tests/unittests/validation/ValidationTest.cpp
+++ b/src/tests/unittests/validation/ValidationTest.cpp
@@ -63,7 +63,7 @@
" [--enable-toggles=toggles] [--disable-toggles=toggles]\n"
" -w, --use-wire: Run the tests through the wire (defaults to no wire)\n"
" --enable-toggles: Comma-delimited list of Dawn toggles to enable.\n"
- " ex.) skip_validation,use_tint_generator,disable_robustness,turn_off_vsync\n"
+ " ex.) skip_validation,disable_robustness,turn_off_vsync\n"
" --disable-toggles: Comma-delimited list of Dawn toggles to disable\n";
continue;
}
diff --git a/src/tests/white_box/D3D12DescriptorHeapTests.cpp b/src/tests/white_box/D3D12DescriptorHeapTests.cpp
index 4050667..513a165 100644
--- a/src/tests/white_box/D3D12DescriptorHeapTests.cpp
+++ b/src/tests/white_box/D3D12DescriptorHeapTests.cpp
@@ -737,9 +737,6 @@
// Verify encoding many sampler and ubo worth of bindgroups.
// Shader-visible heaps should switch out |kNumOfViewHeaps| times.
TEST_P(D3D12DescriptorHeapTests, EncodeManyUBOAndSamplers) {
- // TODO(crbug.com/dawn/571): HLSL emission via SPIRV-Cross produces incorrect results.
- DAWN_TEST_UNSUPPORTED_IF(!HasToggleEnabled("use_tint_generator"));
-
DAWN_TEST_UNSUPPORTED_IF(!mD3DDevice->IsToggleEnabled(
dawn_native::Toggle::UseD3D12SmallShaderVisibleHeapForTesting));