From 2c2aa2a76a1a920cbf3efe233536686ecf1406d3 Mon Sep 17 00:00:00 2001 From: James Price Date: Mon, 12 Jul 2021 16:11:41 +0000 Subject: [PATCH] writer/msl: Implement invariant attribute Report whether one was generated so that Dawn knows to use the `-fpreserve-invariance` compiler option. Bug: tint:772 Change-Id: Ife1eb05265646727dc864f12f983781af4df3777 Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/57644 Kokoro: Kokoro Reviewed-by: Ben Clayton Commit-Queue: Ben Clayton --- src/transform/canonicalize_entry_point_io.cc | 18 +++---- .../canonicalize_entry_point_io_test.cc | 52 +++++++++++++++++++ src/writer/msl/generator.cc | 1 + src/writer/msl/generator.h | 3 ++ src/writer/msl/generator_impl.cc | 3 ++ src/writer/msl/generator_impl.h | 6 +++ src/writer/msl/generator_impl_test.cc | 49 +++++++++++++++++ test/shader_io/invariant.wgsl.expected.msl | 18 ++++--- .../invariant_struct_member.wgsl.expected.msl | 22 +++++--- 9 files changed, 147 insertions(+), 25 deletions(-) diff --git a/src/transform/canonicalize_entry_point_io.cc b/src/transform/canonicalize_entry_point_io.cc index e8bc3dc454..bd30b0d332 100644 --- a/src/transform/canonicalize_entry_point_io.cc +++ b/src/transform/canonicalize_entry_point_io.cc @@ -84,9 +84,9 @@ void CanonicalizeEntryPointIO::Run(CloneContext& ctx, for (auto* member : struct_ty->members()) { ast::DecorationList new_decorations = RemoveDecorations( &ctx, member->decorations(), [](const ast::Decoration* deco) { - return deco - ->IsAnyOf(); + return deco->IsAnyOf< + ast::BuiltinDecoration, ast::InterpolateDecoration, + ast::InvariantDecoration, ast::LocationDecoration>(); }); new_struct_members.push_back( ctx.dst->Member(ctx.Clone(member->symbol()), @@ -156,9 +156,9 @@ void CanonicalizeEntryPointIO::Run(CloneContext& ctx, ast::DecorationList new_decorations = RemoveDecorations( &ctx, member->Declaration()->decorations(), [](const ast::Decoration* deco) { - return !deco->IsAnyOf(); + return !deco->IsAnyOf< + ast::BuiltinDecoration, ast::InterpolateDecoration, + ast::InvariantDecoration, ast::LocationDecoration>(); }); if (cfg->builtin_style == BuiltinStyle::kParameter && @@ -246,9 +246,9 @@ void CanonicalizeEntryPointIO::Run(CloneContext& ctx, ast::DecorationList new_decorations = RemoveDecorations( &ctx, member->Declaration()->decorations(), [](const ast::Decoration* deco) { - return !deco->IsAnyOf(); + return !deco->IsAnyOf< + ast::BuiltinDecoration, ast::InterpolateDecoration, + ast::InvariantDecoration, ast::LocationDecoration>(); }); auto symbol = ctx.Clone(member->Declaration()->symbol()); auto* member_ty = ctx.Clone(member->Declaration()->type()); diff --git a/src/transform/canonicalize_entry_point_io_test.cc b/src/transform/canonicalize_entry_point_io_test.cc index d0a59e34c3..1f12d40e8e 100644 --- a/src/transform/canonicalize_entry_point_io_test.cc +++ b/src/transform/canonicalize_entry_point_io_test.cc @@ -691,6 +691,58 @@ fn frag_main(tint_symbol_2 : tint_symbol_3) { EXPECT_EQ(expect, str(got)); } +TEST_F(CanonicalizeEntryPointIOTest, InvariantAttributes) { + auto* src = R"( +struct VertexOut { + [[builtin(position), invariant]] pos : vec4; +}; + +[[stage(vertex)]] +fn main1() -> VertexOut { + return VertexOut(); +} + +[[stage(vertex)]] +fn main2() -> [[builtin(position), invariant]] vec4 { + return vec4(); +} +)"; + + auto* expect = R"( +struct VertexOut { + pos : vec4; +}; + +struct tint_symbol { + [[builtin(position), invariant]] + pos : vec4; +}; + +[[stage(vertex)]] +fn main1() -> tint_symbol { + let tint_symbol_1 : VertexOut = VertexOut(); + return tint_symbol(tint_symbol_1.pos); +} + +struct tint_symbol_2 { + [[builtin(position), invariant]] + value : vec4; +}; + +[[stage(vertex)]] +fn main2() -> tint_symbol_2 { + return tint_symbol_2(vec4()); +} +)"; + + DataMap data; + data.Add( + CanonicalizeEntryPointIO::BuiltinStyle::kStructMember); + auto got = Run(src, data); + + EXPECT_EQ(expect, str(got)); +} + TEST_F(CanonicalizeEntryPointIOTest, Struct_LayoutDecorations) { auto* src = R"( [[block]] diff --git a/src/writer/msl/generator.cc b/src/writer/msl/generator.cc index 45287febb8..215d6503cb 100644 --- a/src/writer/msl/generator.cc +++ b/src/writer/msl/generator.cc @@ -48,6 +48,7 @@ Result Generate(const Program* program, const Options& options) { result.success = impl->Generate(); result.error = impl->error(); result.msl = impl->result(); + result.has_invariant_attribute = impl->HasInvariant(); return result; } diff --git a/src/writer/msl/generator.h b/src/writer/msl/generator.h index b4891c90f9..50284587d7 100644 --- a/src/writer/msl/generator.h +++ b/src/writer/msl/generator.h @@ -63,6 +63,9 @@ struct Result { /// True if the shader needs a UBO of buffer sizes. bool needs_storage_buffer_sizes = false; + + /// True if the generated shader uses the invariant attribute. + bool has_invariant_attribute = false; }; /// Generate MSL for a program, according to a set of configuration options. The diff --git a/src/writer/msl/generator_impl.cc b/src/writer/msl/generator_impl.cc index 96ebba6af2..05768ae306 100644 --- a/src/writer/msl/generator_impl.cc +++ b/src/writer/msl/generator_impl.cc @@ -2134,6 +2134,9 @@ bool GeneratorImpl::EmitStructType(const sem::Struct* str) { return false; } out << " [[" << attr << "]]"; + } else if (deco->Is()) { + out << " [[invariant]]"; + has_invariant_ = true; } else if (!deco->IsAnyOf()) { diff --git a/src/writer/msl/generator_impl.h b/src/writer/msl/generator_impl.h index 49333e36eb..fde3c9a51a 100644 --- a/src/writer/msl/generator_impl.h +++ b/src/writer/msl/generator_impl.h @@ -61,6 +61,9 @@ class GeneratorImpl : public TextGenerator { /// @returns true on successful generation; false otherwise bool Generate(); + /// @returns true if an invariant attribute was generated + bool HasInvariant() { return has_invariant_; } + /// Handles generating a declared type /// @param ty the declared type to generate /// @returns true if the declared type was emitted @@ -302,6 +305,9 @@ class GeneratorImpl : public TextGenerator { /// Name of atomicCompareExchangeWeak() helper for the given pointer storage /// class. StorageClassToString atomicCompareExchangeWeak_; + + /// True if an invariant attribute has been generated. + bool has_invariant_ = false; }; } // namespace msl diff --git a/src/writer/msl/generator_impl_test.cc b/src/writer/msl/generator_impl_test.cc index 72f32b0914..f7c23e59cb 100644 --- a/src/writer/msl/generator_impl_test.cc +++ b/src/writer/msl/generator_impl_test.cc @@ -88,6 +88,55 @@ INSTANTIATE_TEST_SUITE_P( MslBuiltinData{ast::Builtin::kSampleIndex, "sample_id"}, MslBuiltinData{ast::Builtin::kSampleMask, "sample_mask"})); +TEST_F(MslGeneratorImplTest, HasInvariantAttribute_True) { + auto* out = Structure( + "Out", {Member("pos", ty.vec4(), + {Builtin(ast::Builtin::kPosition), Invariant()})}); + Func("vert_main", ast::VariableList{}, ty.Of(out), + {Return(Construct(ty.Of(out)))}, {Stage(ast::PipelineStage::kVertex)}); + + GeneratorImpl& gen = Build(); + + ASSERT_TRUE(gen.Generate()) << gen.error(); + EXPECT_TRUE(gen.HasInvariant()); + EXPECT_EQ(gen.result(), R"(#include + +using namespace metal; +struct Out { + float4 pos [[position]] [[invariant]]; +}; + +vertex Out vert_main() { + return {}; +} + +)"); +} + +TEST_F(MslGeneratorImplTest, HasInvariantAttribute_False) { + auto* out = Structure("Out", {Member("pos", ty.vec4(), + {Builtin(ast::Builtin::kPosition)})}); + Func("vert_main", ast::VariableList{}, ty.Of(out), + {Return(Construct(ty.Of(out)))}, {Stage(ast::PipelineStage::kVertex)}); + + GeneratorImpl& gen = Build(); + + ASSERT_TRUE(gen.Generate()) << gen.error(); + EXPECT_FALSE(gen.HasInvariant()); + EXPECT_EQ(gen.result(), R"(#include + +using namespace metal; +struct Out { + float4 pos [[position]]; +}; + +vertex Out vert_main() { + return {}; +} + +)"); +} + } // namespace } // namespace msl } // namespace writer diff --git a/test/shader_io/invariant.wgsl.expected.msl b/test/shader_io/invariant.wgsl.expected.msl index 6e1ba46bf0..5543bd0ee7 100644 --- a/test/shader_io/invariant.wgsl.expected.msl +++ b/test/shader_io/invariant.wgsl.expected.msl @@ -1,10 +1,12 @@ -SKIP: FAILED +#include -../../src/writer/msl/generator_impl.cc:1990 internal compiler error: unhandled struct member attribute: invariant -******************************************************************** -* The tint shader compiler has encountered an unexpected error. * -* * -* Please help us fix this issue by submitting a bug report at * -* crbug.com/tint with the source program that triggered the bug. * -******************************************************************** +using namespace metal; +struct tint_symbol_1 { + float4 value [[position]] [[invariant]]; +}; + +vertex tint_symbol_1 tint_symbol() { + tint_symbol_1 const tint_symbol_2 = {.value=float4()}; + return tint_symbol_2; +} diff --git a/test/shader_io/invariant_struct_member.wgsl.expected.msl b/test/shader_io/invariant_struct_member.wgsl.expected.msl index 6e1ba46bf0..545642e6b4 100644 --- a/test/shader_io/invariant_struct_member.wgsl.expected.msl +++ b/test/shader_io/invariant_struct_member.wgsl.expected.msl @@ -1,10 +1,16 @@ -SKIP: FAILED +#include -../../src/writer/msl/generator_impl.cc:1990 internal compiler error: unhandled struct member attribute: invariant -******************************************************************** -* The tint shader compiler has encountered an unexpected error. * -* * -* Please help us fix this issue by submitting a bug report at * -* crbug.com/tint with the source program that triggered the bug. * -******************************************************************** +using namespace metal; +struct Out { + float4 pos; +}; +struct tint_symbol_1 { + float4 pos [[position]] [[invariant]]; +}; + +vertex tint_symbol_1 tint_symbol() { + Out const tint_symbol_2 = {}; + tint_symbol_1 const tint_symbol_3 = {.pos=tint_symbol_2.pos}; + return tint_symbol_3; +}