Vk: Run SingleEntryPoint before BindingRemapper.

This avoids Tint validation errors between transforms caused by unused
entry-points having conflict with the remapped bindings.

Fixed: dawn:1363
Change-Id: If7d22d09905816bfe777ab22211af21513f98698
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/89441
Auto-Submit: Corentin Wallez <cwallez@chromium.org>
Commit-Queue: Austin Eng <enga@chromium.org>
Reviewed-by: Austin Eng <enga@chromium.org>
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Loko Kung <lokokung@google.com>
This commit is contained in:
Corentin Wallez 2022-05-10 03:33:34 +00:00 committed by Dawn LUCI CQ
parent 7c2680badd
commit 65271f6645
2 changed files with 32 additions and 2 deletions

View File

@ -159,15 +159,16 @@ ResultOrError<ShaderModule::ModuleAndSpirv> ShaderModule::GetHandleAndSpirv(
} }
tint::transform::Manager transformManager; tint::transform::Manager transformManager;
transformManager.append(std::make_unique<tint::transform::BindingRemapper>());
// Many Vulkan drivers can't handle multi-entrypoint shader modules. // Many Vulkan drivers can't handle multi-entrypoint shader modules.
transformManager.append(std::make_unique<tint::transform::SingleEntryPoint>()); transformManager.append(std::make_unique<tint::transform::SingleEntryPoint>());
// Run the binding remapper after SingleEntryPoint to avoid collisions with unused entryPoints.
transformManager.append(std::make_unique<tint::transform::BindingRemapper>());
tint::transform::DataMap transformInputs; tint::transform::DataMap transformInputs;
transformInputs.Add<tint::transform::SingleEntryPoint::Config>(entryPointName);
transformInputs.Add<BindingRemapper::Remappings>(std::move(bindingPoints), transformInputs.Add<BindingRemapper::Remappings>(std::move(bindingPoints),
std::move(accessControls), std::move(accessControls),
/* mayCollide */ false); /* mayCollide */ false);
transformInputs.Add<tint::transform::SingleEntryPoint::Config>(entryPointName);
// Transform external textures into the binding locations specified in the bgl // Transform external textures into the binding locations specified in the bgl
// TODO(dawn:1082): Replace this block with ShaderModuleBase::AddExternalTextureTransform. // TODO(dawn:1082): Replace this block with ShaderModuleBase::AddExternalTextureTransform.

View File

@ -732,6 +732,35 @@ fn main(@builtin(vertex_index) VertexIndex : u32)
EXPECT_PIXEL_RGBA8_EQ(RGBA8(255, 255, 255, 255), renderPass.color, 0, 0); EXPECT_PIXEL_RGBA8_EQ(RGBA8(255, 255, 255, 255), renderPass.color, 0, 0);
} }
// This is a regression test for crbug.com/dawn:1363 where the BindingRemapper transform was run
// before the SingleEntryPoint transform, causing one of the other entry points to have conflicting
// bindings.
TEST_P(ShaderTests, ConflictingBindingsDueToTransformOrder) {
wgpu::ShaderModule module = utils::CreateShaderModule(device, R"(
@group(0) @binding(0) var<uniform> b0 : u32;
@group(0) @binding(1) var<uniform> b1 : u32;
@stage(vertex) fn vertex() -> @builtin(position) vec4<f32> {
_ = b0;
return vec4<f32>(0.0);
}
@stage(fragment) fn fragment() -> @location(0) vec4<f32> {
_ = b0;
_ = b1;
return vec4<f32>(0.0);
}
)");
utils::ComboRenderPipelineDescriptor desc;
desc.vertex.module = module;
desc.vertex.entryPoint = "vertex";
desc.cFragment.module = module;
desc.cFragment.entryPoint = "fragment";
device.CreateRenderPipeline(&desc);
}
// TODO(tint:1155): Test overridable constants used for workgroup size // TODO(tint:1155): Test overridable constants used for workgroup size
DAWN_INSTANTIATE_TEST(ShaderTests, DAWN_INSTANTIATE_TEST(ShaderTests,