Require both `blend_src(0)` and `blend_src(1)` must be used in one struct
This patch adds a new check on dual source blending that both
`@location(0) @blend_src(0)` and `@location(0) @blend_src(1)` must be
used in one struct, and other members (if exist) must not have
`@location` attribute, or neither of them are used.
As now this rule is applied to all structs, the original checks on the
entry point are all removed.
Bug: chromium:341973423
Test: tint_unittests
Change-Id: I71a7b088e57bfd67eb1adf9f1c053f1592c3ce5a
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/191766
Commit-Queue: Jiawei Shao <jiawei.shao@intel.com>
Reviewed-by: James Price <jrprice@google.com>
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 ded56eb..5a53f76 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
@@ -123,20 +123,35 @@
EXPECT_EQ(r()->error(), "12:34 error: '@blend_src' can only be used with '@location(0)'");
}
-// Using an index attribute on a struct member should pass.
-TEST_F(DualSourceBlendingExtensionTests, StructMemberBlendSrcAttribute) {
+// Using a @blend_src attribute on a struct member while there is only one member in the struct
+// should fail.
+TEST_F(DualSourceBlendingExtensionTests, StructMemberBlendSrcAttribute_OnlyBlendSrc0) {
Structure("Output", Vector{
Member("a", ty.vec4<f32>(),
Vector{Location(0_a), BlendSrc(Source{{12, 34}}, 0_a)}),
});
- EXPECT_TRUE(r()->Resolve()) << r()->error();
+ EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(r()->error(), "12:34 error: '@blend_src(1)' is missing when '@blend_src' is used");
}
-// Using an index attribute on a global variable should pass. This is needed internally when using
-// @blend_src with the canonicalize_entry_point transform. This test uses an internal attribute to
-// ignore address space, which is how it is used with the canonicalize_entry_point transform.
-TEST_F(DualSourceBlendingExtensionTests, GlobalVariableBlendSrcAttribute) {
+// Using a @blend_src attribute on a struct member while there is only one member in the struct
+// should fail.
+TEST_F(DualSourceBlendingExtensionTests, StructMemberBlendSrcAttribute_OnlyBlendSrc1) {
+ Structure("Output", Vector{
+ Member("a", ty.vec4<f32>(),
+ Vector{Location(0_a), BlendSrc(Source{{12, 34}}, 1_a)}),
+ });
+
+ EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(r()->error(), "12:34 error: '@blend_src(0)' is missing when '@blend_src' is used");
+}
+
+// Using a @blend_src attribute on a global variable should pass. This is needed internally when
+// using @blend_src with the canonicalize_entry_point transform. This test uses an internal
+// attribute to ignore address space, which is how it is used with the canonicalize_entry_point
+// transform.
+TEST_F(DualSourceBlendingExtensionTests, GlobalVariableBlendSrcAttributeAfterInternalTransform) {
GlobalVar(
"var", ty.vec4<f32>(),
Vector{Location(0_a), BlendSrc(0_a), Disable(ast::DisabledValidation::kIgnoreAddressSpace)},
@@ -156,87 +171,139 @@
EXPECT_EQ(r()->error(), "12:34 error: '@blend_src' can only be used with '@location(0)'");
}
-TEST_F(DualSourceBlendingExtensionTests, MixedBlendSrcAndNonBlendSrcOnLocationZero_Struct) {
+// Using @blend_src and @location(0) on two members and having another without @location should not
+// fail.
+TEST_F(DualSourceBlendingExtensionTests, BlendSrcAndNonLocationNonBlendSrc) {
// struct S {
- // @location(0) a : vec4<f32>,
- // @location(0) @blend_src(1) b : vec4<f32>,
+ // a : vec4<f32>,
+ // @location(0) @blend_src(0) b : vec4<f32>,
+ // @location(0) @blend_src(1) c : 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)}),
- });
+ Structure("S",
+ Vector{
+ Member("a", ty.vec4<f32>()),
+ Member("b", ty.vec4<f32>(), Vector{Location(0_a), BlendSrc(Source{{3, 4}}, 0_a)}),
+ Member("c", ty.vec4<f32>(), Vector{Location(0_a), BlendSrc(Source{{5, 6}}, 1_a)}),
+ });
EXPECT_TRUE(r()->Resolve());
}
-TEST_F(DualSourceBlendingExtensionTests,
- MixedBlendSrcAndNonBlendSrcOnLocationZero_StructUsedInEntryPoint) {
+// Using @blend_src and @location(0) on two members and having another member with @location but
+// without @blend_src should fail.
+TEST_F(DualSourceBlendingExtensionTests, ZeroLocationAndNonBlendSrcBeforeBlendSrc) {
// struct S {
// @location(0) a : vec4<f32>,
- // @location(0) @blend_src(1) b : vec4<f32>,
+ // @location(0) @blend_src(0) b : vec4<f32>,
+ // @location(0) @blend_src(1) c : 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)}),
+ Member("a", ty.vec4<f32>(), Vector{Location(0_a)}),
+ Member(Source{{12, 34}}, "b", ty.vec4<f32>(),
+ Vector{Location(0_a), BlendSrc(0_a)}),
+ Member("c", ty.vec4<f32>(), Vector{Location(0_a), BlendSrc(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')");
+ R"(12:34 error: '@blend_src' and '@location' are used on one member while another member with '@location' doesn't use '@blend_src')");
}
+// Using @blend_src and @location(0) on two members and having another member with @location but
+// without @blend_src should fail.
+TEST_F(DualSourceBlendingExtensionTests, ZeroLocationAndNonBlendSrcAfterBlendSrc) {
+ // struct S {
+ // @location(0) @blend_src(0) a : vec4<f32>,
+ // @location(0) b : vec4<f32>,
+ // @location(0) @blend_src(1) c : vec4<f32>,
+ // };
+ Structure("S", Vector{
+ Member("a", ty.vec4<f32>(), Vector{Location(0_a), BlendSrc(0_a)}),
+ Member(Source{{12, 34}}, "b", ty.vec4<f32>(), Vector{Location(0_a)}),
+ Member("c", ty.vec4<f32>(), Vector{Location(0_a), BlendSrc(1_a)}),
+ });
+
+ EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(
+ r()->error(),
+ R"(12:34 error: '@blend_src' and '@location' are used on one member while another member with '@location' doesn't use '@blend_src')");
+}
+
+// Using @blend_src and @location(0) on two members and having another member with @location but
+// without @blend_src should fail.
+TEST_F(DualSourceBlendingExtensionTests, NonZeroLocationAndNonBlendSrcBeforeBlendSrc) {
+ // struct S {
+ // @location(1) a : vec4<f32>,
+ // @location(0) @blend_src(0) b : vec4<f32>,
+ // @location(0) @blend_src(1) c : vec4<f32>,
+ // };
+ Structure("S", Vector{
+ Member("a", ty.vec4<f32>(), Vector{Location(1_a)}),
+ Member(Source{{12, 34}}, "b", ty.vec4<f32>(),
+ Vector{Location(0_a), BlendSrc(0_a)}),
+ Member("c", ty.vec4<f32>(), Vector{Location(0_a), BlendSrc(1_a)}),
+ });
+
+ EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(
+ r()->error(),
+ R"(12:34 error: '@blend_src' and '@location' are used on one member while another member with '@location' doesn't use '@blend_src')");
+}
+
+// Using @blend_src and @location(0) on two members and having another member with @location but
+// without @blend_src should fail.
+TEST_F(DualSourceBlendingExtensionTests, NonZeroLocationAndNonBlendSrcAfterBlendSrc) {
+ // struct S {
+ // @location(0) @blend_src(0) a : vec4<f32>,
+ // @location(1) b : vec4<f32>,
+ // @location(0) @blend_src(1) c : vec4<f32>,
+ // };
+ Structure("S", Vector{
+ Member("a", ty.vec4<f32>(), Vector{Location(0_a), BlendSrc(0_a)}),
+ Member(Source{{12, 34}}, "b", ty.vec4<f32>(), Vector{Location(1_a)}),
+ Member("c", ty.vec4<f32>(), Vector{Location(0_a), BlendSrc(1_a)}),
+ });
+
+ EXPECT_FALSE(r()->Resolve());
+ EXPECT_EQ(
+ r()->error(),
+ R"(12:34 error: '@blend_src' and '@location' are used on one member while another member with '@location' doesn't use '@blend_src')");
+}
+
+// The members with @blend_src and @location(0) must have same type.
TEST_F(DualSourceBlendingExtensionTests, BlendSrcTypes_DifferentWidth) {
// struct S {
// @location(0) @blend_src(0) a : vec4<f32>,
// @location(0) @blend_src(1) b : vec2<f32>,
// };
- // @fragment fn F() -> S { return S(); }
Structure("S",
Vector{
Member("a", ty.vec4<f32>(), Vector{Location(0_a), BlendSrc(0_a)}),
Member("b", ty.vec2<f32>(), Vector{Location(0_a), BlendSrc(Source{{1, 2}}, 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"(1:2 error: Use of '@blend_src' requires all outputs have same type
-note: while analyzing entry point 'F')");
+ EXPECT_EQ(r()->error(), R"(1:2 error: All the outputs with '@blend_src' must have same type)");
}
+// The members with @blend_src and @location(0) must have same type.
TEST_F(DualSourceBlendingExtensionTests, BlendSrcTypes_DifferentElementType) {
// struct S {
// @location(0) @blend_src(0) a : vec4<f32>,
// @location(0) @blend_src(1) b : vec4<i32>,
// };
- // @fragment fn F() -> S { return S(); }
Structure("S",
Vector{
Member("a", ty.vec4<f32>(), Vector{Location(0_a), BlendSrc(0_a)}),
Member("b", ty.vec4<i32>(), Vector{Location(0_a), BlendSrc(Source{{1, 2}}, 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"(1:2 error: Use of '@blend_src' requires all outputs have same type
-note: while analyzing entry point 'F')");
+ EXPECT_EQ(r()->error(), R"(1:2 error: All the outputs with '@blend_src' must have same type)");
}
+// It is not allowed to use a struct with @blend_src as fragment shader input.
TEST_F(DualSourceBlendingExtensionTests, BlendSrcAsFragmentInput) {
// struct S {
// @location(0) @blend_src(0) a : vec4<f32>,
@@ -255,6 +322,7 @@
note: while analyzing entry point 'F')");
}
+// It is not allowed to use @blend_src on the fragment output declaration.
TEST_F(DualSourceBlendingExtensionTest, BlendSrcOnNonStructFragmentOutput) {
// enable dual_source_blending;
// @fragment fn F() -> @location(0) @blend_src(0) vec4<f32> {
@@ -287,7 +355,7 @@
Vector{
Member("a", ty.vec4<f32>(), Vector{Location(0_a), BlendSrc(0_a)}),
Member("b", ty.vec4<f32>(), Vector{Location(0_a), BlendSrc(Source{{1, 2}}, 1_a)}),
- Member("c", ty.vec4<f32>(), Vector{Location(Source{{3, 4}}, AInt(GetParam()))}),
+ Member(Source{{3, 4}}, "c", ty.vec4<f32>(), Vector{Location(AInt(GetParam()))}),
});
Func("F", Empty, ty("S"), Vector{Return(Call("S"))},
Vector{Stage(ast::PipelineStage::kFragment)});
@@ -295,9 +363,7 @@
EXPECT_FALSE(r()->Resolve());
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')");
+ R"(3:4 error: '@blend_src' and '@location' are used on one member while another member with '@location' doesn't use '@blend_src')");
}
TEST_P(DualSourceBlendingExtensionTestWithParams,
@@ -310,8 +376,8 @@
// fn F() -> S { return S(); }
Structure("S",
Vector{
- Member("a", ty.vec4<f32>(), Vector{Location(Source{{1, 2}}, AInt(GetParam()))}),
- Member("b", ty.vec4<f32>(), Vector{Location(0_a), BlendSrc(0_a)}),
+ Member("a", ty.vec4<f32>(), Vector{Location(AInt(GetParam()))}),
+ Member(Source{{1, 2}}, "b", ty.vec4<f32>(), Vector{Location(0_a), BlendSrc(0_a)}),
Member("c", ty.vec4<f32>(), Vector{Location(0_a), BlendSrc(Source{{3, 4}}, 1_a)}),
});
Func(Source{{5, 6}}, "F", Empty, ty("S"), Vector{Return(Call("S"))},
@@ -320,9 +386,7 @@
EXPECT_FALSE(r()->Resolve());
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')");
+ R"(1:2 error: '@blend_src' and '@location' are used on one member while another member with '@location' doesn't use '@blend_src')");
}
INSTANTIATE_TEST_SUITE_P(DualSourceBlendingExtensionTests,
diff --git a/src/tint/lang/wgsl/resolver/validator.cc b/src/tint/lang/wgsl/resolver/validator.cc
index a19c181..6de0336 100644
--- a/src/tint/lang/wgsl/resolver/validator.cc
+++ b/src/tint/lang/wgsl/resolver/validator.cc
@@ -28,6 +28,7 @@
#include "src/tint/lang/wgsl/resolver/validator.h"
#include <algorithm>
+#include <bitset>
#include <limits>
#include <string_view>
#include <tuple>
@@ -1235,10 +1236,6 @@
// sem::Function since it would be useful there too.
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_blend_src = nullptr;
- const core::type::Type* first_blend_src_type = nullptr;
- const ast::LocationAttribute* first_location_without_blend_src = nullptr;
Hashset<uint32_t, 4> colors;
enum class ParamOrRetType {
kParameter,
@@ -1390,54 +1387,7 @@
}
}
- if (blend_src_attribute) {
- if (location.value_or(1) != 0) {
- AddError(blend_src_attribute->source)
- << style::Attribute("@blend_src") << " can only be used with "
- << style::Attribute("@location")
- << style::Code("(", style::Literal("0"), ")");
- return false;
- }
- if (first_blend_src_type == nullptr) {
- first_blend_src_type = ty;
- } else if (!first_blend_src_type->Equals(*ty)) {
- AddError(blend_src_attribute->source)
- << "Use of " << style::Attribute("@blend_src")
- << " requires all outputs have same type";
- return false;
- }
- }
-
- 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(first_location_without_blend_src->source)
- << "use of " << style::Attribute("@blend_src") << " requires all the output "
- << style::Attribute("@location")
- << " attributes of the entry point to be paired with a "
- << style::Attribute("@blend_src") << " attribute";
- AddNote(first_blend_src->source)
- << "use of " << style::Attribute("@blend_src") << " here";
- return false;
- }
-
if (location_attribute) {
- if (!first_nonzero_location && location > 0u) {
- first_nonzero_location = location_attribute;
- }
- if (first_nonzero_location && first_blend_src) {
- AddError(first_blend_src->source)
- << "pipeline cannot use both a " << style::Attribute("@blend_src")
- << " and non-zero " << style::Attribute("@location");
- AddNote(first_nonzero_location->source)
- << "non-zero " << style::Attribute("@location") << " declared here";
- return false;
- }
-
std::pair<uint32_t, uint32_t> location_and_blend_src(location.value(),
blend_src.value_or(0));
if (!locations_and_blend_srcs.Add(location_and_blend_src)) {
@@ -1533,10 +1483,6 @@
// should be validated independently from those used in parameters.
builtins.Clear();
locations_and_blend_srcs.Clear();
- first_nonzero_location = nullptr;
- first_blend_src = nullptr;
- first_blend_src_type = 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(),
@@ -2323,6 +2269,10 @@
return false;
}
+ std::bitset<2> blend_src_appearance_mask;
+ const ast::BlendSrcAttribute* first_blend_src = nullptr;
+ const core::type::Type* first_blend_src_type = nullptr;
+ const ast::LocationAttribute* first_location_without_blend_src = nullptr;
Hashset<std::pair<uint32_t, std::optional<uint32_t>>, 8> locations_and_blend_srcs;
Hashset<uint32_t, 4> colors;
for (auto* member : str->Members()) {
@@ -2411,15 +2361,38 @@
}
if (blend_src_attribute) {
- // Because HLSL specifies dual source blending targets with SV_Target0 and 1, we should
- // restrict targets with index attributes to location 0 for easy translation in the
- // backend writers.
if (member->Attributes().location.value_or(1) != 0) {
AddError(blend_src_attribute->source)
<< style::Attribute("@blend_src") << " can only be used with "
<< style::Attribute("@location") << style::Code("(", style::Literal("0"), ")");
return false;
}
+
+ if (first_blend_src == nullptr) {
+ first_blend_src = blend_src_attribute;
+ first_blend_src_type = member->Type();
+ } else if (!first_blend_src_type->Equals(*member->Type())) {
+ AddError(blend_src_attribute->source)
+ << "All the outputs with " << style::Attribute("@blend_src")
+ << " must have same type";
+ return false;
+ }
+
+ TINT_ASSERT(member->Attributes().blend_src.has_value() &&
+ *member->Attributes().blend_src <= 1u);
+ uint32_t blend_src = *member->Attributes().blend_src;
+ blend_src_appearance_mask.set(blend_src);
+ } else if (location_attribute) {
+ first_location_without_blend_src = location_attribute;
+ }
+
+ if (first_blend_src && first_location_without_blend_src) {
+ AddError(member->Declaration()->source)
+ << style::Attribute("@blend_src") << " and " << style::Attribute("@location")
+ << " are used on one member while another member with "
+ << style::Attribute("@location") << " doesn't use "
+ << style::Attribute("@blend_src");
+ return false;
}
if (interpolate_attribute && !location_attribute) {
@@ -2458,6 +2431,17 @@
}
}
+ if (first_blend_src != nullptr && !blend_src_appearance_mask.all()) {
+ for (uint32_t i = 0; i < blend_src_appearance_mask.size(); ++i) {
+ if (!blend_src_appearance_mask.test(i)) {
+ AddError(first_blend_src->source)
+ << style::Attribute("@blend_src") << style::Code("(", style::Literal(i), ")")
+ << " is missing when " << style::Attribute("@blend_src") << " is used";
+ }
+ }
+ return false;
+ }
+
return true;
}
