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>
diff --git a/src/reader/spirv/entry_point_info.cc b/src/reader/spirv/entry_point_info.cc
index 61f1586..44242a5 100644
--- a/src/reader/spirv/entry_point_info.cc
+++ b/src/reader/spirv/entry_point_info.cc
@@ -22,10 +22,14 @@
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)) {}
diff --git a/src/reader/spirv/entry_point_info.h b/src/reader/spirv/entry_point_info.h
index 8cb11f3..ee819cc 100644
--- a/src/reader/spirv/entry_point_info.h
+++ b/src/reader/spirv/entry_point_info.h
@@ -26,25 +26,39 @@
/// 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.
diff --git a/src/reader/spirv/parser_impl.cc b/src/reader/spirv/parser_impl.cc
index 32b32b0..559136d 100644
--- a/src/reader/spirv/parser_impl.cc
+++ b/src/reader/spirv/parser_impl.cc
@@ -712,8 +712,27 @@
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 @@
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.
diff --git a/src/reader/spirv/parser_impl_user_name_test.cc b/src/reader/spirv/parser_impl_user_name_test.cc
index 99ffbcc..f69be61 100644
--- a/src/reader/spirv/parser_impl_user_name_test.cc
+++ b/src/reader/spirv/parser_impl_user_name_test.cc
@@ -100,7 +100,7 @@
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 @@
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 @@
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
