spirv-reader: remove old-style pipeline IO support

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

src/reader/spirv/function.cc

@@ -3376,28 +3376,18 @@ bool FunctionEmitter::EmitStatement(const spvtools::opt::Instruction& inst) {
             // Convert the LHS to a reference by dereferencing it.
             lhs = Dereference(lhs);
           }
-          if (parser_impl_.UseHLSLStylePipelineIO()) {
+          // The private variable is an array whose element type is already of
-            // In the HLSL-style pipeline IO case, the private variable is an
+          // the same type as the value being stored into it.  Form the
-            // array whose element type is already of the same type as the value
+          // reference into the first element.
-            // being stored into it.  Form the reference into the first element.
+          lhs.expr = create<ast::ArrayAccessorExpression>(
-            lhs.expr = create<ast::ArrayAccessorExpression>(
+              Source{}, lhs.expr, parser_impl_.MakeNullValue(ty_.I32()));
-                Source{}, lhs.expr, parser_impl_.MakeNullValue(ty_.I32()));
+          if (auto* ref = lhs.type->As<Reference>()) {
-            if (auto* ref = lhs.type->As<Reference>()) {
+            lhs.type = ref->type;
-              lhs.type = ref->type;
-            }
-            if (auto* arr = lhs.type->As<Array>()) {
-              lhs.type = arr->type;
-            }
-            TINT_ASSERT(lhs.type);
-          } else {
-            if (!rhs.type->Is<U32>()) {
-              // WGSL requires sample_mask_out to be unsigned.
-              rhs = TypedExpression{ty_.U32(),
-                                    create<ast::TypeConstructorExpression>(
-                                        Source{}, builder_.ty.u32(),
-                                        ast::ExpressionList{rhs.expr})};
-            }
           }
+          if (auto* arr = lhs.type->As<Array>()) {
+            lhs.type = arr->type;
+          }
+          TINT_ASSERT(lhs.type);
           break;
         default:
           break;

src/reader/spirv/parser_impl.cc

@@ -250,7 +250,6 @@ ParserImpl::ParserImpl(const std::vector<uint32_t>& spv_binary)
       namer_(fail_stream_),
       enum_converter_(fail_stream_),
       tools_context_(kInputEnv) {
-  hlsl_style_pipeline_io_ = true;
   // Create a message consumer to propagate error messages from SPIRV-Tools
   // out as our own failures.
   message_consumer_ = [this](spv_message_level_t level, const char* /*source*/,
@@ -823,18 +822,16 @@ bool ParserImpl::RegisterEntryPoints() {
     bool owns_inner_implementation = false;
     std::string inner_implementation_name;
 
-    if (hlsl_style_pipeline_io_) {
+    auto where = function_to_ep_info_.find(function_id);
-      auto where = function_to_ep_info_.find(function_id);
+    if (where == function_to_ep_info_.end()) {
-      if (where == function_to_ep_info_.end()) {
+      // If this is the first entry point to have function_id as its
-        // If this is the first entry point to have function_id as its
+      // implementation, then this entry point is responsible for generating
-        // implementation, then this entry point is responsible for generating
+      // the inner implementation.
-        // the inner implementation.
+      owns_inner_implementation = true;
-        owns_inner_implementation = true;
+      inner_implementation_name = namer_.MakeDerivedName(ep_name);
-        inner_implementation_name = namer_.MakeDerivedName(ep_name);
+    } else {
-      } else {
+      // Reuse the inner implementation owned by the first entry point.
-        // Reuse the inner implementation owned by the first entry point.
+      inner_implementation_name = where->second[0].inner_name;
-        inner_implementation_name = where->second[0].inner_name;
-      }
     }
     TINT_ASSERT(ep_name != inner_implementation_name);
 
@@ -873,9 +870,9 @@ bool ParserImpl::RegisterEntryPoints() {
                           workgroup_size_builtin_.z_value};
       } else {
         // Use the LocalSize execution mode.  This is the second choice.
-        auto where = local_size.find(function_id);
+        auto where_local_size = local_size.find(function_id);
-        if (where != local_size.end()) {
+        if (where_local_size != local_size.end()) {
-          wgsize = where->second;
+          wgsize = where_local_size->second;
         }
       }
     }
@@ -1165,8 +1162,8 @@ const Type* ParserImpl::ConvertType(uint32_t type_id,
 
   if (pointee_type_id == builtin_position_.struct_type_id) {
     builtin_position_.pointer_type_id = type_id;
-    builtin_position_.storage_class =
+    // Pipeline IO builtins map to private variables.
-        hlsl_style_pipeline_io_ ? SpvStorageClassPrivate : storage_class;
+    builtin_position_.storage_class = SpvStorageClassPrivate;
     return nullptr;
   }
   auto* ast_elem_ty = ConvertType(pointee_type_id, PtrAs::Ptr);
@@ -1189,13 +1186,10 @@ const Type* ParserImpl::ConvertType(uint32_t type_id,
     remap_buffer_block_type_.insert(type_id);
   }
 
-  if (hlsl_style_pipeline_io_) {
+  // Pipeline intput and output variables map to private variables.
-    // When using HLSL-style pipeline IO, intput and output variables
+  if (ast_storage_class == ast::StorageClass::kInput ||
-    // are mapped to private variables.
+      ast_storage_class == ast::StorageClass::kOutput) {
-    if (ast_storage_class == ast::StorageClass::kInput ||
+    ast_storage_class = ast::StorageClass::kPrivate;
-        ast_storage_class == ast::StorageClass::kOutput) {
-      ast_storage_class = ast::StorageClass::kPrivate;
-    }
   }
   switch (ptr_as) {
     case PtrAs::Ref:
@@ -1429,13 +1423,6 @@ bool ParserImpl::EmitModuleScopeVariables() {
     // Make sure the variable has a name.
     namer_.SuggestSanitizedName(builtin_position_.per_vertex_var_id,
                                 "gl_Position");
-    ast::DecorationList decos;
-    if (!hlsl_style_pipeline_io_) {
-      // When doing HLSL-style pipeline IO, the decoration goes on the
-      // parameter, not the variable.
-      decos.push_back(
-          create<ast::BuiltinDecoration>(Source{}, ast::Builtin::kPosition));
-    }
     ast::Expression* ast_constructor = nullptr;
     if (builtin_position_.per_vertex_var_init_id) {
       // The initializer is complex.
@@ -1460,7 +1447,7 @@ bool ParserImpl::EmitModuleScopeVariables() {
         builtin_position_.per_vertex_var_id,
         enum_converter_.ToStorageClass(builtin_position_.storage_class),
         ConvertType(builtin_position_.position_member_type_id), false,
-        ast_constructor, std::move(decos));
+        ast_constructor, {});
 
     builder_.AST().AddGlobalVariable(ast_var);
   }

src/reader/spirv/parser_impl.h

@@ -147,14 +147,6 @@ class ParserImpl : Reader {
   /// @returns the accumulated error string
   const std::string error() { return errors_.str(); }
 
-  /// Changes pipeline IO to be HLSL-style: as entry point parameters and
-  /// return.
-  /// TODO(crbug.com/tint/508): Once all this support has landed, switch
-  /// over to that, and remove the old support.
-  void SetHLSLStylePipelineIO() { hlsl_style_pipeline_io_ = true; }
-  /// @returns true if HLSL-style IO should be used.
-  bool UseHLSLStylePipelineIO() const { return hlsl_style_pipeline_io_; }
-
   /// Builds an internal representation of the SPIR-V binary,
   /// and parses it into a Tint AST module.  Diagnostics are emitted
   /// to the error stream.
@@ -822,10 +814,6 @@ class ParserImpl : Reader {
   /// complex case of replacing an entire structure.
   BuiltInsMap special_builtins_;
 
-  /// This is temporary while this module is converted to use the new style
-  /// of pipeline IO.
-  bool hlsl_style_pipeline_io_ = false;
-
   /// Info about the WorkgroupSize builtin. If it's not present, then the 'id'
   /// field will be 0. Sadly, in SPIR-V right now, there's only one workgroup
   /// size object in the module.

src/reader/spirv/parser_impl_module_var_test.cc

@@ -5260,10 +5260,6 @@ TEST_F(SpvModuleScopeVarParserTest, InputVarsConvertedToPrivate) {
   )" + MainBody();
   auto p = parser(test::Assemble(assembly));
 
-  // TODO(crbug.com/tint/508): Remove this when everything is converted
-  // to HLSL style pipeline IO.
-  p->SetHLSLStylePipelineIO();
-
   ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions());
   EXPECT_TRUE(p->error().empty());
   const auto got = p->program().to_str();
@@ -5285,10 +5281,6 @@ TEST_F(SpvModuleScopeVarParserTest, OutputVarsConvertedToPrivate) {
   )" + MainBody();
   auto p = parser(test::Assemble(assembly));
 
-  // TODO(crbug.com/tint/508): Remove this when everything is converted
-  // to HLSL style pipeline IO.
-  p->SetHLSLStylePipelineIO();
-
   ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions());
   EXPECT_TRUE(p->error().empty());
   const auto got = p->program().to_str();
@@ -5311,10 +5303,6 @@ TEST_F(SpvModuleScopeVarParserTest,
   )" + MainBody();
   auto p = parser(test::Assemble(assembly));
 
-  // TODO(crbug.com/tint/508): Remove this when everything is converted
-  // to HLSL style pipeline IO.
-  p->SetHLSLStylePipelineIO();
-
   ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions());
   EXPECT_TRUE(p->error().empty());
   const auto got = p->program().to_str();
@@ -5346,10 +5334,6 @@ TEST_F(SpvModuleScopeVarParserTest,
   )" + MainBody();
   auto p = parser(test::Assemble(assembly));
 
-  // TODO(crbug.com/tint/508): Remove this when everything is converted
-  // to HLSL style pipeline IO.
-  p->SetHLSLStylePipelineIO();
-
   ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions());
   EXPECT_TRUE(p->error().empty());
   const auto got = p->program().to_str();
@@ -5384,10 +5368,6 @@ TEST_F(SpvModuleScopeVarParserTest,
   )" + MainBody();
   auto p = parser(test::Assemble(assembly));
 
-  // TODO(crbug.com/tint/508): Remove this when everything is converted
-  // to HLSL style pipeline IO.
-  p->SetHLSLStylePipelineIO();
-
   ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions());
   EXPECT_TRUE(p->error().empty());
   const auto got = p->program().to_str();
@@ -5418,10 +5398,6 @@ TEST_F(SpvModuleScopeVarParserTest, Builtin_Input_SameSignednessAsWGSL) {
   )" + MainBody();
   auto p = parser(test::Assemble(assembly));
 
-  // TODO(crbug.com/tint/508): Remove this when everything is converted
-  // to HLSL style pipeline IO.
-  p->SetHLSLStylePipelineIO();
-
   ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions());
   EXPECT_TRUE(p->error().empty());
   const auto got = p->program().to_str();
@@ -5446,10 +5422,6 @@ TEST_F(SpvModuleScopeVarParserTest, Builtin_Input_OppositeSignednessAsWGSL) {
   )" + MainBody();
   auto p = parser(test::Assemble(assembly));
 
-  // TODO(crbug.com/tint/508): Remove this when everything is converted
-  // to HLSL style pipeline IO.
-  p->SetHLSLStylePipelineIO();
-
   ASSERT_TRUE(p->BuildAndParseInternalModuleExceptFunctions());
   EXPECT_TRUE(p->error().empty());
   const auto got = p->program().to_str();
@@ -5487,10 +5459,6 @@ TEST_F(SpvModuleScopeVarParserTest, EntryPointWrapping_IOLocations) {
   )";
   auto p = parser(test::Assemble(assembly));
 
-  // TODO(crbug.com/tint/508): Remove this when everything is converted
-  // to HLSL style pipeline IO.
-  p->SetHLSLStylePipelineIO();
-
   ASSERT_TRUE(p->BuildAndParseInternalModule());
   EXPECT_TRUE(p->error().empty());
   const auto got = p->program().to_str();
@@ -5603,10 +5571,6 @@ TEST_F(SpvModuleScopeVarParserTest,
   )";
   auto p = parser(test::Assemble(assembly));
 
-  // TODO(crbug.com/tint/508): Remove this when everything is converted
-  // to HLSL style pipeline IO.
-  p->SetHLSLStylePipelineIO();
-
   ASSERT_TRUE(p->Parse()) << p->error() << assembly;
   EXPECT_TRUE(p->error().empty());
   const auto got = p->program().to_str();
@@ -5682,10 +5646,6 @@ TEST_F(SpvModuleScopeVarParserTest,
   )";
   auto p = parser(test::Assemble(assembly));
 
-  // TODO(crbug.com/tint/508): Remove this when everything is converted
-  // to HLSL style pipeline IO.
-  p->SetHLSLStylePipelineIO();
-
   ASSERT_TRUE(p->Parse()) << p->error() << assembly;
   EXPECT_TRUE(p->error().empty());
   const auto got = p->program().to_str();
@@ -5765,10 +5725,6 @@ TEST_F(SpvModuleScopeVarParserTest,
   )";
   auto p = parser(test::Assemble(assembly));
 
-  // TODO(crbug.com/tint/508): Remove this when everything is converted
-  // to HLSL style pipeline IO.
-  p->SetHLSLStylePipelineIO();
-
   ASSERT_TRUE(p->Parse()) << p->error() << assembly;
   EXPECT_TRUE(p->error().empty());
   const auto got = p->program().to_str();
@@ -5837,10 +5793,6 @@ TEST_F(SpvModuleScopeVarParserTest,
   )";
   auto p = parser(test::Assemble(assembly));
 
-  // TODO(crbug.com/tint/508): Remove this when everything is converted
-  // to HLSL style pipeline IO.
-  p->SetHLSLStylePipelineIO();
-
   ASSERT_TRUE(p->Parse()) << p->error() << assembly;
   EXPECT_TRUE(p->error().empty());
   const auto got = p->program().to_str();
@@ -5912,10 +5864,6 @@ TEST_F(SpvModuleScopeVarParserTest,
   )";
   auto p = parser(test::Assemble(assembly));
 
-  // TODO(crbug.com/tint/508): Remove this when everything is converted
-  // to HLSL style pipeline IO.
-  p->SetHLSLStylePipelineIO();
-
   ASSERT_TRUE(p->Parse()) << p->error() << assembly;
   EXPECT_TRUE(p->error().empty());
   const auto got = p->program().to_str();
@@ -5989,10 +5937,6 @@ TEST_F(SpvModuleScopeVarParserTest,
   )";
   auto p = parser(test::Assemble(assembly));
 
-  // TODO(crbug.com/tint/508): Remove this when everything is converted
-  // to HLSL style pipeline IO.
-  p->SetHLSLStylePipelineIO();
-
   ASSERT_TRUE(p->Parse()) << p->error() << assembly;
   EXPECT_TRUE(p->error().empty());
   const auto got = p->program().to_str();
@@ -6067,10 +6011,6 @@ TEST_F(SpvModuleScopeVarParserTest,
   )";
   auto p = parser(test::Assemble(assembly));
 
-  // TODO(crbug.com/tint/508): Remove this when everything is converted
-  // to HLSL style pipeline IO.
-  p->SetHLSLStylePipelineIO();
-
   ASSERT_TRUE(p->Parse()) << p->error() << assembly;
   EXPECT_TRUE(p->error().empty());
   const auto got = p->program().to_str();
@@ -6126,10 +6066,6 @@ TEST_F(SpvModuleScopeVarParserTest, BuiltinPosition_BuiltIn_Position) {
 )";
   auto p = parser(test::Assemble(assembly));
 
-  // TODO(crbug.com/tint/508): Remove this when everything is converted
-  // to HLSL style pipeline IO.
-  p->SetHLSLStylePipelineIO();
-
   ASSERT_TRUE(p->Parse()) << p->error() << assembly;
   EXPECT_TRUE(p->error().empty());
 
@@ -6217,10 +6153,6 @@ TEST_F(SpvModuleScopeVarParserTest,
 )";
   auto p = parser(test::Assemble(assembly));
 
-  // TODO(crbug.com/tint/508): Remove this when everything is converted
-  // to HLSL style pipeline IO.
-  p->SetHLSLStylePipelineIO();
-
   ASSERT_TRUE(p->Parse()) << p->error() << assembly;
   EXPECT_TRUE(p->error().empty());
 

src/reader/spirv/parser_impl_test_helper.h

@@ -248,15 +248,6 @@ class ParserImplWrapperForTest {
     return impl_.GetSourceForResultIdForTest(id);
   }
 
-  /// Changes pipeline IO to be HLSL-style: as entry point parameters and
-  /// return.
-  /// TODO(crbug.com/tint/508): Once all this support has landed, switch
-  /// over to that, and remove the old support.
-  void SetHLSLStylePipelineIO() { impl_.SetHLSLStylePipelineIO(); }
-
-  /// @returns true if HLSL-style IO should be used.
-  bool UseHLSLStylePipelineIO() const { return impl_.UseHLSLStylePipelineIO(); }
-
  private:
   ParserImpl impl_;
   /// When true, indicates the input SPIR-V module should not be emitted.

src/reader/spirv/parser_impl_user_name_test.cc

@@ -186,9 +186,6 @@ TEST_F(SpvParserUserNameTest, EntryPointNames_DistinctFromInnerNames) {
 )";
   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"));