[spirv-writer] Remove push_preamble

The push_preamble method was dealing with multiple sections of the
SPIR-V binary layout. As we changed the way things write (like
extensions getting written later) the preamble section was ending up in
incorrect order.

This CL replaces push_preamble with push methods for each of the
sections at the start of the SPIR-V module which should fixup the
ordering issue.

Bug: tint:267
Change-Id: Ib73a66d0fdb2c67dd6e80582289dd18475fad9f9
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/29841
Commit-Queue: Ryan Harrison <rharrison@chromium.org>
Reviewed-by: Ryan Harrison <rharrison@chromium.org>

src/writer/spirv/binary_writer_test.cc

@@ -45,7 +45,7 @@ TEST_F(BinaryWriterTest, Preamble) {
 TEST_F(BinaryWriterTest, Float) {
   ast::Module mod;
   Builder b(&mod);
-  b.push_preamble(spv::Op::OpKill, {Operand::Float(2.4f)});
+  b.push_annot(spv::Op::OpKill, {Operand::Float(2.4f)});
   BinaryWriter bw;
   bw.WriteBuilder(&b);
 
@@ -59,7 +59,7 @@ TEST_F(BinaryWriterTest, Float) {
 TEST_F(BinaryWriterTest, Int) {
   ast::Module mod;
   Builder b(&mod);
-  b.push_preamble(spv::Op::OpKill, {Operand::Int(2)});
+  b.push_annot(spv::Op::OpKill, {Operand::Int(2)});
   BinaryWriter bw;
   bw.WriteBuilder(&b);
 
@@ -71,7 +71,7 @@ TEST_F(BinaryWriterTest, Int) {
 TEST_F(BinaryWriterTest, String) {
   ast::Module mod;
   Builder b(&mod);
-  b.push_preamble(spv::Op::OpKill, {Operand::String("my_string")});
+  b.push_annot(spv::Op::OpKill, {Operand::String("my_string")});
   BinaryWriter bw;
   bw.WriteBuilder(&b);
 
@@ -96,7 +96,7 @@ TEST_F(BinaryWriterTest, String) {
 TEST_F(BinaryWriterTest, String_Multiple4Length) {
   ast::Module mod;
   Builder b(&mod);
-  b.push_preamble(spv::Op::OpKill, {Operand::String("mystring")});
+  b.push_annot(spv::Op::OpKill, {Operand::String("mystring")});
   BinaryWriter bw;
   bw.WriteBuilder(&b);
 

src/writer/spirv/builder.cc

@@ -279,12 +279,12 @@ bool Builder::Build() {
 
   // TODO(dneto): Stop using the Vulkan memory model. crbug.com/tint/63
   push_capability(SpvCapabilityVulkanMemoryModel);
-  push_preamble(spv::Op::OpExtension,
-                {Operand::String("SPV_KHR_vulkan_memory_model")});
+  push_extension(spv::Op::OpExtension,
+                 {Operand::String("SPV_KHR_vulkan_memory_model")});
 
-  push_preamble(spv::Op::OpMemoryModel,
-                {Operand::Int(SpvAddressingModelLogical),
-                 Operand::Int(SpvMemoryModelVulkanKHR)});
+  push_memory_model(spv::Op::OpMemoryModel,
+                    {Operand::Int(SpvAddressingModelLogical),
+                     Operand::Int(SpvMemoryModelVulkanKHR)});
 
   for (const auto& var : mod_->global_variables()) {
     if (!GenerateGlobalVariable(var.get())) {
@@ -310,7 +310,9 @@ uint32_t Builder::total_size() const {
   uint32_t size = 5;
 
   size += size_of(capabilities_);
-  size += size_of(preamble_);
+  size += size_of(extensions_);
+  size += size_of(ext_imports_);
+  size += size_of(memory_model_);
   size += size_of(entry_points_);
   size += size_of(execution_modes_);
   size += size_of(debug_);
@@ -327,7 +329,13 @@ void Builder::iterate(std::function<void(const Instruction&)> cb) const {
   for (const auto& inst : capabilities_) {
     cb(inst);
   }
-  for (const auto& inst : preamble_) {
+  for (const auto& inst : extensions_) {
+    cb(inst);
+  }
+  for (const auto& inst : ext_imports_) {
+    cb(inst);
+  }
+  for (const auto& inst : memory_model_) {
     cb(inst);
   }
   for (const auto& inst : entry_points_) {
@@ -1064,8 +1072,8 @@ void Builder::GenerateGLSLstd450Import() {
   auto result = result_op();
   auto id = result.to_i();
 
-  push_preamble(spv::Op::OpExtInstImport,
-                {result, Operand::String(kGLSLstd450)});
+  push_ext_import(spv::Op::OpExtInstImport,
+                  {result, Operand::String(kGLSLstd450)});
 
   import_name_to_id_[kGLSLstd450] = id;
 }

src/writer/spirv/builder.h

@@ -95,14 +95,30 @@ class Builder {
   void push_capability(uint32_t cap);
   /// @returns the capabilities
   const InstructionList& capabilities() const { return capabilities_; }
-  /// Adds an instruction to the preamble
+  /// Adds an instruction to the extensions
   /// @param op the op to set
   /// @param operands the operands for the instruction
-  void push_preamble(spv::Op op, const OperandList& operands) {
-    preamble_.push_back(Instruction{op, operands});
+  void push_extension(spv::Op op, const OperandList& operands) {
+    extensions_.push_back(Instruction{op, operands});
   }
-  /// @returns the preamble
-  const InstructionList& preamble() const { return preamble_; }
+  /// @returns the extensions
+  const InstructionList& extensions() const { return extensions_; }
+  /// Adds an instruction to the ext import
+  /// @param op the op to set
+  /// @param operands the operands for the instruction
+  void push_ext_import(spv::Op op, const OperandList& operands) {
+    ext_imports_.push_back(Instruction{op, operands});
+  }
+  /// @returns the ext imports
+  const InstructionList& ext_imports() const { return ext_imports_; }
+  /// Adds an instruction to the memory model
+  /// @param op the op to set
+  /// @param operands the operands for the instruction
+  void push_memory_model(spv::Op op, const OperandList& operands) {
+    memory_model_.push_back(Instruction{op, operands});
+  }
+  /// @returns the memory model
+  const InstructionList& memory_model() const { return memory_model_; }
   /// Adds an instruction to the entry points
   /// @param op the op to set
   /// @param operands the operands for the instruction
@@ -441,7 +457,9 @@ class Builder {
   uint32_t next_id_ = 1;
   uint32_t current_label_id_ = 0;
   InstructionList capabilities_;
-  InstructionList preamble_;
+  InstructionList extensions_;
+  InstructionList ext_imports_;
+  InstructionList memory_model_;
   InstructionList entry_points_;
   InstructionList execution_modes_;
   InstructionList debug_;