From dd69ac3505dae00dc0475b9fcbd87f4b73e35214 Mon Sep 17 00:00:00 2001 From: Ben Clayton Date: Wed, 27 Jan 2021 19:23:55 +0000 Subject: [PATCH] Automatically run the TypeDeterminer when building programs Removes the need for Dawn to use the TypeDeterminer directly. TypeDeterminer errors will be added to the Program diagnostics list. Change-Id: I4cfb405e7e6b0e94727296eea872a3ddc4412b66 Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/38921 Commit-Queue: Ben Clayton Reviewed-by: dan sinclair --- fuzzers/tint_common_fuzzer.cc | 9 --- samples/main.cc | 7 -- src/inspector/inspector_test.cc | 92 ++-------------------- src/program.cc | 10 ++- src/program_builder.h | 13 +++ src/reader/spirv/parser_impl_test_helper.h | 6 +- src/transform/first_index_offset.cc | 16 ---- src/transform/manager.cc | 4 +- src/transform/test_helper.h | 8 -- 9 files changed, 33 insertions(+), 132 deletions(-) diff --git a/fuzzers/tint_common_fuzzer.cc b/fuzzers/tint_common_fuzzer.cc index fb5408a2cf..93b84fa7f6 100644 --- a/fuzzers/tint_common_fuzzer.cc +++ b/fuzzers/tint_common_fuzzer.cc @@ -85,15 +85,6 @@ int CommonFuzzer::Run(const uint8_t* data, size_t size) { return 0; } - { - ProgramBuilder builder = program.CloneAsBuilder(); - TypeDeterminer td(&builder); - if (!td.Determine()) { - return 0; - } - program = Program(std::move(builder)); - } - Validator v; if (!v.Validate(&program)) { return 0; diff --git a/samples/main.cc b/samples/main.cc index c90fe95bcb..a0678caa29 100644 --- a/samples/main.cc +++ b/samples/main.cc @@ -498,13 +498,6 @@ int main(int argc, const char** argv) { return 1; } - auto diags = tint::TypeDeterminer::Run(program.get()); - if (diags.contains_errors()) { - std::cerr << "Type Determination: "; - diag_formatter.format(diags, diag_printer.get()); - return 1; - } - if (options.dump_ast) { auto ast_str = program->to_str(); if (options.demangle) { diff --git a/src/inspector/inspector_test.cc b/src/inspector/inspector_test.cc index cd5e026160..572b6c0cf0 100644 --- a/src/inspector/inspector_test.cc +++ b/src/inspector/inspector_test.cc @@ -70,8 +70,7 @@ namespace { class InspectorHelper : public ProgramBuilder { public: InspectorHelper() - : td_(std::make_unique(this)), - sampler_type_(type::SamplerKind::kSampler), + : sampler_type_(type::SamplerKind::kSampler), comparison_sampler_type_(type::SamplerKind::kComparisonSampler) {} /// Generates an empty function @@ -623,8 +622,6 @@ class InspectorHelper : public ProgramBuilder { return *inspector_; } - TypeDeterminer* td() { return td_.get(); } - type::Array* u32_array_type(uint32_t count) { if (array_type_memo_.find(count) == array_type_memo_.end()) { array_type_memo_[count] = @@ -808,11 +805,10 @@ TEST_F(InspectorGetEntryPointTest, MixFunctionsAndEntryPoints) { } TEST_F(InspectorGetEntryPointTest, DefaultWorkgroupSize) { - auto* foo = MakeCallerBodyFunction( - "foo", "func", - ast::FunctionDecorationList{ - create(ast::PipelineStage::kVertex), - }); + auto* foo = MakeEmptyBodyFunction( + "foo", ast::FunctionDecorationList{ + create(ast::PipelineStage::kVertex), + }); AST().Functions().Add(foo); Inspector& inspector = Build(); @@ -880,8 +876,6 @@ TEST_F(InspectorGetEntryPointTest, EntryPointInOutVariables) { }); AST().Functions().Add(foo); - ASSERT_TRUE(td()->Determine()) << td()->error(); - Inspector& inspector = Build(); auto result = inspector.GetEntryPoints(); @@ -913,8 +907,6 @@ TEST_F(InspectorGetEntryPointTest, FunctionInOutVariables) { }); AST().Functions().Add(foo); - ASSERT_TRUE(td()->Determine()) << td()->error(); - Inspector& inspector = Build(); auto result = inspector.GetEntryPoints(); @@ -946,8 +938,6 @@ TEST_F(InspectorGetEntryPointTest, RepeatedInOutVariables) { }); AST().Functions().Add(foo); - ASSERT_TRUE(td()->Determine()) << td()->error(); - Inspector& inspector = Build(); auto result = inspector.GetEntryPoints(); @@ -975,8 +965,6 @@ TEST_F(InspectorGetEntryPointTest, EntryPointMultipleInOutVariables) { }); AST().Functions().Add(foo); - ASSERT_TRUE(td()->Determine()) << td()->error(); - Inspector& inspector = Build(); auto result = inspector.GetEntryPoints(); @@ -1014,8 +1002,6 @@ TEST_F(InspectorGetEntryPointTest, FunctionMultipleInOutVariables) { }); AST().Functions().Add(foo); - ASSERT_TRUE(td()->Determine()) << td()->error(); - Inspector& inspector = Build(); auto result = inspector.GetEntryPoints(); @@ -1057,8 +1043,6 @@ TEST_F(InspectorGetEntryPointTest, MultipleEntryPointsInOutVariables) { }); AST().Functions().Add(bar); - ASSERT_TRUE(td()->Determine()) << td()->error(); - // TODO(dsinclair): Update to run the namer transform when // available. @@ -1113,8 +1097,6 @@ TEST_F(InspectorGetEntryPointTest, MultipleEntryPointsSharedInOutVariables) { }); AST().Functions().Add(bar); - ASSERT_TRUE(td()->Determine()) << td()->error(); - // TODO(dsinclair): Update to run the namer transform when // available. @@ -1175,8 +1157,6 @@ TEST_F(InspectorGetEntryPointTest, BuiltInsNotStageVariables) { }); AST().Functions().Add(foo); - ASSERT_TRUE(td()->Determine()) << td()->error(); - // TODO(dsinclair): Update to run the namer transform when available. Inspector& inspector = Build(); @@ -1395,8 +1375,6 @@ TEST_F(InspectorGetUniformBufferResourceBindingsTest, NonEntryPointFunc) { }); AST().Functions().Add(ep_func); - ASSERT_TRUE(td()->Determine()) << td()->error(); - Inspector& inspector = Build(); auto result = inspector.GetUniformBufferResourceBindings("ub_func"); @@ -1425,8 +1403,6 @@ TEST_F(InspectorGetUniformBufferResourceBindingsTest, MissingBlockDeco) { }); AST().Functions().Add(ep_func); - ASSERT_TRUE(td()->Determine()) << td()->error(); - Inspector& inspector = Build(); auto result = inspector.GetUniformBufferResourceBindings("ep_func"); @@ -1452,8 +1428,6 @@ TEST_F(InspectorGetUniformBufferResourceBindingsTest, Simple) { }); AST().Functions().Add(ep_func); - ASSERT_TRUE(td()->Determine()) << td()->error(); - Inspector& inspector = Build(); auto result = inspector.GetUniformBufferResourceBindings("ep_func"); @@ -1483,8 +1457,6 @@ TEST_F(InspectorGetUniformBufferResourceBindingsTest, MultipleMembers) { }); AST().Functions().Add(ep_func); - ASSERT_TRUE(td()->Determine()) << td()->error(); - Inspector& inspector = Build(); auto result = inspector.GetUniformBufferResourceBindings("ep_func"); @@ -1529,8 +1501,6 @@ TEST_F(InspectorGetUniformBufferResourceBindingsTest, MultipleUniformBuffers) { }); AST().Functions().Add(func); - ASSERT_TRUE(td()->Determine()) << td()->error(); - Inspector& inspector = Build(); auto result = inspector.GetUniformBufferResourceBindings("ep_func"); @@ -1568,8 +1538,6 @@ TEST_F(InspectorGetUniformBufferResourceBindingsTest, ContainingArray) { }); AST().Functions().Add(ep_func); - ASSERT_TRUE(td()->Determine()) << td()->error(); - Inspector& inspector = Build(); auto result = inspector.GetUniformBufferResourceBindings("ep_func"); @@ -1599,8 +1567,6 @@ TEST_F(InspectorGetStorageBufferResourceBindingsTest, Simple) { }); AST().Functions().Add(ep_func); - ASSERT_TRUE(td()->Determine()) << td()->error(); - Inspector& inspector = Build(); auto result = inspector.GetStorageBufferResourceBindings("ep_func"); @@ -1630,8 +1596,6 @@ TEST_F(InspectorGetStorageBufferResourceBindingsTest, MultipleMembers) { }); AST().Functions().Add(ep_func); - ASSERT_TRUE(td()->Determine()) << td()->error(); - Inspector& inspector = Build(); auto result = inspector.GetStorageBufferResourceBindings("ep_func"); @@ -1679,8 +1643,6 @@ TEST_F(InspectorGetStorageBufferResourceBindingsTest, MultipleStorageBuffers) { }); AST().Functions().Add(func); - ASSERT_TRUE(td()->Determine()) << td()->error(); - Inspector& inspector = Build(); auto result = inspector.GetStorageBufferResourceBindings("ep_func"); @@ -1718,8 +1680,6 @@ TEST_F(InspectorGetStorageBufferResourceBindingsTest, ContainingArray) { }); AST().Functions().Add(ep_func); - ASSERT_TRUE(td()->Determine()) << td()->error(); - Inspector& inspector = Build(); auto result = inspector.GetStorageBufferResourceBindings("ep_func"); @@ -1749,8 +1709,6 @@ TEST_F(InspectorGetStorageBufferResourceBindingsTest, ContainingRuntimeArray) { }); AST().Functions().Add(ep_func); - ASSERT_TRUE(td()->Determine()) << td()->error(); - Inspector& inspector = Build(); auto result = inspector.GetStorageBufferResourceBindings("ep_func"); @@ -1780,8 +1738,6 @@ TEST_F(InspectorGetStorageBufferResourceBindingsTest, SkipReadOnly) { }); AST().Functions().Add(ep_func); - ASSERT_TRUE(td()->Determine()) << td()->error(); - Inspector& inspector = Build(); auto result = inspector.GetStorageBufferResourceBindings("ep_func"); @@ -1807,8 +1763,6 @@ TEST_F(InspectorGetReadOnlyStorageBufferResourceBindingsTest, Simple) { }); AST().Functions().Add(ep_func); - ASSERT_TRUE(td()->Determine()) << td()->error(); - Inspector& inspector = Build(); auto result = inspector.GetReadOnlyStorageBufferResourceBindings("ep_func"); @@ -1857,8 +1811,6 @@ TEST_F(InspectorGetReadOnlyStorageBufferResourceBindingsTest, }); AST().Functions().Add(func); - ASSERT_TRUE(td()->Determine()) << td()->error(); - Inspector& inspector = Build(); auto result = inspector.GetReadOnlyStorageBufferResourceBindings("ep_func"); @@ -1896,8 +1848,6 @@ TEST_F(InspectorGetReadOnlyStorageBufferResourceBindingsTest, ContainingArray) { }); AST().Functions().Add(ep_func); - ASSERT_TRUE(td()->Determine()) << td()->error(); - Inspector& inspector = Build(); auto result = inspector.GetReadOnlyStorageBufferResourceBindings("ep_func"); @@ -1928,8 +1878,6 @@ TEST_F(InspectorGetReadOnlyStorageBufferResourceBindingsTest, }); AST().Functions().Add(ep_func); - ASSERT_TRUE(td()->Determine()) << td()->error(); - Inspector& inspector = Build(); auto result = inspector.GetReadOnlyStorageBufferResourceBindings("ep_func"); @@ -1959,8 +1907,6 @@ TEST_F(InspectorGetReadOnlyStorageBufferResourceBindingsTest, SkipNonReadOnly) { }); AST().Functions().Add(ep_func); - ASSERT_TRUE(td()->Determine()) << td()->error(); - Inspector& inspector = Build(); auto result = inspector.GetReadOnlyStorageBufferResourceBindings("ep_func"); @@ -1982,8 +1928,6 @@ TEST_F(InspectorGetSamplerResourceBindingsTest, Simple) { }); AST().Functions().Add(func); - ASSERT_TRUE(td()->Determine()) << td()->error(); - Inspector& inspector = Build(); auto result = inspector.GetSamplerResourceBindings("ep"); @@ -2001,8 +1945,6 @@ TEST_F(InspectorGetSamplerResourceBindingsTest, NoSampler) { }); AST().Functions().Add(func); - ASSERT_TRUE(td()->Determine()) << td()->error(); - Inspector& inspector = Build(); auto result = inspector.GetSamplerResourceBindings("ep_func"); @@ -2029,8 +1971,6 @@ TEST_F(InspectorGetSamplerResourceBindingsTest, InFunction) { }); AST().Functions().Add(ep_func); - ASSERT_TRUE(td()->Determine()) << td()->error(); - Inspector& inspector = Build(); auto result = inspector.GetSamplerResourceBindings("ep_func"); @@ -2055,8 +1995,6 @@ TEST_F(InspectorGetSamplerResourceBindingsTest, UnknownEntryPoint) { }); AST().Functions().Add(func); - ASSERT_TRUE(td()->Determine()) << td()->error(); - Inspector& inspector = Build(); auto result = inspector.GetSamplerResourceBindings("foo"); @@ -2077,8 +2015,6 @@ TEST_F(InspectorGetSamplerResourceBindingsTest, SkipsComparisonSamplers) { }); AST().Functions().Add(func); - ASSERT_TRUE(td()->Determine()) << td()->error(); - Inspector& inspector = Build(); auto result = inspector.GetSamplerResourceBindings("ep"); @@ -2101,8 +2037,6 @@ TEST_F(InspectorGetComparisonSamplerResourceBindingsTest, Simple) { }); AST().Functions().Add(func); - ASSERT_TRUE(td()->Determine()) << td()->error(); - Inspector& inspector = Build(); auto result = inspector.GetComparisonSamplerResourceBindings("ep"); @@ -2120,8 +2054,6 @@ TEST_F(InspectorGetComparisonSamplerResourceBindingsTest, NoSampler) { }); AST().Functions().Add(func); - ASSERT_TRUE(td()->Determine()) << td()->error(); - Inspector& inspector = Build(); auto result = inspector.GetComparisonSamplerResourceBindings("ep_func"); @@ -2149,8 +2081,6 @@ TEST_F(InspectorGetComparisonSamplerResourceBindingsTest, InFunction) { }); AST().Functions().Add(ep_func); - ASSERT_TRUE(td()->Determine()) << td()->error(); - Inspector& inspector = Build(); auto result = inspector.GetComparisonSamplerResourceBindings("ep_func"); @@ -2175,8 +2105,6 @@ TEST_F(InspectorGetComparisonSamplerResourceBindingsTest, UnknownEntryPoint) { }); AST().Functions().Add(func); - ASSERT_TRUE(td()->Determine()) << td()->error(); - Inspector& inspector = Build(); auto result = inspector.GetSamplerResourceBindings("foo"); @@ -2197,8 +2125,6 @@ TEST_F(InspectorGetComparisonSamplerResourceBindingsTest, SkipsSamplers) { }); AST().Functions().Add(func); - ASSERT_TRUE(td()->Determine()) << td()->error(); - Inspector& inspector = Build(); auto result = inspector.GetComparisonSamplerResourceBindings("ep"); @@ -2239,8 +2165,6 @@ TEST_P(InspectorGetSampledTextureResourceBindingsTestWithParam, textureSample) { }); AST().Functions().Add(func); - ASSERT_TRUE(td()->Determine()) << td()->error(); - Inspector& inspector = Build(); auto result = inspector.GetSampledTextureResourceBindings("ep"); @@ -2332,8 +2256,6 @@ TEST_P(InspectorGetSampledArrayTextureResourceBindingsTestWithParam, }); AST().Functions().Add(func); - ASSERT_TRUE(td()->Determine()) << td()->error(); - Inspector& inspector = Build(); auto result = inspector.GetSampledTextureResourceBindings("ep"); @@ -2405,8 +2327,6 @@ TEST_P(InspectorGetMultisampledTextureResourceBindingsTestWithParam, }); AST().Functions().Add(func); - ASSERT_TRUE(td()->Determine()) << td()->error(); - Inspector& inspector = Build(); auto result = inspector.GetMultisampledTextureResourceBindings("ep"); @@ -2489,8 +2409,6 @@ TEST_P(InspectorGetMultisampledArrayTextureResourceBindingsTestWithParam, }); AST().Functions().Add(func); - ASSERT_TRUE(td()->Determine()) << td()->error(); - Inspector& inspector = Build(); auto result = inspector.GetMultisampledTextureResourceBindings("ep"); diff --git a/src/program.cc b/src/program.cc index d8393a7624..6b25bdfdb7 100644 --- a/src/program.cc +++ b/src/program.cc @@ -38,7 +38,15 @@ Program::Program(Program&& program) } Program::Program(ProgramBuilder&& builder) { - is_valid_ = builder.IsValid(); // must be called before the std::move()s + is_valid_ = builder.IsValid(); + if (builder.ResolveOnBuild() && builder.IsValid()) { + TypeDeterminer td(&builder); + if (!td.Determine()) { + diagnostics_.add_error(td.error()); + } + } + + // The above must be called *before* the calls to std::move() below types_ = std::move(builder.Types()); nodes_ = std::move(builder.Nodes()); diff --git a/src/program_builder.h b/src/program_builder.h index 9d712364c1..dfac2c9e7f 100644 --- a/src/program_builder.h +++ b/src/program_builder.h @@ -125,6 +125,15 @@ class ProgramBuilder { return diagnostics_; } + /// Controls whether the TypeDeterminer will be run on the program when it is + /// built. + /// @param enable the new flag value (defaults to true) + void SetResolveOnBuild(bool enable) { resolve_on_build_ = enable; } + + /// @return true if the TypeDeterminer will be run on the program when it is + /// built. + bool ResolveOnBuild() const { return resolve_on_build_; } + /// @returns true if the program has no error diagnostics and is not missing /// information bool IsValid() const; @@ -851,6 +860,10 @@ class ProgramBuilder { /// the first argument. Source source_; + /// Set by SetResolveOnBuild(). If set, the TypeDeterminer will be run on the + /// program when built. + bool resolve_on_build_ = true; + /// Set by MarkAsMoved(). Once set, no methods may be called on this builder. bool moved_ = false; }; diff --git a/src/reader/spirv/parser_impl_test_helper.h b/src/reader/spirv/parser_impl_test_helper.h index 0b51e9e091..3a3acad484 100644 --- a/src/reader/spirv/parser_impl_test_helper.h +++ b/src/reader/spirv/parser_impl_test_helper.h @@ -40,7 +40,11 @@ class SpvParserTestBase : public T { /// @param input the SPIR-V binary to parse /// @returns a parser for the given binary std::unique_ptr parser(const std::vector& input) { - return std::make_unique(input); + auto parser = std::make_unique(input); + // Don't run the TypeDeterminer when building the program. + // We're not interested in type information with these tests. + parser->builder().SetResolveOnBuild(false); + return parser; } /// Gets the internal representation of the function with the given ID. diff --git a/src/transform/first_index_offset.cc b/src/transform/first_index_offset.cc index 95d652e864..64b25dce5d 100644 --- a/src/transform/first_index_offset.cc +++ b/src/transform/first_index_offset.cc @@ -96,22 +96,6 @@ Transform::Output FirstIndexOffset::Run(const Program* in) { } } - // Running TypeDeterminer as we require local_referenced_builtin_variables() - // to be populated. TODO(bclayton) - it should not be necessary to re-run the - // type determiner if semantic information is already generated. Remove. - Program program; - { - ProgramBuilder builder = in->CloneAsBuilder(); - TypeDeterminer td(&builder); - if (!td.Determine()) { - Output out; - out.diagnostics.add_error(td.error()); - return out; - } - program = Program(std::move(builder)); - in = &program; - } - Symbol vertex_index_sym; Symbol instance_index_sym; diff --git a/src/transform/manager.cc b/src/transform/manager.cc index 22e865fe96..31fe9ac290 100644 --- a/src/transform/manager.cc +++ b/src/transform/manager.cc @@ -36,11 +36,9 @@ Transform::Output Manager::Run(const Program* program) { program = &out.program; } } else { - out.program = Program(program->Clone()); + out.program = program->Clone(); } - out.diagnostics.add(TypeDeterminer::Run(&out.program)); - return out; } diff --git a/src/transform/test_helper.h b/src/transform/test_helper.h index a1417915f3..78c84aca20 100644 --- a/src/transform/test_helper.h +++ b/src/transform/test_helper.h @@ -56,14 +56,6 @@ class TransformTest : public testing::Test { return diag::Formatter(style).format(program.Diagnostics()); } - { - auto diagnostics = TypeDeterminer::Run(&program); - if (diagnostics.contains_errors()) { - return "Type determination failed:\n" + - diag::Formatter(style).format(diagnostics); - } - } - { Manager manager; for (auto& transform : transforms) {