spirv-reader: Register a name for an entry point inner implementation

It's tricky when mulitple entry points share the same implementation
function.

Bug: tint:508
Change-Id: Ie3084d4af8d6eb3aa9f0d25babb8ebfaed7b2fae
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/48902
Kokoro: Kokoro <noreply+kokoro@google.com>
Commit-Queue: David Neto <dneto@google.com>
Auto-Submit: David Neto <dneto@google.com>
Reviewed-by: James Price <jrprice@google.com>
Reviewed-by: Ben Clayton <bclayton@google.com>

src/reader/spirv/entry_point_info.cc

@@ -22,10 +22,14 @@ namespace spirv {
 
 EntryPointInfo::EntryPointInfo(std::string the_name,
                                ast::PipelineStage the_stage,
+                               bool the_owns_inner_implementation,
+                               std::string the_inner_name,
                                std::vector<uint32_t>&& the_inputs,
                                std::vector<uint32_t>&& the_outputs)
     : name(the_name),
       stage(the_stage),
+      owns_inner_implementation(the_owns_inner_implementation),
+      inner_name(std::move(the_inner_name)),
       inputs(std::move(the_inputs)),
       outputs(std::move(the_outputs)) {}
 

src/reader/spirv/entry_point_info.h

@@ -26,25 +26,39 @@ namespace spirv {
 
 /// Entry point information for a function
 struct EntryPointInfo {
-  // Constructor.
-  // @param the_name the name of the entry point
-  // @param the_stage the pipeline stage
-  // @param the_inputs list of IDs for Input variables used by the shader
-  // @param the_outputs list of IDs for Output variables used by the shader
+  /// Constructor.
+  /// @param the_name the name of the entry point
+  /// @param the_stage the pipeline stage
+  /// @param the_inputs list of IDs for Input variables used by the shader
+  /// @param the_outputs list of IDs for Output variables used by the shader
   EntryPointInfo(std::string the_name,
                  ast::PipelineStage the_stage,
+                 bool the_owns_inner_implementation,
+                 std::string the_inner_name,
                  std::vector<uint32_t>&& the_inputs,
                  std::vector<uint32_t>&& the_outputs);
-  // Copy constructor
-  // @param other the other entry point info to be built from
+  /// Copy constructor
+  /// @param other the other entry point info to be built from
   EntryPointInfo(const EntryPointInfo& other);
-  // Destructor
+  /// Destructor
   ~EntryPointInfo();
 
-  /// The entry point name
+  /// The entry point name.
+  /// In the WGSL output, this function will have pipeline inputs and outputs
+  /// as parameters. This function will store them into Private variables,
+  /// and then call the "inner" function, named by the next memeber.
+  /// Then outputs are copied from the private variables to the return value.
   std::string name;
   /// The entry point stage
   ast::PipelineStage stage = ast::PipelineStage::kNone;
+  /// True when this entry point is responsible for generating the
+  /// inner implementation function.  False when this is the second entry
+  /// point encountered for the same function in SPIR-V. It's unusual, but
+  /// possible for the same function to be the implementation for multiple
+  /// entry points.
+  bool owns_inner_implementation;
+  /// The name of the inner implementation function of the entry point.
+  std::string inner_name;
   /// IDs of pipeline input variables, sorted and without duplicates.
   std::vector<uint32_t> inputs;
   /// IDs of pipeline output variables, sorted and without duplicates.

src/reader/spirv/parser_impl.cc

@@ -712,8 +712,27 @@ bool ParserImpl::RegisterEntryPoints() {
        module_->entry_points()) {
     const auto stage = SpvExecutionModel(entry_point.GetSingleWordInOperand(0));
     const uint32_t function_id = entry_point.GetSingleWordInOperand(1);
+
     const std::string ep_name = entry_point.GetOperand(2).AsString();
 
+    bool owns_inner_implementation = false;
+    std::string inner_implementation_name;
+
+    if (hlsl_style_pipeline_io_) {
+      auto where = function_to_ep_info_.find(function_id);
+      if (where == function_to_ep_info_.end()) {
+        // If this is the first entry point to have function_id as its
+        // implementation, then this entry point is responsible for generating
+        // the inner implementation.
+        owns_inner_implementation = true;
+        inner_implementation_name = namer_.MakeDerivedName(ep_name);
+      } else {
+        // Reuse the inner implementation owned by the first entry point.
+        inner_implementation_name = where->second[0].inner_name;
+      }
+    }
+    TINT_ASSERT(ep_name != inner_implementation_name);
+
     tint::UniqueVector<uint32_t> inputs;
     tint::UniqueVector<uint32_t> outputs;
     for (unsigned iarg = 3; iarg < entry_point.NumInOperands(); iarg++) {
@@ -739,6 +758,7 @@ bool ParserImpl::RegisterEntryPoints() {
 
     function_to_ep_info_[function_id].emplace_back(
         ep_name, enum_converter_.ToPipelineStage(stage),
+        owns_inner_implementation, inner_implementation_name,
         std::move(sorted_inputs), std::move(sorted_outputs));
   }
   // The enum conversion could have failed, so return the existing status value.

src/reader/spirv/parser_impl_user_name_test.cc

@@ -100,7 +100,7 @@ TEST_F(SpvParserTest, UserName_MemberNamesMixUserAndSynthesized) {
   EXPECT_THAT(p->namer().GetMemberName(3, 2), Eq("field2"));
 }
 
-TEST_F(SpvParserTest, EntryPointNamesAlwaysTakePrecedence) {
+TEST_F(SpvParserTest, EntryPointNames_AlwaysTakePrecedence) {
   const std::string assembly = R"(
    OpCapability Shader
    OpMemoryModel Logical Simple
@@ -126,7 +126,7 @@ TEST_F(SpvParserTest, EntryPointNamesAlwaysTakePrecedence) {
   EXPECT_TRUE(p->BuildAndParseInternalModule());
   // The first entry point grabs the best name, "main"
   EXPECT_THAT(p->namer().Name(100), Eq("main"));
-  // The OpName on %1 is overriden becuase the second entry point
+  // The OpName on %1 is overriden because the second entry point
   // has grabbed "main_1" first.
   EXPECT_THAT(p->namer().Name(1), Eq("main_1_1"));
 
@@ -136,6 +136,48 @@ TEST_F(SpvParserTest, EntryPointNamesAlwaysTakePrecedence) {
   EXPECT_EQ(ep_info[1].name, "main_1");
 }
 
+TEST_F(SpvParserTest, EntryPointNames_DistinctFromInnerNames) {
+  const std::string assembly = R"(
+   OpCapability Shader
+   OpMemoryModel Logical Simple
+   OpEntryPoint GLCompute %100 "main"
+   OpEntryPoint Fragment %100 "main_1"
+
+   ; attempt to grab the "main_1" that would be the derived name
+   ; for the second entry point.
+   OpName %1 "main_1"
+
+   %void = OpTypeVoid
+   %voidfn = OpTypeFunction %void
+   %uint = OpTypeInt 32 0
+   %uint_0 = OpConstant %uint 0
+
+   %100 = OpFunction %void None %voidfn
+   %100_entry = OpLabel
+   %1 = OpCopyObject %uint %uint_0
+   OpReturn
+   OpFunctionEnd
+)";
+  auto p = parser(test::Assemble(assembly));
+
+  // TODO(crbug.com/tint/508): Remove this when switchover complete.
+  p->SetHLSLStylePipelineIO();
+
+  EXPECT_TRUE(p->BuildAndParseInternalModule());
+  // The first entry point grabs the best name, "main"
+  EXPECT_THAT(p->namer().Name(100), Eq("main"));
+  EXPECT_THAT(p->namer().Name(1), Eq("main_1_1"));
+
+  const auto ep_info = p->GetEntryPointInfo(100);
+  ASSERT_EQ(2u, ep_info.size());
+  EXPECT_EQ(ep_info[0].name, "main");
+  EXPECT_EQ(ep_info[0].inner_name, "main_2");
+  // The second entry point retains its name...
+  EXPECT_EQ(ep_info[1].name, "main_1");
+  // ...but will use the same implementation function.
+  EXPECT_EQ(ep_info[1].inner_name, "main_2");
+}
+
 }  // namespace
 }  // namespace spirv
 }  // namespace reader