Rely on Tint to validate shader IO and bindings have decorations.

This removes a little bit of noise from the reflection of shader
metadata in Dawn. Tests are added to make sure that Tint does the
correct validation (it does).

Bug: None
Change-Id: I334e7c23b723cf5b5985c9914cc9f8d79a7c0568
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/85502
Reviewed-by: Austin Eng <enga@chromium.org>
Reviewed-by: Loko Kung <lokokung@google.com>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
This commit is contained in:
Corentin Wallez 2022-04-04 18:57:52 +00:00 committed by Dawn LUCI CQ
parent 2f2977b728
commit 2787a4c64c
2 changed files with 84 additions and 20 deletions

View File

@ -703,11 +703,6 @@ namespace dawn::native {
if (metadata->stage == SingleShaderStage::Vertex) { if (metadata->stage == SingleShaderStage::Vertex) {
for (const auto& inputVar : entryPoint.input_variables) { for (const auto& inputVar : entryPoint.input_variables) {
DAWN_INVALID_IF(
!inputVar.has_location_decoration,
"Vertex input variable \"%s\" doesn't have a location decoration.",
inputVar.name);
uint32_t unsanitizedLocation = inputVar.location_decoration; uint32_t unsanitizedLocation = inputVar.location_decoration;
if (DelayedInvalidIf(unsanitizedLocation >= kMaxVertexAttributes, if (DelayedInvalidIf(unsanitizedLocation >= kMaxVertexAttributes,
"Vertex input variable \"%s\" has a location (%u) that " "Vertex input variable \"%s\" has a location (%u) that "
@ -742,11 +737,6 @@ namespace dawn::native {
outputVar.interpolation_sampling)); outputVar.interpolation_sampling));
totalInterStageShaderComponents += variable.componentCount; totalInterStageShaderComponents += variable.componentCount;
DAWN_INVALID_IF(
!outputVar.has_location_decoration,
"Vertex ouput variable \"%s\" doesn't have a location decoration.",
outputVar.name);
uint32_t location = outputVar.location_decoration; uint32_t location = outputVar.location_decoration;
if (DelayedInvalidIf(location > kMaxInterStageShaderLocation, if (DelayedInvalidIf(location > kMaxInterStageShaderLocation,
"Vertex output variable \"%s\" has a location (%u) that " "Vertex output variable \"%s\" has a location (%u) that "
@ -782,11 +772,6 @@ namespace dawn::native {
inputVar.interpolation_sampling)); inputVar.interpolation_sampling));
totalInterStageShaderComponents += variable.componentCount; totalInterStageShaderComponents += variable.componentCount;
DAWN_INVALID_IF(
!inputVar.has_location_decoration,
"Fragment input variable \"%s\" doesn't have a location decoration.",
inputVar.name);
uint32_t location = inputVar.location_decoration; uint32_t location = inputVar.location_decoration;
if (DelayedInvalidIf(location > kMaxInterStageShaderLocation, if (DelayedInvalidIf(location > kMaxInterStageShaderLocation,
"Fragment input variable \"%s\" has a location (%u) that " "Fragment input variable \"%s\" has a location (%u) that "
@ -826,11 +811,6 @@ namespace dawn::native {
TintCompositionTypeToInterStageComponentCount(outputVar.composition_type)); TintCompositionTypeToInterStageComponentCount(outputVar.composition_type));
ASSERT(variable.componentCount <= 4); ASSERT(variable.componentCount <= 4);
DAWN_INVALID_IF(
!outputVar.has_location_decoration,
"Fragment input variable \"%s\" doesn't have a location decoration.",
outputVar.name);
uint32_t unsanitizedAttachment = outputVar.location_decoration; uint32_t unsanitizedAttachment = outputVar.location_decoration;
if (DelayedInvalidIf(unsanitizedAttachment >= kMaxColorAttachments, if (DelayedInvalidIf(unsanitizedAttachment >= kMaxColorAttachments,
"Fragment output variable \"%s\" has a location (%u) that " "Fragment output variable \"%s\" has a location (%u) that "

View File

@ -575,3 +575,87 @@ TEST_F(ShaderModuleValidationTest, MaxBindingNumber) {
)"); )");
ASSERT_DEVICE_ERROR(device.CreateComputePipeline(&desc)); ASSERT_DEVICE_ERROR(device.CreateComputePipeline(&desc));
} }
// Test that missing decorations on shader IO or bindings causes a validation error.
TEST_F(ShaderModuleValidationTest, MissingDecorations) {
// Vertex input.
utils::CreateShaderModule(device, R"(
@stage(vertex) fn main(@location(0) a : vec4<f32>) -> @builtin(position) vec4<f32> {
return vec4(1.0);
}
)");
ASSERT_DEVICE_ERROR(utils::CreateShaderModule(device, R"(
@stage(vertex) fn main(a : vec4<f32>) -> @builtin(position) vec4<f32> {
return vec4(1.0);
}
)"));
// Vertex output
utils::CreateShaderModule(device, R"(
struct Output {
@builtin(position) pos : vec4<f32>,
@location(0) a : f32,
}
@stage(vertex) fn main() -> Output {
var output : Output;
return output;
}
)");
ASSERT_DEVICE_ERROR(utils::CreateShaderModule(device, R"(
struct Output {
@builtin(position) pos : vec4<f32>,
a : f32,
}
@stage(vertex) fn main() -> Output {
var output : Output;
return output;
}
)"));
// Fragment input
utils::CreateShaderModule(device, R"(
@stage(fragment) fn main(@location(0) a : vec4<f32>) -> @location(0) f32 {
return 1.0;
}
)");
ASSERT_DEVICE_ERROR(utils::CreateShaderModule(device, R"(
@stage(fragment) fn main(a : vec4<f32>) -> @location(0) f32 {
return 1.0;
}
)"));
// Fragment input
utils::CreateShaderModule(device, R"(
@stage(fragment) fn main() -> @location(0) f32 {
return 1.0;
}
)");
ASSERT_DEVICE_ERROR(utils::CreateShaderModule(device, R"(
@stage(fragment) fn main() -> f32 {
return 1.0;
}
)"));
// Binding decorations
utils::CreateShaderModule(device, R"(
@group(0) @binding(0) var s : sampler;
@stage(fragment) fn main() -> @location(0) f32 {
_ = s;
return 1.0;
}
)");
ASSERT_DEVICE_ERROR(utils::CreateShaderModule(device, R"(
@binding(0) var s : sampler;
@stage(fragment) fn main() -> @location(0) f32 {
_ = s;
return 1.0;
}
)"));
ASSERT_DEVICE_ERROR(utils::CreateShaderModule(device, R"(
@group(0) var s : sampler;
@stage(fragment) fn main() -> @location(0) f32 {
_ = s;
return 1.0;
}
)"));
}