D3D12: Fix order of Tint transforms
FirstIndexOffset must happen after BindingRemapper otherwise some
invalid AST is produced after FirstIndexOffset.
This was found running a CTS test on Windows:
validation,vertex_state:vertex_shader_type_matches_attribute_format
Also adds a regression test.
Bug: dawn:1014
Change-Id: Icbe4b3adb5e5844ffc8435e0e67056c7dff23970
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/59281
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
Auto-Submit: Corentin Wallez <cwallez@chromium.org>
diff --git a/src/dawn_native/d3d12/ShaderModuleD3D12.cpp b/src/dawn_native/d3d12/ShaderModuleD3D12.cpp
index d2f0ed5..265725e 100644
--- a/src/dawn_native/d3d12/ShaderModuleD3D12.cpp
+++ b/src/dawn_native/d3d12/ShaderModuleD3D12.cpp
@@ -251,13 +251,19 @@
if (GetDevice()->IsRobustnessEnabled()) {
transformManager.Add<tint::transform::BoundArrayAccessors>();
}
+ transformManager.Add<tint::transform::BindingRemapper>();
+
+ // The FirstIndexOffset transform must be done after the BindingRemapper because it assumes
+ // that the register space has already flattened (and uses the next register). Otherwise
+ // intermediate ASTs can be produced where the extra registers conflict with one of the
+ // user-declared bind points.
if (stage == SingleShaderStage::Vertex) {
transformManager.Add<tint::transform::FirstIndexOffset>();
transformInputs.Add<tint::transform::FirstIndexOffset::BindingPoint>(
layout->GetFirstIndexOffsetShaderRegister(),
layout->GetFirstIndexOffsetRegisterSpace());
}
- transformManager.Add<tint::transform::BindingRemapper>();
+
transformManager.Add<tint::transform::Renamer>();
if (GetDevice()->IsToggleEnabled(Toggle::DisableSymbolRenaming)) {
diff --git a/src/tests/end2end/ShaderTests.cpp b/src/tests/end2end/ShaderTests.cpp
index 4fb95b3..5c53bd5 100644
--- a/src/tests/end2end/ShaderTests.cpp
+++ b/src/tests/end2end/ShaderTests.cpp
@@ -322,6 +322,50 @@
ASSERT_DEVICE_ERROR(utils::CreateShaderModule(device, shader.c_str()));
}
+// This is a regression test for an issue caused by the FirstIndexOffset transfrom being done before
+// the BindingRemapper, causing an intermediate AST to be invalid (and fail the overall
+// compilation).
+TEST_P(ShaderTests, FirstIndexOffsetRegisterConflictInHLSLTransforms) {
+ // TODO(crbug.com/dawn/658): Crashes on bots because there are two entrypoints in the shader.
+ DAWN_SUPPRESS_TEST_IF(IsOpenGL() || IsOpenGLES());
+
+ const char* shader = R"(
+// Dumped WGSL:
+
+struct Inputs {
+ [[location(1)]] attrib1 : u32;
+ // The extra register added to handle base_vertex for vertex_index conflicts with [1]
+ [[builtin(vertex_index)]] vertexIndex: u32;
+};
+
+// [1] a binding point that conflicts with the regitster
+[[block]] struct S1 { data : array<vec4<u32>, 20>; };
+[[group(0), binding(1)]] var<uniform> providedData1 : S1;
+
+[[stage(vertex)]] fn vsMain(input : Inputs) -> [[builtin(position)]] vec4<f32> {
+ ignore(providedData1.data[input.vertexIndex][0]);
+ return vec4<f32>();
+}
+
+[[stage(fragment)]] fn fsMain() -> [[location(0)]] vec4<f32> {
+ return vec4<f32>();
+}
+ )";
+ auto module = utils::CreateShaderModule(device, shader);
+
+ utils::ComboRenderPipelineDescriptor rpDesc;
+ rpDesc.vertex.module = module;
+ rpDesc.vertex.entryPoint = "vsMain";
+ rpDesc.cFragment.module = module;
+ rpDesc.cFragment.entryPoint = "fsMain";
+ rpDesc.vertex.bufferCount = 1;
+ rpDesc.cBuffers[0].attributeCount = 1;
+ rpDesc.cBuffers[0].arrayStride = 16;
+ rpDesc.cAttributes[0].shaderLocation = 1;
+ rpDesc.cAttributes[0].format = wgpu::VertexFormat::Uint8x2;
+ device.CreateRenderPipeline(&rpDesc);
+}
+
DAWN_INSTANTIATE_TEST(ShaderTests,
D3D12Backend(),
MetalBackend(),
