[tint][ast][msl] Remove the need for SingleEntryPoint to be run
Before the PixelLocal transform. As chromium:343072269 points out, this dependency wasn't really required.
Fixed: 343072269
Change-Id: Ic4de89ffbda2fc1b7e5d1f9cff95dc2b98584e5e
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/190401
Auto-Submit: Ben Clayton <bclayton@google.com>
Commit-Queue: James Price <jrprice@google.com>
Reviewed-by: James Price <jrprice@google.com>
diff --git a/src/tint/lang/msl/writer/ast_raise/pixel_local.cc b/src/tint/lang/msl/writer/ast_raise/pixel_local.cc
index 16d849b..63c58b2 100644
--- a/src/tint/lang/msl/writer/ast_raise/pixel_local.cc
+++ b/src/tint/lang/msl/writer/ast_raise/pixel_local.cc
@@ -89,25 +89,26 @@
}
}
- // Find the single entry point
- const sem::Function* entry_point = nullptr;
+ /// The pixel local struct
+ Hashset<const sem::Struct*, 1> pixel_local_structs;
+
+ // Find the entry points
for (auto* fn : src.AST().Functions()) {
- if (fn->IsEntryPoint()) {
- if (entry_point != nullptr) {
- TINT_ICE() << "PixelLocal transform requires that the SingleEntryPoint "
- "transform has already been run";
+ if (!fn->IsEntryPoint()) {
+ continue;
+ }
+
+ auto* entry_point = sem.Get(fn);
+
+ // Look for a `var<pixel_local>` used by the entry point...
+ for (auto* global : entry_point->TransitivelyReferencedGlobals()) {
+ if (global->AddressSpace() != core::AddressSpace::kPixelLocal) {
+ continue;
}
- entry_point = sem.Get(fn);
- // Look for a `var<pixel_local>` used by the entry point...
- for (auto* global : entry_point->TransitivelyReferencedGlobals()) {
- if (global->AddressSpace() != core::AddressSpace::kPixelLocal) {
- continue;
- }
-
- // Obtain struct of the pixel local.
- auto* pixel_local_str = global->Type()->UnwrapRef()->As<sem::Struct>();
-
+ // Obtain struct of the pixel local.
+ auto* pixel_local_str = global->Type()->UnwrapRef()->As<sem::Struct>();
+ if (pixel_local_structs.Add(pixel_local_str)) {
// Add an Color attribute to each member of the pixel_local structure.
for (auto* member : pixel_local_str->Members()) {
ctx.InsertBack(member->Declaration()->attributes,
@@ -115,12 +116,11 @@
ctx.InsertBack(member->Declaration()->attributes,
b.Disable(ast::DisabledValidation::kEntryPointParameter));
}
-
- TransformEntryPoint(entry_point, global, pixel_local_str);
- made_changes = true;
-
- break; // Only a single `var<pixel_local>` can be used by an entry point.
}
+
+ TransformEntryPoint(entry_point, global, pixel_local_str);
+ made_changes = true;
+ break; // Only a single `var<pixel_local>` can be used by an entry point.
}
}
@@ -217,14 +217,6 @@
auto& member_attrs = member->Declaration()->attributes;
add_member(member->Type(), ctx.Clone(member_attrs));
return_args.Push(b.MemberAccessor(call_result, ctx.Clone(member->Name())));
- if (auto* location = ast::GetAttribute<ast::LocationAttribute>(member_attrs)) {
- // Remove the @location attribute from the member of the inner function's
- // output structure.
- // Note: This will break other entry points that share the same output
- // structure, however this transform assumes that the SingleEntryPoint
- // transform will have already been run.
- ctx.Remove(member_attrs, location);
- }
}
} else {
// The entry point returned a non-structure
diff --git a/src/tint/lang/msl/writer/ast_raise/pixel_local.h b/src/tint/lang/msl/writer/ast_raise/pixel_local.h
index 4459e59..2fa06d5 100644
--- a/src/tint/lang/msl/writer/ast_raise/pixel_local.h
+++ b/src/tint/lang/msl/writer/ast_raise/pixel_local.h
@@ -48,7 +48,6 @@
/// copied to the module-scope var before calling the 'inner' function.
/// * The outer function will have a new struct return type which holds both the pixel local members
/// and the returned value(s) of the 'inner' function.
-/// @note PixelLocal requires that the SingleEntryPoint transform has already been run
class PixelLocal final : public Castable<PixelLocal, ast::transform::Transform> {
public:
/// Transform configuration options
diff --git a/src/tint/lang/msl/writer/ast_raise/pixel_local_test.cc b/src/tint/lang/msl/writer/ast_raise/pixel_local_test.cc
index fba0af3..09776c1 100644
--- a/src/tint/lang/msl/writer/ast_raise/pixel_local_test.cc
+++ b/src/tint/lang/msl/writer/ast_raise/pixel_local_test.cc
@@ -84,7 +84,7 @@
EXPECT_EQ(expect, str(got));
}
-TEST_F(PixelLocalTest, UseInEntryPoint) {
+TEST_F(PixelLocalTest, UsedInSingleEntryPoint) {
auto* src = R"(
enable chromium_experimental_pixel_local;
@@ -133,6 +133,231 @@
EXPECT_EQ(expect, str(got));
}
+TEST_F(PixelLocalTest, SameVarUsedInMultipleEntryPoints) {
+ auto* src = R"(
+enable chromium_experimental_pixel_local;
+
+struct PixelLocal {
+ a : u32,
+}
+
+var<pixel_local> P : PixelLocal;
+
+@fragment
+fn F1() {
+ P.a += 10;
+}
+
+@fragment
+fn F2() {
+ P.a += 20;
+}
+)";
+
+ auto* expect =
+ R"(
+enable chromium_experimental_framebuffer_fetch;
+
+struct F1_res {
+ @location(1)
+ output_0 : u32,
+}
+
+@fragment
+fn F1(pixel_local_1 : PixelLocal) -> F1_res {
+ P = pixel_local_1;
+ F1_inner();
+ return F1_res(P.a);
+}
+
+struct F2_res {
+ @location(1)
+ output_0 : u32,
+}
+
+@fragment
+fn F2(pixel_local_2 : PixelLocal) -> F2_res {
+ P = pixel_local_2;
+ F2_inner();
+ return F2_res(P.a);
+}
+
+struct PixelLocal {
+ @color(1u) @internal(disable_validation__entry_point_parameter)
+ a : u32,
+}
+
+var<private> P : PixelLocal;
+
+fn F1_inner() {
+ P.a += 10;
+}
+
+fn F2_inner() {
+ P.a += 20;
+}
+)";
+
+ auto got = Run<PixelLocal>(src, Bindings({{0, 1}}));
+
+ EXPECT_EQ(expect, str(got));
+}
+
+TEST_F(PixelLocalTest, DifferentVarUsedInMultipleEntryPoints) {
+ auto* src = R"(
+enable chromium_experimental_pixel_local;
+
+struct PixelLocal {
+ a : u32,
+}
+
+var<pixel_local> P1 : PixelLocal;
+var<pixel_local> P2 : PixelLocal;
+
+@fragment
+fn F1() {
+ P1.a += 10;
+}
+
+@fragment
+fn F2() {
+ P2.a += 20;
+}
+)";
+
+ auto* expect =
+ R"(
+enable chromium_experimental_framebuffer_fetch;
+
+struct F1_res {
+ @location(1)
+ output_0 : u32,
+}
+
+@fragment
+fn F1(pixel_local_1 : PixelLocal) -> F1_res {
+ P1 = pixel_local_1;
+ F1_inner();
+ return F1_res(P1.a);
+}
+
+struct F2_res {
+ @location(1)
+ output_0 : u32,
+}
+
+@fragment
+fn F2(pixel_local_2 : PixelLocal) -> F2_res {
+ P2 = pixel_local_2;
+ F2_inner();
+ return F2_res(P2.a);
+}
+
+struct PixelLocal {
+ @color(1u) @internal(disable_validation__entry_point_parameter)
+ a : u32,
+}
+
+var<private> P1 : PixelLocal;
+
+var<private> P2 : PixelLocal;
+
+fn F1_inner() {
+ P1.a += 10;
+}
+
+fn F2_inner() {
+ P2.a += 20;
+}
+)";
+
+ auto got = Run<PixelLocal>(src, Bindings({{0, 1}}));
+
+ EXPECT_EQ(expect, str(got));
+}
+
+TEST_F(PixelLocalTest, DifferentStructUsedInMultipleEntryPoints) {
+ auto* src = R"(
+enable chromium_experimental_pixel_local;
+
+struct PixelLocal1 {
+ a : u32,
+}
+
+struct PixelLocal2 {
+ a : u32,
+}
+
+var<pixel_local> P1 : PixelLocal1;
+var<pixel_local> P2 : PixelLocal2;
+
+@fragment
+fn F1() {
+ P1.a += 10;
+}
+
+@fragment
+fn F2() {
+ P2.a += 20;
+}
+)";
+
+ auto* expect =
+ R"(
+enable chromium_experimental_framebuffer_fetch;
+
+struct F1_res {
+ @location(1)
+ output_0 : u32,
+}
+
+@fragment
+fn F1(pixel_local_1 : PixelLocal1) -> F1_res {
+ P1 = pixel_local_1;
+ F1_inner();
+ return F1_res(P1.a);
+}
+
+struct F2_res {
+ @location(1)
+ output_0 : u32,
+}
+
+@fragment
+fn F2(pixel_local_2 : PixelLocal2) -> F2_res {
+ P2 = pixel_local_2;
+ F2_inner();
+ return F2_res(P2.a);
+}
+
+struct PixelLocal1 {
+ @color(1u) @internal(disable_validation__entry_point_parameter)
+ a : u32,
+}
+
+struct PixelLocal2 {
+ @color(1u) @internal(disable_validation__entry_point_parameter)
+ a : u32,
+}
+
+var<private> P1 : PixelLocal1;
+
+var<private> P2 : PixelLocal2;
+
+fn F1_inner() {
+ P1.a += 10;
+}
+
+fn F2_inner() {
+ P2.a += 20;
+}
+)";
+
+ auto got = Run<PixelLocal>(src, Bindings({{0, 1}}));
+
+ EXPECT_EQ(expect, str(got));
+}
+
TEST_F(PixelLocalTest, UseInCallee) {
auto* src = R"(
enable chromium_experimental_pixel_local;
@@ -793,7 +1018,9 @@
var<private> P : PixelLocal;
struct Output {
+ @location(0)
x : vec4f,
+ @location(2)
y : vec4f,
}
@@ -808,5 +1035,87 @@
EXPECT_EQ(expect, str(got));
}
+TEST_F(PixelLocalTest, OutputStructUsedByMultipleEntryPoints) {
+ auto* src = R"(
+enable chromium_experimental_pixel_local;
+
+struct PixelLocal {
+ a : u32,
+ b : u32,
+}
+
+var<pixel_local> P : PixelLocal;
+
+struct Output {
+ @location(0) x : vec4f,
+ @location(2) y : vec4f,
+}
+
+@fragment
+fn F1() -> Output {
+ P.a += 42;
+ return Output(vec4f(1), vec4f(9));
+}
+
+@fragment
+fn F2() -> Output {
+ return Output(vec4f(1), vec4f(9));
+}
+)";
+
+ auto* expect =
+ R"(
+enable chromium_experimental_framebuffer_fetch;
+
+struct F1_res {
+ @location(1)
+ output_0 : u32,
+ @location(5)
+ output_1 : u32,
+ @location(0)
+ output_2 : vec4<f32>,
+ @location(2)
+ output_3 : vec4<f32>,
+}
+
+@fragment
+fn F1(pixel_local_1 : PixelLocal) -> F1_res {
+ P = pixel_local_1;
+ let result = F1_inner();
+ return F1_res(P.a, P.b, result.x, result.y);
+}
+
+struct PixelLocal {
+ @color(1u) @internal(disable_validation__entry_point_parameter)
+ a : u32,
+ @color(5u) @internal(disable_validation__entry_point_parameter)
+ b : u32,
+}
+
+var<private> P : PixelLocal;
+
+struct Output {
+ @location(0)
+ x : vec4f,
+ @location(2)
+ y : vec4f,
+}
+
+fn F1_inner() -> Output {
+ P.a += 42;
+ return Output(vec4f(1), vec4f(9));
+}
+
+@fragment
+fn F2() -> Output {
+ return Output(vec4f(1), vec4f(9));
+}
+)";
+
+ auto got = Run<PixelLocal>(src, Bindings({{0, 1}, {1, 5}}));
+
+ EXPECT_EQ(expect, str(got));
+}
+
} // namespace
} // namespace tint::msl::writer
diff --git a/src/tint/lang/msl/writer/writer_ast_fuzz.cc b/src/tint/lang/msl/writer/writer_ast_fuzz.cc
index 12c1a55..1c6e09a 100644
--- a/src/tint/lang/msl/writer/writer_ast_fuzz.cc
+++ b/src/tint/lang/msl/writer/writer_ast_fuzz.cc
@@ -36,8 +36,7 @@
namespace {
void ASTFuzzer(const tint::Program& program, const fuzz::wgsl::Context& context, Options options) {
- if (program.AST().HasOverrides() ||
- context.program_properties.Contains(fuzz::wgsl::ProgramProperties::kMultipleEntryPoints)) {
+ if (program.AST().HasOverrides()) {
// MSL writer assumes the SubstituteOverride and SingleEntryPoint transforms have been run
return;
}
