[tint][resolver]: Require @blend_src on all or none of the IO
Fixed: dawn:2380
Change-Id: I106719ad6e2a6f8dd75d9a8b00cc29f54595f3b6
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/172420
Kokoro: Kokoro <noreply+kokoro@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>
Commit-Queue: Corentin Wallez <cwallez@chromium.org>
diff --git a/src/tint/lang/wgsl/resolver/dual_source_blending_extension_test.cc b/src/tint/lang/wgsl/resolver/dual_source_blending_extension_test.cc
index 67a3aab..ab8677e 100644
--- a/src/tint/lang/wgsl/resolver/dual_source_blending_extension_test.cc
+++ b/src/tint/lang/wgsl/resolver/dual_source_blending_extension_test.cc
@@ -215,6 +215,49 @@
EXPECT_TRUE(r()->Resolve()) << r()->error();
}
+TEST_F(DualSourceBlendingExtensionTests, MixedBlendSrcAndNonBlendSrcOnLocationZero_Struct) {
+ // struct S {
+ // @location(0) a : vec4<f32>,
+ // @location(0) @blend_src(1) b : vec4<f32>,
+ // };
+ Structure("S", Vector{
+ Member("a", ty.vec4<f32>(),
+ Vector{
+ Location(Source{{12, 34}}, 0_a),
+ }),
+ Member("b", ty.vec4<f32>(),
+ Vector{Location(0_a), BlendSrc(Source{{56, 78}}, 1_a)}),
+ });
+
+ EXPECT_TRUE(r()->Resolve());
+}
+
+TEST_F(DualSourceBlendingExtensionTests,
+ MixedBlendSrcAndNonBlendSrcOnLocationZero_StructUsedInEntryPoint) {
+ // struct S {
+ // @location(0) a : vec4<f32>,
+ // @location(0) @blend_src(1) b : vec4<f32>,
+ // };
+ // fn F() -> S { return S(); }
+ Structure("S", Vector{
+ Member("a", ty.vec4<f32>(),
+ Vector{
+ Location(Source{{12, 34}}, 0_a),
+ }),
+ Member("b", ty.vec4<f32>(),
+ Vector{Location(0_a), BlendSrc(Source{{56, 78}}, 1_a)}),
+ });
+ Func("F", Empty, ty("S"), Vector{Return(Call("S"))},
+ Vector{Stage(ast::PipelineStage::kFragment)});
+
+ EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(
+ r()->error(),
+ R"(12:34 error: use of @blend_src requires all the output @location attributes of the entry point to be paired with a @blend_src attribute
+56:78 note: use of @blend_src here
+note: while analyzing entry point 'F')");
+}
+
class DualSourceBlendingExtensionTestWithParams : public ResolverTestWithParam<int> {
public:
DualSourceBlendingExtensionTestWithParams() {
@@ -224,7 +267,7 @@
// Rendering to multiple render targets while using dual source blending should fail.
TEST_P(DualSourceBlendingExtensionTestWithParams,
- MultipleRenderTargetsNotAllowed_BlendSrcThenNonZeroLocation) {
+ MultipleRenderTargetsNotAllowed_BlendSrcAndNoBlendSrc) {
// struct S {
// @location(0) @blend_src(0) a : vec4<f32>,
// @location(0) @blend_src(1) b : vec4<f32>,
@@ -241,9 +284,10 @@
Vector{Stage(ast::PipelineStage::kFragment)});
EXPECT_FALSE(r()->Resolve());
- EXPECT_EQ(r()->error(),
- R"(1:2 error: pipeline cannot use both non-zero @blend_src and non-zero @location
-3:4 note: non-zero @location declared here
+ EXPECT_EQ(
+ r()->error(),
+ R"(3:4 error: use of @blend_src requires all the output @location attributes of the entry point to be paired with a @blend_src attribute
+1:2 note: use of @blend_src here
note: while analyzing entry point 'F')");
}
@@ -265,9 +309,10 @@
Vector{Stage(ast::PipelineStage::kFragment)});
EXPECT_FALSE(r()->Resolve());
- EXPECT_EQ(r()->error(),
- R"(3:4 error: pipeline cannot use both non-zero @blend_src and non-zero @location
-1:2 note: non-zero @location declared here
+ EXPECT_EQ(
+ r()->error(),
+ R"(1:2 error: use of @blend_src requires all the output @location attributes of the entry point to be paired with a @blend_src attribute
+note: use of @blend_src here
5:6 note: while analyzing entry point 'F')");
}
diff --git a/src/tint/lang/wgsl/resolver/validator.cc b/src/tint/lang/wgsl/resolver/validator.cc
index 907a3c4..0201bb4 100644
--- a/src/tint/lang/wgsl/resolver/validator.cc
+++ b/src/tint/lang/wgsl/resolver/validator.cc
@@ -1149,7 +1149,8 @@
Hashset<core::BuiltinValue, 4> builtins;
Hashset<std::pair<uint32_t, uint32_t>, 8> locations_and_blend_srcs;
const ast::LocationAttribute* first_nonzero_location = nullptr;
- const ast::BlendSrcAttribute* first_nonzero_blend_src = nullptr;
+ const ast::BlendSrcAttribute* first_blend_src = nullptr;
+ const ast::LocationAttribute* first_location_without_blend_src = nullptr;
Hashset<uint32_t, 4> colors;
enum class ParamOrRetType {
kParameter,
@@ -1311,16 +1312,28 @@
}
}
+ if (blend_src_attribute) {
+ first_blend_src = blend_src_attribute;
+ } else if (location_attribute) {
+ first_location_without_blend_src = location_attribute;
+ }
+
+ if (first_blend_src && first_location_without_blend_src) {
+ AddError(
+ "use of @blend_src requires all the output @location attributes of the entry "
+ "point to be paired with a @blend_src attribute",
+ first_location_without_blend_src->source);
+ AddNote("use of @blend_src here", first_blend_src->source);
+ return false;
+ }
+
if (location_attribute) {
if (!first_nonzero_location && location > 0u) {
first_nonzero_location = location_attribute;
}
- if (!first_nonzero_blend_src && blend_src > 0u) {
- first_nonzero_blend_src = blend_src_attribute;
- }
- if (first_nonzero_location && first_nonzero_blend_src) {
- AddError("pipeline cannot use both non-zero @blend_src and non-zero @location",
- first_nonzero_blend_src->source);
+ if (first_nonzero_location && first_blend_src) {
+ AddError("pipeline cannot use both a @blend_src and non-zero @location",
+ first_blend_src->source);
AddNote("non-zero @location declared here", first_nonzero_location->source);
return false;
}
@@ -1416,7 +1429,8 @@
builtins.Clear();
locations_and_blend_srcs.Clear();
first_nonzero_location = nullptr;
- first_nonzero_blend_src = nullptr;
+ first_blend_src = nullptr;
+ first_location_without_blend_src = nullptr;
if (!func->ReturnType()->Is<core::type::Void>()) {
if (!validate_entry_point_attributes(decl->return_type_attributes, func->ReturnType(),
@@ -2226,7 +2240,7 @@
return false;
}
- Hashset<std::pair<uint32_t, uint32_t>, 8> locations_and_blend_srcs;
+ Hashset<std::pair<uint32_t, std::optional<uint32_t>>, 8> locations_and_blend_srcs;
Hashset<uint32_t, 4> colors;
for (auto* member : str->Members()) {
if (auto* r = member->Type()->As<sem::Array>()) {
@@ -2330,17 +2344,16 @@
return false;
}
- // Ensure all locations and index pairs are unique
+ // Ensure all locations and optional blend_src pairs are unique
if (location_attribute) {
uint32_t location = member->Attributes().location.value();
- uint32_t blend_src = member->Attributes().blend_src.value_or(0);
+ std::optional<uint32_t> blend_src = member->Attributes().blend_src;
- std::pair<uint32_t, uint32_t> location_and_blend_src(location, blend_src);
- if (!locations_and_blend_srcs.Add(location_and_blend_src)) {
+ if (!locations_and_blend_srcs.Add(std::make_pair(location, blend_src))) {
StringStream err;
err << "@location(" << location << ") ";
- if (blend_src_attribute) {
- err << "@blend_src(" << blend_src << ") ";
+ if (blend_src) {
+ err << "@blend_src(" << blend_src.value() << ") ";
}
err << "appears multiple times";
AddError(err.str(), location_attribute->source);
