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 <bclayton@google.com> Reviewed-by: dan sinclair <dsinclair@chromium.org>
2025-07-03 19:55:56 +00:00 · 2021-01-27 19:23:55 +00:00 · 2021-01-27 19:23:55 +00:00 · dd69ac3505
commit dd69ac3505
parent 844217fa34
9 changed files with 33 additions and 132 deletions
--- 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;
--- 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) {
--- 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<TypeDeterminer>(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::StageDecoration>(ast::PipelineStage::kVertex),
-      });
+  auto* foo = MakeEmptyBodyFunction(
+      "foo", ast::FunctionDecorationList{
+                 create<ast::StageDecoration>(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");
--- 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());
--- 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;
 };
--- 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<ParserImpl> parser(const std::vector<uint32_t>& input) {
-    return std::make_unique<ParserImpl>(input);
+    auto parser = std::make_unique<ParserImpl>(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.
--- 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;

--- 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;
 }

--- 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) {